On 04/10/2018 10:58 AM, Michal Privoznik wrote:
When plugging a disk into domain we need to start qemu-pr-helper
process iff this is the first disk with PR enabled for the
domain. Otherwise the helper is already running (or not needed).
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/qemu/qemu_hotplug.c | 51 +++++++++++++++++++++++++++++++++++++++++++++++++
src/qemu/qemu_process.c | 4 ++--
src/qemu/qemu_process.h | 5 +++++
3 files changed, 58 insertions(+), 2 deletions(-)
Personally not a fan of splitting up the hotplug need for PR daemon from
need for adding the object to the disk and the separate unplug patch,
but I understand why it's done this way.
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index f0d549de38..468153c79c 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -348,6 +348,49 @@ qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver,
}
+/**
+ * qemuDomainMaybeStartPRDaemon:
+ * @vm: domain object
+ * @disk: disk to hotplug
+ *
+ * Checks if it's needed to start qemu-pr-helper and starts it.
+ *
+ * Returns: 0 if qemu-pr-helper is not needed
+ * 1 if it is needed and was started
+ * -1 otherwise.
+ */
+static int
+qemuDomainMaybeStartPRDaemon(virDomainObjPtr vm,
+ virDomainDiskDefPtr disk)
+{
+ size_t i;
+ qemuDomainStorageSourcePrivatePtr srcPriv;
+
+ if (!virStoragePRDefIsManaged(disk->src->pr)) {
+ /* @disk itself does not require qemu-pr-helper. */
+ return 0;
+ }
+
If we used the fact that the thing was already started for the domain in
some sort of domain private data (which could even store the PID-file
path), then we wouldn't need the following...
+ for (i = 0; i < vm->def->ndisks; i++) {
+ const virDomainDiskDef *domainDisk = vm->def->disks[i];
+
+ if (virStoragePRDefIsManaged(domainDisk->src->pr)) {
+ /* qemu-pr-helper should be already started because
+ * another disk in domain requires it. */
+ return 0;
+ }
+ }
+
+ /* @disk requires qemu-pr-helper but none is running.
+ * Start it now. */
+ srcPriv = QEMU_DOMAIN_STORAGE_SOURCE_PRIVATE(disk->src);
+ if (qemuProcessStartPRDaemon(vm, srcPriv->prd) < 0)
+ return -1;
and the above would set things for us (conceivably)
John
+
+ return 1;
+}
+
+
/**
* qemuDomainAttachDiskGeneric:
*
@@ -368,6 +411,7 @@ qemuDomainAttachDiskGeneric(virQEMUDriverPtr driver,
bool driveAdded = false;
bool secobjAdded = false;
bool encobjAdded = false;
+ bool prdStarted = false;
virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
virJSONValuePtr secobjProps = NULL;
virJSONValuePtr encobjProps = NULL;
@@ -384,6 +428,11 @@ qemuDomainAttachDiskGeneric(virQEMUDriverPtr driver,
if (qemuDomainPrepareDiskSource(disk, priv, cfg) < 0)
goto error;
+ if ((rv = qemuDomainMaybeStartPRDaemon(vm, disk)) < 0)
+ goto cleanup;
+ else if (rv > 0)
+ prdStarted = true;
+
srcPriv = QEMU_DOMAIN_STORAGE_SOURCE_PRIVATE(disk->src);
if (srcPriv) {
secinfo = srcPriv->secinfo;
@@ -481,6 +530,8 @@ qemuDomainAttachDiskGeneric(virQEMUDriverPtr driver,
error:
qemuDomainDelDiskSrcTLSObject(driver, vm, disk->src);
ignore_value(qemuHotplugPrepareDiskAccess(driver, vm, disk, NULL, true));
+ if (prdStarted)
+ qemuProcessKillPRDaemon(vm);
goto cleanup;
}
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 982c989a0a..11aaeabb38 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -2571,7 +2571,7 @@ qemuProcessBuildPRHelperPidfilePath(virDomainObjPtr vm,
}
-static void
+void
qemuProcessKillPRDaemon(virDomainObjPtr vm)
{
virErrorPtr orig_err;
@@ -2629,7 +2629,7 @@ qemuProcessStartPRDaemonHook(void *opaque)
}
-static int
+int
qemuProcessStartPRDaemon(virDomainObjPtr vm,
qemuDomainDiskPRDPtr prd)
{
diff --git a/src/qemu/qemu_process.h b/src/qemu/qemu_process.h
index 2741115673..8df5832e23 100644
--- a/src/qemu/qemu_process.h
+++ b/src/qemu/qemu_process.h
@@ -203,4 +203,9 @@ int qemuProcessRefreshDisks(virQEMUDriverPtr driver,
virDomainObjPtr vm,
qemuDomainAsyncJob asyncJob);
+int qemuProcessStartPRDaemon(virDomainObjPtr vm,
+ qemuDomainDiskPRDPtr prd);
+
+void qemuProcessKillPRDaemon(virDomainObjPtr vm);
+
#endif /* __QEMU_PROCESS_H__ */