qemu: domain: Don't call namespace setup for storage already accessed by vm

When doing block commit we need to allow write for members of the
backing chain so that we can commit the data into them.

qemuDomainDiskChainElementPrepare was used for this which since commit
786d8d91b4 calls qemuDomainNamespaceSetupDisk which has very adverse
side-effects, namely it relabels the nodes to the same label it has in
the main namespace. This was messing up permissions for the commit
operation since its touching various parts of a single backing chain.

Since we are are actually not introducing new images at that point add a
flag for qemuDomainDiskChainElementPrepare which will refrain from
calling to the namespace setup function.

Calls from qemuDomainSnapshotCreateSingleDiskActive and
qemuDomainBlockCopyCommon do introduce new members all calls from
qemuDomainBlockCommit do not, so the calls are anotated accordingly.

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1506072

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index cc7596bad108ea3a2c369378e7b9deb615034f61..f12450cc69b97682010fbbf605d15600d4292123 100644 (file)
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -6146,15 +6146,25 @@ qemuDomainDiskChainElementRevoke(virQEMUDriverPtr driver,
 
 /**
  * qemuDomainDiskChainElementPrepare:
+ * @driver: qemu driver data
+ * @vm: domain object
+ * @elem: source structure to set access for
+ * @readonly: setup read-only access if true
+ * @newSource: @elem describes a storage source which @vm can't access yet
  *
  * Allow a VM access to a single element of a disk backing chain; this helper
  * ensures that the lock manager, cgroup device controller, and security manager
- * labelling are all aware of each new file before it is added to a chain */
+ * labelling are all aware of each new file before it is added to a chain.
+ *
+ * When modifying permissions of @elem which @vm can already access (is in the
+ * backing chain) @newSource needs to be set to false.
+ */
 int
 qemuDomainDiskChainElementPrepare(virQEMUDriverPtr driver,
                                   virDomainObjPtr vm,
                                   virStorageSourcePtr elem,
-                                  bool readonly)
+                                  bool readonly,
+                                  bool newSource)
 {
     bool was_readonly = elem->readonly;
     virQEMUDriverConfigPtr cfg = NULL;
@@ -6167,7 +6177,8 @@ qemuDomainDiskChainElementPrepare(virQEMUDriverPtr driver,
     if (virDomainLockImageAttach(driver->lockManager, cfg->uri, vm, elem) < 0)
         goto cleanup;
 
-    if (qemuDomainNamespaceSetupDisk(driver, vm, elem) < 0)
+    if (newSource &&
+        qemuDomainNamespaceSetupDisk(driver, vm, elem) < 0)
         goto cleanup;
 
     if (qemuSetupImageCgroup(vm, elem) < 0)

diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index e021da51fc8e3825c93f1ab9e5c5c6a9bdb79ded..9066f5d0f54fcaf2494f9a57695b0b7d8ad433f3 100644 (file)
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -689,7 +689,8 @@ void qemuDomainDiskChainElementRevoke(virQEMUDriverPtr driver,
 int qemuDomainDiskChainElementPrepare(virQEMUDriverPtr driver,
                                       virDomainObjPtr vm,
                                       virStorageSourcePtr elem,
-                                      bool readonly);
+                                      bool readonly,
+                                      bool newSource);
 
 int qemuDomainCleanupAdd(virDomainObjPtr vm,
                          qemuDomainCleanupCallback cb);

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 3a0e3b6cec98bb72bd01d527d72e7e6715271506..809863be57fe33d6e3f81f789d6768fa4a4b34cf 100644 (file)
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -14570,7 +14570,7 @@ qemuDomainSnapshotCreateSingleDiskActive(virQEMUDriverPtr driver,
     }
 
     /* set correct security, cgroup and locking options on the new image */
-    if (qemuDomainDiskChainElementPrepare(driver, vm, dd->src, false) < 0) {
+    if (qemuDomainDiskChainElementPrepare(driver, vm, dd->src, false, true) < 0) {
         qemuDomainDiskChainElementRevoke(driver, vm, dd->src);
         goto cleanup;
     }
@@ -17165,7 +17165,7 @@ qemuDomainBlockCopyCommon(virDomainObjPtr vm,
                                          keepParentLabel) < 0)
         goto endjob;
 
-    if (qemuDomainDiskChainElementPrepare(driver, vm, mirror, false) < 0) {
+    if (qemuDomainDiskChainElementPrepare(driver, vm, mirror, false, true) < 0) {
         qemuDomainDiskChainElementRevoke(driver, vm, mirror);
         goto endjob;
     }
@@ -17558,9 +17558,9 @@ qemuDomainBlockCommit(virDomainPtr dom,
      * operation succeeds, but doing that requires tracking the
      * operation in XML across libvirtd restarts.  */
     clean_access = true;
-    if (qemuDomainDiskChainElementPrepare(driver, vm, baseSource, false) < 0 ||
+    if (qemuDomainDiskChainElementPrepare(driver, vm, baseSource, false, false) < 0 ||
         (top_parent && top_parent != disk->src &&
-         qemuDomainDiskChainElementPrepare(driver, vm, top_parent, false) < 0))
+         qemuDomainDiskChainElementPrepare(driver, vm, top_parent, false, false) < 0))
         goto endjob;
 
     /* Start the commit operation.  Pass the user's original spelling,
@@ -17604,9 +17604,9 @@ qemuDomainBlockCommit(virDomainPtr dom,
     if (ret < 0 && clean_access) {
         virErrorPtr orig_err = virSaveLastError();
         /* Revert access to read-only, if possible.  */
-        qemuDomainDiskChainElementPrepare(driver, vm, baseSource, true);
+        qemuDomainDiskChainElementPrepare(driver, vm, baseSource, true, false);
         if (top_parent && top_parent != disk->src)
-            qemuDomainDiskChainElementPrepare(driver, vm, top_parent, true);
+            qemuDomainDiskChainElementPrepare(driver, vm, top_parent, true, false);
 
         if (orig_err) {
             virSetError(orig_err);