Thanks for tracking this one down!
> -----Original Message-----
> From: xen-api-bounces@xxxxxxxxxxxxxxxxxxx [mailto:xen-api-
> bounces@xxxxxxxxxxxxxxxxxxx] On Behalf Of Jonathan Knowles
> Sent: 04 March 2010 12:23
> To: xen-api@xxxxxxxxxxxxxxxxxxx
> Subject: [Xen-API] [PATCH] [CA-38359] Restarting Xapi no longer changes
> "PV-drivers-up-to-date" from "true" to "false" for running VMs
>
> # HG changeset patch
> # User Jonathan Knowles <jonathan.knowles@xxxxxxxxxxxxx> # Date
> 1267705085 0 # Node ID 0e7ab109bf92783d1f248ac39eed3d142e19178e
> # Parent e97f45cd743e4caf381de15b53865b91a847464a
> [CA-38359] Restarting Xapi no longer changes "PV-drivers-up-to-date"
> from "true" to "false" for running VMs.
>
> Signed-off-by: Jonathan Knowles <jonathan.knowles@xxxxxxxxxxxxx>
> Acked-by: Magnus Therning <Magnus.Therning@xxxxxxxxxxxxx>
>
> This change fixes a regression introduced by changeset 22cd3f304b9e
> (originally to fix CA-35549).
>
> During Xapi startup, the "Events.guest_agent_update" function iterates
> through every VM in the Xapi database. For each VM, it calls the
> "Xapi_pv_driver_version.compare_vsn_with_product_vsn" function to
> determine whether its PV drivers are up to date, and then writes the
> result into that VM's "PV-drivers-up-to-date" field. The
> "compare_vsn_with_product_vsn" function works by comparing a PV driver
> version with the host product version.
>
> Unfortunately:
> * The "compare_vsn_with_product_vsn" function would read the product
> version from a global mutable association list
> "Xapi_globs.localhost_software_version".
> * The "Xapi_globs.localhost_software_version" variable had the empty
> list as its default value, but was initialised by the
> "Dbsync_slave.refresh_localhost_info" function during Xapi startup.
> * Changeset 22cd3f304b9e altered Xapi's startup order, making it call
> the "compare_vsn_with_product_vsn" function before the
> "Dbsync_slave.refresh_localhost_info" function.
> * This caused the "compare_vsn_with_product_vsn" function to read an
> empty list from the "Xapi_globs.localhost_software_version" variable.
> * On attempting to extract the product version from the empty list
> (doomed to failure), the "compare_vsn_with_product_vsn" function would
> trigger an exception.
> * Unfortunately, the "compare_vsn_with_product_vsn" function had an
> exception handler that swallowed all exceptions without prejudice,
> changing them into the "safe" value of "false".
> * The "Events.guest_agent_update" function would obediently write the
> value of "false" into the PV-drivers-up-to-date field for every VM.
>
> Ironically, the "compare_vsn_with_product_vsn" function need not have
> relied on a mutable variable in this way, since the value it was
> interested in already existed as the static "Version.product_version"
> constant!
>
> This change:
> * Modifies the "compare_vsn_with_product_vsn" function to rely on the
> static "Version.product_version" constant.
> * Removes the swallow-all exception handler from the
> "compare_vsn_with_product_vsn" function, allowing any exceptions (rare)
> to trickle up.
> * Removes the global variable "Xapi_globs.localhost_software_version"
> along with the code that initialises it, since it's no longer used by
> anything.
>
> diff -r e97f45cd743e -r 0e7ab109bf92 ocaml/xapi/dbsync_slave.ml
> --- a/ocaml/xapi/dbsync_slave.ml Thu Mar 04 12:03:40 2010 +0000
> +++ b/ocaml/xapi/dbsync_slave.ml Thu Mar 04 12:18:05 2010 +0000
> @@ -83,7 +83,6 @@
> let host = !Xapi_globs.localhost_ref in
> let info = read_localhost_info () in
> let software_version = Create_misc.make_software_version () in
> - Xapi_globs.localhost_software_version := software_version; (* Cache
> this *)
>
> (* Xapi_ha_flags.resync_host_armed_flag __context host; *)
> debug "Updating host software_version"; diff -r e97f45cd743e -r
> 0e7ab109bf92 ocaml/xapi/xapi_globs.ml
> --- a/ocaml/xapi/xapi_globs.ml Thu Mar 04 12:03:40 2010 +0000
> +++ b/ocaml/xapi/xapi_globs.ml Thu Mar 04 12:18:05 2010 +0000
> @@ -66,9 +66,6 @@
>
> let unix_domain_socket = "/var/xapi/xapi"
> let local_database = "/var/xapi/local.db"
> -
> -(* Cached localhost info *)
> -let localhost_software_version : ((string * string) list) ref = ref []
>
> (* amount of time to retry master_connection before (if
> restart_on_connection_timeout is set) restarting xapi; -ve means don't
> timeout: *) let master_connect_retry_timeout = -1. (* never timeout *)
> diff -r e97f45cd743e -r 0e7ab109bf92
> ocaml/xapi/xapi_pv_driver_version.ml
> --- a/ocaml/xapi/xapi_pv_driver_version.ml Thu Mar 04 12:03:40
> 2010 +0000
> +++ b/ocaml/xapi/xapi_pv_driver_version.ml Thu Mar 04 12:18:05
> 2010 +0000
> @@ -74,24 +74,25 @@
> | Windows(major, minor, micro, build) -> Printf.sprintf "Windows
> %d.%d.%d-%d" major minor micro build
> | Unknown -> "Unknown"
>
> -(* Returns -1 if PV drivers are out-of-date wrt product version on
> this host;
> - returns 0 if PV drivers match product version on this host;
> - returns 1 if PV drivers are a newer version than the product
> version on this host *)
> -let compare_vsn_with_product_vsn (pv_maj,pv_min,pv_mic) =
> - try
> - let my_software_version = !Xapi_globs.localhost_software_version
> in
> - let my_product_version = List.assoc "product_version"
> my_software_version in
> - let (prod_maj, prod_min, prod_mic) =
> - match (Stringext.String.split '.' my_product_version) with
> - | [ prod_maj; prod_min; prod_mic] -> int_of_string prod_maj,
> int_of_string prod_min, int_of_string prod_mic
> - | _ -> warn "xapi product
> version is wrong format: %s" my_product_version; assert false;
> - in
> - if pv_mic = -1 then -1 (* out of date if micro version not
> specified -- reqd since Miami Beta1 was shipped without micro versions!
> *)
> - else if pv_maj<prod_maj || (pv_maj=prod_maj && pv_min<prod_min)
> || (pv_maj=prod_maj && pv_min=prod_min && pv_mic<prod_mic) then -1
> - else if pv_maj=prod_maj && pv_min=prod_min && pv_mic=prod_mic
> then 0
> - else 1
> - with e ->
> - -1 (* return out-of-date - if something goes wrong here "fail
> safe". *)
> +(** Compares the given version tuple with the product version on this
> host.
> + ** @return -1: if the given version is older;
> + ** @return 0: if the given version is equal;
> + ** @return +1: if the given version is newer;
> + ** @raise Assert_failure: if this host does not have a valid product
> version.
> + **)
> +let compare_vsn_with_product_vsn (pv_maj, pv_min, pv_mic) =
> + let (prod_maj, prod_min, prod_mic) =
> + match (Stringext.String.split '.' Version.product_version)
> with
> + | [maj; min; mic] ->
> + (int_of_string maj, int_of_string min,
> int_of_string mic)
> + | _ ->
> + warn "xapi product version is wrong format: %s"
> + Version.product_version; assert false;
> + in
> + if pv_mic = -1 then -1 (* out of date if micro version not
> specified -- reqd since Miami Beta1 was shipped without micro versions!
> *)
> + else if pv_maj<prod_maj || (pv_maj=prod_maj && pv_min<prod_min)
> || (pv_maj=prod_maj && pv_min=prod_min && pv_mic<prod_mic) then -1
> + else if pv_maj=prod_maj && pv_min=prod_min && pv_mic=prod_mic
> then 0
> + else 1
>
> (* Returns -1 if PV drivers are out-of-date wrt tools version on this
> host;
> returns 0 if the PV drivers match the tools version on this host;
> 3 files changed, 19 insertions(+), 22 deletions(-)
> ocaml/xapi/dbsync_slave.ml | 1
> ocaml/xapi/xapi_globs.ml | 3 --
> ocaml/xapi/xapi_pv_driver_version.ml | 37 +++++++++++++++++----------
> -------
>
_______________________________________________
xen-api mailing list
xen-api@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/mailman/listinfo/xen-api
|
Home