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

[libvirt] [BUG, PATCH-RFC] libvirt localtime and rtc_timeoffset handling in xen-sexpr/sxpr/sxp



Hello,

I'm currently tracking a problem in libvirt regarding Xens handling of 
localtime and rtc_timeoffset. My current understanding (Xen-3.4.3 and 
Xen-4.1.2 under Linux) of Xend (the depcrecated Python one still used by 
libvirt) is as this:
- for HV domains, the RTC gets setup to either UTC or localtime depending 
on "/domain/image/hvm/localtime" ± "/domain/image/hvm/rtc_offset".
- if the OS of a domU changes its RTC, the rtc_offset gets adjusted and is 
saved in XenStore as "/vm/$UUID/rtc/timeoffset".
- if the dom0 accesses its RTC, is accesses the real HW-RTC.
- the Xen-Hypervisor initially read the HW-RTC to setup its Wallclock once, 
which is than used to simulate the domU RTCs. (The HW-RTC is otherwise only 
accessed on (ACPI-)Suspend and Resume, and with NTP-drift-correction from 
dom0).
- on shut-down the rtc_offset is stored by Xend in 
the "/var/lib/xend/domains/$uuid/config.sxp" file 
in "/domain/image/hvm/rtc_timeoffset", from where it is loaded again on next 
start.
- since PV domains don't have a RTC, they somehow(?) get either initialized to 
the localtime or UTC time depending on "/domain/image/linux/localtime".

@xen:
Did I figure out that correct?

@xen:
Is there some documentation on the Xen-sxp domain configuration? For the 
Python based xen-xm format, I found (and updated) 
<http://wiki.xen.org/wiki/XenConfigurationFileOptions>, but for Xen-sxp I so 
far found no documentation, especially on what changed between xen-1, xen-2, 
xen-3.x, xen-4.x.

@libvirt:
Comparing Xend handling to <http://libvirt.org/formatdomain.html#elementsTime> 
the current translation done by libvirt looks wrong; I think is mandates back 
to the time when Xen supported only PV-domUs:
libvirt translates the Xen configuration to "localtime" and "utc" ignoring 
the "rtc_offset", which exists for HV domains. For localtime=0 this 
translates to libvirts offset="variable"-case, but for localtime=1 there is 
no matching mapping in libvirt.
Since for PV domains no rtc_timeoffset is tracked, there the mapping to "utc" 
and "localtime" looks right.


For libvirt there was a patch 
<http://www.redhat.com/archives/libvir-list/2009-January/msg00757.html> which 
added some special handling for "localtime" to be either placed 
in "/domain/localtime" or "/domain/image/{hvm,linux}/localtime". Xend from 
3.4.3 und 4.1.2 seems to accept either one, but /domain/image/hvm/localtime 
is preferred and overwrites the first one. When reading back the 
configuration the setting is always returned 
as /domain/image/{hvm,linux}/localtime.

@John:
Is there a case, where /domain/localtime is returned or is that key 
always translated to /domain/linux/{hvm,linux}/localtime? As you had a 
sun.com email address, was this some special case when using Xen with 
Solaris?

@libvirt:
The attached patch (for 0.8.7) would change the implementation to match the 
following:
1. For Xen-PV-domUs, use clock/@offset='utc' and clock/@offset='localtime'.
2. For Xen-HV-domUs, use clock/@offset='variable'.
3. For backward compatibility with old libvirt-XML-files convert 
clock/@offset='utc' → (localtime 0)(rtc_timeoffset 0) and 
clock/@offset='localtime' → (localtime 1)(rtc_timeosset 0). On readback that 
will be returned as clock/@offset='variable'!
4. For Xen-HV-domUs with (localtime=1)(rtc_timeoffset≠0) print a warning that 
there is no mapping to libvirts XML.
5. Always put the (localtime)(rtc_offset)-SEXPRs in "(image ({linux,hvm})", 
since this is where Xend-3.4 and Xend-4.1 return them.
I also checked Xen-3.2, where this is okay, but the I don't have any older 
versions of Xen available (and running), the I can't verify that it still 
works there.

Which leads me to a another question: Which versions of Xen are still 
supported by libvirt (and must be checked for regressions)? I don't want so 
actively remove the code for old Xen versions, but it gets harder and harder 
to maintain all those versions. So a statement like "Xen-3.x and Xen-4.y are 
actively supported by libvirt-0.a.b; older versions might still work (by 
accident ;-)"

Before I forward-port that change to 0.9.10 I'd like to get some comments. 
Thanks in advance.

Sincerely
Philipp Hahn
-- 
Philipp Hahn           Open Source Software Engineer      hahn univention de
Univention GmbH        Linux for Your Business        fon: +49 421 22 232- 0
Mary-Somerville-Str.1  D-28359 Bremen                 fax: +49 421 22 232-99
                                                   http://www.univention.de/
diff --git a/src/xen/xend_internal.c b/src/xen/xend_internal.c
index 835c230..e112aa3 100644
--- a/src/xen/xend_internal.c
+++ b/src/xen/xend_internal.c
@@ -903,7 +903,6 @@ xenDaemonListDomainsOld(virConnectPtr xend)
  *
  * Returns 0 for success, -1 (with errno) on error
  */
-
 int
 xenDaemonDomainCreateXML(virConnectPtr xend, const char *sexpr)
 {
@@ -2148,7 +2147,6 @@ xenDaemonParseSxpr(virConnectPtr conn,
     } 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);
