[libvirt] [PATCH v5 3/4] blockcommit: track job type in xml

Eric Blake eblake at redhat.com
Tue Jul 29 04:20:55 UTC 2014


A future patch is going to wire up qemu active block commit jobs;
but as they have similar events and are canceled/pivoted in the
same way as block copy jobs, it is easiest to track all bookkeeping
for the commit job by reusing the <mirror> element.  This patch
adds domain XML to track which job was responsible for creating a
mirroring situation, and adds a job='copy' attribute to all
existing uses of <mirror>.  Along the way, it also massages the
qemu monitor backend to read the new field in order to generate
the correct type of libvirt job (even though it requires a
future patch to actually cause a qemu event that can be reported
as an active commit).  It also prepares to update persistent XML
to match changes made to live XML when a copy completes.

* docs/schemas/domaincommon.rng: Enhance schema.
* docs/formatdomain.html.in: Document it.
* src/conf/domain_conf.h (_virDomainDiskDef): Add a field.
* src/conf/domain_conf.c (virDomainBlockJobType): String conversion.
(virDomainDiskDefParseXML): Parse job type.
(virDomainDiskDefFormat): Output job type.
* src/qemu/qemu_process.c (qemuProcessHandleBlockJob): Distinguish
active from regular commit.
* src/qemu/qemu_driver.c (qemuDomainBlockCopy): Set job type.
(qemuDomainBlockPivot, qemuDomainBlockJobImpl): Clean up job type
on completion.
* tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-mirror-old.xml:
Update tests.
* tests/qemuxml2argvdata/qemuxml2argv-disk-mirror.xml: Likewise.
* tests/qemuxml2argvdata/qemuxml2argv-disk-active-commit.xml: New
file.
* tests/qemuxml2xmltest.c (mymain): Drive new test.

Signed-off-by: Eric Blake <eblake at redhat.com>
---
 docs/formatdomain.html.in                          | 15 +++++----
 docs/schemas/domaincommon.rng                      | 13 ++++++++
 src/conf/domain_conf.c                             | 33 ++++++++++++++++++-
 src/conf/domain_conf.h                             |  1 +
 src/qemu/qemu_driver.c                             |  2 ++
 src/qemu/qemu_process.c                            | 37 +++++++++++++++++++++-
 .../qemuxml2argv-disk-active-commit.xml            | 37 ++++++++++++++++++++++
 .../qemuxml2argvdata/qemuxml2argv-disk-mirror.xml  |  6 ++--
 .../qemuxml2xmlout-disk-mirror-old.xml             |  4 +--
 tests/qemuxml2xmltest.c                            |  1 +
 10 files changed, 136 insertions(+), 13 deletions(-)
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-active-commit.xml

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 4cb4289..0e7fad2 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -1898,16 +1898,19 @@
       </dd>
       <dt><code>mirror</code></dt>
       <dd>
-        This element is present if the hypervisor has started a block
-        copy operation (via the <code>virDomainBlockRebase</code>
-        API), where the mirror location in the <code>source</code>
-        sub-element will eventually have the same contents as the
-        source, and with the file format in the
+        This element is present if the hypervisor has started a
+        long-running block job operation, where the mirror location in
+        the <code>source</code> sub-element will eventually have the
+        same contents as the source, and with the file format in the
         sub-element <code>format</code> (which might differ from the
         format of the source).  The details of the <code>source</code>
         sub-element are determined by the <code>type</code> attribute
         of the mirror, similar to what is done for the
-        overall <code>disk</code> device element. The
+        overall <code>disk</code> device element. The <code>job</code>
+        attribute mentions which API started the operation ("copy" for
+        the <code>virDomainBlockRebase</code> API, or "active-commit"
+        for the <code>virDomainBlockCommit</code>
+        API), <span class="since">since 1.2.7</span>.  The
         attribute <code>ready</code>, if present, tracks progress of
         the job: <code>yes</code> if the disk is known to be ready to
         pivot, or, <span class="since">since
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 389c8c9..3911a9f 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -4265,6 +4265,13 @@
             </attribute>
           </optional>
           <optional>
+            <attribute name='job'>
+              <choice>
+                <value>copy</value>
+              </choice>
+            </attribute>
+          </optional>
+          <optional>
             <interleave>
               <ref name='diskSourceFile'/>
               <optional>
@@ -4274,6 +4281,12 @@
           </optional>
         </group>
         <group> <!-- preferred format -->
