[libvirt] [PATCHv5 2/3] Xen: Fix <clock> handling

[Eric Blake]

On 02/08/2012 09:32 AM, Philipp Hahn wrote:
> XenD-3.1 introduced managed domains. HV-domains have rtc_timeoffset
> (hgd24f37b31030 from 2007-04-03), which tracks the offset between the
> hypervisors clock and the domains RTC, and is persisted by XenD.
> In combination with localtime=1 this had a bug until XenD-3.4
> (hg5d701be7c37b from 2009-04-01) (I'm not 100% sure how that bug
> manifests, but at least for me in TZ=Europe/Berlin I see the previous
> offset relative to utc being applied to localtime again, which manifests
> in an extra hour being added)
> 

Applying this patch causes 'make check' to fail on xml2sexprtest,
sexpr2xmltest, and xmconfigtest for me.  I didn't look into it, other
than seeing lots of:

$ make -C tests check TESTS=xmconfigtest VIR_TEST_DEBUG=1
...
64) Xen XM-2-XML Format no-source-cdrom                               ...
Offset 414
Expect [/]
Actual [ adjustment='reset'/]

... FAILED

If patch 3/3 fixes things, then I will squash the two together (we want
'git bisect' to succeed as often as possible).

>  
> -        /* Old XenD only allows localtime here for HVM */
> -        if (sexpr_int(root, "domain/image/hvm/localtime"))
> -            def->clock.offset = VIR_DOMAIN_CLOCK_OFFSET_LOCALTIME;
> +    /* 12aaf4a2486b (3.0.3) added a second low-priority 'localtime' setting */
> +    vmlocaltime = sexpr_int(root, "domain/localtime");
> +    if (hvm) {
> +        const char *value = sexpr_node(root, "domain/image/hvm/localtime");
> +        if (value)
> +            vmlocaltime = strtol(value, NULL, 0);

strtol() is awkward (I'm surprised 'make syntax-check' isn't flagging it).

> @@ -1279,14 +1298,17 @@ xenParseSxpr(const struct sexpr *root,
>              def->clock.ntimers = 1;
>              def->clock.timers[0] = timer;
>          }
> -    } else { /* !hvm */
> -        if (sexpr_int(root, "domain/image/linux/localtime"))
> +    } else {
> +        const char *value = sexpr_node(root, "domain/image/linux/localtime");
> +        if (value)
> +            vmlocaltime = strtol(value, NULL, 0);

another strtol().

I didn't spot any other problems, so conditional ack once I review 3/3
and squash this in:


diff --git i/src/xenxs/xen_sxpr.c w/src/xenxs/xen_sxpr.c
index c9bacb2..b26b2bc 100644
--- i/src/xenxs/xen_sxpr.c
+++ w/src/xenxs/xen_sxpr.c
@@ -1263,8 +1263,13 @@ xenParseSxpr(const struct sexpr *root,
     vmlocaltime = sexpr_int(root, "domain/localtime");
     if (hvm) {
         const char *value = sexpr_node(root, "domain/image/hvm/localtime");
-        if (value)
-            vmlocaltime = strtol(value, NULL, 0);
+        if (value) {
+            if (virStrToLong_i(value, NULL, 0, &vmlocaltime) < 0) {
+                XENXS_ERROR(VIR_ERR_INTERNAL_ERROR,
+                            _("unknown localtime offset %s"), value);
+                goto error;
+            }
+        }
         /* only managed HVM domains since 3.1.0 have persistent
rtc_timeoffset */
         if (xendConfigVersion < XEND_CONFIG_VERSION_3_1_0) {
             if (vmlocaltime)
@@ -1300,8 +1305,13 @@ xenParseSxpr(const struct sexpr *root,
         }
     } else {
         const char *value = sexpr_node(root,
"domain/image/linux/localtime");
-        if (value)
-            vmlocaltime = strtol(value, NULL, 0);
+        if (value) {
+            if (virStrToLong_i(value, NULL, 0, &vmlocaltime) < 0) {
+                XENXS_ERROR(VIR_ERR_INTERNAL_ERROR,
+                            _("unknown localtime offset %s"), value);
+                goto error;
+            }
+        }
         /* PV domains do not have an emulated RTC and the offset is
fixed. */
         if (vmlocaltime)
             def->clock.offset = VIR_DOMAIN_CLOCK_OFFSET_LOCALTIME;

-- 
Eric Blake   eblake at redhat.com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 620 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20120402/b43119e1/attachment-0001.sig>