On 04/10/2018 10:58 AM, Michal Privoznik wrote:
If we are the last one to use pr-manager object we need to remove it and also kill the qemu-pr-helper process.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_hotplug.c | 38 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+)
Again, my opinion this is combined too - patch is larger, but everything is covered at one time.
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 43bb910ed6..98e1bf7082 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -3894,6 +3894,34 @@ static bool qemuIsMultiFunctionDevice(virDomainDefPtr def, }
+static qemuDomainDiskPRDPtr +qemuDomainDiskNeedRemovePR(virDomainObjPtr vm, + virDomainDiskDefPtr disk) +{ + qemuDomainStorageSourcePrivatePtr srcPriv; + size_t i; + + if (!virStoragePRDefIsManaged(disk->src->pr)) + return NULL; + + for (i = 0; i < vm->def->ndisks; i++) { + const virDomainDiskDef *domainDisk = vm->def->disks[i]; + + if (domainDisk == disk) + continue; + + if (virStoragePRDefIsManaged(domainDisk->src->pr)) + break; + } + + if (i != vm->def->ndisks) + return NULL; + + srcPriv = QEMU_DOMAIN_STORAGE_SOURCE_PRIVATE(disk->src); + return srcPriv->prd; +} + +
Trying to combine two separate things into one? Thing 1 -> Remove the object from the domain Thing 2 -> Kill the PRD helper *iff* this is the last one using it
static int qemuDomainRemoveDiskDevice(virQEMUDriverPtr driver, virDomainObjPtr vm, @@ -3907,6 +3935,7 @@ qemuDomainRemoveDiskDevice(virQEMUDriverPtr driver, char *drivestr; char *objAlias = NULL; char *encAlias = NULL; + qemuDomainDiskPRDPtr prd = NULL;
VIR_DEBUG("Removing disk %s from domain %p %s", disk->info.alias, vm, vm->def->name); @@ -3944,6 +3973,8 @@ qemuDomainRemoveDiskDevice(virQEMUDriverPtr driver, } }
+ prd = qemuDomainDiskNeedRemovePR(vm, disk); + qemuDomainObjEnterMonitor(driver, vm);
qemuMonitorDriveDel(priv->mon, drivestr); @@ -3959,6 +3990,10 @@ qemuDomainRemoveDiskDevice(virQEMUDriverPtr driver, ignore_value(qemuMonitorDelObject(priv->mon, encAlias)); VIR_FREE(encAlias);
+ /* If it fails, then so be it - it was a best shot */ + if (prd) + ignore_value(qemuMonitorDelObject(priv->mon, prd->alias)); +
OK - remove the object if we have one...
if (disk->src->haveTLS) ignore_value(qemuMonitorDelObject(priv->mon, disk->src->tlsAlias));
@@ -3977,6 +4012,9 @@ qemuDomainRemoveDiskDevice(virQEMUDriverPtr driver, } }
+ if (prd) + qemuProcessKillPRDaemon(vm); +
But we only want to kill if this if there are no other disk's needing the pr-helper, right? So we need to have a routine that would be run after the disk is really removed. The two phase removal process always gets me ;-) - that is Detach vs. Remove and which one is last *after* the disk is removed from the list of disks *and* when we know the object has been removed from the domain, then we can stop the PR process iff this is the last one using it. John
qemuDomainReleaseDeviceAddress(vm, &disk->info, src);
if (qemuSecurityRestoreDiskLabel(driver, vm, disk) < 0)