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@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);