Re: [libvirt] [PATCH v4 12/14] qemu_hotplug: Hotplug of reservations
On 04/10/2018 10:58 AM, Michal Privoznik wrote:
When attaching a disk that requires pr-manager we might need to
plug the pr-manager object too.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/qemu/qemu_hotplug.c | 41 +++++++++++++++++++++++++++++++++++++++++
1 file changed, 41 insertions(+)
My opinion - combine w/ previous patch.
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 468153c79c..43bb910ed6 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -391,6 +391,29 @@ qemuDomainMaybeStartPRDaemon(virDomainObjPtr vm,
}
+static int
+qemuMaybeBuildPRManagerInfoProps(virDomainObjPtr vm,
+ qemuDomainDiskPRDPtr prd,
+ virJSONValuePtr *propsret)
+{
+ size_t i;
+
+ *propsret = NULL;
+
+ 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;
+ }
+ }
I don't understand the need for the above hunk - that was done in
patch11 wasn't it? This is why I like things combined - going back and
forth and changing logic as patches are developed means something may be
missed.
+
+ return qemuBuildPRManagerInfoProps(prd, propsret);
+}
+
+
/**
* qemuDomainAttachDiskGeneric:
*
@@ -411,13 +434,16 @@ qemuDomainAttachDiskGeneric(virQEMUDriverPtr driver,
bool driveAdded = false;
bool secobjAdded = false;
bool encobjAdded = false;
+ bool prmgrAdded = false;
bool prdStarted = false;
virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
virJSONValuePtr secobjProps = NULL;
virJSONValuePtr encobjProps = NULL;
+ virJSONValuePtr prmgrProps = NULL;
qemuDomainStorageSourcePrivatePtr srcPriv;
qemuDomainSecretInfoPtr secinfo = NULL;
qemuDomainSecretInfoPtr encinfo = NULL;
+ qemuDomainDiskPRDPtr prd = NULL;
if (qemuHotplugPrepareDiskAccess(driver, vm, disk, NULL, false) < 0)
goto cleanup;
@@ -437,6 +463,7 @@ qemuDomainAttachDiskGeneric(virQEMUDriverPtr driver,
if (srcPriv) {
secinfo = srcPriv->secinfo;
encinfo = srcPriv->encinfo;
+ prd = srcPriv->prd;
As noted much earlier - the private data isn't needed here as the prd is
in the disk storage definition.
}
if (secinfo && secinfo->type == VIR_DOMAIN_SECRET_INFO_TYPE_AES) {
@@ -447,6 +474,9 @@ qemuDomainAttachDiskGeneric(virQEMUDriverPtr driver,
if (encinfo && qemuBuildSecretInfoProps(encinfo, &encobjProps) < 0)
goto error;
+ if (qemuMaybeBuildPRManagerInfoProps(vm, prd, &prmgrProps) < 0)
+ goto error;
+
This should just call qemuBuildPRManagerInfoProps directly since it
already checks if !prd || !prd->alias
John
if (disk->src->haveTLS &&
qemuDomainAddDiskSrcTLSObject(driver, vm, disk->src,
disk->info.alias) < 0)
@@ -484,6 +514,15 @@ qemuDomainAttachDiskGeneric(virQEMUDriverPtr driver,
encobjAdded = true;
}
+ if (prmgrProps) {
+ rv = qemuMonitorAddObject(priv->mon, "pr-manager-helper",
prd->alias,
+ prmgrProps);
+ prmgrProps = NULL; /* qemuMonitorAddObject consumes */
+ if (rv < 0)
+ goto exit_monitor;
+ prmgrAdded = true;
+ }
+
if (qemuMonitorAddDrive(priv->mon, drivestr) < 0)
goto exit_monitor;
driveAdded = true;
@@ -521,6 +560,8 @@ qemuDomainAttachDiskGeneric(virQEMUDriverPtr driver,
ignore_value(qemuMonitorDelObject(priv->mon, secinfo->s.aes.alias));
if (encobjAdded)
ignore_value(qemuMonitorDelObject(priv->mon, encinfo->s.aes.alias));
+ if (prmgrAdded)
+ ignore_value(qemuMonitorDelObject(priv->mon, prd->alias));
if (qemuDomainObjExitMonitor(driver, vm) < 0)
ret = -2;
virErrorRestore(&orig_err);