[libvirt] [PATCH v4 4/5] blockcommit: track job type in xml

Eric Blake eblake at redhat.com
Mon Jun 23 23:30: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).

* 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                          | 20 ++++++------
 docs/schemas/domaincommon.rng                      | 13 ++++++++
 src/conf/domain_conf.c                             | 33 ++++++++++++++++++-
 src/conf/domain_conf.h                             |  1 +
 src/qemu/qemu_driver.c                             |  3 ++
 src/qemu/qemu_process.c                            | 18 +++++++----
 .../qemuxml2argv-disk-active-commit.xml            | 37 ++++++++++++++++++++++
 .../qemuxml2argvdata/qemuxml2argv-disk-mirror.xml  |  4 +--
 .../qemuxml2xmlout-disk-mirror-old.xml             |  4 +--
 tests/qemuxml2xmltest.c                            |  1 +
 10 files changed, 113 insertions(+), 21 deletions(-)
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-active-commit.xml

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 3075e16..16061b8 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -1878,16 +1878,18 @@
       </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. If
+        format of the source).  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).  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. If
         attribute <code>ready</code> is present, then it is known the
         disk is ready to pivot; otherwise, the disk is probably still
         copying.  For now, this element only valid in output; it is
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 33d0308..fe943f9 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -4186,6 +4186,13 @@
             </attribute>
           </optional>
           <optional>
+            <attribute name='job'>
+              <choice>
+                <value>copy</value>
+              </choice>
+            </attribute>
+          </optional>
+          <optional>
             <interleave>
               <ref name='diskSourceFile'/>
               <optional>
@@ -4195,6 +4202,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 02c394f..65a41c9 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -760,6 +760,12 @@ VIR_ENUM_IMPL(virDomainDiskDiscard, VIR_DOMAIN_DISK_DISCARD_LAST,
               "unmap",
               "ignore")

+/* 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

@@ -5377,10 +5383,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);
@@ -5403,6 +5425,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) {
@@ -15198,10 +15226,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->mirroring)
             virBufferAddLit(buf, " ready='yes'");
         virBufferAddLit(buf, ">\n");
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 6779a41..c03fe64 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -608,6 +608,7 @@ struct _virDomainDiskDef {

     virStorageSourcePtr mirror;
     bool mirroring;
+    int mirrorJob; /* virDomainBlockJobType */

     struct {
         unsigned int cylinders;
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index b57f77b..79554e9 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -14993,6 +14993,7 @@ qemuDomainBlockPivot(virConnectPtr conn,
     virStorageSourceFree(disk->mirror);
     disk->mirror = NULL;
     disk->mirroring = false;
+    disk->mirrorJob = VIR_DOMAIN_BLOCK_JOB_TYPE_UNKNOWN;

  cleanup:
     if (resume && virDomainObjIsActive(vm) &&
@@ -15120,6 +15121,7 @@ qemuDomainBlockJobImpl(virDomainObjPtr vm,
         virStorageSourceFree(disk->mirror);
         disk->mirror = NULL;
         disk->mirroring = false;
+        disk->mirrorJob = VIR_DOMAIN_BLOCK_JOB_TYPE_UNKNOWN;
     }

     /* With synchronous block cancel, we must synthesize an event, and
@@ -15397,6 +15399,7 @@ qemuDomainBlockCopy(virDomainObjPtr vm,
     need_unlink = false;
     disk->mirror = mirror;
     mirror = NULL;
+    disk->mirrorJob = VIR_DOMAIN_BLOCK_JOB_TYPE_COPY;

  endjob:
     if (need_unlink && unlink(dest))
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 0b8155b..0c5a3ac 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -1023,28 +1023,32 @@ 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,
                                                    status);
         /* XXX If we completed a block pull or commit, then recompute
          * the cached backing chain to match.  Better would be storing
-         * the chain ourselves rather than reprobing, but this
-         * requires modifying domain_conf and our XML to fully track
-         * the chain across libvirtd restarts.  For that matter, if
-         * qemu gains support for committing the active layer, we have
-         * to update disk->src.  */
+         * the chain ourselves rather than reprobing, but we haven't
+         * quite completed that conversion to use our XML tracking. */
         if ((type == VIR_DOMAIN_BLOCK_JOB_TYPE_PULL ||
-             type == VIR_DOMAIN_BLOCK_JOB_TYPE_COMMIT) &&
+             type == VIR_DOMAIN_BLOCK_JOB_TYPE_COMMIT ||
+             type == VIR_DOMAIN_BLOCK_JOB_TYPE_ACTIVE_COMMIT) &&
             status == VIR_DOMAIN_BLOCK_JOB_COMPLETED)
             qemuDomainDetermineDiskChain(driver, vm, disk, true);
-        if (disk->mirror && type == VIR_DOMAIN_BLOCK_JOB_TYPE_COPY) {
+        if (disk->mirror &&
+            (type == VIR_DOMAIN_BLOCK_JOB_TYPE_COPY ||
+             type == VIR_DOMAIN_BLOCK_JOB_TYPE_ACTIVE_COMMIT)) {
             if (status == VIR_DOMAIN_BLOCK_JOB_READY) {
                 disk->mirroring = true;
             } else if (status == VIR_DOMAIN_BLOCK_JOB_FAILED) {
                 virStorageSourceFree(disk->mirror);
                 disk->mirror = NULL;
                 disk->mirroring = false;
+                disk->mirrorJob = VIR_DOMAIN_BLOCK_JOB_TYPE_UNKNOWN;
             }
         }
     }
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 72b03c9..abec180 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>
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 200d50f..98b2c15 100644
--- a/tests/qemuxml2xmltest.c
+++ b/tests/qemuxml2xmltest.c
@@ -227,6 +227,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