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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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)

XenD implements the following variants for clock/@offset:
- PV domains don't have a RTC → 'localtime' | 'utc'
- <3.1: no managed domains → 'localtime' | 'utc'
- ≥3.1: the offset is tracked for HV → 'variable'
        due to the localtime=1 bug → 'localtime' | 'utc'
- ≥3.4: the offset is tracked for HV → 'variable'

Current libvirtd still thinks XenD only implements <clock offset='utc'/>
and <clock offset='localtime'/>, which is wrong, since the semantic of
'utc' and 'localtime' specifies, that the offset will be reset on
domain-restart, while with 'variable' the offset is kept. (keeping the
offset over "virsh edit" is important, since otherwise the clock might
jump, which confuses certain guest OSs)

xendConfigVersion was last incremented to 4 by the xen-folks for
xen-3.1.0. I know of no way to reliably detect the version of XenD
(user space tools), which may be different from the version of the
hypervisor (kernel) version! Because of this only the change from
'utc'/'localtime' to 'variable' in XenD-3.1 is handled, not the buggy
behaviour of XenD-3.1 until XenD-3.4.

For backward compatibility with previous versions of libvirt Xen-HV
still accepts 'utc' and 'localtime', but they are returned as 'variable'
on the next read-back from Xend to libvirt, since this is what XenD
implements: The RTC is NOT reset back to the specified time on next
restart, but the previous offset is kept.

Signed-off-by: Philipp Hahn <hahn@xxxxxxxxxxxxx>
---
 src/xenxs/xen_sxpr.c |  161 +++++++++++++++++++++++++++++++++++++------------
 src/xenxs/xen_xm.c   |  116 ++++++++++++++++++++++++++++--------
 2 files changed, 212 insertions(+), 65 deletions(-)

