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 --- 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 cc7596bad1..f12450cc69 100644 --- 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 e021da51fc..9066f5d0f5 100644 --- 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 3a0e3b6cec..809863be57 100644 --- 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); -- 2.14.3