diff --git a/src/qemu/qemu_backup.c b/src/qemu/qemu_backup.c index 2cc6ff7a42..8b66ee8d1f 100644 --- a/src/qemu/qemu_backup.c +++ b/src/qemu/qemu_backup.c @@ -469,8 +469,8 @@ qemuBackupDiskPrepareOneStorage(virDomainObjPtr vm, dd->created = true; } - if (qemuDomainStorageSourceAccessAllow(priv->driver, vm, dd->store, false, - true) < 0) + if (qemuDomainStorageSourceAccessAllow(priv->driver, vm, dd->store, + false, true, true) < 0) return -1; dd->labelled = true; diff --git a/src/qemu/qemu_blockjob.c b/src/qemu/qemu_blockjob.c index 71df0d1ab2..e894e1634d 100644 --- a/src/qemu/qemu_blockjob.c +++ b/src/qemu/qemu_blockjob.c @@ -1105,9 +1105,11 @@ qemuBlockJobProcessEventCompletedCommit(virQEMUDriverPtr driver, return; /* revert access to images */ - qemuDomainStorageSourceAccessAllow(driver, vm, job->data.commit.base, true, false); + qemuDomainStorageSourceAccessAllow(driver, vm, job->data.commit.base, + true, false, false); if (job->data.commit.topparent != job->disk->src) - qemuDomainStorageSourceAccessAllow(driver, vm, job->data.commit.topparent, true, false); + qemuDomainStorageSourceAccessAllow(driver, vm, job->data.commit.topparent, + true, false, true); baseparent->backingStore = NULL; job->data.commit.topparent->backingStore = job->data.commit.base; diff --git a/src/qemu/qemu_checkpoint.c b/src/qemu/qemu_checkpoint.c index a387e7dfe7..ea87b09aa0 100644 --- a/src/qemu/qemu_checkpoint.c +++ b/src/qemu/qemu_checkpoint.c @@ -296,7 +296,8 @@ qemuCheckpointDiscardBitmaps(virDomainObjPtr vm, for (next = reopenimages; next; next = next->next) { virStorageSourcePtr src = next->data; - if (qemuDomainStorageSourceAccessAllow(driver, vm, src, false, false) < 0) + if (qemuDomainStorageSourceAccessAllow(driver, vm, src, + false, false, false) < 0) goto relabel; relabelimages = g_slist_prepend(relabelimages, src); @@ -311,7 +312,8 @@ qemuCheckpointDiscardBitmaps(virDomainObjPtr vm, for (next = relabelimages; next; next = next->next) { virStorageSourcePtr src = next->data; - ignore_value(qemuDomainStorageSourceAccessAllow(driver, vm, src, true, false)); + ignore_value(qemuDomainStorageSourceAccessAllow(driver, vm, src, + true, false, false)); } return rc; diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 33c2158eb5..3d3f796d85 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -11668,6 +11668,8 @@ typedef enum { QEMU_DOMAIN_STORAGE_SOURCE_ACCESS_SKIP_REVOKE = 1 << 4, /* VM already has access to the source and we are just modifying it */ QEMU_DOMAIN_STORAGE_SOURCE_ACCESS_MODIFY_ACCESS = 1 << 5, + /* whether the image is the top image of the backing chain (e.g. disk source) */ + QEMU_DOMAIN_STORAGE_SOURCE_ACCESS_CHAIN_TOP = 1 << 6, } qemuDomainStorageSourceAccessFlags; @@ -11745,6 +11747,7 @@ qemuDomainStorageSourceAccessModify(virQEMUDriverPtr driver, bool force_ro = flags & QEMU_DOMAIN_STORAGE_SOURCE_ACCESS_FORCE_READ_ONLY; bool force_rw = flags & QEMU_DOMAIN_STORAGE_SOURCE_ACCESS_FORCE_READ_WRITE; bool revoke = flags & QEMU_DOMAIN_STORAGE_SOURCE_ACCESS_REVOKE; + bool chain_top = flags & QEMU_DOMAIN_STORAGE_SOURCE_ACCESS_CHAIN_TOP; int rc; bool was_readonly = src->readonly; bool revoke_cgroup = false; @@ -11791,7 +11794,7 @@ qemuDomainStorageSourceAccessModify(virQEMUDriverPtr driver, revoke_namespace = true; } - if (qemuSecuritySetImageLabel(driver, vm, src, chain) < 0) + if (qemuSecuritySetImageLabel(driver, vm, src, chain, chain_top) < 0) goto revoke; revoke_label = true; @@ -11854,7 +11857,8 @@ qemuDomainStorageSourceChainAccessAllow(virQEMUDriverPtr driver, virDomainObjPtr vm, virStorageSourcePtr src) { - qemuDomainStorageSourceAccessFlags flags = QEMU_DOMAIN_STORAGE_SOURCE_ACCESS_CHAIN; + qemuDomainStorageSourceAccessFlags flags = QEMU_DOMAIN_STORAGE_SOURCE_ACCESS_CHAIN | + QEMU_DOMAIN_STORAGE_SOURCE_ACCESS_CHAIN_TOP; return qemuDomainStorageSourceAccessModify(driver, vm, src, flags); } @@ -11866,7 +11870,8 @@ qemuDomainStorageSourceChainAccessRevoke(virQEMUDriverPtr driver, virStorageSourcePtr src) { qemuDomainStorageSourceAccessFlags flags = QEMU_DOMAIN_STORAGE_SOURCE_ACCESS_REVOKE | - QEMU_DOMAIN_STORAGE_SOURCE_ACCESS_CHAIN; + QEMU_DOMAIN_STORAGE_SOURCE_ACCESS_CHAIN | + QEMU_DOMAIN_STORAGE_SOURCE_ACCESS_CHAIN_TOP; return qemuDomainStorageSourceAccessModify(driver, vm, src, flags); } @@ -11896,6 +11901,7 @@ qemuDomainStorageSourceAccessRevoke(virQEMUDriverPtr driver, * @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 + * @chainTop: @elem is top parent of backing chain * * 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 @@ -11903,13 +11909,20 @@ qemuDomainStorageSourceAccessRevoke(virQEMUDriverPtr driver, * * When modifying permissions of @elem which @vm can already access (is in the * backing chain) @newSource needs to be set to false. + * + * The @chainTop flag must be set if the @elem image is the topmost image of a + * given backing chain or meant to become the topmost image (for e.g. + * snapshots, or blockcopy or even in the end for active layer block commit, + * where we discard the top of the backing chain so one of the intermediates + * (the base) becomes the top of the chain). */ int qemuDomainStorageSourceAccessAllow(virQEMUDriverPtr driver, virDomainObjPtr vm, virStorageSourcePtr elem, bool readonly, - bool newSource) + bool newSource, + bool chainTop) { qemuDomainStorageSourceAccessFlags flags = QEMU_DOMAIN_STORAGE_SOURCE_ACCESS_SKIP_REVOKE; @@ -11921,6 +11934,9 @@ qemuDomainStorageSourceAccessAllow(virQEMUDriverPtr driver, if (!newSource) flags |= QEMU_DOMAIN_STORAGE_SOURCE_ACCESS_MODIFY_ACCESS; + if (chainTop) + flags |= QEMU_DOMAIN_STORAGE_SOURCE_ACCESS_CHAIN_TOP; + return qemuDomainStorageSourceAccessModify(driver, vm, elem, flags); } diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index 476056c73f..3929ee9ca1 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -896,7 +896,8 @@ int qemuDomainStorageSourceAccessAllow(virQEMUDriverPtr driver, virDomainObjPtr vm, virStorageSourcePtr elem, bool readonly, - bool newSource); + bool newSource, + bool chainTop); int qemuDomainPrepareStorageSourceBlockdev(virDomainDiskDefPtr disk, virStorageSourcePtr src, diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 3707448f49..cd761f87b5 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -15142,7 +15142,8 @@ qemuDomainSnapshotDiskPrepareOne(virQEMUDriverPtr driver, } /* set correct security, cgroup and locking options on the new image */ - if (qemuDomainStorageSourceAccessAllow(driver, vm, dd->src, false, true) < 0) + if (qemuDomainStorageSourceAccessAllow(driver, vm, dd->src, + false, true, true) < 0) return -1; dd->prepared = true; @@ -18490,11 +18491,19 @@ qemuDomainBlockCommit(virDomainPtr dom, * operation succeeds, but doing that requires tracking the * operation in XML across libvirtd restarts. */ clean_access = true; - if (qemuDomainStorageSourceAccessAllow(driver, vm, baseSource, false, false) < 0 || - (top_parent && top_parent != disk->src && - qemuDomainStorageSourceAccessAllow(driver, vm, top_parent, false, false) < 0)) + if (qemuDomainStorageSourceAccessAllow(driver, vm, baseSource, + false, false, false) < 0) goto endjob; + if (top_parent && top_parent != disk->src) { + /* While top_parent is topmost image, we don't need to remember its + * owner as it will be overwritten upon finishing the commit. Hence, + * pass chainTop = false. */ + if (qemuDomainStorageSourceAccessAllow(driver, vm, top_parent, + false, false, false) < 0) + goto endjob; + } + if (!(job = qemuBlockJobDiskNewCommit(vm, disk, top_parent, topSource, baseSource, flags & VIR_DOMAIN_BLOCK_COMMIT_DELETE, @@ -18552,9 +18561,11 @@ qemuDomainBlockCommit(virDomainPtr dom, virErrorPtr orig_err; virErrorPreserveLast(&orig_err); /* Revert access to read-only, if possible. */ - qemuDomainStorageSourceAccessAllow(driver, vm, baseSource, true, false); + qemuDomainStorageSourceAccessAllow(driver, vm, baseSource, + true, false, false); if (top_parent && top_parent != disk->src) - qemuDomainStorageSourceAccessAllow(driver, vm, top_parent, true, false); + qemuDomainStorageSourceAccessAllow(driver, vm, top_parent, + true, false, false); virErrorRestore(&orig_err); } diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index bec822a2ae..499d39a920 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -7856,7 +7856,7 @@ qemuProcessRefreshLegacyBlockjob(void *payload, (qemuDomainNamespaceSetupDisk(vm, disk->mirror) < 0 || qemuSetupImageChainCgroup(vm, disk->mirror) < 0 || qemuSecuritySetImageLabel(priv->driver, vm, disk->mirror, - true) < 0)) + true, true) < 0)) goto cleanup; } } diff --git a/src/qemu/qemu_security.c b/src/qemu/qemu_security.c index 2aa2b5b9c6..484fc34552 100644 --- a/src/qemu/qemu_security.c +++ b/src/qemu/qemu_security.c @@ -98,7 +98,8 @@ int qemuSecuritySetImageLabel(virQEMUDriverPtr driver, virDomainObjPtr vm, virStorageSourcePtr src, - bool backingChain) + bool backingChain, + bool chainTop) { qemuDomainObjPrivatePtr priv = vm->privateData; pid_t pid = -1; @@ -108,6 +109,9 @@ qemuSecuritySetImageLabel(virQEMUDriverPtr driver, if (backingChain) labelFlags |= VIR_SECURITY_DOMAIN_IMAGE_LABEL_BACKING_CHAIN; + if (chainTop) + labelFlags |= VIR_SECURITY_DOMAIN_IMAGE_PARENT_CHAIN_TOP; + if (qemuDomainNamespaceEnabled(vm, QEMU_DOMAIN_NS_MOUNT)) pid = vm->pid; diff --git a/src/qemu/qemu_security.h b/src/qemu/qemu_security.h index a8c648ece1..c8516005ac 100644 --- a/src/qemu/qemu_security.h +++ b/src/qemu/qemu_security.h @@ -36,7 +36,8 @@ void qemuSecurityRestoreAllLabel(virQEMUDriverPtr driver, int qemuSecuritySetImageLabel(virQEMUDriverPtr driver, virDomainObjPtr vm, virStorageSourcePtr src, - bool backingChain); + bool backingChain, + bool chainTop); int qemuSecurityRestoreImageLabel(virQEMUDriverPtr driver, virDomainObjPtr vm,