[libvirt] [PATCHv9 4/9] blockjob: support pivot operation on cancel

[Eric Blake]

On 10/27/2012 07:04 AM, Eric Blake wrote:
> On 10/26/2012 07:07 AM, Peter Krempa wrote:
>> On 10/23/12 04:10, Eric Blake wrote:
>>> This is the bare minimum to end a copy job (of course, until a
>>> later patch adds the ability to start a copy job, this patch
>>> doesn't do much in isolation; I've just split the patches to
>>> ease the review).
>>>
>>
>> ACK if the RHEL stuff will be pulled in, otherwise it will require a few
>> changes.
> 
> Here's what I'm squashing in for the non-RHEL change:

Hit send too soon.. In addition to the compile fixes,

> 
> diff --git i/src/qemu/qemu_driver.c w/src/qemu/qemu_driver.c
> index f812ac1..65df4f0 100644
> --- i/src/qemu/qemu_driver.c
> +++ w/src/qemu/qemu_driver.c
> @@ -12554,7 +12554,6 @@ qemuDomainBlockPivot(struct qemud_driver
> *driver, virDomainObjPtr vm,
>      int ret = -1;
>      qemuDomainObjPrivatePtr priv = vm->privateData;
>      virDomainBlockJobInfo info;
> -    bool reopen = qemuCapsGet(priv->caps, QEMU_CAPS_DRIVE_REOPEN);
>      const char *format =
> virStorageFileFormatTypeToString(disk->mirrorFormat);
> 
>      /* Probe the status, if needed.  */
> @@ -12584,8 +12583,7 @@ qemuDomainBlockPivot(struct qemud_driver
> *driver, virDomainObjPtr vm,
> 
>      /* Attempt the pivot.  */
>      qemuDomainObjEnterMonitorWithDriver(driver, vm);
> -    ret = qemuMonitorDrivePivot(priv->mon, device, disk->mirror, format,
> -                                reopen);
> +    ret = qemuMonitorDrivePivot(priv->mon, device, disk->mirror, format);
>      qemuDomainObjExitMonitorWithDriver(driver, vm);
> 
>      /* Note that RHEL 6.3 'drive-reopen' has the remote risk of a

I'm also squashing in some comment simplifications:

diff --git i/src/qemu/qemu_driver.c w/src/qemu/qemu_driver.c
index 65df4f0..5309745 100644
--- i/src/qemu/qemu_driver.c
+++ w/src/qemu/qemu_driver.c
@@ -12545,8 +12545,7 @@ cleanup:

 /* Called while holding the VM job lock, to implement a block job
  * abort with pivot; this updates the VM definition as appropriate, on
- * either success or failure (although there are some forms of
- * catastrophic failure that will leave the VM unusable).  */
+ * either success or failure.  */
 static int
 qemuDomainBlockPivot(struct qemud_driver *driver, virDomainObjPtr vm,
                      const char *device, virDomainDiskDefPtr disk)
@@ -12586,11 +12585,6 @@ qemuDomainBlockPivot(struct qemud_driver
*driver, virDomainObjPtr vm,
     ret = qemuMonitorDrivePivot(priv->mon, device, disk->mirror, format);
     qemuDomainObjExitMonitorWithDriver(driver, vm);

-    /* Note that RHEL 6.3 'drive-reopen' has the remote risk of a
-     * catastrophic failure, where the it fails but can't recover by
-     * reopening the source.  Not much we can do about it.  qemu 1.3
-     * 'block-job-complete' is safer by design.  */
-
     if (ret == 0) {
         /* XXX We want to revoke security labels and disk lease, as
          * well as audit that revocation, before dropping the original
@@ -12607,9 +12601,10 @@ qemuDomainBlockPivot(struct qemud_driver
*driver, virDomainObjPtr vm,
         disk->mirroring = false;
         qemuDomainDetermineDiskChain(driver, disk, true);
     } else {
-        /* On failure, qemu abandons the mirror, and attempts to
-         * revert back to the source disk.  Hopefully it was able to
-         * reopen things.  */
+        /* On failure, qemu abandons the mirror, and reverts back to
+         * the source disk (RHEL 6.3 has a bug where the revert could
+         * cause catastrophic failure in qemu, but we don't need to
+         * worry about it here as it is not an upstream qemu problem.  */
         /* XXX should we be parsing the exact qemu error, or calling
          * 'query-block', to see what state we really got left in
          * before killing the mirroring job?  And just as on the

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

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 617 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20121027/1ed3c5bb/attachment-0001.sig>