qemu: Label backing chain of user-provided target of blockCopy when starting the job

Be more sensible when setting labels of the target of a
virDomainBlockCopy operation. Previously we'd relabel everything in case
it's a copy job even if there's no unlabelled backing chain. Since we
are also not sure whether the backing chain is shared we don't relabel
the chain on completion of the blockjob. This certainly won't play nice
with the image permission relabelling feature.

While this does not fix the case where the image is reused and has
backing chain it certainly sanitizes all the other cases. Later on it
will also allow to do the correct thing in cases where only one layer
was introduced.

The change is necessary as in case when -blockdev will be used we will
need to hotplug the backing chain and thus labeling needs to be setup in
advance and not only at the time of pivot.  To avoid multiple code paths
move the labeling now.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 007c46fd992af86892f485f8e1fdabc11a2bf155..96cb108a0ed7f9662b4fb23f9bf53d89f414a4b7 100644 (file)
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -17150,26 +17150,6 @@ qemuDomainBlockPivot(virQEMUDriverPtr driver,
         goto cleanup;
     }
 
-    /* For active commit, the mirror is part of the already labeled
-     * chain.  For blockcopy, we previously labeled only the top-level
-     * image; but if the user is reusing an external image that
-     * includes a 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->mirrorJob == VIR_DOMAIN_BLOCK_JOB_TYPE_COPY) {
-        if (qemuDomainDetermineDiskChain(driver, vm, disk, disk->mirror, true) < 0)
-            goto cleanup;
-
-        if (disk->mirror->format &&
-            disk->mirror->format != VIR_STORAGE_FILE_RAW &&
-            (qemuDomainNamespaceSetupDisk(vm, disk->mirror) < 0 ||
-             qemuSetupImageChainCgroup(vm, disk->mirror) < 0 ||
-             qemuSecuritySetImageLabel(driver, vm, disk->mirror, true) < 0))
-            goto cleanup;
-    }
-
     /* Attempt the pivot.  Record the attempt now, to prevent duplicate
      * attempts; but the actual disk change will be made when emitting
      * the event.
@@ -17816,9 +17796,28 @@ qemuDomainBlockCopyCommon(virDomainObjPtr vm,
                                          keepParentLabel) < 0)
         goto endjob;
 
-    if (qemuDomainDiskChainElementPrepare(driver, vm, mirror, false, true) < 0) {
-        qemuDomainDiskChainElementRevoke(driver, vm, mirror);
+    /* If reusing an external image that includes a backing file, the pivot may
+     * result in qemu needing to open the entire backing chain, so we need to
+     * label the full backing chain of the mirror instead of just the top image */
+    if (flags & VIR_DOMAIN_BLOCK_COPY_REUSE_EXT &&
+        mirror->format >= VIR_STORAGE_FILE_BACKING &&
+        qemuDomainDetermineDiskChain(driver, vm, disk, mirror, true) < 0)
         goto endjob;
+
+    if (flags & VIR_DOMAIN_BLOCK_COPY_REUSE_EXT &&
+        virStorageSourceHasBacking(mirror)) {
+        /* note that we don't really know whether a part of the backing chain
+         * is shared so rolling this back is not as easy. Thus we do it only
+         * if there's a backing chain */
+        if (qemuDomainNamespaceSetupDisk(vm, mirror) < 0 ||
+            qemuSetupImageChainCgroup(vm, mirror) < 0 ||
+            qemuSecuritySetImageLabel(driver, vm, mirror, true) < 0)
+            goto endjob;
+    } else {
+        if (qemuDomainDiskChainElementPrepare(driver, vm, mirror, false, true) < 0) {
+            qemuDomainDiskChainElementRevoke(driver, vm, mirror);
+            goto endjob;
+        }
     }
 
     if (!(job = qemuBlockJobDiskNew(disk, QEMU_BLOCKJOB_TYPE_COPY, device)))

diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index fa83d9fc0e3ec63f1dd490b5c33f5982641615d2..7592c98b74d64c2d558290cc8519188d2b34e3bd 100644 (file)
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -7856,6 +7856,7 @@ qemuProcessRefreshLegacyBlockjob(void *payload,
     virDomainDiskDefPtr disk;
     qemuBlockJobDataPtr job;
     qemuBlockJobType jobtype = info->type;
+    qemuDomainObjPrivatePtr priv = vm->privateData;
 
     if (!(disk = qemuProcessFindDomainDiskByAliasOrQOM(vm, jobname, jobname))) {
         VIR_DEBUG("could not find disk for block job '%s'", jobname);
@@ -7877,8 +7878,29 @@ qemuProcessRefreshLegacyBlockjob(void *payload,
             disk->mirrorState = VIR_DOMAIN_DISK_MIRROR_STATE_READY;
             job->state = VIR_DOMAIN_BLOCK_JOB_READY;
         }
+
+        /* Pre-blockdev block copy labelled the chain of the mirrored device
+         * just before pivoting. At that point it was no longer known whether
+         * it's even necessary (e.g. disk is being reused). This code fixes
+         * the labelling in case the job was started in a libvirt version
+         * which did not label the chain when the block copy is being started.
+         * Note that we can't do much on failure. */
+        if (disk->mirrorJob == VIR_DOMAIN_BLOCK_JOB_TYPE_COPY) {
+            if (qemuDomainDetermineDiskChain(priv->driver, vm, disk,
+                                             disk->mirror, true) < 0)
+                goto cleanup;
+
+            if (disk->mirror->format &&
+                disk->mirror->format != VIR_STORAGE_FILE_RAW &&
+                (qemuDomainNamespaceSetupDisk(vm, disk->mirror) < 0 ||
+                 qemuSetupImageChainCgroup(vm, disk->mirror) < 0 ||
+                 qemuSecuritySetImageLabel(priv->driver, vm, disk->mirror,
+                                           true) < 0))
+                goto cleanup;
+        }
     }
 
+ cleanup:
     qemuBlockJobStartupFinalize(job);
 
     return 0;