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

Re: [libvirt] [PATCH v2] qemu: Report the offset from host UTC for RTC_CHANGE event



On 06/06/13 05:28, Eric Blake wrote:
On 06/05/2013 04:32 AM, Osier Yang wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=964177

Though both libvirt and QEMU's document say RTC_CHANGE returns
the offset from the host UTC, qemu actually returns the offset
from the specified date instead when specific date is provided
(-rtc base=$date).

It's not safe for qemu to fix it in code, it worked like that
for 3 years, changing it now may break other QEMU use cases.
What qemu tries to do is to fix the document:

http://lists.gnu.org/archive/html/qemu-devel/2013-05/msg04782.html

And in libvirt side, instead of reply on the qemu, this convert
Unclear grammar.  Not sure if you meant:

instead of replaying the value from qemu

or:

instead of relying on the value from qemu

Will use this.


but either alternative makes more sense.

s/convert/converts/

the offset returned from qemu to the offset from host UTC, by:

   /*
    * a: the offset from qemu RTC_CHANGE event
    * b: The specified date (-rtc base=$date)
    * c: the host date when libvirt gets the RTC_CHANGE event
    * offset: What libvirt will report
    */

   offset = a + (b - c);

The specified date (-rtc base=$date) is recorded in clock's def as
an internal only member (may be useful to exposed outside?).

Internal only XML tag "starttime" is introduced to not lose the
domain process's starttime after libvirt restarting/reloading:

