[PATCHv8 11/11] blockjob: relabel entire existing chain

[Eric Blake <eblake@xxxxxxxxxx>]

When using block copy to pivot over to a new chain, the backing files
for the new chain might still need labeling (particularly if the user
passes --reuse-ext with a relative backing file name).  Relabeling a
file that is already labeled won't hurt, so this just labels the entire
chain at the point of the pivot.  But to do the relabel, we must probe
the file type of an external file (the API already promised to do this,
and the probe is safe; we are just moving the probe from qemu into
libvirt).

* src/qemu/qemu_driver.c (qemuDomainBlockPivot): Relabel chain before
asking qemu to pivot.
---

v8: rebase on top of block-commit work

 src/qemu/qemu_driver.c | 49 +++++++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 45 insertions(+), 4 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index f2ff931..3316e4b 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -12550,6 +12550,10 @@ qemuDomainBlockPivot(virConnectPtr conn,
     bool reopen = qemuCapsGet(priv->caps, QEMU_CAPS_DRIVE_REOPEN);
     const char *format = virStorageFileFormatTypeToString(disk->mirrorFormat);
     bool resume = false;
+    virCgroupPtr cgroup = NULL;
+    char *oldsrc = NULL;
+    int oldformat;
+    virStorageFileMetadataPtr oldchain = NULL;

     /* Probe the status, if needed.  */
     if (!disk->mirroring) {
@@ -12599,6 +12603,39 @@ qemuDomainBlockPivot(virConnectPtr conn,
         }
     }

+    /* We previously labeled only the top-level image; but if the
+     * image includes a relative backing file, the pivot may result in
+     * qemu needing to open the entire backing chain, so we need to
+     * label the entire chain.  This action is safe even if the
+     * backing chain has already been labeled; but only necessary when
+     * we know for sure that there is a backing chain.  */
+    if (disk->mirrorFormat && disk->mirrorFormat != VIR_STORAGE_FILE_RAW &&
+        qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_DEVICES) &&
+        virCgroupForDomain(driver->cgroup, vm->def->name, &cgroup, 0) < 0) {
+        virReportError(VIR_ERR_INTERNAL_ERROR,
+                       _("Unable to find cgroup for %s"),
+                       vm->def->name);
+        goto cleanup;
+    }
+    oldsrc = disk->src;
+    oldformat = disk->format;
+    oldchain = disk->backingChain;
+    disk->src = disk->mirror;
+    disk->format = disk->mirrorFormat;
+    disk->backingChain = NULL;
+    if (qemuDomainDetermineDiskChain(driver, disk, false) < 0)
+        goto cleanup;
+    if (disk->mirrorFormat && disk->mirrorFormat != VIR_STORAGE_FILE_RAW &&
+        (virDomainLockDiskAttach(driver->lockManager, driver->uri,
+                                 vm, disk) < 0 ||
+         (cgroup && qemuSetupDiskCgroup(vm, cgroup, disk) < 0) ||
+         virSecurityManagerSetImageLabel(driver->securityManager, vm->def,
+                                         disk) < 0)) {
+        disk->src = oldsrc;
+        disk->format = oldformat;
+        goto cleanup;
+    }
+
     /* Attempt the pivot.  */
     qemuDomainObjEnterMonitorWithDriver(driver, vm);
     ret = qemuMonitorDrivePivot(priv->mon, device, disk->mirror, format,
@@ -12618,13 +12655,11 @@ qemuDomainBlockPivot(virConnectPtr conn,
          * portion of the chain, and is made more difficult by the
          * fact that we aren't tracking the full chain ourselves; so
          * for now, we leak the access to the original.  */
-        VIR_FREE(disk->src);
-        disk->src = disk->mirror;
-        disk->format = disk->mirrorFormat;
+        VIR_FREE(oldsrc);
+        virStorageFileFreeMetadata(oldchain);
         disk->mirror = NULL;
         disk->mirrorFormat = VIR_STORAGE_FILE_NONE;
         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
@@ -12633,12 +12668,18 @@ qemuDomainBlockPivot(virConnectPtr conn,
          * 'query-block', to see what state we really got left in
          * before killing the mirroring job?  And just as on the
          * success case, there's security labeling to worry about.  */
+        disk->src = oldsrc;
+        disk->format = oldformat;
+        virStorageFileFreeMetadata(disk->backingChain);
+        disk->backingChain = oldchain;
         VIR_FREE(disk->mirror);
         disk->mirrorFormat = VIR_STORAGE_FILE_NONE;
         disk->mirroring = false;
     }

 cleanup:
+    if (cgroup)
+        virCgroupFree(&cgroup);
     if (resume && virDomainObjIsActive(vm) &&
         qemuProcessStartCPUs(driver, vm, conn,
                              VIR_DOMAIN_RUNNING_UNPAUSED,
-- 
1.7.11.7

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