diff --git a/src/xenxs/xen_sxpr.c b/src/xenxs/xen_sxpr.c
index f8390ea..2d6cddb 100644
--- a/src/xenxs/xen_sxpr.c
+++ b/src/xenxs/xen_sxpr.c
@@ -1126,7 +1126,7 @@ xenParseSxpr(const struct sexpr *root,
 {
     const char *tmp;
     virDomainDefPtr def;
-    int hvm = 0;
+    int hvm = 0, vmlocaltime;
 
     if (VIR_ALLOC(def) < 0)
         goto no_memory;
@@ -1247,7 +1247,6 @@ xenParseSxpr(const struct sexpr *root,
     } else
         def->onCrash = VIR_DOMAIN_LIFECYCLE_DESTROY;
 
-    def->clock.offset = VIR_DOMAIN_CLOCK_OFFSET_UTC;
     if (hvm) {
         if (sexpr_int(root, "domain/image/hvm/acpi"))
             def->features |= (1 << VIR_DOMAIN_FEATURE_ACPI);
@@ -1259,10 +1258,29 @@ xenParseSxpr(const struct sexpr *root,
             def->features |= (1 << VIR_DOMAIN_FEATURE_HAP);
         if (sexpr_int(root, "domain/image/hvm/viridian"))
             def->features |= (1 << VIR_DOMAIN_FEATURE_VIRIDIAN);
+    }
 
-        /* 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);
+        /* only managed HVM domains since 3.1.0 have persistent rtc_timeoffset */
+        if (xendConfigVersion < XEND_CONFIG_VERSION_3_1_0) {
+            if (vmlocaltime)
+                def->clock.offset = VIR_DOMAIN_CLOCK_OFFSET_LOCALTIME;
+            else
+                def->clock.offset = VIR_DOMAIN_CLOCK_OFFSET_UTC;
+        } else {
+            int rtc_offset;
+            def->clock.offset = VIR_DOMAIN_CLOCK_OFFSET_VARIABLE;
+            rtc_offset =  sexpr_int(root, "domain/image/hvm/rtc_timeoffset");
+            def->clock.data.variable.adjustment = rtc_offset;
+            def->clock.data.variable.basis = vmlocaltime ?
+                VIR_DOMAIN_CLOCK_BASIS_LOCALTIME :
+                VIR_DOMAIN_CLOCK_BASIS_UTC;
+        }
 
         if (sexpr_lookup(root, "domain/image/hvm/hpet")) {
             virDomainTimerDefPtr timer;
@@ -1280,14 +1298,16 @@ 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);
+        /* PV domains do not have an emulated RTC and the offset is fixed. */
+        if (vmlocaltime)
             def->clock.offset = VIR_DOMAIN_CLOCK_OFFSET_LOCALTIME;
-    }
-
-    /* Current XenD allows localtime here, for PV and HVM */
-    if (sexpr_int(root, "domain/localtime"))
-        def->clock.offset = VIR_DOMAIN_CLOCK_OFFSET_LOCALTIME;
+        else
+            def->clock.offset = VIR_DOMAIN_CLOCK_OFFSET_UTC;
+    } /* !hvm */
 
     if (sexpr_node_copy(root, hvm ?
                         "domain/image/hvm/device_model" :
@@ -2195,7 +2215,8 @@ xenFormatSxpr(virConnectPtr conn,
     char uuidstr[VIR_UUID_STRING_BUFLEN];
     const char *tmp;
     char *bufout;
-    int hvm = 0, i;
+    int hvm = 0, i, vmlocaltime = -1;
+    bool in_image = false;
 
     VIR_DEBUG("Formatting domain sexpr");
 
@@ -2255,30 +2276,15 @@ xenFormatSxpr(virConnectPtr conn,
     }
     virBufferAsprintf(&buf, "(on_crash '%s')", tmp);
 
-    /* Set localtime here for current XenD (both PV & HVM) */
-    if (def->clock.offset == VIR_DOMAIN_CLOCK_OFFSET_LOCALTIME) {
-        if (def->clock.data.timezone) {
-            XENXS_ERROR(VIR_ERR_CONFIG_UNSUPPORTED,
-                         "%s", _("configurable timezones are not supported"));
-            goto error;
-        }
-
-        virBufferAddLit(&buf, "(localtime 1)");
-    } else if (def->clock.offset != VIR_DOMAIN_CLOCK_OFFSET_UTC) {
-        XENXS_ERROR(VIR_ERR_CONFIG_UNSUPPORTED,
-                     _("unsupported clock offset '%s'"),
-                     virDomainClockOffsetTypeToString(def->clock.offset));
-        goto error;
-    }
+    if (STREQ(def->os.type, "hvm"))
+        hvm = 1;
 
     if (!def->os.bootloader) {
-        if (STREQ(def->os.type, "hvm"))
-            hvm = 1;
-
         if (hvm)
             virBufferAddLit(&buf, "(image (hvm ");
         else
             virBufferAddLit(&buf, "(image (linux ");
+        in_image = true;
 
         if (hvm &&
             def->os.loader == NULL) {
@@ -2424,17 +2430,13 @@ xenFormatSxpr(virConnectPtr conn,
                 virBufferAddLit(&buf, "(serial none)");
             }
 
-            /* Set localtime here to keep old XenD happy for HVM */
-            if (def->clock.offset == VIR_DOMAIN_CLOCK_OFFSET_LOCALTIME)
-                virBufferAddLit(&buf, "(localtime 1)");
-
             if (def->sounds) {
                 virBufferAddLit(&buf, "(soundhw '");
                 if (xenFormatSxprSound(def, &buf) < 0)
                     goto error;
                 virBufferAddLit(&buf, "')");
             }
-        }
+        } /* hvm */
 
         /* get the device emulation model */
         if (def->emulator && (hvm || xendConfigVersion >= XEND_CONFIG_VERSION_3_0_4))
@@ -2458,15 +2460,94 @@ xenFormatSxpr(virConnectPtr conn,
                                          &buf, xendConfigVersion) < 0)
                 goto error;
         }
-
-        virBufferAddLit(&buf, "))");
     } else {
         /* PV domains accept kernel cmdline args */
         if (def->os.cmdline) {
-            virBufferEscapeSexpr(&buf, "(image (linux (args '%s')))",
-                                 def->os.cmdline);
+            virBufferEscapeSexpr(&buf, "(image (linux (args '%s')", def->os.cmdline);
+            in_image = true;
+        }
+    } /* os.bootloader */
+
+
+    if (xendConfigVersion < XEND_CONFIG_VERSION_3_1_0) {
+        /* <3.1: UTC and LOCALTIME */
+        switch (def->clock.offset) {
+        case VIR_DOMAIN_CLOCK_OFFSET_UTC:
+            vmlocaltime = 0;
+            break;
+        case VIR_DOMAIN_CLOCK_OFFSET_LOCALTIME:
+            vmlocaltime = 1;
+            break;
+        default:
+            vmlocaltime = -1; /* goto clock_error */
         }
+    } else {
+        if (!in_image) {
+            if (hvm)
+                virBufferAddLit(&buf, "(image (hvm ");
+            else
+                virBufferAddLit(&buf, "(image (linux ");
+            in_image = true;
+        }
+        if (hvm) {
+            /* >=3.1 HV: VARIABLE */
+            int rtc_timeoffset;
+            switch (def->clock.offset) {
+            case VIR_DOMAIN_CLOCK_OFFSET_VARIABLE:
+                vmlocaltime = (int)def->clock.data.variable.basis;
+                rtc_timeoffset = def->clock.data.variable.adjustment;
+                break;
+/* Previously libvirt ≤0.9.9 returned UTC and LOCALTIME while XenD ≥3.1
+ * (d24f37b31030 from 2007-04-03) really implemented VARIABLE.
+ * For backward compatibility (snapshots!) old XML is silently accepted and
+ * converted. If the behaviour to reset the offset back to 0 on restart is
+ * important and an error is preferred, define STRICT. */
+#ifndef LIBVIRT_XEND_CLOCK_STRICT
+            case VIR_DOMAIN_CLOCK_OFFSET_UTC:
+                vmlocaltime = 0;
+                rtc_timeoffset = 0;
+                break;
+            case VIR_DOMAIN_CLOCK_OFFSET_LOCALTIME:
+                vmlocaltime = 1;
+                rtc_timeoffset = 0;
+                break;
+#endif
+            default:
+                vmlocaltime = -1; /* goto clock_error */
+            }
+            if (vmlocaltime >= 0)
+                virBufferAsprintf(&buf, "(rtc_timeoffset %d)", rtc_timeoffset);
+        } else {
+            /* >=3.1 PV: UTC and LOCALTIME */
+            switch (def->clock.offset) {
+            case VIR_DOMAIN_CLOCK_OFFSET_UTC:
+                vmlocaltime = 0;
+                break;
+            case VIR_DOMAIN_CLOCK_OFFSET_LOCALTIME:
+                vmlocaltime = 1;
+                break;
+            default:
+                vmlocaltime = -1; /* goto clock_error */
+            }
+        } /* !hvm */
+        /* default post-XenD-3.1 location: */
+        if (vmlocaltime >= 0)
+            virBufferAsprintf(&buf, "(localtime %d)", vmlocaltime);
     }
+    if (vmlocaltime < 0) { /* clock_error: */
+        XENXS_ERROR(VIR_ERR_CONFIG_UNSUPPORTED,
+                    _("unsupported clock offset '%s'"),
+                    virDomainClockOffsetTypeToString(def->clock.offset));
+        goto error;
+    }
+    if (in_image) {
+        /* closes (image(hvm|linux */
+        virBufferAddLit(&buf, "))");
+        in_image = false;
+    }
+    /* pre-XenD-3.1 and compatibility location */
+    virBufferAsprintf(&buf, "(localtime %d)", vmlocaltime);
+
 
     for (i = 0 ; i < def->ndisks ; i++)
         if (xenFormatSxprDisk(def->disks[i],
diff --git a/src/xenxs/xen_xm.c b/src/xenxs/xen_xm.c
index e580a3e..15190de 100644
--- a/src/xenxs/xen_xm.c
+++ b/src/xenxs/xen_xm.c
@@ -414,9 +414,29 @@ xenParseXM(virConfPtr conf, int xendConfigVersion,
     if (xenXMConfigGetBool(conf, "localtime", &vmlocaltime, 0) < 0)
         goto cleanup;
 
-    def->clock.offset = vmlocaltime ?
-        VIR_DOMAIN_CLOCK_OFFSET_LOCALTIME :
-        VIR_DOMAIN_CLOCK_OFFSET_UTC;
+    if (hvm) {
+        /* only managed HVM domains since 3.1.0 have persistent rtc_timeoffset */
+        if (xendConfigVersion < XEND_CONFIG_VERSION_3_1_0) {
+            if (vmlocaltime)
+                def->clock.offset = VIR_DOMAIN_CLOCK_OFFSET_LOCALTIME;
+            else
+                def->clock.offset = VIR_DOMAIN_CLOCK_OFFSET_UTC;
+        } else {
+            unsigned long rtc_timeoffset;
+            def->clock.offset = VIR_DOMAIN_CLOCK_OFFSET_VARIABLE;
+            if (xenXMConfigGetULong(conf, "rtc_timeoffset", &rtc_timeoffset, 0) < 0)
+                goto cleanup;
+            def->clock.data.variable.adjustment = (int)rtc_timeoffset;
+            def->clock.data.variable.basis = vmlocaltime ?
+                VIR_DOMAIN_CLOCK_BASIS_LOCALTIME :
+                VIR_DOMAIN_CLOCK_BASIS_UTC;
+        }
+    } else {
+        /* PV domains do not have an emulated RTC and the offset is fixed. */
+        def->clock.offset = vmlocaltime ?
+            VIR_DOMAIN_CLOCK_OFFSET_LOCALTIME :
+            VIR_DOMAIN_CLOCK_OFFSET_UTC;
+    } /* !hvm */
 
     if (xenXMConfigCopyStringOpt(conf, "device_model", &def->emulator) < 0)
         goto cleanup;
@@ -1450,7 +1470,7 @@ virConfPtr xenFormatXM(virConnectPtr conn,
                                    virDomainDefPtr def,
                                    int xendConfigVersion) {
     virConfPtr conf = NULL;
-    int hvm = 0, i;
+    int hvm = 0, i, vmlocaltime = 0;
     char *cpus = NULL;
     const char *lifecycle;
     char uuid[VIR_UUID_STRING_BUFLEN];
@@ -1557,26 +1577,6 @@ virConfPtr xenFormatXM(virConnectPtr conn,
                 goto no_memory;
         }
 
-        if (def->clock.offset == VIR_DOMAIN_CLOCK_OFFSET_LOCALTIME) {
-            if (def->clock.data.timezone) {
-                XENXS_ERROR(VIR_ERR_CONFIG_UNSUPPORTED,
-                           "%s", _("configurable timezones are not supported"));
-                goto cleanup;
-            }
-
-            if (xenXMConfigSetInt(conf, "localtime", 1) < 0)
-                goto no_memory;
-        } else if (def->clock.offset == VIR_DOMAIN_CLOCK_OFFSET_UTC) {
-            if (xenXMConfigSetInt(conf, "localtime", 0) < 0)
-                goto no_memory;
-        } else {
-            /* XXX We could support Xen's rtc clock offset */
-            XENXS_ERROR(VIR_ERR_CONFIG_UNSUPPORTED,
-                       _("unsupported clock offset '%s'"),
-                       virDomainClockOffsetTypeToString(def->clock.offset));
-            goto cleanup;
-        }
-
         for (i = 0; i < def->clock.ntimers; i++) {
             if (def->clock.timers[i]->name == VIR_DOMAIN_TIMER_NAME_HPET &&
                 def->clock.timers[i]->present != -1 &&
@@ -1615,8 +1615,74 @@ virConfPtr xenFormatXM(virConnectPtr conn,
         if (def->os.cmdline &&
             xenXMConfigSetString(conf, "extra", def->os.cmdline) < 0)
             goto no_memory;
-
+    } /* !hvm */
+
+
+    if (xendConfigVersion < XEND_CONFIG_VERSION_3_1_0) {
+        /* <3.1: UTC and LOCALTIME */
+        switch (def->clock.offset) {
+        case VIR_DOMAIN_CLOCK_OFFSET_UTC:
+            vmlocaltime = 0;
+            break;
+        case VIR_DOMAIN_CLOCK_OFFSET_LOCALTIME:
+            vmlocaltime = 1;
+            break;
+        default:
+            vmlocaltime = -1; /* goto clock_error */
+        }
+    } else {
+        if (hvm) {
+            /* >=3.1 HV: VARIABLE */
+            int rtc_timeoffset;
+            switch (def->clock.offset) {
+            case VIR_DOMAIN_CLOCK_OFFSET_VARIABLE:
+                vmlocaltime = (int)def->clock.data.variable.basis;
+                rtc_timeoffset = def->clock.data.variable.adjustment;
+                break;
+/* Previously libvirt ≤0.9.9 returned UTC and LOCALTIME while XenD ≥3.1
+ * (d24f37b31030 from 2007-04-03) really implemented VARIABLE.
+ * For backward compatibility (snapshots!) old XML is silently accepted and
+ * converted. If the behaviour to reset the offset back to 0 on restart is
+ * important and an error is preferred, define STRICT. */
+#ifndef LIBVIRT_XEND_CLOCK_STRICT
+            case VIR_DOMAIN_CLOCK_OFFSET_UTC:
+                vmlocaltime = 0;
+                rtc_timeoffset = 0;
+                break;
+            case VIR_DOMAIN_CLOCK_OFFSET_LOCALTIME:
+                vmlocaltime = 1;
+                rtc_timeoffset = 0;
+                break;
+#endif
+            default:
+                vmlocaltime = -1; /* goto clock_error */
+            }
+            if (vmlocaltime >= 0)
+                if (xenXMConfigSetInt(conf, "rtc_timeoffset", rtc_timeoffset) < 0)
+                    goto no_memory;
+        } else {
+            /* >=3.1 PV: UTC and LOCALTIME */
+            switch (def->clock.offset) {
+            case VIR_DOMAIN_CLOCK_OFFSET_UTC:
+                vmlocaltime = 0;
+                break;
+            case VIR_DOMAIN_CLOCK_OFFSET_LOCALTIME:
+                vmlocaltime = 1;
+                break;
+            default:
+                vmlocaltime = -1; /* goto clock_error */
+            }
+        } /* !hvm */
+    }
+    if (vmlocaltime < 0) { /* clock_error: */
+        XENXS_ERROR(VIR_ERR_CONFIG_UNSUPPORTED,
+                    _("unsupported clock offset '%s'"),
+                    virDomainClockOffsetTypeToString(def->clock.offset));
+        goto cleanup;
     }
+    if (xenXMConfigSetInt(conf, "localtime", vmlocaltime) < 0)
+        goto no_memory;
+
 
     if (!(lifecycle = virDomainLifecycleTypeToString(def->onPoweroff))) {
         XENXS_ERROR(VIR_ERR_INTERNAL_ERROR,
-- 
1.7.1

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list



[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]