+          <attribute name='job'>
+            <choice>
+              <value>copy</value>
+              <value>active-commit</value>
+            </choice>
+          </attribute>
           <interleave>
             <ref name="diskSource"/>
             <optional>
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 1c8f8cf..1a76ce7 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -753,6 +753,12 @@ VIR_ENUM_IMPL(virDomainDiskMirror, VIR_DOMAIN_DISK_MIRROR_LAST,
               "abort",
               "pivot")

+/* Internal mapping: subset of block job types that can be present in
+ * <mirror> XML (remaining types are not two-phase). */
+VIR_ENUM_DECL(virDomainBlockJob)
+VIR_ENUM_IMPL(virDomainBlockJob, VIR_DOMAIN_BLOCK_JOB_TYPE_LAST,
+              "", "", "copy", "", "active-commit")
+
 #define VIR_DOMAIN_XML_WRITE_FLAGS  VIR_DOMAIN_XML_SECURE
 #define VIR_DOMAIN_XML_READ_FLAGS   VIR_DOMAIN_XML_INACTIVE

@@ -5437,10 +5443,26 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt,
                        xmlStrEqual(cur->name, BAD_CAST "mirror") &&
                        !(flags & VIR_DOMAIN_XML_INACTIVE)) {
                 char *ready;
+                char *blockJob;

                 if (VIR_ALLOC(def->mirror) < 0)
                     goto error;

+                blockJob = virXMLPropString(cur, "job");
+                if (blockJob) {
+                    def->mirrorJob = virDomainBlockJobTypeFromString(blockJob);
+                    if (def->mirrorJob <= 0) {
+                        virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+                                       _("unknown mirror job type '%s'"),
+                                       blockJob);
+                        VIR_FREE(blockJob);
+                        goto error;
+                    }
+                    VIR_FREE(blockJob);
+                } else {
+                    def->mirrorJob = VIR_DOMAIN_BLOCK_JOB_TYPE_COPY;
+                }
+
                 mirrorType = virXMLPropString(cur, "type");
                 if (mirrorType) {
                     def->mirror->type = virStorageTypeFromString(mirrorType);
@@ -5463,6 +5485,12 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt,
                                        _("mirror requires file name"));
                         goto error;
                     }
