blockcopy: add more XML for state tracking

Doing a blockcopy operation across a libvirtd restart is not very
robust at the moment.  In particular, we are clearing the <mirror>
element prior to telling qemu to finish the job.  Also, thanks to the
ability to request async completion, the user can easily regain
control prior to qemu actually finishing the effort, and they should
be able to poll the domain XML to see if the job is still going.

A future patch will fix things to actually wait until qemu is done
before modifying the XML to reflect the job completion.  But since
qemu issues identical BLOCK_JOB_COMPLETE events regardless of whether
the job was cancelled (kept the original disk) or completed (pivoted
to the new disk), we have to track which of the two operations were
used to end the job.  Furthermore, we'd like to avoid attempts to
end a job where we are already waiting on an earlier request to qemu
to end the job.  Likewise, if we miss the qemu event (perhaps because
it arrived during a libvirtd restart), we still need enough state
recorded to be able to determine how to modify the domain XML once
we reconnect to qemu and manually learn whether the job still exists.

Although this patch doesn't actually fix the problem, it is a
preliminary step that makes it possible to track whether a job
has already begun steps towards completion.

* src/conf/domain_conf.h (virDomainDiskMirrorState): New enum.
(_virDomainDiskDef): Convert bool mirroring to new enum.
* src/conf/domain_conf.c (virDomainDiskDefParseXML)
(virDomainDiskDefFormat): Handle new values.
* src/qemu/qemu_process.c (qemuProcessHandleBlockJob): Adjust
client.
* src/qemu/qemu_driver.c (qemuDomainBlockPivot)
(qemuDomainBlockJobImpl): Likewise.
* docs/schemas/domaincommon.rng (diskMirror): Expose new values.
* docs/formatdomain.html.in (elementsDisks): Document it.
* tests/qemuxml2argvdata/qemuxml2argv-disk-mirror.xml: Test it.

Signed-off-by: Eric Blake <eblake@redhat.com>

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index bce58859d1a3e830472b45faf4b354c6439615a3..2f0be20a6e26389122056c20526f70e7ccb5604d 100644 (file)
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -1921,9 +1921,13 @@
         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
-        attribute <code>ready</code> is present, then it is known the
-        disk is ready to pivot; otherwise, the disk is probably still
+        overall <code>disk</code> device element. 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
+        1.2.7</span>, <code>abort</code> or <code>pivot</code> if the
+        job is in the process of completing.  If <code>ready</code> is
+        not present, the disk is probably still
         copying.  For now, this element only valid in output; it is
         ignored on input.  The <code>source</code> sub-element exists
         for all two-phase jobs <span class="since">since 1.2.6</span>.

diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index aa0dcd5814819d34e69a3aee62db0450cdce4dbb..7cb5a9eb1180184826016cdc383cb5307139d0f3 100644 (file)
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -4309,7 +4309,11 @@
       </choice>
       <optional>
         <attribute name='ready'>
-          <value>yes</value>
+          <choice>
+            <value>yes</value>
+            <value>abort</value>
+            <value>pivot</value>
+          </choice>
         </attribute>
       </optional>
     </element>

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 65b1857f90f8340601b6bc749cc50826b71196c4..2a8cdeb03b901e9470be6eecc287a9cf22304847 100644 (file)
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -747,6 +747,12 @@ VIR_ENUM_IMPL(virDomainDiskDiscard, VIR_DOMAIN_DISK_DISCARD_LAST,
               "unmap",
               "ignore")
 
+VIR_ENUM_IMPL(virDomainDiskMirrorState, VIR_DOMAIN_DISK_MIRROR_STATE_LAST,
+              "none",
+              "yes",
+              "abort",
+              "pivot")
+
 #define VIR_DOMAIN_XML_WRITE_FLAGS  VIR_DOMAIN_XML_SECURE
 #define VIR_DOMAIN_XML_READ_FLAGS   VIR_DOMAIN_XML_INACTIVE
 
