RE: [Xen-API] [PATCH] [CA-38359] Restarting Xapi no longer changes "PV-drivers-up-to-date" from "true" to "false" for running VMs

[Dave Scott <Dave.Scott@xxxxxxxxxxxxx>]

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