[PATCH] blockjob: wait for pivot to complete

[Eric Blake <eblake@xxxxxxxxxx>]

https://bugzilla.redhat.com/show_bug.cgi?id=1119173 documents that
commit eaba79d was flawed in the implementation of the
VIR_DOMAIN_BLOCK_JOB_ABORT_ASYNC flag when it comes to completing
a blockcopy.  Basically, the qemu pivot action is async (the QMP
command returns immediately, but the user must wait for the
BLOCK_JOB_COMPLETE event to know that all I/O related to the job
has finally been flushed), but the libvirt command was documented
as synchronous by default.  As active block commit will also be
using this code, it is worth fixing now.

* src/qemu/qemu_driver.c (qemuDomainBlockJobImpl): Don't skip wait
loop after pivot.

Signed-off-by: Eric Blake <eblake@xxxxxxxxxx>
---

Shown here with extra context, to hopefully aid the review.

 src/qemu/qemu_driver.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index ecccf6c..300e49e 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -14973,45 +14973,45 @@ qemuDomainBlockJobImpl(virDomainObjPtr vm,
     if (!device)
         goto endjob;
     disk = vm->def->disks[idx];

     if (mode == BLOCK_JOB_PULL && disk->mirror) {
         virReportError(VIR_ERR_BLOCK_COPY_ACTIVE,
                        _("disk '%s' already in active block job"),
                        disk->dst);
         goto endjob;
     }
     if (mode == BLOCK_JOB_ABORT &&
         (flags & VIR_DOMAIN_BLOCK_JOB_ABORT_PIVOT) &&
         !(async && disk->mirror)) {
         virReportError(VIR_ERR_OPERATION_INVALID,
                        _("pivot of disk '%s' requires an active copy job"),
                        disk->dst);
         goto endjob;
     }

     if (disk->mirror && mode == BLOCK_JOB_ABORT &&
         (flags & VIR_DOMAIN_BLOCK_JOB_ABORT_PIVOT)) {
         ret = qemuDomainBlockPivot(conn, driver, vm, device, disk);
-        goto endjob;
+        goto waitjob;
     }

     if (base &&
         (virStorageFileParseChainIndex(disk->dst, base, &baseIndex) < 0 ||
          !(baseSource = virStorageFileChainLookup(disk->src, disk->src,
                                                   base, baseIndex, NULL))))
         goto endjob;

     if (baseSource) {
         if (qemuGetDriveSourceString(baseSource, NULL, &basePath) < 0)
             goto endjob;

         if (flags & VIR_DOMAIN_BLOCK_REBASE_RELATIVE) {
             if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_CHANGE_BACKING_FILE)) {
                 virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
                                _("this QEMU binary doesn't support relative "
                                  "block pull/rebase"));
                 goto endjob;
             }

             if (virStorageFileGetRelativeBackingPath(disk->src->backingStore,
                                                      baseSource,
@@ -15030,44 +15030,45 @@ qemuDomainBlockJobImpl(virDomainObjPtr vm,
     qemuDomainObjEnterMonitor(driver, vm);
     ret = qemuMonitorBlockJob(priv->mon, device, basePath, backingPath,
                               bandwidth, info, mode, async);
     qemuDomainObjExitMonitor(driver, vm);
     if (ret < 0)
         goto endjob;

     /* Snoop block copy operations, so future cancel operations can
      * 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;

     /* A successful block job cancelation stops any mirroring.  */
     if (mode == BLOCK_JOB_ABORT && disk->mirror) {
         /* XXX We should also revoke security labels and disk lease on
          * the mirror, and audit that fact, before dropping things.  */
         virStorageSourceFree(disk->mirror);
         disk->mirror = NULL;
         disk->mirroring = false;
     }

+ waitjob:
     /* With synchronous block cancel, we must synthesize an event, and
      * we silently ignore the ABORT_ASYNC flag.  With asynchronous
      * block cancel, the event will come from qemu, but without the
      * ABORT_ASYNC flag, we must block to guarantee synchronous
      * operation.  We do the waiting while still holding the VM job,
      * to prevent newly scheduled block jobs from confusing us.  */
     if (mode == BLOCK_JOB_ABORT) {
         if (!async) {
             /* Older qemu that lacked async reporting also lacked
              * active commit, so we can hardcode the event to pull.
              * We have to generate two variants of the event. */
             int type = VIR_DOMAIN_BLOCK_JOB_TYPE_PULL;
             int status = VIR_DOMAIN_BLOCK_JOB_CANCELED;
             event = virDomainEventBlockJobNewFromObj(vm, disk->src->path, type,
                                                      status);
             event2 = virDomainEventBlockJob2NewFromObj(vm, disk->dst, type,
                                                        status);
         } else if (!(flags & VIR_DOMAIN_BLOCK_JOB_ABORT_ASYNC)) {
             while (1) {
                 /* Poll every 50ms */
                 static struct timespec ts = { .tv_sec = 0,
                                               .tv_nsec = 50 * 1000 * 1000ull };
-- 
1.9.3

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