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

[Jonathan Knowles <jonathan.knowles@xxxxxxxxxxxxx>]

# 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 +++++++++++++++++-----------------

Attachment: xen-api.hg.patch
Description: Text Data

_______________________________________________
xen-api mailing list
xen-api@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/mailman/listinfo/xen-api