From 232a31bea3bfde7118bbb0c6594949b62b9bd2a0 Mon Sep 17 00:00:00 2001
From: Eric Blake <eblake@redhat.com>
Date: Mon, 28 Jul 2014 21:46:44 -0600
Subject: [PATCH] blockcommit: track job type in xml

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@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                       | 39 ++++++++++++++++++-
 .../qemuxml2argv-disk-active-commit.xml       | 37 ++++++++++++++++++
 .../qemuxml2argv-disk-mirror.xml              |  6 +--
 .../qemuxml2xmlout-disk-mirror-old.xml        |  4 +-
 tests/qemuxml2xmltest.c                       |  1 +
 10 files changed, 138 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 df3fc4af84..e5b1adbb59 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -1913,16 +1913,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 7cb5a9eb11..11f0fd02d7 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -4289,6 +4289,13 @@
               <ref name='storageFormat'/>
             </attribute>
           </optional>
+          <optional>
+            <attribute name='job'>
+              <choice>
+                <value>copy</value>
+              </choice>
+            </attribute>
+          </optional>
           <optional>
             <interleave>
               <ref name='diskSourceFile'/>
@@ -4299,6 +4306,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 421a44af6c..c25c74b627 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -753,6 +753,12 @@ VIR_ENUM_IMPL(virDomainDiskMirrorState, VIR_DOMAIN_DISK_MIRROR_STATE_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) {
@@ -15401,10 +15429,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 c77c85c470..bffc0a58bd 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -632,6 +632,7 @@ struct _virDomainDiskDef {
 
     virStorageSourcePtr mirror;
     int mirrorState; /* enum virDomainDiskMirrorState */
+    int mirrorJob; /* virDomainBlockJobType */
 
     struct {
         unsigned int cylinders;
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 883f10d22d..9ee62ef2e8 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -14938,6 +14938,7 @@ qemuDomainBlockPivot(virConnectPtr conn,
         virStorageSourceFree(disk->mirror);
         disk->mirror = NULL;
         disk->mirrorState = VIR_DOMAIN_DISK_MIRROR_STATE_NONE;
+        disk->mirrorJob = VIR_DOMAIN_BLOCK_JOB_TYPE_UNKNOWN;
     }
     if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm) < 0)
         ret = -1;
@@ -15413,6 +15414,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 d0132e3b66..407da5e1f0 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,
@@ -1034,6 +1038,31 @@ qemuProcessHandleBlockJob(qemuMonitorPtr mon ATTRIBUTE_UNUSED,
          * to match.  */
         if (status == VIR_DOMAIN_BLOCK_JOB_COMPLETED) {
             if (disk->mirrorState == VIR_DOMAIN_DISK_MIRROR_STATE_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
@@ -1054,8 +1083,11 @@ qemuProcessHandleBlockJob(qemuMonitorPtr mon ATTRIBUTE_UNUSED,
             disk->mirror = NULL;
             save = disk->mirrorState != VIR_DOMAIN_DISK_MIRROR_STATE_NONE;
             disk->mirrorState = VIR_DOMAIN_DISK_MIRROR_STATE_NONE;
+            disk->mirrorJob = VIR_DOMAIN_BLOCK_JOB_TYPE_UNKNOWN;
             qemuDomainDetermineDiskChain(driver, vm, disk, true);
-        } else if (disk->mirror && type == VIR_DOMAIN_BLOCK_JOB_TYPE_COPY) {
+        } else 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->mirrorState = VIR_DOMAIN_DISK_MIRROR_STATE_READY;
                 save = true;
@@ -1063,6 +1095,7 @@ qemuProcessHandleBlockJob(qemuMonitorPtr mon ATTRIBUTE_UNUSED,
                 virStorageSourceFree(disk->mirror);
                 disk->mirror = NULL;
                 disk->mirrorState = VIR_DOMAIN_DISK_MIRROR_STATE_NONE;
+                disk->mirrorJob = VIR_DOMAIN_BLOCK_JOB_TYPE_UNKNOWN;
                 save = true;
             }
         }
@@ -1072,6 +1105,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 0000000000..6ba572473b
--- /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 fa7af31d3f..46f2a3eef0 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 72b03c96b0..abec1806e3 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 8d46b4051c..451dedcff4 100644
--- a/tests/qemuxml2xmltest.c
+++ b/tests/qemuxml2xmltest.c
@@ -232,6 +232,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");