Re: [PATCH v5 1/4] blockcopy: add more XML for state tracking

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

 



On 07/29/2014 05:20 AM, Peter Krempa wrote:
> On 07/29/14 06:20, Eric Blake wrote:
>> 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; and 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.
>>

>>
>> +typedef enum {
>> +    VIR_DOMAIN_DISK_MIRROR_DEFAULT = 0, /* No job, or job still not synced */
> 
> I'd go with _NONE here and ...
> 
>> +    VIR_DOMAIN_DISK_MIRROR_READY, /* Job in second phase */
>> +    VIR_DOMAIN_DISK_MIRROR_ABORT, /* Job aborted, waiting for event */
>> +    VIR_DOMAIN_DISK_MIRROR_PIVOT, /* Job pivoted, waiting for event */
>> +
>> +    VIR_DOMAIN_DISK_MIRROR_LAST
>> +} virDomainDiskMirror;
> 
> ... as this describes the ready state of the mirroring operation I'd go
> with virDomainDiskMirrorState and rename those fields accordingly.
> Without that it's not clear what the enum describes.
> 

> Otherwise looks good. ACK if you change the enum name and default value
> name.

I'm squashing this in, and pushing.

diff --git i/src/conf/domain_conf.c w/src/conf/domain_conf.c
index 1c8f8cf..c2a81f1 100644
--- i/src/conf/domain_conf.c
+++ w/src/conf/domain_conf.c
@@ -747,8 +747,8 @@ VIR_ENUM_IMPL(virDomainDiskDiscard,
VIR_DOMAIN_DISK_DISCARD_LAST,
               "unmap",
               "ignore")

