9:42 a.m.
With new blockjob handling we'll need to modify permissions for chains
and individual images. The individual image code was universally
accessible but the chain setting code reimplemented it mostly only in
qemu_hotplug.h.
Refactor the handling by moving the code to qemu_domain.c and making it
universal.
Peter Krempa (12):
qemu: Rename qemuDomainDiskChainElement(Revoke|Prepare)
qemu: Move and rename qemuHotplugPrepareDiskSourceAccess
qemu: Split entry points to qemuDomainStorageSourceChainAccessPrepare
qemu: domain: Rename qemuDomainStorageSourceChainAccessPrepare
qemu: Convert boolean flags to enum flags in
qemuDomainStorageSourceAccessModify
qemu: Allow using qemuDomainStorageSourceAccessModify on singe images
qemu: Refactor/simplify qemuDomainStorageSourceAccessRevoke
qemu: Allow forcing read-only mode in
qemuDomainStorageSourceAccessModify
qemu: Use bools rather than labels in
qemuDomainStorageSourceAccessModify
qemu: Allow skipping the revoke step in
qemuDomainStorageSourceAccessModify
qemu: Mark when modifying access to existing source in
qemuDomainStorageSourceAccessModify
qemu: Refactor/simplify qemuDomainStorageSourceAccessAllow
src/qemu/qemu_domain.c | 212 +++++++++++++++++++++++++++++++---------
src/qemu/qemu_domain.h | 23 +++--
src/qemu/qemu_driver.c | 24 ++---
src/qemu/qemu_hotplug.c | 80 ++-------------
4 files changed, 200 insertions(+), 139 deletions(-)
--
2.20.1
9:42 a.m.
New subject: [libvirt] [PATCH 01/12] qemu: Rename qemuDomainDiskChainElement(Revoke|Prepare)
Use qemuDomainStorageSourceAccess(Allow|Revoke) instead.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_domain.c | 20 ++++++++++----------
src/qemu/qemu_domain.h | 16 ++++++++--------
src/qemu/qemu_driver.c | 24 ++++++++++++------------
3 files changed, 30 insertions(+), 30 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 960aaff3c7..ce8f4a0dca 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -9212,15 +9212,15 @@ qemuDomainDiskGetBackendAlias(virDomainDiskDefPtr disk,
/**
- * qemuDomainDiskChainElementRevoke:
+ * qemuDomainStorageSourceAccessRevoke:
*
* Revoke access to a single backing chain element. This restores the labels,
* removes cgroup ACLs for devices and removes locks.
*/
void
-qemuDomainDiskChainElementRevoke(virQEMUDriverPtr driver,
- virDomainObjPtr vm,
- virStorageSourcePtr elem)
+qemuDomainStorageSourceAccessRevoke(virQEMUDriverPtr driver,
+ virDomainObjPtr vm,
+ virStorageSourcePtr elem)
{
if (qemuTeardownImageCgroup(vm, elem) < 0)
VIR_WARN("Failed to teardown cgroup for disk path %s",
@@ -9238,7 +9238,7 @@ qemuDomainDiskChainElementRevoke(virQEMUDriverPtr driver,
/**
- * qemuDomainDiskChainElementPrepare:
+ * qemuDomainStorageSourceAccessAllow:
* @driver: qemu driver data
* @vm: domain object
* @elem: source structure to set access for
@@ -9253,11 +9253,11 @@ qemuDomainDiskChainElementRevoke(virQEMUDriverPtr driver,
* backing chain) @newSource needs to be set to false.
*/
int
-qemuDomainDiskChainElementPrepare(virQEMUDriverPtr driver,
- virDomainObjPtr vm,
- virStorageSourcePtr elem,
- bool readonly,
- bool newSource)
+qemuDomainStorageSourceAccessAllow(virQEMUDriverPtr driver,
+ virDomainObjPtr vm,
+ virStorageSourcePtr elem,
+ bool readonly,
+ bool newSource)
{
bool was_readonly = elem->readonly;
virQEMUDriverConfigPtr cfg = NULL;
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index 06640a9510..9a48b5b69d 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -770,14 +770,14 @@ int qemuDomainDiskGetBackendAlias(virDomainDiskDefPtr disk,
ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2)
ATTRIBUTE_NONNULL(3) ATTRIBUTE_RETURN_CHECK;
-void qemuDomainDiskChainElementRevoke(virQEMUDriverPtr driver,
- virDomainObjPtr vm,
- virStorageSourcePtr elem);
-int qemuDomainDiskChainElementPrepare(virQEMUDriverPtr driver,
- virDomainObjPtr vm,
- virStorageSourcePtr elem,
- bool readonly,
- bool newSource);
+void qemuDomainStorageSourceAccessRevoke(virQEMUDriverPtr driver,
+ virDomainObjPtr vm,
+ virStorageSourcePtr elem);
+int qemuDomainStorageSourceAccessAllow(virQEMUDriverPtr driver,
+ virDomainObjPtr vm,
+ virStorageSourcePtr elem,
+ 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 c443c881d5..b038bfc360 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -15025,7 +15025,7 @@ struct _qemuDomainSnapshotDiskData {
virStorageSourcePtr src;
bool initialized; /* @src was initialized in the storage driver */
bool created; /* @src was created by the snapshot code */
- bool prepared; /* @src was prepared using qemuDomainDiskChainElementPrepare */
+ bool prepared; /* @src was prepared using qemuDomainStorageSourceAccessAllow */
virDomainDiskDefPtr disk;
char *relPath; /* relative path component to fill into original disk */
@@ -15056,7 +15056,7 @@ qemuDomainSnapshotDiskDataFree(qemuDomainSnapshotDiskDataPtr
data,
virStorageFileDeinit(data[i].src);
if (data[i].prepared)
- qemuDomainDiskChainElementRevoke(driver, vm, data[i].src);
+ qemuDomainStorageSourceAccessRevoke(driver, vm, data[i].src);
virObjectUnref(data[i].src);
}
@@ -15216,8 +15216,8 @@ qemuDomainSnapshotCreateSingleDiskActive(virQEMUDriverPtr driver,
}
/* set correct security, cgroup and locking options on the new image */
- if (qemuDomainDiskChainElementPrepare(driver, vm, dd->src, false, true) < 0) {
- qemuDomainDiskChainElementRevoke(driver, vm, dd->src);
+ if (qemuDomainStorageSourceAccessAllow(driver, vm, dd->src, false, true) < 0)
{
+ qemuDomainStorageSourceAccessRevoke(driver, vm, dd->src);
goto cleanup;
}
@@ -15314,7 +15314,7 @@ qemuDomainSnapshotCreateDiskActive(virQEMUDriverPtr driver,
continue;
if (diskdata[i].prepared)
- qemuDomainDiskChainElementRevoke(driver, vm, diskdata[i].src);
+ qemuDomainStorageSourceAccessRevoke(driver, vm, diskdata[i].src);
if (diskdata[i].created &&
virStorageFileUnlink(diskdata[i].src) < 0)
@@ -17752,8 +17752,8 @@ qemuDomainBlockCopyCommon(virDomainObjPtr vm,
qemuSecuritySetImageLabel(driver, vm, mirror, true) < 0)
goto endjob;
} else {
- if (qemuDomainDiskChainElementPrepare(driver, vm, mirror, false, true) < 0) {
- qemuDomainDiskChainElementRevoke(driver, vm, mirror);
+ if (qemuDomainStorageSourceAccessAllow(driver, vm, mirror, false, true) < 0)
{
+ qemuDomainStorageSourceAccessRevoke(driver, vm, mirror);
goto endjob;
}
}
@@ -17774,7 +17774,7 @@ qemuDomainBlockCopyCommon(virDomainObjPtr vm,
ret = -1;
if (ret < 0) {
monitor_error = virSaveLastError();
- qemuDomainDiskChainElementRevoke(driver, vm, mirror);
+ qemuDomainStorageSourceAccessRevoke(driver, vm, mirror);
goto endjob;
}
@@ -18149,9 +18149,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, false) < 0
||
+ if (qemuDomainStorageSourceAccessAllow(driver, vm, baseSource, false, false) < 0
||
(top_parent && top_parent != disk->src &&
- qemuDomainDiskChainElementPrepare(driver, vm, top_parent, false, false) <
0))
+ qemuDomainStorageSourceAccessAllow(driver, vm, top_parent, false, false) <
0))
goto endjob;
if (!(job = qemuBlockJobDiskNew(disk, jobtype, device)))
@@ -18192,9 +18192,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, false);
+ qemuDomainStorageSourceAccessAllow(driver, vm, baseSource, true, false);
if (top_parent && top_parent != disk->src)
- qemuDomainDiskChainElementPrepare(driver, vm, top_parent, true, false);
+ qemuDomainStorageSourceAccessAllow(driver, vm, top_parent, true, false);
if (orig_err) {
virSetError(orig_err);
--
2.20.1
10:01 a.m.
New subject: [libvirt] [PATCH 01/12] qemu: Rename qemuDomainDiskChainElement(Revoke|Prepare)
On Thu, Apr 18, 2019 at 04:42:56PM +0200, Peter Krempa wrote:
Use qemuDomainStorageSourceAccess(Allow|Revoke) instead.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_domain.c | 20 ++++++++++----------
src/qemu/qemu_domain.h | 16 ++++++++--------
src/qemu/qemu_driver.c | 24 ++++++++++++------------
3 files changed, 30 insertions(+), 30 deletions(-)
Reviewed-by: Ján Tomko <jtomko(a)redhat.com>
Jano
9:42 a.m.
New subject: [libvirt] [PATCH 02/12] qemu: Move and rename qemuHotplugPrepareDiskSourceAccess
Move it to qemu_domain.c and call it
qemuDomainStorageSourceChainAccessPrepare.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_domain.c | 68 +++++++++++++++++++++++++++++++++++
src/qemu/qemu_domain.h | 5 +++
src/qemu/qemu_hotplug.c | 80 ++++-------------------------------------
3 files changed, 79 insertions(+), 74 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index ce8f4a0dca..ab96688a65 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -9211,6 +9211,74 @@ qemuDomainDiskGetBackendAlias(virDomainDiskDefPtr disk,
}
+/**
+ * qemuDomainStorageSourceChainAccessPrepare:
+ * @driver: qemu driver struct
+ * @vm: domain object
+ * @src: Source to prepare
+ * @teardown: Teardown the access to @src instead of adding it to a vm
+ *
+ * Setup the locks, cgroups and security permissions on a disk source and its
+ * backing chain. If @teardown is true, then the labels and cgroups are removed
+ * instead.
+ *
+ * Returns 0 on success and -1 on error. Reports libvirt error.
+ */
+int
+qemuDomainStorageSourceChainAccessPrepare(virQEMUDriverPtr driver,
+ virDomainObjPtr vm,
+ virStorageSourcePtr src,
+ bool teardown)
+{
+ VIR_AUTOUNREF(virQEMUDriverConfigPtr) cfg = virQEMUDriverGetConfig(driver);
+ const char *srcstr = NULLSTR(src->path);
+ int ret = -1;
+ virErrorPtr orig_err = NULL;
+
+ /* just tear down the disk access */
+ if (teardown) {
+ virErrorPreserveLast(&orig_err);
+ ret = 0;
+ goto rollback_cgroup;
+ }
+
+ if (virDomainLockImageAttach(driver->lockManager, cfg->uri, vm, src) < 0)
+ goto cleanup;
+
+ if (qemuDomainNamespaceSetupDisk(vm, src) < 0)
+ goto rollback_lock;
+
+ if (qemuSecuritySetImageLabel(driver, vm, src, true) < 0)
+ goto rollback_namespace;
+
+ if (qemuSetupImageChainCgroup(vm, src) < 0)
+ goto rollback_label;
+
+ ret = 0;
+ goto cleanup;
+
+ rollback_cgroup:
+ if (qemuTeardownImageChainCgroup(vm, src) < 0)
+ VIR_WARN("Unable to tear down cgroup access on %s", srcstr);
+ rollback_label:
+ if (qemuSecurityRestoreImageLabel(driver, vm, src, true) < 0)
+ VIR_WARN("Unable to restore security label on %s", srcstr);
+
+ rollback_namespace:
+ if (qemuDomainNamespaceTeardownDisk(vm, src) < 0)
+ VIR_WARN("Unable to remove /dev entry for %s", srcstr);
+
+ rollback_lock:
+ if (virDomainLockImageDetach(driver->lockManager, vm, src) < 0)
+ VIR_WARN("Unable to release lock on %s", srcstr);
+
+ cleanup:
+ virErrorRestore(&orig_err);
+
+ return ret;
+}
+
+
/**
* qemuDomainStorageSourceAccessRevoke:
*
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index 9a48b5b69d..65b0e8c39d 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -770,6 +770,11 @@ int qemuDomainDiskGetBackendAlias(virDomainDiskDefPtr disk,
ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2)
ATTRIBUTE_NONNULL(3) ATTRIBUTE_RETURN_CHECK;
+int qemuDomainStorageSourceChainAccessPrepare(virQEMUDriverPtr driver,
+ virDomainObjPtr vm,
+ virStorageSourcePtr src,
+ bool teardown);
+
void qemuDomainStorageSourceAccessRevoke(virQEMUDriverPtr driver,
virDomainObjPtr vm,
virStorageSourcePtr elem);
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index a4f7d111b1..dd5571aea3 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -132,74 +132,6 @@ qemuDomainDeleteDevice(virDomainObjPtr vm,
}
-/**
- * qemuHotplugPrepareDiskSourceAccess:
- * @driver: qemu driver struct
- * @vm: domain object
- * @src: Source to prepare
- * @teardown: Teardown the access to @src instead of adding it to a vm
- *
- * Setup the locks, cgroups and security permissions on a disk source and its
- * backing chain. If @teardown is true, then the labels and cgroups are removed
- * instead.
- *
- * Returns 0 on success and -1 on error. Reports libvirt error.
- */
-static int
-qemuHotplugPrepareDiskSourceAccess(virQEMUDriverPtr driver,
- virDomainObjPtr vm,
- virStorageSourcePtr src,
- bool teardown)
-{
- VIR_AUTOUNREF(virQEMUDriverConfigPtr) cfg = virQEMUDriverGetConfig(driver);
- const char *srcstr = NULLSTR(src->path);
- int ret = -1;
- virErrorPtr orig_err = NULL;
-
- /* just tear down the disk access */
- if (teardown) {
- virErrorPreserveLast(&orig_err);
- ret = 0;
- goto rollback_cgroup;
- }
-
- if (virDomainLockImageAttach(driver->lockManager, cfg->uri, vm, src) < 0)
- goto cleanup;
-
- if (qemuDomainNamespaceSetupDisk(vm, src) < 0)
- goto rollback_lock;
-
- if (qemuSecuritySetImageLabel(driver, vm, src, true) < 0)
- goto rollback_namespace;
-
- if (qemuSetupImageChainCgroup(vm, src) < 0)
- goto rollback_label;
-
- ret = 0;
- goto cleanup;
-
- rollback_cgroup:
- if (qemuTeardownImageChainCgroup(vm, src) < 0)
- VIR_WARN("Unable to tear down cgroup access on %s", srcstr);
- rollback_label:
- if (qemuSecurityRestoreImageLabel(driver, vm, src, true) < 0)
- VIR_WARN("Unable to restore security label on %s", srcstr);
-
- rollback_namespace:
- if (qemuDomainNamespaceTeardownDisk(vm, src) < 0)
- VIR_WARN("Unable to remove /dev entry for %s", srcstr);
-
- rollback_lock:
- if (virDomainLockImageDetach(driver->lockManager, vm, src) < 0)
- VIR_WARN("Unable to release lock on %s", srcstr);
-
- cleanup:
- virErrorRestore(&orig_err);
-
- return ret;
-}
-
-
static int
qemuDomainAttachZPCIDevice(qemuMonitorPtr mon,
virDomainDeviceInfoPtr info)
@@ -877,7 +809,7 @@ qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver,
if (qemuDomainPrepareDiskSource(disk, priv, cfg) < 0)
goto cleanup;
- if (qemuHotplugPrepareDiskSourceAccess(driver, vm, newsrc, false) < 0)
+ if (qemuDomainStorageSourceChainAccessPrepare(driver, vm, newsrc, false) < 0)
goto cleanup;
if (qemuHotplugAttachManagedPR(driver, vm, newsrc, QEMU_ASYNC_JOB_NONE) < 0)
@@ -896,7 +828,7 @@ qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver,
/* remove the old source from shared device list */
disk->src = oldsrc;
ignore_value(qemuRemoveSharedDisk(driver, disk, vm->def->name));
- ignore_value(qemuHotplugPrepareDiskSourceAccess(driver, vm, oldsrc, true));
+ ignore_value(qemuDomainStorageSourceChainAccessPrepare(driver, vm, oldsrc, true));
/* media was changed, so we can remove the old media definition now */
virObjectUnref(oldsrc);
@@ -911,7 +843,7 @@ qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver,
if (sharedAdded)
ignore_value(qemuRemoveSharedDisk(driver, disk, vm->def->name));
- ignore_value(qemuHotplugPrepareDiskSourceAccess(driver, vm, newsrc, true));
+ ignore_value(qemuDomainStorageSourceChainAccessPrepare(driver, vm, newsrc,
true));
}
/* remove PR manager object if unneeded */
@@ -941,7 +873,7 @@ qemuDomainAttachDiskGeneric(virQEMUDriverPtr driver,
char *devstr = NULL;
VIR_AUTOUNREF(virQEMUDriverConfigPtr) cfg = virQEMUDriverGetConfig(driver);
- if (qemuHotplugPrepareDiskSourceAccess(driver, vm, disk->src, false) < 0)
+ if (qemuDomainStorageSourceChainAccessPrepare(driver, vm, disk->src, false) <
0)
goto cleanup;
if (qemuAssignDeviceDiskAlias(vm->def, disk, priv->qemuCaps) < 0)
@@ -1003,7 +935,7 @@ qemuDomainAttachDiskGeneric(virQEMUDriverPtr driver,
virDomainAuditDisk(vm, NULL, disk->src, "attach", false);
error:
- ignore_value(qemuHotplugPrepareDiskSourceAccess(driver, vm, disk->src, true));
+ ignore_value(qemuDomainStorageSourceChainAccessPrepare(driver, vm, disk->src,
true));
goto cleanup;
}
@@ -4532,7 +4464,7 @@ qemuDomainRemoveDiskDevice(virQEMUDriverPtr driver,
qemuDomainReleaseDeviceAddress(vm, &disk->info);
/* tear down disk security access */
- qemuHotplugPrepareDiskSourceAccess(driver, vm, disk->src, true);
+ qemuDomainStorageSourceChainAccessPrepare(driver, vm, disk->src, true);
dev.type = VIR_DOMAIN_DEVICE_DISK;
dev.data.disk = disk;
--
2.20.1
10:04 a.m.
New subject: [libvirt] [PATCH 02/12] qemu: Move and rename qemuHotplugPrepareDiskSourceAccess
On Thu, Apr 18, 2019 at 04:42:57PM +0200, Peter Krempa wrote:
Move it to qemu_domain.c and call it
qemuDomainStorageSourceChainAccessPrepare.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_domain.c | 68 +++++++++++++++++++++++++++++++++++
src/qemu/qemu_domain.h | 5 +++
src/qemu/qemu_hotplug.c | 80 ++++-------------------------------------
3 files changed, 79 insertions(+), 74 deletions(-)
Reviewed-by: Ján Tomko <jtomko(a)redhat.com>
Jano
9:42 a.m.
New subject: [libvirt] [PATCH 03/12] qemu: Split entry points to qemuDomainStorageSourceChainAccessPrepare
Introduce qemuDomainStorageSourceChainAccess(Allow|Revoke) as entry
points to qemuDomainStorageSourceChainAccessPrepare for symmetry with
the functions for single backing chain elements.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_domain.c | 20 +++++++++++++++++++-
src/qemu/qemu_domain.h | 10 ++++++----
src/qemu/qemu_hotplug.c | 12 ++++++------
3 files changed, 31 insertions(+), 11 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index ab96688a65..85e43027d5 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -9224,7 +9224,7 @@ qemuDomainDiskGetBackendAlias(virDomainDiskDefPtr disk,
*
* Returns 0 on success and -1 on error. Reports libvirt error.
*/
-int
+static int
qemuDomainStorageSourceChainAccessPrepare(virQEMUDriverPtr driver,
virDomainObjPtr vm,
virStorageSourcePtr src,
@@ -9279,6 +9279,24 @@ qemuDomainStorageSourceChainAccessPrepare(virQEMUDriverPtr driver,
}
+int
+qemuDomainStorageSourceChainAccessAllow(virQEMUDriverPtr driver,
+ virDomainObjPtr vm,
+ virStorageSourcePtr src)
+{
+ return qemuDomainStorageSourceChainAccessPrepare(driver, vm, src, false);
+}
+
+
+int
+qemuDomainStorageSourceChainAccessRevoke(virQEMUDriverPtr driver,
+ virDomainObjPtr vm,
+ virStorageSourcePtr src)
+{
+ return qemuDomainStorageSourceChainAccessPrepare(driver, vm, src, true);
+}
+
+
/**
* qemuDomainStorageSourceAccessRevoke:
*
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index 65b0e8c39d..f92f0dbc27 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -770,10 +770,12 @@ int qemuDomainDiskGetBackendAlias(virDomainDiskDefPtr disk,
ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2)
ATTRIBUTE_NONNULL(3) ATTRIBUTE_RETURN_CHECK;
-int qemuDomainStorageSourceChainAccessPrepare(virQEMUDriverPtr driver,
- virDomainObjPtr vm,
- virStorageSourcePtr src,
- bool teardown);
+int qemuDomainStorageSourceChainAccessAllow(virQEMUDriverPtr driver,
+ virDomainObjPtr vm,
+ virStorageSourcePtr src);
+int qemuDomainStorageSourceChainAccessRevoke(virQEMUDriverPtr driver,
+ virDomainObjPtr vm,
+ virStorageSourcePtr src);
void qemuDomainStorageSourceAccessRevoke(virQEMUDriverPtr driver,
virDomainObjPtr vm,
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index dd5571aea3..ce5ca015ab 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -809,7 +809,7 @@ qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver,
if (qemuDomainPrepareDiskSource(disk, priv, cfg) < 0)
goto cleanup;
- if (qemuDomainStorageSourceChainAccessPrepare(driver, vm, newsrc, false) < 0)
+ if (qemuDomainStorageSourceChainAccessAllow(driver, vm, newsrc) < 0)
goto cleanup;
if (qemuHotplugAttachManagedPR(driver, vm, newsrc, QEMU_ASYNC_JOB_NONE) < 0)
@@ -828,7 +828,7 @@ qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver,
/* remove the old source from shared device list */
disk->src = oldsrc;
ignore_value(qemuRemoveSharedDisk(driver, disk, vm->def->name));
- ignore_value(qemuDomainStorageSourceChainAccessPrepare(driver, vm, oldsrc, true));
+ ignore_value(qemuDomainStorageSourceChainAccessRevoke(driver, vm, oldsrc));
/* media was changed, so we can remove the old media definition now */
virObjectUnref(oldsrc);
@@ -843,7 +843,7 @@ qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver,
if (sharedAdded)
ignore_value(qemuRemoveSharedDisk(driver, disk, vm->def->name));
- ignore_value(qemuDomainStorageSourceChainAccessPrepare(driver, vm, newsrc,
true));
+ ignore_value(qemuDomainStorageSourceChainAccessRevoke(driver, vm, newsrc));
}
/* remove PR manager object if unneeded */
@@ -873,7 +873,7 @@ qemuDomainAttachDiskGeneric(virQEMUDriverPtr driver,
char *devstr = NULL;
VIR_AUTOUNREF(virQEMUDriverConfigPtr) cfg = virQEMUDriverGetConfig(driver);
- if (qemuDomainStorageSourceChainAccessPrepare(driver, vm, disk->src, false) <
0)
+ if (qemuDomainStorageSourceChainAccessAllow(driver, vm, disk->src) < 0)
goto cleanup;
if (qemuAssignDeviceDiskAlias(vm->def, disk, priv->qemuCaps) < 0)
@@ -935,7 +935,7 @@ qemuDomainAttachDiskGeneric(virQEMUDriverPtr driver,
virDomainAuditDisk(vm, NULL, disk->src, "attach", false);
error:
- ignore_value(qemuDomainStorageSourceChainAccessPrepare(driver, vm, disk->src,
true));
+ ignore_value(qemuDomainStorageSourceChainAccessRevoke(driver, vm, disk->src));
goto cleanup;
}
@@ -4464,7 +4464,7 @@ qemuDomainRemoveDiskDevice(virQEMUDriverPtr driver,
qemuDomainReleaseDeviceAddress(vm, &disk->info);
/* tear down disk security access */
- qemuDomainStorageSourceChainAccessPrepare(driver, vm, disk->src, true);
+ qemuDomainStorageSourceChainAccessRevoke(driver, vm, disk->src);
dev.type = VIR_DOMAIN_DEVICE_DISK;
dev.data.disk = disk;
--
2.20.1
10:05 a.m.
New subject: [libvirt] [PATCH 03/12] qemu: Split entry points to qemuDomainStorageSourceChainAccessPrepare
On Thu, Apr 18, 2019 at 04:42:58PM +0200, Peter Krempa wrote:
Introduce qemuDomainStorageSourceChainAccess(Allow|Revoke) as entry
points to qemuDomainStorageSourceChainAccessPrepare for symmetry with
the functions for single backing chain elements.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_domain.c | 20 +++++++++++++++++++-
src/qemu/qemu_domain.h | 10 ++++++----
src/qemu/qemu_hotplug.c | 12 ++++++------
3 files changed, 31 insertions(+), 11 deletions(-)
Reviewed-by: Ján Tomko <jtomko(a)redhat.com>
Thank you,
Jano
9:42 a.m.
New subject: [libvirt] [PATCH 04/12] qemu: domain: Rename qemuDomainStorageSourceChainAccessPrepare
The function will be able to deal with non-chains too so drop 'Chain'
and also change the suffix to 'Modify' as it's used both for setup and
teardown.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_domain.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 85e43027d5..b4c59b9a74 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -9212,7 +9212,7 @@ qemuDomainDiskGetBackendAlias(virDomainDiskDefPtr disk,
/**
- * qemuDomainStorageSourceChainAccessPrepare:
+ * qemuDomainStorageSourceAccessModify:
* @driver: qemu driver struct
* @vm: domain object
* @src: Source to prepare
@@ -9225,10 +9225,10 @@ qemuDomainDiskGetBackendAlias(virDomainDiskDefPtr disk,
* Returns 0 on success and -1 on error. Reports libvirt error.
*/
static int
-qemuDomainStorageSourceChainAccessPrepare(virQEMUDriverPtr driver,
- virDomainObjPtr vm,
- virStorageSourcePtr src,
- bool teardown)
+qemuDomainStorageSourceAccessModify(virQEMUDriverPtr driver,
+ virDomainObjPtr vm,
+ virStorageSourcePtr src,
+ bool teardown)
{
VIR_AUTOUNREF(virQEMUDriverConfigPtr) cfg = virQEMUDriverGetConfig(driver);
const char *srcstr = NULLSTR(src->path);
@@ -9284,7 +9284,7 @@ qemuDomainStorageSourceChainAccessAllow(virQEMUDriverPtr driver,
virDomainObjPtr vm,
virStorageSourcePtr src)
{
- return qemuDomainStorageSourceChainAccessPrepare(driver, vm, src, false);
+ return qemuDomainStorageSourceAccessModify(driver, vm, src, false);
}
@@ -9293,7 +9293,7 @@ qemuDomainStorageSourceChainAccessRevoke(virQEMUDriverPtr driver,
virDomainObjPtr vm,
virStorageSourcePtr src)
{
- return qemuDomainStorageSourceChainAccessPrepare(driver, vm, src, true);
+ return qemuDomainStorageSourceAccessModify(driver, vm, src, true);
}
--
2.20.1
7:49 a.m.
New subject: [libvirt] [PATCH 04/12] qemu: domain: Rename qemuDomainStorageSourceChainAccessPrepare
On Thu, Apr 18, 2019 at 04:42:59PM +0200, Peter Krempa wrote:
The function will be able to deal with non-chains too so drop
'Chain'
and also change the suffix to 'Modify' as it's used both for setup and
teardown.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_domain.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
Reviewed-by: Ján Tomko <jtomko(a)redhat.com>
Jano
9:43 a.m.
New subject: [libvirt] [PATCH 05/12] qemu: Convert boolean flags to enum flags in qemuDomainStorageSourceAccessModify
Upcoming patches will add a few more flags. Add an enum to collect them
so that we don't end up with multiple bools.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_domain.c | 23 ++++++++++++++++-------
1 file changed, 16 insertions(+), 7 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index b4c59b9a74..6644c1418b 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -9211,16 +9211,21 @@ qemuDomainDiskGetBackendAlias(virDomainDiskDefPtr disk,
}
+typedef enum {
+ /* revoke access to the image instead of allowing it */
+ QEMU_DOMAIN_STORAGE_SOURCE_ACCESS_REVOKE = 1 << 0,
+} qemuDomainStorageSourceAccessFlags;
+
+
/**
* qemuDomainStorageSourceAccessModify:
* @driver: qemu driver struct
* @vm: domain object
* @src: Source to prepare
- * @teardown: Teardown the access to @src instead of adding it to a vm
+ * @flags: bitwise or of qemuDomainStorageSourceAccessFlags
*
* Setup the locks, cgroups and security permissions on a disk source and its
- * backing chain. If @teardown is true, then the labels and cgroups are removed
- * instead.
+ * backing chain.
*
* Returns 0 on success and -1 on error. Reports libvirt error.
*/
@@ -9228,7 +9233,7 @@ static int
qemuDomainStorageSourceAccessModify(virQEMUDriverPtr driver,
virDomainObjPtr vm,
virStorageSourcePtr src,
- bool teardown)
+ qemuDomainStorageSourceAccessFlags flags)
{
VIR_AUTOUNREF(virQEMUDriverConfigPtr) cfg = virQEMUDriverGetConfig(driver);
const char *srcstr = NULLSTR(src->path);
@@ -9236,7 +9241,7 @@ qemuDomainStorageSourceAccessModify(virQEMUDriverPtr driver,
virErrorPtr orig_err = NULL;
/* just tear down the disk access */
- if (teardown) {
+ if (flags & QEMU_DOMAIN_STORAGE_SOURCE_ACCESS_REVOKE) {
virErrorPreserveLast(&orig_err);
ret = 0;
goto rollback_cgroup;
@@ -9284,7 +9289,9 @@ qemuDomainStorageSourceChainAccessAllow(virQEMUDriverPtr driver,
virDomainObjPtr vm,
virStorageSourcePtr src)
{
- return qemuDomainStorageSourceAccessModify(driver, vm, src, false);
+ qemuDomainStorageSourceAccessFlags flags = 0;
+
+ return qemuDomainStorageSourceAccessModify(driver, vm, src, flags);
}
@@ -9293,7 +9300,9 @@ qemuDomainStorageSourceChainAccessRevoke(virQEMUDriverPtr driver,
virDomainObjPtr vm,
virStorageSourcePtr src)
{
- return qemuDomainStorageSourceAccessModify(driver, vm, src, true);
+ qemuDomainStorageSourceAccessFlags flags = QEMU_DOMAIN_STORAGE_SOURCE_ACCESS_REVOKE;
+
+ return qemuDomainStorageSourceAccessModify(driver, vm, src, flags);
}
--
2.20.1
9:24 a.m.
New subject: [libvirt] [PATCH 05/12] qemu: Convert boolean flags to enum flags in qemuDomainStorageSourceAccessModify
On Thu, Apr 18, 2019 at 04:43:00PM +0200, Peter Krempa wrote:
Upcoming patches will add a few more flags. Add an enum to collect
them
so that we don't end up with multiple bools.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_domain.c | 23 ++++++++++++++++-------
1 file changed, 16 insertions(+), 7 deletions(-)
Reviewed-by: Ján Tomko <jtomko(a)redhat.com>
Jano
9:43 a.m.
New subject: [libvirt] [PATCH 06/12] qemu: Allow using qemuDomainStorageSourceAccessModify on singe images
Add a new flag QEMU_DOMAIN_STORAGE_SOURCE_ACCESS_CHAIN to select whether
to work on single image or full chain.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_domain.c | 27 +++++++++++++++++++++------
1 file changed, 21 insertions(+), 6 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 6644c1418b..30acba3c47 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -9214,6 +9214,8 @@ qemuDomainDiskGetBackendAlias(virDomainDiskDefPtr disk,
typedef enum {
/* revoke access to the image instead of allowing it */
QEMU_DOMAIN_STORAGE_SOURCE_ACCESS_REVOKE = 1 << 0,
+ /* operate on full backing chain rather than single image */
+ QEMU_DOMAIN_STORAGE_SOURCE_ACCESS_CHAIN = 1 << 1,
} qemuDomainStorageSourceAccessFlags;
@@ -9239,6 +9241,8 @@ qemuDomainStorageSourceAccessModify(virQEMUDriverPtr driver,
const char *srcstr = NULLSTR(src->path);
int ret = -1;
virErrorPtr orig_err = NULL;
+ bool chain = flags & QEMU_DOMAIN_STORAGE_SOURCE_ACCESS_CHAIN;
+ int rc;
/* just tear down the disk access */
if (flags & QEMU_DOMAIN_STORAGE_SOURCE_ACCESS_REVOKE) {
@@ -9253,20 +9257,30 @@ qemuDomainStorageSourceAccessModify(virQEMUDriverPtr driver,
if (qemuDomainNamespaceSetupDisk(vm, src) < 0)
goto rollback_lock;
- if (qemuSecuritySetImageLabel(driver, vm, src, true) < 0)
+ if (qemuSecuritySetImageLabel(driver, vm, src, chain) < 0)
goto rollback_namespace;
- if (qemuSetupImageChainCgroup(vm, src) < 0)
+ if (chain)
+ rc = qemuSetupImageChainCgroup(vm, src);
+ else
+ rc = qemuSetupImageCgroup(vm, src);
+
+ if (rc < 0)
goto rollback_label;
ret = 0;
goto cleanup;
rollback_cgroup:
- if (qemuTeardownImageChainCgroup(vm, src) < 0)
+ if (chain)
+ rc = qemuTeardownImageChainCgroup(vm, src);
+ else
+ rc = qemuTeardownImageCgroup(vm, src);
+
+ if (rc < 0)
VIR_WARN("Unable to tear down cgroup access on %s", srcstr);
rollback_label:
- if (qemuSecurityRestoreImageLabel(driver, vm, src, true) < 0)
+ if (qemuSecurityRestoreImageLabel(driver, vm, src, chain) < 0)
VIR_WARN("Unable to restore security label on %s", srcstr);
rollback_namespace:
@@ -9289,7 +9303,7 @@ qemuDomainStorageSourceChainAccessAllow(virQEMUDriverPtr driver,
virDomainObjPtr vm,
virStorageSourcePtr src)
{
- qemuDomainStorageSourceAccessFlags flags = 0;
+ qemuDomainStorageSourceAccessFlags flags = QEMU_DOMAIN_STORAGE_SOURCE_ACCESS_CHAIN;
return qemuDomainStorageSourceAccessModify(driver, vm, src, flags);
}
@@ -9300,7 +9314,8 @@ qemuDomainStorageSourceChainAccessRevoke(virQEMUDriverPtr driver,
virDomainObjPtr vm,
virStorageSourcePtr src)
{
- qemuDomainStorageSourceAccessFlags flags = QEMU_DOMAIN_STORAGE_SOURCE_ACCESS_REVOKE;
+ qemuDomainStorageSourceAccessFlags flags = QEMU_DOMAIN_STORAGE_SOURCE_ACCESS_REVOKE
|
+ QEMU_DOMAIN_STORAGE_SOURCE_ACCESS_CHAIN;
return qemuDomainStorageSourceAccessModify(driver, vm, src, flags);
}
--
2.20.1
9:29 a.m.
New subject: [libvirt] [PATCH 06/12] qemu: Allow using qemuDomainStorageSourceAccessModify on singe images
On Thu, Apr 18, 2019 at 04:43:01PM +0200, Peter Krempa wrote:
Add a new flag QEMU_DOMAIN_STORAGE_SOURCE_ACCESS_CHAIN to select
whether
to work on single image or full chain.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_domain.c | 27 +++++++++++++++++++++------
1 file changed, 21 insertions(+), 6 deletions(-)
Reviewed-by: Ján Tomko <jtomko(a)redhat.com>
Jano
9:43 a.m.
New subject: [libvirt] [PATCH 07/12] qemu: Refactor/simplify qemuDomainStorageSourceAccessRevoke
Use qemuDomainStorageSourceAccessModify instead of the individual calls.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_domain.c | 13 ++-----------
1 file changed, 2 insertions(+), 11 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 30acba3c47..f7d1250f09 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -9332,18 +9332,9 @@ qemuDomainStorageSourceAccessRevoke(virQEMUDriverPtr driver,
virDomainObjPtr vm,
virStorageSourcePtr elem)
{
- if (qemuTeardownImageCgroup(vm, elem) < 0)
- VIR_WARN("Failed to teardown cgroup for disk path %s",
- NULLSTR(elem->path));
+ qemuDomainStorageSourceAccessFlags flags = QEMU_DOMAIN_STORAGE_SOURCE_ACCESS_REVOKE;
- if (qemuSecurityRestoreImageLabel(driver, vm, elem, false) < 0)
- VIR_WARN("Unable to restore security label on %s",
NULLSTR(elem->path));
-
- if (qemuDomainNamespaceTeardownDisk(vm, elem) < 0)
- VIR_WARN("Unable to remove /dev entry for %s",
NULLSTR(elem->path));
-
- if (virDomainLockImageDetach(driver->lockManager, vm, elem) < 0)
- VIR_WARN("Unable to release lock on %s", NULLSTR(elem->path));
+ ignore_value(qemuDomainStorageSourceAccessModify(driver, vm, elem, flags));
}
--
2.20.1
9:34 a.m.
New subject: [libvirt] [PATCH 07/12] qemu: Refactor/simplify qemuDomainStorageSourceAccessRevoke
On Thu, Apr 18, 2019 at 04:43:02PM +0200, Peter Krempa wrote:
Use qemuDomainStorageSourceAccessModify instead of the individual
calls.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_domain.c | 13 ++-----------
1 file changed, 2 insertions(+), 11 deletions(-)
Reviewed-by: Ján Tomko <jtomko(a)redhat.com>
Jano
9:43 a.m.
New subject: [libvirt] [PATCH 08/12] qemu: Allow forcing read-only mode in qemuDomainStorageSourceAccessModify
Add a new flag which will set the image as read-only even if the image
data allows writing.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_domain.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index f7d1250f09..e8f6ee548a 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -9216,6 +9216,8 @@ typedef enum {
QEMU_DOMAIN_STORAGE_SOURCE_ACCESS_REVOKE = 1 << 0,
/* operate on full backing chain rather than single image */
QEMU_DOMAIN_STORAGE_SOURCE_ACCESS_CHAIN = 1 << 1,
+ /* force permissions to read-only when allowing */
+ QEMU_DOMAIN_STORAGE_SOURCE_ACCESS_READ_ONLY = 1 << 2,
} qemuDomainStorageSourceAccessFlags;
@@ -9243,6 +9245,10 @@ qemuDomainStorageSourceAccessModify(virQEMUDriverPtr driver,
virErrorPtr orig_err = NULL;
bool chain = flags & QEMU_DOMAIN_STORAGE_SOURCE_ACCESS_CHAIN;
int rc;
+ bool was_readonly = src->readonly;
+
+ if (flags & QEMU_DOMAIN_STORAGE_SOURCE_ACCESS_READ_ONLY)
+ src->readonly = true;
/* just tear down the disk access */
if (flags & QEMU_DOMAIN_STORAGE_SOURCE_ACCESS_REVOKE) {
@@ -9292,6 +9298,7 @@ qemuDomainStorageSourceAccessModify(virQEMUDriverPtr driver,
VIR_WARN("Unable to release lock on %s", srcstr);
cleanup:
+ src->readonly = was_readonly;
virErrorRestore(&orig_err);
return ret;
--
2.20.1
8:17 a.m.
New subject: [libvirt] [PATCH 08/12] qemu: Allow forcing read-only mode in qemuDomainStorageSourceAccessModify
On Thu, Apr 18, 2019 at 04:43:03PM +0200, Peter Krempa wrote:
Add a new flag which will set the image as read-only even if the
image
data allows writing.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_domain.c | 7 +++++++
1 file changed, 7 insertions(+)
Reviewed-by: Ján Tomko <jtomko(a)redhat.com>
Jano
9:43 a.m.
New subject: [libvirt] [PATCH 09/12] qemu: Use bools rather than labels in qemuDomainStorageSourceAccessModify
Rather than jumping to the correct label use a set of booleans to
determine which operation needs to be rolled back. This will allow more
flexibility when e.g. rollback after a failed operation will not be
necessary/desired.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_domain.c | 68 ++++++++++++++++++++++++++++--------------
1 file changed, 45 insertions(+), 23 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index e8f6ee548a..aec094d8f9 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -9246,6 +9246,10 @@ qemuDomainStorageSourceAccessModify(virQEMUDriverPtr driver,
bool chain = flags & QEMU_DOMAIN_STORAGE_SOURCE_ACCESS_CHAIN;
int rc;
bool was_readonly = src->readonly;
+ bool revoke_cgroup = false;
+ bool revoke_label = false;
+ bool revoke_namespace = false;
+ bool revoke_lockspace = false;
if (flags & QEMU_DOMAIN_STORAGE_SOURCE_ACCESS_READ_ONLY)
src->readonly = true;
@@ -9253,18 +9257,28 @@ qemuDomainStorageSourceAccessModify(virQEMUDriverPtr driver,
/* just tear down the disk access */
if (flags & QEMU_DOMAIN_STORAGE_SOURCE_ACCESS_REVOKE) {
virErrorPreserveLast(&orig_err);
+ revoke_cgroup = true;
+ revoke_label = true;
+ revoke_namespace = true;
+ revoke_lockspace = true;
ret = 0;
- goto rollback_cgroup;
+ goto revoke;
}
if (virDomainLockImageAttach(driver->lockManager, cfg->uri, vm, src) < 0)
- goto cleanup;
+ goto revoke;
+
+ revoke_lockspace = true;
if (qemuDomainNamespaceSetupDisk(vm, src) < 0)
- goto rollback_lock;
+ goto revoke;
+
+ revoke_namespace = true;
if (qemuSecuritySetImageLabel(driver, vm, src, chain) < 0)
- goto rollback_namespace;
+ goto revoke;
+
+ revoke_label = true;
if (chain)
rc = qemuSetupImageChainCgroup(vm, src);
@@ -9272,30 +9286,38 @@ qemuDomainStorageSourceAccessModify(virQEMUDriverPtr driver,
rc = qemuSetupImageCgroup(vm, src);
if (rc < 0)
- goto rollback_label;
+ goto revoke;
+
+ revoke_cgroup = true;
ret = 0;
goto cleanup;
- rollback_cgroup:
- if (chain)
- rc = qemuTeardownImageChainCgroup(vm, src);
- else
- rc = qemuTeardownImageCgroup(vm, src);
+ revoke:
+ if (revoke_cgroup) {
+ if (chain)
+ rc = qemuTeardownImageChainCgroup(vm, src);
+ else
+ rc = qemuTeardownImageCgroup(vm, src);
- if (rc < 0)
- VIR_WARN("Unable to tear down cgroup access on %s", srcstr);
- rollback_label:
- if (qemuSecurityRestoreImageLabel(driver, vm, src, chain) < 0)
- VIR_WARN("Unable to restore security label on %s", srcstr);
-
- rollback_namespace:
- if (qemuDomainNamespaceTeardownDisk(vm, src) < 0)
- VIR_WARN("Unable to remove /dev entry for %s", srcstr);
-
- rollback_lock:
- if (virDomainLockImageDetach(driver->lockManager, vm, src) < 0)
- VIR_WARN("Unable to release lock on %s", srcstr);
+ if (rc < 0)
+ VIR_WARN("Unable to tear down cgroup access on %s", srcstr);
+ }
+
+ if (revoke_label) {
+ if (qemuSecurityRestoreImageLabel(driver, vm, src, chain) < 0)
+ VIR_WARN("Unable to restore security label on %s", srcstr);
+ }
+
+ if (revoke_namespace) {
+ if (qemuDomainNamespaceTeardownDisk(vm, src) < 0)
+ VIR_WARN("Unable to remove /dev entry for %s", srcstr);
+ }
+
+ if (revoke_lockspace) {
+ if (virDomainLockImageDetach(driver->lockManager, vm, src) < 0)
+ VIR_WARN("Unable to release lock on %s", srcstr);
+ }
cleanup:
src->readonly = was_readonly;
--
2.20.1
8:17 a.m.
New subject: [libvirt] [PATCH 09/12] qemu: Use bools rather than labels in qemuDomainStorageSourceAccessModify
On Thu, Apr 18, 2019 at 04:43:04PM +0200, Peter Krempa wrote:
Rather than jumping to the correct label use a set of booleans to
determine which operation needs to be rolled back. This will allow more
flexibility when e.g. rollback after a failed operation will not be
necessary/desired.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_domain.c | 68 ++++++++++++++++++++++++++++--------------
1 file changed, 45 insertions(+), 23 deletions(-)
Reviewed-by: Ján Tomko <jtomko(a)redhat.com>
Jano
9:43 a.m.
New subject: [libvirt] [PATCH 10/12] qemu: Allow skipping the revoke step in qemuDomainStorageSourceAccessModify
In some cases when we need to modify access permissions for a storage
source which is already used by the VM we should not revoke all
permissions on a failure. Allow this in qemuDomainStorageSourceAccessModify
by adding a new flag.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_domain.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index aec094d8f9..82ebf3c324 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -9218,6 +9218,8 @@ typedef enum {
QEMU_DOMAIN_STORAGE_SOURCE_ACCESS_CHAIN = 1 << 1,
/* force permissions to read-only when allowing */
QEMU_DOMAIN_STORAGE_SOURCE_ACCESS_READ_ONLY = 1 << 2,
+ /* don't revoke permissions when modification has failed */
+ QEMU_DOMAIN_STORAGE_SOURCE_ACCESS_SKIP_REVOKE = 1 << 3,
} qemuDomainStorageSourceAccessFlags;
@@ -9294,6 +9296,9 @@ qemuDomainStorageSourceAccessModify(virQEMUDriverPtr driver,
goto cleanup;
revoke:
+ if (flags & QEMU_DOMAIN_STORAGE_SOURCE_ACCESS_SKIP_REVOKE)
+ goto cleanup;
+
if (revoke_cgroup) {
if (chain)
rc = qemuTeardownImageChainCgroup(vm, src);
--
2.20.1
8:21 a.m.
New subject: [libvirt] [PATCH 10/12] qemu: Allow skipping the revoke step in qemuDomainStorageSourceAccessModify
On Thu, Apr 18, 2019 at 04:43:05PM +0200, Peter Krempa wrote:
In some cases when we need to modify access permissions for a storage
source which is already used by the VM we should not revoke all
permissions on a failure. Allow this in qemuDomainStorageSourceAccessModify
by adding a new flag.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_domain.c | 5 +++++
1 file changed, 5 insertions(+)
Reviewed-by: Ján Tomko <jtomko(a)redhat.com>
Jano
9:43 a.m.
New subject: [libvirt] [PATCH 11/12] qemu: Mark when modifying access to existing source in qemuDomainStorageSourceAccessModify
Some operations e.g. namespace setup are not necessary when modifying
access to a file which the VM can already access. Add a flag which
allows to skip them.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_domain.c | 11 ++++++++---
1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 82ebf3c324..75d0b34e42 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -9220,6 +9220,8 @@ typedef enum {
QEMU_DOMAIN_STORAGE_SOURCE_ACCESS_READ_ONLY = 1 << 2,
/* don't revoke permissions when modification has failed */
QEMU_DOMAIN_STORAGE_SOURCE_ACCESS_SKIP_REVOKE = 1 << 3,
+ /* VM already has access to the source and we are just modifying it */
+ QEMU_DOMAIN_STORAGE_SOURCE_ACCESS_MODIFY_ACCESS = 1 << 4,
} qemuDomainStorageSourceAccessFlags;
@@ -9272,10 +9274,13 @@ qemuDomainStorageSourceAccessModify(virQEMUDriverPtr driver,
revoke_lockspace = true;
- if (qemuDomainNamespaceSetupDisk(vm, src) < 0)
- goto revoke;
+ /* When modifying access of existing @src namespace does not need update */
+ if (!(flags & QEMU_DOMAIN_STORAGE_SOURCE_ACCESS_MODIFY_ACCESS)) {
+ if (qemuDomainNamespaceSetupDisk(vm, src) < 0)
+ goto revoke;
- revoke_namespace = true;
+ revoke_namespace = true;
+ }
if (qemuSecuritySetImageLabel(driver, vm, src, chain) < 0)
goto revoke;
--
2.20.1
8:21 a.m.
New subject: [libvirt] [PATCH 11/12] qemu: Mark when modifying access to existing source in qemuDomainStorageSourceAccessModify
On Thu, Apr 18, 2019 at 04:43:06PM +0200, Peter Krempa wrote:
Some operations e.g. namespace setup are not necessary when modifying
access to a file which the VM can already access. Add a flag which
allows to skip them.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_domain.c | 11 ++++++++---
1 file changed, 8 insertions(+), 3 deletions(-)
Reviewed-by: Ján Tomko <jtomko(a)redhat.com>
Jano
9:43 a.m.
New subject: [libvirt] [PATCH 12/12] qemu: Refactor/simplify qemuDomainStorageSourceAccessAllow
Use qemuDomainStorageSourceAccessModify with correct flags to do the
job.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_domain.c | 30 ++++++------------------------
1 file changed, 6 insertions(+), 24 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 75d0b34e42..c2b96b825e 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -9399,33 +9399,15 @@ qemuDomainStorageSourceAccessAllow(virQEMUDriverPtr driver,
bool readonly,
bool newSource)
{
- bool was_readonly = elem->readonly;
- virQEMUDriverConfigPtr cfg = NULL;
- int ret = -1;
-
- cfg = virQEMUDriverGetConfig(driver);
-
- elem->readonly = readonly;
-
- if (virDomainLockImageAttach(driver->lockManager, cfg->uri, vm, elem) < 0)
- goto cleanup;
-
- if (newSource &&
- qemuDomainNamespaceSetupDisk(vm, elem) < 0)
- goto cleanup;
-
- if (qemuSetupImageCgroup(vm, elem) < 0)
- goto cleanup;
+ qemuDomainStorageSourceAccessFlags flags =
QEMU_DOMAIN_STORAGE_SOURCE_ACCESS_SKIP_REVOKE;
- if (qemuSecuritySetImageLabel(driver, vm, elem, false) < 0)
- goto cleanup;
+ if (readonly)
+ flags &= QEMU_DOMAIN_STORAGE_SOURCE_ACCESS_READ_ONLY;
- ret = 0;
+ if (!newSource)
+ flags &= QEMU_DOMAIN_STORAGE_SOURCE_ACCESS_MODIFY_ACCESS;
- cleanup:
- elem->readonly = was_readonly;
- virObjectUnref(cfg);
- return ret;
+ return qemuDomainStorageSourceAccessModify(driver, vm, elem, flags);
}
--
2.20.1
8:24 a.m.
New subject: [libvirt] [PATCH 12/12] qemu: Refactor/simplify qemuDomainStorageSourceAccessAllow
On Thu, Apr 18, 2019 at 04:43:07PM +0200, Peter Krempa wrote:
Use qemuDomainStorageSourceAccessModify with correct flags to do the
job.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_domain.c | 30 ++++++------------------------
1 file changed, 6 insertions(+), 24 deletions(-)
Reviewed-by: Ján Tomko <jtomko(a)redhat.com>
Jano
2:30 p.m.
On 4/18/19 9:42 AM, Peter Krempa wrote:
With new blockjob handling we'll need to modify permissions for
chains
and individual images. The individual image code was universally
accessible but the chain setting code reimplemented it mostly only in
qemu_hotplug.h.
Refactor the handling by moving the code to qemu_domain.c and making it
universal.
Oh fun - competes with my work to get incremental backup in. But from
the subject lines alone, it seems reasonable; I can see how hard it is
to rebase my code on top of yours.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
2:59 a.m.
On Thu, Apr 18, 2019 at 14:30:04 -0500, Eric Blake wrote:
On 4/18/19 9:42 AM, Peter Krempa wrote:
> With new blockjob handling we'll need to modify permissions for chains
> and individual images. The individual image code was universally
> accessible but the chain setting code reimplemented it mostly only in
> qemu_hotplug.h.
>
> Refactor the handling by moving the code to qemu_domain.c and making it
> universal.
Oh fun - competes with my work to get incremental backup in. But from
the subject lines alone, it seems reasonable; I can see how hard it is
to rebase my code on top of yours.
These are the necessary changes which are needed in:
backup: Wire up qemu full pull backup commands over QMP
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index c9f1996b2a..d3307bfc4f 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -17796,7 +17796,7 @@ qemuDomainBackupDiskCleanup(virQEMUDriverPtr driver,
virDomainObjPtr vm,
ret = -1;
}
if (disk->state >= VIR_DOMAIN_BACKUP_DISK_STATE_LABEL)
- qemuDomainDiskChainElementRevoke(driver, vm, disk->store);
+ qemuDomainStorageSourceAccessRevoke(driver, vm, disk->store);
if ((!push || !completed) &&
disk->state >= VIR_DOMAIN_BACKUP_DISK_STATE_CREATED &&
disk->store->detected && unlink(disk->store->path) < 0) {
@@ -17997,8 +17997,8 @@ qemuDomainBackupBegin(virDomainPtr domain, const char *diskXml,
}
disk->state = VIR_DOMAIN_BACKUP_DISK_STATE_CREATED;
}
- if (qemuDomainDiskChainElementPrepare(driver, vm, disk->store, false,
- true) < 0)
+ if (qemuDomainStorageSourceAccessAllow(driver, vm, disk->store, false,
+ true) < 0)
goto endmon;
disk->state = VIR_DOMAIN_BACKUP_DISK_STATE_LABEL;
if (disk->store->detected) {
7:41 a.m.
On Thu, Apr 18, 2019 at 16:42:55 +0200, Peter Krempa wrote:
With new blockjob handling we'll need to modify permissions for
chains
and individual images. The individual image code was universally
accessible but the chain setting code reimplemented it mostly only in
qemu_hotplug.h.
Refactor the handling by moving the code to qemu_domain.c and making it
universal.
Ping?
2024
days inactive
2045
days old
27 comments
3 participants
participants (3)
-
Eric Blake -
Ján Tomko -
Peter Krempa