+                    if (def->mirrorJob != VIR_DOMAIN_BLOCK_JOB_TYPE_COPY) {
+                        virReportError(VIR_ERR_XML_ERROR, "%s",
+                                       _("mirror without type only supported "
+                                         "by copy job"));
+                        goto error;
+                    }
                     mirrorFormat = virXMLPropString(cur, "format");
                 }
                 if (mirrorFormat) {
@@ -15282,10 +15310,13 @@ virDomainDiskDefFormat(virBufferPtr buf,
             formatStr = virStorageFileFormatTypeToString(def->mirror->format);
         virBufferAsprintf(buf, "<mirror type='%s'",
                           virStorageTypeToString(def->mirror->type));
-        if (def->mirror->type == VIR_STORAGE_TYPE_FILE) {
+        if (def->mirror->type == VIR_STORAGE_TYPE_FILE &&
+            def->mirrorJob == VIR_DOMAIN_BLOCK_JOB_TYPE_COPY) {
             virBufferEscapeString(buf, " file='%s'", def->mirror->path);
             virBufferEscapeString(buf, " format='%s'", formatStr);
         }
+        virBufferEscapeString(buf, " job='%s'",
+                              virDomainBlockJobTypeToString(def->mirrorJob));
         if (def->mirrorState) {
             const char *mirror;

diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index bc8966d..00ad137 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -632,6 +632,7 @@ struct _virDomainDiskDef {

     virStorageSourcePtr mirror;
     int mirrorState; /* enum virDomainDiskMirror */
+    int mirrorJob; /* virDomainBlockJobType */

     struct {
         unsigned int cylinders;
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 180333c..683eaf3 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -14943,6 +14943,7 @@ qemuDomainBlockPivot(virConnectPtr conn,
         virStorageSourceFree(disk->mirror);
         disk->mirror = NULL;
         disk->mirrorState = VIR_DOMAIN_DISK_MIRROR_DEFAULT;
+        disk->mirrorJob = VIR_DOMAIN_BLOCK_JOB_TYPE_UNKNOWN;
     }
     if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm) < 0)
         ret = -1;
@@ -15418,6 +15419,7 @@ qemuDomainBlockCopy(virDomainObjPtr vm,
     need_unlink = false;
     disk->mirror = mirror;
     mirror = NULL;
+    disk->mirrorJob = VIR_DOMAIN_BLOCK_JOB_TYPE_COPY;

     if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm) < 0)
         VIR_WARN("Unable to save status on vm %s after state change",
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index e770a77..4ea5493 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -1017,6 +1017,7 @@ qemuProcessHandleBlockJob(qemuMonitorPtr mon ATTRIBUTE_UNUSED,
     const char *path;
     virDomainDiskDefPtr disk;
     virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
+    virDomainDiskDefPtr persistDisk = NULL;
     bool save = false;

     virObjectLock(vm);
@@ -1025,6 +1026,9 @@ qemuProcessHandleBlockJob(qemuMonitorPtr mon ATTRIBUTE_UNUSED,
     if (disk) {
         /* Have to generate two variants of the event for old vs. new
          * client callbacks */
+        if (type == VIR_DOMAIN_BLOCK_JOB_TYPE_COMMIT &&
+            disk->mirrorJob == VIR_DOMAIN_BLOCK_JOB_TYPE_ACTIVE_COMMIT)
+            type = disk->mirrorJob;
         path = virDomainDiskGetSource(disk);
         event = virDomainEventBlockJobNewFromObj(vm, path, type, status);
         event2 = virDomainEventBlockJob2NewFromObj(vm, disk->dst, type,
@@ -1036,6 +1040,31 @@ qemuProcessHandleBlockJob(qemuMonitorPtr mon ATTRIBUTE_UNUSED,
             bool has_mirror = !!disk->mirror;

             if (disk->mirrorState == VIR_DOMAIN_DISK_MIRROR_PIVOT) {
+                if (vm->newDef) {
+                    int indx = virDomainDiskIndexByName(vm->newDef, disk->dst,
+                                                        false);
+                    virStorageSourcePtr copy = NULL;
+
+                    if (indx >= 0) {
+                        persistDisk = vm->newDef->disks[indx];
+                        copy = virStorageSourceCopy(disk->mirror, false);
+                        if (virStorageSourceInitChainElement(copy,
+                                                             persistDisk->src,
+                                                             false) < 0) {
+                            VIR_WARN("Unable to update persistent definition "
+                                     "on vm %s after block job",
+                                     vm->def->name);
+                            virStorageSourceFree(copy);
+                            copy = NULL;
+                            persistDisk = NULL;
+                        }
+                    }
+                    if (copy) {
+                        virStorageSourceFree(persistDisk->src);
+                        persistDisk->src = copy;
+                    }
+                }
+
                 /* XXX We want to revoke security labels and disk
                  * lease, as well as audit that revocation, before
                  * dropping the original source.  But it gets tricky
@@ -1061,7 +1090,8 @@ qemuProcessHandleBlockJob(qemuMonitorPtr mon ATTRIBUTE_UNUSED,
         }

         if (disk->mirror &&
-            type == VIR_DOMAIN_BLOCK_JOB_TYPE_COPY) {
+            (type == VIR_DOMAIN_BLOCK_JOB_TYPE_COPY ||
+             type == VIR_DOMAIN_BLOCK_JOB_TYPE_ACTIVE_COMMIT)) {
             if (status == VIR_DOMAIN_BLOCK_JOB_READY) {
                 disk->mirrorState = VIR_DOMAIN_DISK_MIRROR_READY;
                 save = true;
@@ -1069,6 +1099,7 @@ qemuProcessHandleBlockJob(qemuMonitorPtr mon ATTRIBUTE_UNUSED,
                 virStorageSourceFree(disk->mirror);
                 disk->mirror = NULL;
                 disk->mirrorState = VIR_DOMAIN_DISK_MIRROR_DEFAULT;
+                disk->mirrorJob = VIR_DOMAIN_BLOCK_JOB_TYPE_UNKNOWN;
                 save = true;
             }
         }
@@ -1078,6 +1109,10 @@ qemuProcessHandleBlockJob(qemuMonitorPtr mon ATTRIBUTE_UNUSED,
         if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm) < 0)
             VIR_WARN("Unable to save status on vm %s after block job",
                      vm->def->name);
+        if (persistDisk && virDomainSaveConfig(cfg->configDir,
+                                               vm->newDef) < 0)
+            VIR_WARN("Unable to update persistent definition on vm %s "
+                     "after block job", vm->def->name);
     }
     virObjectUnlock(vm);
     virObjectUnref(cfg);
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-active-commit.xml b/tests/qemuxml2argvdata/qemuxml2argv-disk-active-commit.xml
new file mode 100644
index 0000000..6ba5724
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-active-commit.xml
@@ -0,0 +1,37 @@
+<domain type='qemu' id='1'>
+  <name>QEMUGuest1</name>
+  <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
+  <memory unit='KiB'>219136</memory>
+  <currentMemory unit='KiB'>219136</currentMemory>
+  <vcpu placement='static'>1</vcpu>
+  <os>
+    <type arch='i686' machine='pc'>hvm</type>
+    <boot dev='hd'/>
+  </os>
+  <clock offset='utc'/>
+  <on_poweroff>destroy</on_poweroff>
+  <on_reboot>restart</on_reboot>
+  <on_crash>destroy</on_crash>
+  <devices>
+    <emulator>/usr/bin/qemu</emulator>
+    <disk type='file' device='disk'>
+      <driver name='qemu' type='qcow2'/>
+      <source file='/tmp/HostVG/QEMUGuest1-snap'/>
+      <backingStore type='block' index='1'>
+        <format type='raw'/>
+        <source dev='/dev/HostVG/QEMUGuest1'/>
+        <backingStore/>
+      </backingStore>
+      <mirror type='block' job='active-commit'>
+        <format type='raw'/>
+        <source dev='/dev/HostVG/QEMUGuest1'/>
+      </mirror>
+      <target dev='hda' bus='ide'/>
+      <address type='drive' controller='0' bus='0' target='0' unit='0'/>
+    </disk>
+    <controller type='usb' index='0'/>
+    <controller type='ide' index='0'/>
+    <controller type='pci' index='0' model='pci-root'/>
+    <memballoon model='virtio'/>
+  </devices>
+</domain>
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-mirror.xml b/tests/qemuxml2argvdata/qemuxml2argv-disk-mirror.xml
index fa7af31..46f2a3e 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-disk-mirror.xml
+++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-mirror.xml
@@ -17,7 +17,7 @@
     <disk type='block' device='disk'>
       <source dev='/dev/HostVG/QEMUGuest1'/>
       <backingStore/>
-      <mirror type='file' file='/dev/HostVG/QEMUGuest1Copy' ready='yes'>
+      <mirror type='file' file='/dev/HostVG/QEMUGuest1Copy' job='copy' ready='yes'>
         <source file='/dev/HostVG/QEMUGuest1Copy'/>
       </mirror>
       <target dev='hda' bus='ide'/>
@@ -33,7 +33,7 @@
     <disk type='file' device='disk'>
       <source file='/tmp/data.img'/>
       <backingStore/>
-      <mirror type='file' file='/tmp/copy.img' format='qcow2'>
+      <mirror type='file' file='/tmp/copy.img' format='qcow2' job='copy'>
         <format type='qcow2'/>
         <source file='/tmp/copy.img'/>
       </mirror>
@@ -42,7 +42,7 @@
     <disk type='file' device='disk'>
       <source file='/tmp/logs.img'/>
       <backingStore/>
-      <mirror type='file' file='/tmp/logcopy.img' format='qcow2' ready='abort'>
+      <mirror type='file' file='/tmp/logcopy.img' format='qcow2' job='copy' ready='abort'>
         <format type='qcow2'/>
         <source file='/tmp/logcopy.img'/>
       </mirror>
diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-mirror-old.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-mirror-old.xml
index 72b03c9..abec180 100644
--- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-mirror-old.xml
+++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-mirror-old.xml
@@ -17,7 +17,7 @@
     <disk type='block' device='disk'>
       <source dev='/dev/HostVG/QEMUGuest1'/>
       <backingStore/>
-      <mirror type='file' file='/dev/HostVG/QEMUGuest1Copy' ready='yes'>
+      <mirror type='file' file='/dev/HostVG/QEMUGuest1Copy' job='copy' ready='yes'>
         <source file='/dev/HostVG/QEMUGuest1Copy'/>
       </mirror>
       <target dev='hda' bus='ide'/>
@@ -33,7 +33,7 @@
     <disk type='file' device='disk'>
       <source file='/tmp/data.img'/>
       <backingStore/>
-      <mirror type='file' file='/tmp/copy.img' format='qcow2'>
+      <mirror type='file' file='/tmp/copy.img' format='qcow2' job='copy'>
         <format type='qcow2'/>
         <source file='/tmp/copy.img'/>
       </mirror>
diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
index cefe05b..8966b9a 100644
--- a/tests/qemuxml2xmltest.c
+++ b/tests/qemuxml2xmltest.c
@@ -229,6 +229,7 @@ mymain(void)
     DO_TEST_DIFFERENT("disk-mirror-old");
     DO_TEST_FULL("disk-mirror", false, WHEN_ACTIVE);
     DO_TEST_FULL("disk-mirror", true, WHEN_INACTIVE);
+    DO_TEST_FULL("disk-active-commit", false, WHEN_ACTIVE);
     DO_TEST("graphics-listen-network");
     DO_TEST("graphics-vnc");
     DO_TEST("graphics-vnc-websocket");
-- 
1.9.3




More information about the libvir-list mailing list