@@ -2157,15 +2155,18 @@ xenDaemonParseSxpr(virConnectPtr conn,
         if (sexpr_int(root, "domain/image/hvm/pae"))
             def->features |= (1 << VIR_DOMAIN_FEATURE_PAE);
 
-        /* Old XenD only allows localtime here for HVM */
-        if (sexpr_int(root, "domain/image/hvm/localtime"))
+        def->clock.offset = VIR_DOMAIN_CLOCK_OFFSET_VARIABLE;
+        def->clock.data.adjustment = sexpr_int(root, "domain/image/hvm/rtc_timeoffset");
+        if (def->clock.data.adjustment && sexpr_int(root, "domain/image/hvm/localtime")) {
+            virXendError(VIR_ERR_INTERNAL_ERROR, _("Ignoring localtime=1 while rtc_timeoffset!=0"));
+        }
+    } else { /* !hvm */
+        if (sexpr_int(root, "domain/image/linux/localtime"))
             def->clock.offset = VIR_DOMAIN_CLOCK_OFFSET_LOCALTIME;
+        else
+            def->clock.offset = VIR_DOMAIN_CLOCK_OFFSET_UTC;
     }
 
-    /* Current XenD allows localtime here, for PV and HVM */
-    if (sexpr_int(root, "domain/localtime"))
-        def->clock.offset = VIR_DOMAIN_CLOCK_OFFSET_LOCALTIME;
-
     if (sexpr_node_copy(root, hvm ?
                         "domain/image/hvm/device_model" :
                         "domain/image/linux/device_model",
@@ -5759,31 +5760,15 @@ xenDaemonFormatSxpr(virConnectPtr conn,
     }
     virBufferVSprintf(&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) {
-            virXendError(VIR_ERR_CONFIG_UNSUPPORTED,
-                         "%s", _("configurable timezones are not supported"));
-            goto error;
-        }
+    if (STREQ(def->os.type, "hvm"))
+        hvm = 1;
 
-        virBufferAddLit(&buf, "(localtime 1)");
-    } else if (def->clock.offset != VIR_DOMAIN_CLOCK_OFFSET_UTC) {
-        virXendError(VIR_ERR_CONFIG_UNSUPPORTED,
-                     _("unsupported clock offset '%s'"),
-                     virDomainClockOffsetTypeToString(def->clock.offset));
-        goto error;
-    }
+    if (hvm)
+        virBufferAddLit(&buf, "(image (hvm ");
+    else
+        virBufferAddLit(&buf, "(image (linux ");
 
     if (!def->os.bootloader) {
-        if (STREQ(def->os.type, "hvm"))
-            hvm = 1;
-
-        if (hvm)
-            virBufferAddLit(&buf, "(image (hvm ");
-        else
-            virBufferAddLit(&buf, "(image (linux ");
-
         if (hvm &&
             def->os.loader == NULL) {
             virXendError(VIR_ERR_INTERNAL_ERROR,
@@ -5893,17 +5878,13 @@ xenDaemonFormatSxpr(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 (xenDaemonFormatSxprSound(def, &buf) < 0)
                     goto error;
                 virBufferAddLit(&buf, "')");
             }
-        }
+        } /* hvm */
 
         /* get the device emulation model */
         if (def->emulator && (hvm || xendConfigVersion >= 3))
@@ -5918,10 +5899,37 @@ xenDaemonFormatSxpr(virConnectPtr conn,
                                                &buf, xendConfigVersion) < 0)
                 goto error;
         }
+    } /* os.bootloader */
+
+    if (hvm) {
+        /* Set localtime here to keep old XenD happy for HVM */
+        if (def->clock.offset == VIR_DOMAIN_CLOCK_OFFSET_VARIABLE) {
+            virBufferVSprintf(&buf, "(localtime 0)(rtc_timeoffset %d)", def->clock.data.adjustment);
+        } else if (def->clock.offset == VIR_DOMAIN_CLOCK_OFFSET_LOCALTIME) {
+            if (def->clock.data.timezone) {
+                virXendError(VIR_ERR_CONFIG_UNSUPPORTED,
+                             "%s", _("configurable timezones are not supported"));
+                goto error;
+            }
 
-        virBufferAddLit(&buf, "))");
+            virBufferAddLit(&buf, "(localtime 1)(rtc_timeoffset 0)");
+        } else if (def->clock.offset == VIR_DOMAIN_CLOCK_OFFSET_UTC) {
+            virBufferAddLit(&buf, "(localtime 0)(rtc_timeoffset 0)");
+        } else {
+            virXendError(VIR_ERR_CONFIG_UNSUPPORTED,
+                         _("unsupported clock offset '%s'"),
+                         virDomainClockOffsetTypeToString(def->clock.offset));
+            goto error;
+        }
+    } else { /* !hvm */
+        if (def->clock.offset == VIR_DOMAIN_CLOCK_OFFSET_LOCALTIME)
+            virBufferAddLit(&buf, "(localtime 1)");
+        else if (def->clock.offset == VIR_DOMAIN_CLOCK_OFFSET_UTC)
+            virBufferAddLit(&buf, "(localtime 0)");
     }
 
+    virBufferAddLit(&buf, "))"); /* image/{kvm,linux}/ */
+
     for (i = 0 ; i < def->ndisks ; i++)
         if (xenDaemonFormatSxprDisk(conn, def->disks[i],
                                     &buf, hvm, xendConfigVersion, 0) < 0)

Attachment: signature.asc
Description: This is a digitally signed message part.


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