# HG changeset patch # User Jonathan Knowles # 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 Acked-by: Magnus Therning 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 - -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