-VIR_ENUM_IMPL(virDomainDiskMirror, VIR_DOMAIN_DISK_MIRROR_LAST,
-              "default",
+VIR_ENUM_IMPL(virDomainDiskMirrorState, VIR_DOMAIN_DISK_MIRROR_STATE_LAST,
+              "none",
               "yes",
               "abort",
               "pivot")
@@ -5488,8 +5488,8 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt,
                 }
                 ready = virXMLPropString(cur, "ready");
                 if (ready) {
-                    def->mirrorState =
virDomainDiskMirrorTypeFromString(ready);
-                    if (def->mirrorState < 0) {
+                    if ((def->mirrorState =
+                         virDomainDiskMirrorStateTypeFromString(ready))
< 0) {
                         virReportError(VIR_ERR_XML_ERROR,
                                        _("unknown mirror ready state %s"),
                                        ready);
@@ -15289,7 +15289,7 @@ virDomainDiskDefFormat(virBufferPtr buf,
         if (def->mirrorState) {
             const char *mirror;

-            mirror = virDomainDiskMirrorTypeToString(def->mirrorState);
+            mirror =
virDomainDiskMirrorStateTypeToString(def->mirrorState);
             virBufferEscapeString(buf, " ready='%s'", mirror);
         }
         virBufferAddLit(buf, ">\n");
diff --git i/src/conf/domain_conf.h w/src/conf/domain_conf.h
index bc8966d..2df3ae2 100644
--- i/src/conf/domain_conf.h
+++ w/src/conf/domain_conf.h
@@ -611,13 +611,13 @@ typedef virDomainBlockIoTuneInfo
*virDomainBlockIoTuneInfoPtr;


 typedef enum {
-    VIR_DOMAIN_DISK_MIRROR_DEFAULT = 0, /* No job, or job still not
synced */
-    VIR_DOMAIN_DISK_MIRROR_READY, /* Job in second phase */
-    VIR_DOMAIN_DISK_MIRROR_ABORT, /* Job aborted, waiting for event */
-    VIR_DOMAIN_DISK_MIRROR_PIVOT, /* Job pivoted, waiting for event */
+    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_LAST
-} virDomainDiskMirror;
+    VIR_DOMAIN_DISK_MIRROR_STATE_LAST
+} virDomainDiskMirrorState;


 /* Stores the virtual disk configuration */
@@ -631,7 +631,7 @@ struct _virDomainDiskDef {
     int removable; /* enum virTristateSwitch */

     virStorageSourcePtr mirror;
-    int mirrorState; /* enum virDomainDiskMirror */
+    int mirrorState; /* enum virDomainDiskMirrorState */

     struct {
         unsigned int cylinders;
@@ -2566,7 +2566,7 @@ VIR_ENUM_DECL(virDomainDiskIo)
 VIR_ENUM_DECL(virDomainDeviceSGIO)
 VIR_ENUM_DECL(virDomainDiskTray)
 VIR_ENUM_DECL(virDomainDiskDiscard)
-VIR_ENUM_DECL(virDomainDiskMirror)
+VIR_ENUM_DECL(virDomainDiskMirrorState)
 VIR_ENUM_DECL(virDomainController)
 VIR_ENUM_DECL(virDomainControllerModelPCI)
 VIR_ENUM_DECL(virDomainControllerModelSCSI)
diff --git i/src/qemu/qemu_driver.c w/src/qemu/qemu_driver.c
index acc9ef0..836adba 100644
--- i/src/qemu/qemu_driver.c
+++ w/src/qemu/qemu_driver.c
@@ -14860,10 +14860,10 @@ qemuDomainBlockPivot(virConnectPtr conn,
         }
         if (rc == 1 && info.cur == info.end &&
             info.type == VIR_DOMAIN_BLOCK_JOB_TYPE_COPY)
-            disk->mirrorState = VIR_DOMAIN_DISK_MIRROR_READY;
+            disk->mirrorState = VIR_DOMAIN_DISK_MIRROR_STATE_READY;
     }

-    if (disk->mirrorState != VIR_DOMAIN_DISK_MIRROR_READY) {
+    if (disk->mirrorState != VIR_DOMAIN_DISK_MIRROR_STATE_READY) {
         virReportError(VIR_ERR_BLOCK_COPY_ACTIVE,
                        _("disk '%s' not ready for pivot yet"),
                        disk->dst);
@@ -14939,7 +14939,7 @@ qemuDomainBlockPivot(virConnectPtr conn,
     }

     disk->mirror = NULL;
-    disk->mirrorState = VIR_DOMAIN_DISK_MIRROR_DEFAULT;
+    disk->mirrorState = VIR_DOMAIN_DISK_MIRROR_STATE_NONE;

  cleanup:
     /* revert to original disk def on failure */
@@ -15096,7 +15096,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->mirrorState = VIR_DOMAIN_DISK_MIRROR_READY;
+        disk->mirrorState = VIR_DOMAIN_DISK_MIRROR_STATE_READY;

     /* A successful block job cancelation stops any mirroring.  */
     if (mode == BLOCK_JOB_ABORT && disk->mirror) {
@@ -15104,7 +15104,7 @@ qemuDomainBlockJobImpl(virDomainObjPtr vm,
          * the mirror, and audit that fact, before dropping things.  */
         virStorageSourceFree(disk->mirror);
         disk->mirror = NULL;
-        disk->mirrorState = VIR_DOMAIN_DISK_MIRROR_DEFAULT;
+        disk->mirrorState = VIR_DOMAIN_DISK_MIRROR_STATE_NONE;
     }

  waitjob:
diff --git i/src/qemu/qemu_process.c w/src/qemu/qemu_process.c
index 73ad27d..378c6d3 100644
--- i/src/qemu/qemu_process.c
+++ w/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->mirrorState = VIR_DOMAIN_DISK_MIRROR_READY;
+                disk->mirrorState = VIR_DOMAIN_DISK_MIRROR_STATE_READY;
             } else if (status == VIR_DOMAIN_BLOCK_JOB_FAILED) {
                 virStorageSourceFree(disk->mirror);
                 disk->mirror = NULL;
-                disk->mirrorState = VIR_DOMAIN_DISK_MIRROR_DEFAULT;
+                disk->mirrorState = VIR_DOMAIN_DISK_MIRROR_STATE_NONE;
             }
         }
     }


-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]