<clock offset='variable' adjustment='304' basis='utc' starttime='1370423588'/>
---
  src/conf/domain_conf.c  | 27 +++++++++++++++++++++++----
  src/conf/domain_conf.h  |  3 +++
  src/qemu/qemu_command.c |  3 +++
  src/qemu/qemu_process.c | 13 +++++++++++++
  4 files changed, 42 insertions(+), 4 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index a16ebd1..7773abf 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -96,6 +96,7 @@ typedef enum {
     VIR_DOMAIN_XML_INTERNAL_PCI_ORIG_STATES = (1<<18),
     VIR_DOMAIN_XML_INTERNAL_ALLOW_ROM = (1<<19),
     VIR_DOMAIN_XML_INTERNAL_ALLOW_BOOT = (1<<20),
+   VIR_DOMAIN_XML_INTERNAL_STARTTIME = (1 << 21)
Please add the trailing comma, so that the next edit to this code can
also be a one-liner.

+++ b/src/conf/domain_conf.h
@@ -1767,6 +1767,9 @@ struct _virDomainClockDef {
          struct {
              long long adjustment;
              int basis;
+
+            /* Store the start time of guest process, internal only */
+            unsigned long long starttime;
Might be worth mentioning how to interpret the value - is it seconds
since Epoch?

Hm, just realized that "starttime" may be confused with the start time of
guest process, what we want actually is the date of "-rtc base=$date". And
only valid for clock.offset == VARIABLE.

So I change it to "basedate", with the nits fixed:
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 7773abf..65edf71 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -96,7 +96,7 @@ typedef enum {
    VIR_DOMAIN_XML_INTERNAL_PCI_ORIG_STATES = (1<<18),
    VIR_DOMAIN_XML_INTERNAL_ALLOW_ROM = (1<<19),
    VIR_DOMAIN_XML_INTERNAL_ALLOW_BOOT = (1<<20),
-   VIR_DOMAIN_XML_INTERNAL_STARTTIME = (1 << 21)
+   VIR_DOMAIN_XML_INTERNAL_BASEDATE = (1 << 21),
 } virDomainXMLInternalFlags;
 
 VIR_ENUM_IMPL(virDomainTaint, VIR_DOMAIN_TAINT_LAST,
@@ -11195,11 +11195,11 @@ virDomainDefParseXML(xmlDocPtr xml,
     }
 
     if (def->clock.offset == VIR_DOMAIN_CLOCK_OFFSET_VARIABLE &&
-        flags & VIR_DOMAIN_XML_INTERNAL_STARTTIME) {
-        if (virXPathULongLong("number(./clock/@starttime)", ctxt,
-                              &def->clock.data.variable.starttime) < 0) {
+        flags & VIR_DOMAIN_XML_INTERNAL_BASEDATE) {
+        if (virXPathULongLong("number(./clock/@basedate)", ctxt,
+                              &def->clock.data.variable.basedate) < 0) {
             virReportError(VIR_ERR_XML_ERROR, "%s",
-                           _("invalid starttime"));
+                           _("invalid basedate"));
             goto error;
         }
     }
@@ -15800,7 +15800,7 @@ virDomainResourceDefFormat(virBufferPtr buf,
 verify(((VIR_DOMAIN_XML_INTERNAL_STATUS |
          VIR_DOMAIN_XML_INTERNAL_ACTUAL_NET |
          VIR_DOMAIN_XML_INTERNAL_PCI_ORIG_STATES |
-         VIR_DOMAIN_XML_INTERNAL_STARTTIME)
+         VIR_DOMAIN_XML_INTERNAL_BASEDATE)
         & DUMPXML_FLAGS) == 0);
 
 /* This internal version can accept VIR_DOMAIN_XML_INTERNAL_*,
@@ -15823,7 +15823,7 @@ virDomainDefFormatInternal(virDomainDefPtr def,
                   VIR_DOMAIN_XML_INTERNAL_STATUS |
                   VIR_DOMAIN_XML_INTERNAL_ACTUAL_NET |
                   VIR_DOMAIN_XML_INTERNAL_PCI_ORIG_STATES |
-                  VIR_DOMAIN_XML_INTERNAL_STARTTIME,
+                  VIR_DOMAIN_XML_INTERNAL_BASEDATE,
                   -1);
 
     if (!(type = virDomainVirtTypeToString(def->virtType))) {
@@ -16222,9 +16222,9 @@ virDomainDefFormatInternal(virDomainDefPtr def,
                           def->clock.data.variable.adjustment,
                           virDomainClockBasisTypeToString(def->clock.data.variable.basis));
 
-        if (flags & VIR_DOMAIN_XML_INTERNAL_STARTTIME)
-            virBufferAsprintf(buf, " starttime='%llu'",
-                              def->clock.data.variable.starttime);
+        if (flags & VIR_DOMAIN_XML_INTERNAL_BASEDATE)
+            virBufferAsprintf(buf, " basedate='%llu'",
+                              def->clock.data.variable.basedate);
         break;
     case VIR_DOMAIN_CLOCK_OFFSET_TIMEZONE:
         virBufferEscapeString(buf, " timezone='%s'", def->clock.data.timezone);
@@ -16604,7 +16604,7 @@ virDomainSaveStatus(virDomainXMLOptionPtr xmlopt,
                           VIR_DOMAIN_XML_INTERNAL_STATUS |
                           VIR_DOMAIN_XML_INTERNAL_ACTUAL_NET |
                           VIR_DOMAIN_XML_INTERNAL_PCI_ORIG_STATES |
-                          VIR_DOMAIN_XML_INTERNAL_STARTTIME);
+                          VIR_DOMAIN_XML_INTERNAL_BASEDATE);
 
     int ret = -1;
     char *xml;
@@ -16693,7 +16693,7 @@ virDomainObjListLoadStatus(virDomainObjListPtr doms,
                                       VIR_DOMAIN_XML_INTERNAL_STATUS |
                                       VIR_DOMAIN_XML_INTERNAL_ACTUAL_NET |
                                       VIR_DOMAIN_XML_INTERNAL_PCI_ORIG_STATES |
-                                      VIR_DOMAIN_XML_INTERNAL_STARTTIME)))
+                                      VIR_DOMAIN_XML_INTERNAL_BASEDATE)))
         goto error;
 
     virUUIDFormat(obj->def->uuid, uuidstr);
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index cca92b4..b8edb1e 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -1768,8 +1768,10 @@ struct _virDomainClockDef {
             long long adjustment;
             int basis;
 
-            /* Store the start time of guest process, internal only */
-            unsigned long long starttime;
+            /* Store the base date (-rtc base=$date, in seconds
+             * since the Epoch) of guest process, internal only
+             */
+            unsigned long long basedate;
         } variable;
 
         /* Timezone name, when
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 9254525..7a445cf 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -5519,7 +5519,7 @@ qemuBuildClockArgStr(virDomainClockDefPtr def)
         gmtime_r(&now, &nowbits);
 
         /* Store the starttime of qemu process */
-        def->data.variable.starttime = now;
+        def->data.variable.basedate = now;
 
         virBufferAsprintf(&buf, "base=%d-%02d-%02dT%02d:%02d:%02d",
                           nowbits.tm_year + 1900,
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 39c62b3..3d6c06d 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -805,7 +805,7 @@ qemuProcessHandleRTCChange(qemuMonitorPtr mon ATTRIBUTE_UNUSED,
     if (vm->def->clock.offset == VIR_DOMAIN_CLOCK_OFFSET_VARIABLE) {
         time_t now = time(NULL);
 
-        offset += vm->def->clock.data.variable.starttime -
+        offset += vm->def->clock.data.variable.basedate -
                   (unsigned long long)now;
     }
 

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