qemu: Add -blockdev support for block commit job

Introduce the handler for finalizing a block commit and active bloc
commit job which will allow to use it with blockdev.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>

src/qemu/qemu_blockjob.c

@@ -237,6 +237,43 @@ qemuBlockJobDiskNewPull(virDomainObjPtr vm,
 }
 
 
+qemuBlockJobDataPtr
+qemuBlockJobDiskNewCommit(virDomainObjPtr vm,
+                          virDomainDiskDefPtr disk,
+                          virStorageSourcePtr topparent,
+                          virStorageSourcePtr top,
+                          virStorageSourcePtr base)
+{
+    qemuDomainObjPrivatePtr priv = vm->privateData;
+    VIR_AUTOUNREF(qemuBlockJobDataPtr) job = NULL;
+    VIR_AUTOFREE(char *) jobname = NULL;
+    qemuBlockJobType jobtype = QEMU_BLOCKJOB_TYPE_COMMIT;
+
+    if (topparent == NULL)
+        jobtype = QEMU_BLOCKJOB_TYPE_ACTIVE_COMMIT;
+
+    if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_BLOCKDEV)) {
+        if (virAsprintf(&jobname, "commit-%s-%s", disk->dst, top->nodeformat) < 0)
+            return NULL;
+    } else {
+        if (!(jobname = qemuAliasDiskDriveFromDisk(disk)))
+            return NULL;
+    }
+
+    if (!(job = qemuBlockJobDataNew(jobtype, jobname)))
+        return NULL;
+
+    job->data.commit.topparent = topparent;
+    job->data.commit.top = top;
+    job->data.commit.base = base;
+
+    if (qemuBlockJobRegister(job, vm, disk, true) < 0)
+        return NULL;
+
+    VIR_RETURN_PTR(job);
+}
+
+
 /**
  * qemuBlockJobDiskRegisterMirror:
  * @job: block job to register 'mirror' chain on
@@ -816,6 +853,167 @@ qemuBlockJobProcessEventCompletedPull(virQEMUDriverPtr driver,
 }
 
 
+/**
+ * qemuBlockJobProcessEventCompletedCommit:
+ * @driver: qemu driver object
+ * @vm: domain object
+ * @job: job data
+ * @asyncJob: qemu asynchronous job type (for monitor interaction)
+ *
+ * This function executes the finalizing steps after a successful block commit
+ * job. The commit job moves the blocks from backing chain images starting from
+ * 'top' into the 'base' image. The overlay of the 'top' image ('topparent')
+ * then directly references the 'base' image. All intermediate images can be
+ * removed/deleted.
+ */
+static void
+qemuBlockJobProcessEventCompletedCommit(virQEMUDriverPtr driver,
+                                        virDomainObjPtr vm,
+                                        qemuBlockJobDataPtr job,
+                                        qemuDomainAsyncJob asyncJob)
+{
+    virStorageSourcePtr baseparent = NULL;
+    virDomainDiskDefPtr cfgdisk = NULL;
+    virStorageSourcePtr cfgnext = NULL;
+    virStorageSourcePtr cfgtopparent = NULL;
+    virStorageSourcePtr cfgtop = NULL;
+    virStorageSourcePtr cfgbase = NULL;
+    virStorageSourcePtr cfgbaseparent = NULL;
+    virStorageSourcePtr n;
+
+    VIR_DEBUG("commit job '%s' on VM '%s' completed", job->name, vm->def->name);
+
+    /* if the job isn't associated with a disk there's nothing to do */
+    if (!job->disk)
+        return;
+
+    if ((cfgdisk = qemuBlockJobGetConfigDisk(vm, job->disk, job->data.commit.base)))
+        cfgnext = cfgdisk->src;
+
+    if (!cfgdisk)
+        qemuBlockJobClearConfigChain(vm, job->disk);
+
+    for (n = job->disk->src; n && n != job->data.commit.base; n = n->backingStore) {
+        if (cfgnext) {
+            if (n == job->data.commit.topparent)
+                cfgtopparent = cfgnext;
+
+            if (n == job->data.commit.top)
+                cfgtop = cfgnext;
+
+            cfgbaseparent = cfgnext;
+            cfgnext = cfgnext->backingStore;
+        }
+        baseparent = n;
+    }
+
+    if (!n)
+        return;
+
+    /* revert access to images */
+    qemuDomainStorageSourceAccessAllow(driver, vm, job->data.commit.base, true, false);
+    if (job->data.commit.topparent != job->disk->src)
+        qemuDomainStorageSourceAccessAllow(driver, vm, job->data.commit.topparent, true, false);
+
+    baseparent->backingStore = NULL;
+    job->data.commit.topparent->backingStore = job->data.commit.base;
+
+    qemuBlockJobEventProcessConcludedRemoveChain(driver, vm, asyncJob, job->data.commit.top);
+    virObjectUnref(job->data.commit.top);
+    job->data.commit.top = NULL;
+
+    if (cfgbaseparent) {
+        cfgbase = cfgbaseparent->backingStore;
+        cfgbaseparent->backingStore = NULL;
+
+        if (cfgtopparent)
+            cfgtopparent->backingStore = cfgbase;
+        else
+            cfgdisk->src = cfgbase;
+
+        virObjectUnref(cfgtop);
+    }
+}
+
+
+/**
+ * qemuBlockJobProcessEventCompletedActiveCommit:
+ * @driver: qemu driver object
+ * @vm: domain object
+ * @job: job data
+ * @asyncJob: qemu asynchronous job type (for monitor interaction)
+ *
+ * This function executes the finalizing steps after a successful active layer
+ * block commit job. The commit job moves the blocks from backing chain images
+ * starting from the active disk source image into the 'base' image. The disk
+ * source then changes to the 'base' image. All intermediate images can be
+ * removed/deleted.
+ */
+static void
+qemuBlockJobProcessEventCompletedActiveCommit(virQEMUDriverPtr driver,
+                                              virDomainObjPtr vm,
+                                              qemuBlockJobDataPtr job,
+                                              qemuDomainAsyncJob asyncJob)
+{
+    virStorageSourcePtr baseparent = NULL;
+    virDomainDiskDefPtr cfgdisk = NULL;
+    virStorageSourcePtr cfgnext = NULL;
+    virStorageSourcePtr cfgtop = NULL;
+    virStorageSourcePtr cfgbase = NULL;
+    virStorageSourcePtr cfgbaseparent = NULL;
+    virStorageSourcePtr n;
+
+    VIR_DEBUG("active commit job '%s' on VM '%s' completed", job->name, vm->def->name);
+
+    /* if the job isn't associated with a disk there's nothing to do */
+    if (!job->disk)
+        return;
+
+    if ((cfgdisk = qemuBlockJobGetConfigDisk(vm, job->disk, job->data.commit.base)))
+        cfgnext = cfgdisk->src;
+
+    for (n = job->disk->src; n && n != job->data.commit.base; n = n->backingStore) {
+        if (cfgnext) {
+            if (n == job->data.commit.top)
+                cfgtop = cfgnext;
+
+            cfgbaseparent = cfgnext;
+            cfgnext = cfgnext->backingStore;
+        }
+        baseparent = n;
+    }
+
+    if (!n)
+        return;
+
+    if (!cfgdisk) {
+        /* in case when the config disk chain didn't match but the disk top seems
+         * to be identical we need to modify the disk source since the active
+         * commit makes the top level image invalid.
+         */
+        qemuBlockJobRewriteConfigDiskSource(vm, job->disk, job->data.commit.base);
+    } else {
+        cfgbase = cfgbaseparent->backingStore;
+        cfgbaseparent->backingStore = NULL;
+        cfgdisk->src = cfgbase;
+        virObjectUnref(cfgtop);
+    }
+
+    /* Move security driver metadata */
+    if (qemuSecurityMoveImageMetadata(driver, vm, job->disk->src, job->data.commit.base) < 0)
+        VIR_WARN("Unable to move disk metadata on vm %s", vm->def->name);
+
+    baseparent->backingStore = NULL;
+    job->disk->src = job->data.commit.base;
+
+    qemuBlockJobEventProcessConcludedRemoveChain(driver, vm, asyncJob, job->data.commit.top);
+    virObjectUnref(job->data.commit.top);
+    job->data.commit.top = NULL;
+    /* the mirror element does not serve functional purpose for the commit job */
+    virObjectUnref(job->disk->mirror);
+    job->disk->mirror = NULL;
+}
+
 static void
 qemuBlockJobEventProcessConcludedTransition(qemuBlockJobDataPtr job,
                                             virQEMUDriverPtr driver,
@@ -830,7 +1028,13 @@ qemuBlockJobEventProcessConcludedTransition(qemuBlockJobDataPtr job,
             break;
 
         case QEMU_BLOCKJOB_TYPE_COMMIT:
+            qemuBlockJobProcessEventCompletedCommit(driver, vm, job, asyncJob);
+            break;
+
         case QEMU_BLOCKJOB_TYPE_ACTIVE_COMMIT:
+            qemuBlockJobProcessEventCompletedActiveCommit(driver, vm, job, asyncJob);
+            break;
+
         case QEMU_BLOCKJOB_TYPE_COPY:
         case QEMU_BLOCKJOB_TYPE_NONE:
         case QEMU_BLOCKJOB_TYPE_INTERNAL:
@@ -845,7 +1049,15 @@ qemuBlockJobEventProcessConcludedTransition(qemuBlockJobDataPtr job,
         switch ((qemuBlockJobType) job->type) {
         case QEMU_BLOCKJOB_TYPE_PULL:
         case QEMU_BLOCKJOB_TYPE_COMMIT:
+            break;
+
         case QEMU_BLOCKJOB_TYPE_ACTIVE_COMMIT:
+            if (job->disk) {
+                virObjectUnref(job->disk->mirror);
+                job->disk->mirror = NULL;
+            }
+            break;
+
         case QEMU_BLOCKJOB_TYPE_COPY:
         case QEMU_BLOCKJOB_TYPE_NONE:
         case QEMU_BLOCKJOB_TYPE_INTERNAL:

src/qemu/qemu_blockjob.h

@@ -77,6 +77,16 @@ struct _qemuBlockJobPullData {
 };
 
 
+typedef struct _qemuBlockJobCommitData qemuBlockJobCommitData;
+typedef qemuBlockJobCommitData *qemuBlockJobDataCommitPtr;
+
+struct _qemuBlockJobCommitData {
+    virStorageSourcePtr topparent;
+    virStorageSourcePtr top;
+    virStorageSourcePtr base;
+};
+
+
 typedef struct _qemuBlockJobData qemuBlockJobData;
 typedef qemuBlockJobData *qemuBlockJobDataPtr;
 
@@ -91,6 +101,7 @@ struct _qemuBlockJobData {
 
     union {
         qemuBlockJobPullData pull;
+        qemuBlockJobCommitData commit;
     } data;
 
     int type; /* qemuBlockJobType */
@@ -132,6 +143,13 @@ qemuBlockJobDiskNewPull(virDomainObjPtr vm,
                         virDomainDiskDefPtr disk,
                         virStorageSourcePtr base);
 
+qemuBlockJobDataPtr
+qemuBlockJobDiskNewCommit(virDomainObjPtr vm,
+                          virDomainDiskDefPtr disk,
+                          virStorageSourcePtr topparent,
+                          virStorageSourcePtr top,
+                          virStorageSourcePtr base);
+
 qemuBlockJobDataPtr
 qemuBlockJobDiskGetJob(virDomainDiskDefPtr disk)
     ATTRIBUTE_NONNULL(1);

src/qemu/qemu_domain.c

@@ -2398,6 +2398,12 @@ qemuDomainObjPrivateXMLFormatBlockjobIterator(void *payload,
 
         case QEMU_BLOCKJOB_TYPE_COMMIT:
         case QEMU_BLOCKJOB_TYPE_ACTIVE_COMMIT:
+            if (job->data.commit.base)
+                virBufferAsprintf(&childBuf, "<base node='%s'/>\n", job->data.commit.base->nodeformat);
+            if (job->data.commit.top)
+                virBufferAsprintf(&childBuf, "<top node='%s'/>\n", job->data.commit.top->nodeformat);
+            if (job->data.commit.topparent)
+                virBufferAsprintf(&childBuf, "<topparent node='%s'/>\n", job->data.commit.topparent->nodeformat);
         case QEMU_BLOCKJOB_TYPE_COPY:
         case QEMU_BLOCKJOB_TYPE_NONE:
         case QEMU_BLOCKJOB_TYPE_INTERNAL:
@@ -2854,7 +2860,29 @@ qemuDomainObjPrivateXMLParseBlockjobDataSpecific(qemuBlockJobDataPtr job,
             break;
 
         case QEMU_BLOCKJOB_TYPE_COMMIT:
+            qemuDomainObjPrivateXMLParseBlockjobNodename(job,
+                                                         "string(./topparent/@node)",
+                                                         &job->data.commit.topparent,
+                                                         ctxt);
+
+            if (!job->data.commit.topparent)
+                goto broken;
+
+            ATTRIBUTE_FALLTHROUGH;
         case QEMU_BLOCKJOB_TYPE_ACTIVE_COMMIT:
+            qemuDomainObjPrivateXMLParseBlockjobNodename(job,
+                                                         "string(./top/@node)",
+                                                         &job->data.commit.top,
+                                                         ctxt);
+            qemuDomainObjPrivateXMLParseBlockjobNodename(job,
+                                                         "string(./base/@node)",
+                                                         &job->data.commit.base,
+                                                         ctxt);
+            if (!job->data.commit.top ||
+                !job->data.commit.base)
+                goto broken;
+            break;
+
         case QEMU_BLOCKJOB_TYPE_COPY:
         case QEMU_BLOCKJOB_TYPE_NONE:
         case QEMU_BLOCKJOB_TYPE_INTERNAL:
@@ -2863,6 +2891,10 @@ qemuDomainObjPrivateXMLParseBlockjobDataSpecific(qemuBlockJobDataPtr job,
     }
 
     return;
+
+ broken:
+    VIR_DEBUG("marking block job '%s' as invalid: malformed job data", job->name);
+    job->invalidData = true;
 }
 
 

src/qemu/qemu_driver.c

@@ -17971,7 +17971,8 @@ qemuDomainBlockCommit(virDomainPtr dom,
     virQEMUDriverPtr driver = dom->conn->privateData;
     qemuDomainObjPrivatePtr priv;
     virDomainObjPtr vm = NULL;
-    VIR_AUTOFREE(char *) device = NULL;
+    const char *device = NULL;
+    const char *jobname = NULL;
     int ret = -1;
     virDomainDiskDefPtr disk = NULL;
     virStorageSourcePtr topSource;
@@ -17985,8 +17986,11 @@ qemuDomainBlockCommit(virDomainPtr dom,
     VIR_AUTOFREE(char *) backingPath = NULL;
     unsigned long long speed = bandwidth;
     qemuBlockJobDataPtr job = NULL;
-    qemuBlockJobType jobtype = QEMU_BLOCKJOB_TYPE_COMMIT;
     VIR_AUTOUNREF(virStorageSourcePtr) mirror = NULL;
+    const char *nodetop = NULL;
+    const char *nodebase = NULL;
+    bool persistjob = false;
+    bool blockdev = false;
 
     /* XXX Add support for COMMIT_DELETE */
     virCheckFlags(VIR_DOMAIN_BLOCK_COMMIT_SHALLOW |
@@ -18007,6 +18011,8 @@ qemuDomainBlockCommit(virDomainPtr dom,
     if (virDomainObjCheckActive(vm) < 0)
         goto endjob;
 
+    blockdev = virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_BLOCKDEV);
+
     /* Convert bandwidth MiB to bytes, if necessary */
     if (!(flags & VIR_DOMAIN_BLOCK_COMMIT_BANDWIDTH_BYTES)) {
         if (speed > LLONG_MAX >> 20) {
@@ -18021,9 +18027,6 @@ qemuDomainBlockCommit(virDomainPtr dom,
     if (!(disk = qemuDomainDiskByName(vm->def, path)))
         goto endjob;
 
-    if (!(device = qemuAliasDiskDriveFromDisk(disk)))
-        goto endjob;
-
     if (virStorageSourceIsEmpty(disk->src)) {
         virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
                        _("disk %s has no source file to be committed"),
@@ -18055,8 +18058,6 @@ qemuDomainBlockCommit(virDomainPtr dom,
                            disk->dst);
             goto endjob;
         }
-
-        jobtype = QEMU_BLOCKJOB_TYPE_ACTIVE_COMMIT;
     } else if (flags & VIR_DOMAIN_BLOCK_COMMIT_ACTIVE) {
         virReportError(VIR_ERR_INVALID_ARG,
                        _("active commit requested but '%s' is not active"),
@@ -18129,7 +18130,8 @@ qemuDomainBlockCommit(virDomainPtr dom,
          qemuDomainStorageSourceAccessAllow(driver, vm, top_parent, false, false) < 0))
         goto endjob;
 
-    if (!(job = qemuBlockJobDiskNew(vm, disk, jobtype, device)))
+    if (!(job = qemuBlockJobDiskNewCommit(vm, disk, top_parent, topSource,
+                                          baseSource)))
         goto endjob;
 
     disk->mirrorState = VIR_DOMAIN_DISK_MIRROR_STATE_NONE;
@@ -18139,15 +18141,34 @@ qemuDomainBlockCommit(virDomainPtr dom,
      * depending on whether the input was specified as relative or
      * absolute (that is, our absolute top_canon may do the wrong
      * thing if the user specified a relative name).  */
+
+    if (blockdev) {
+        persistjob = true;
+        jobname = job->name;
+        nodetop = topSource->nodeformat;
+        nodebase = baseSource->nodeformat;
+        device = disk->src->nodeformat;
+        if (!backingPath && top_parent &&
+            !(backingPath = qemuBlockGetBackingStoreString(baseSource)))
+            goto endjob;
+    } else {
+        device = job->name;
+    }
+
     qemuDomainObjEnterMonitor(driver, vm);
-    basePath = qemuMonitorDiskNameLookup(priv->mon, device, disk->src,
+
-                                         baseSource);
+    if (!blockdev) {
-    topPath = qemuMonitorDiskNameLookup(priv->mon, device, disk->src,
+        basePath = qemuMonitorDiskNameLookup(priv->mon, device, disk->src,
-                                        topSource);
+                                             baseSource);
-    if (basePath && topPath)
+        topPath = qemuMonitorDiskNameLookup(priv->mon, device, disk->src,
-        ret = qemuMonitorBlockCommit(priv->mon, device, NULL, false,
+                                            topSource);
-                                     topPath, NULL, basePath, NULL, backingPath,
+    }
-                                     speed);
+
+    if (blockdev || (basePath && topPath))
+        ret = qemuMonitorBlockCommit(priv->mon, device, jobname, persistjob,
+                                     topPath, nodetop, basePath, nodebase,
+                                     backingPath, speed);
+
     if (qemuDomainObjExitMonitor(driver, vm) < 0 || ret < 0) {
         ret = -1;
         goto endjob;

tests/qemustatusxml2xmldata/blockjob-blockdev-in.xml

@@ -238,6 +238,17 @@
       <disk dst='vdb'/>
       <base node='libvirt-14-format'/>
     </blockjob>
+    <blockjob name='pull-vdd-libvirt-17-format' type='active-commit' state='ready'>
+      <disk dst='vdd'/>
+      <base node='libvirt-19-format'/>
+      <top node='libvirt-17-format'/>
+    </blockjob>
+    <blockjob name='pull-vdc-libvirt-9-format' type='commit' state='running'>
+      <disk dst='vdc'/>
+      <base node='libvirt-11-format'/>
+      <top node='libvirt-9-format'/>
+      <topparent node='libvirt-2-format'/>
+    </blockjob>
     <blockjob name='drive-virtio-disk0' type='copy' state='ready'>
       <disk dst='vda' mirror='yes'/>
     </blockjob>
@@ -581,6 +592,10 @@
             </backingStore>
           </backingStore>
         </backingStore>
+        <mirror type='file' job='active-commit' ready='yes'>
+          <format type='qcow2'/>
+          <source file='/tmp/activecommit2.qcow2'/>
+        </mirror>
         <target dev='vdd' bus='virtio'/>
         <alias name='virtio-disk3'/>
         <address type='pci' domain='0x0000' bus='0x00' slot='0x0e' function='0x0'/>