From 2570986655a119553082702dad8f256fb72ead75 Mon Sep 17 00:00:00 2001
Message-Id: <2570986655a119553082702dad8f256fb72ead75@dist-git>
From: Peter Krempa <pkrempa@redhat.com>
Date: Wed, 22 Nov 2017 18:21:35 +0100
Subject: [PATCH] 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
(cherry picked from commit 3746a38e7b9ae5342675547624122d55e73d6c81)
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
---
src/qemu/qemu_domain.c | 17 ++++++++++++++---
src/qemu/qemu_domain.h | 3 ++-
src/qemu/qemu_driver.c | 12 ++++++------
3 files changed, 22 insertions(+), 10 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 485e085cea..72d67adfe3 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -6074,15 +6074,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;
@@ -6095,7 +6105,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 ff5328277c..caf583373f 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -688,7 +688,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 6096e00b2a..b35ab6d30e 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -14578,7 +14578,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;
}
@@ -17173,7 +17173,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;
}
@@ -17566,9 +17566,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,
@@ -17612,9 +17612,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);
--
2.15.0