@@ -5482,7 +5488,14 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt,
                 }
                 ready = virXMLPropString(cur, "ready");
                 if (ready) {
-                    def->mirroring = true;
+                    if ((def->mirrorState =
+                         virDomainDiskMirrorStateTypeFromString(ready)) < 0) {
+                        virReportError(VIR_ERR_XML_ERROR,
+                                       _("unknown mirror ready state %s"),
+                                       ready);
+                        VIR_FREE(ready);
+                        goto error;
+                    }
                     VIR_FREE(ready);
                 }
             } else if (xmlStrEqual(cur->name, BAD_CAST "auth")) {
@@ -15392,8 +15405,12 @@ virDomainDiskDefFormat(virBufferPtr buf,
             virBufferEscapeString(buf, " file='%s'", def->mirror->path);
             virBufferEscapeString(buf, " format='%s'", formatStr);
         }
-        if (def->mirroring)
-            virBufferAddLit(buf, " ready='yes'");
+        if (def->mirrorState) {
+            const char *mirror;
+
+            mirror = virDomainDiskMirrorStateTypeToString(def->mirrorState);
+            virBufferEscapeString(buf, " ready='%s'", mirror);
+        }
         virBufferAddLit(buf, ">\n");
         virBufferAdjustIndent(buf, 2);
         virBufferEscapeString(buf, "<format type='%s'/>\n", formatStr);

diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 85e1da29ee31007884ea17b5deca7bb769892a6b..c77c85c470c9f5d0d1f66c43c718acd394c6a494 100644 (file)
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -610,6 +610,16 @@ struct _virDomainBlockIoTuneInfo {
 typedef virDomainBlockIoTuneInfo *virDomainBlockIoTuneInfoPtr;
 
 
+typedef enum {
+    VIR_DOMAIN_DISK_MIRROR_STATE_NONE = 0, /* No job, or job still not synced */
+    VIR_DOMAIN_DISK_MIRROR_STATE_READY, /* Job in second phase */
+    VIR_DOMAIN_DISK_MIRROR_STATE_ABORT, /* Job aborted, waiting for event */
+    VIR_DOMAIN_DISK_MIRROR_STATE_PIVOT, /* Job pivoted, waiting for event */
+
+    VIR_DOMAIN_DISK_MIRROR_STATE_LAST
+} virDomainDiskMirrorState;
+
+
 /* Stores the virtual disk configuration */
 struct _virDomainDiskDef {
     virStorageSourcePtr src; /* non-NULL.  XXX Allow NULL for empty cdrom? */
@@ -621,7 +631,7 @@ struct _virDomainDiskDef {
     int removable; /* enum virTristateSwitch */
 
     virStorageSourcePtr mirror;
-    bool mirroring;
+    int mirrorState; /* enum virDomainDiskMirrorState */
 
     struct {
         unsigned int cylinders;
@@ -2567,6 +2577,7 @@ VIR_ENUM_DECL(virDomainDiskIo)
 VIR_ENUM_DECL(virDomainDeviceSGIO)
 VIR_ENUM_DECL(virDomainDiskTray)
 VIR_ENUM_DECL(virDomainDiskDiscard)
+VIR_ENUM_DECL(virDomainDiskMirrorState)
 VIR_ENUM_DECL(virDomainController)
 VIR_ENUM_DECL(virDomainControllerModelPCI)
 VIR_ENUM_DECL(virDomainControllerModelSCSI)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index c80753947391ed019229df32b0c1b975f95cb79a..b581d345651d6e51a1579d43899572b048d0b770 100644 (file)
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -14841,7 +14841,7 @@ qemuDomainBlockPivot(virConnectPtr conn,
     format = virStorageFileFormatTypeToString(disk->mirror->format);
 
     /* Probe the status, if needed.  */
-    if (!disk->mirroring) {
+    if (!disk->mirrorState) {
         qemuDomainObjEnterMonitor(driver, vm);
         rc = qemuMonitorBlockJob(priv->mon, device, NULL, NULL, 0, &info,
                                   BLOCK_JOB_INFO, true);
@@ -14855,10 +14855,10 @@ qemuDomainBlockPivot(virConnectPtr conn,
         }
         if (rc == 1 && info.cur == info.end &&
             info.type == VIR_DOMAIN_BLOCK_JOB_TYPE_COPY)
-            disk->mirroring = true;
+            disk->mirrorState = VIR_DOMAIN_DISK_MIRROR_STATE_READY;
     }
 
-    if (!disk->mirroring) {
+    if (disk->mirrorState != VIR_DOMAIN_DISK_MIRROR_STATE_READY) {
         virReportError(VIR_ERR_BLOCK_COPY_ACTIVE,
                        _("disk '%s' not ready for pivot yet"),
                        disk->dst);
@@ -14934,7 +14934,7 @@ qemuDomainBlockPivot(virConnectPtr conn,
     }
 
     disk->mirror = NULL;
-    disk->mirroring = false;
+    disk->mirrorState = VIR_DOMAIN_DISK_MIRROR_STATE_NONE;
 
  cleanup:
     /* revert to original disk def on failure */
@@ -15091,7 +15091,7 @@ qemuDomainBlockJobImpl(virDomainObjPtr vm,
      * avoid checking if pivot is safe.  */
     if (mode == BLOCK_JOB_INFO && ret == 1 && disk->mirror &&
         info->cur == info->end && info->type == VIR_DOMAIN_BLOCK_JOB_TYPE_COPY)
-        disk->mirroring = true;
+        disk->mirrorState = VIR_DOMAIN_DISK_MIRROR_STATE_READY;
 
     /* A successful block job cancelation stops any mirroring.  */
     if (mode == BLOCK_JOB_ABORT && disk->mirror) {
@@ -15099,7 +15099,7 @@ qemuDomainBlockJobImpl(virDomainObjPtr vm,
          * the mirror, and audit that fact, before dropping things.  */
         virStorageSourceFree(disk->mirror);
         disk->mirror = NULL;
-        disk->mirroring = false;
+        disk->mirrorState = VIR_DOMAIN_DISK_MIRROR_STATE_NONE;
     }
 
  waitjob:

diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 36922cbd261f9c5786787b0a586228796a84d32a..c66326539e6e4107ea2fb40c3d4424ce2dd26143 100644 (file)
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -1040,11 +1040,11 @@ qemuProcessHandleBlockJob(qemuMonitorPtr mon ATTRIBUTE_UNUSED,
             qemuDomainDetermineDiskChain(driver, vm, disk, true);
         if (disk->mirror && type == VIR_DOMAIN_BLOCK_JOB_TYPE_COPY) {
             if (status == VIR_DOMAIN_BLOCK_JOB_READY) {
-                disk->mirroring = true;
+                disk->mirrorState = VIR_DOMAIN_DISK_MIRROR_STATE_READY;
             } else if (status == VIR_DOMAIN_BLOCK_JOB_FAILED) {
                 virStorageSourceFree(disk->mirror);
                 disk->mirror = NULL;
-                disk->mirroring = false;
+                disk->mirrorState = VIR_DOMAIN_DISK_MIRROR_STATE_NONE;
             }
         }
     }

diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-mirror.xml b/tests/qemuxml2argvdata/qemuxml2argv-disk-mirror.xml
index 72b03c96b05de6e3bcc7135dc5866c267d2b32c1..fa7af31d3f8d0effb7be20c7c2eea73d2a180858 100644 (file)
--- a/tests/qemuxml2argvdata/qemuxml2argv-disk-mirror.xml
+++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-mirror.xml
@@ -42,6 +42,10 @@
     <disk type='file' device='disk'>
       <source file='/tmp/logs.img'/>
       <backingStore/>
+      <mirror type='file' file='/tmp/logcopy.img' format='qcow2' ready='abort'>
+        <format type='qcow2'/>
+        <source file='/tmp/logcopy.img'/>
+      </mirror>
       <target dev='vdb' bus='virtio'/>
     </disk>
     <controller type='usb' index='0'/>