Re: [libvirt] [PATCH v11 3/5] qemu: Need to remove TLS object in RemoveRNGDevice

On 10/25/2016 08:53 AM, Pavel Hrdina wrote: > On Mon, Oct 24, 2016 at 06:46:19PM -0400, John Ferlan wrote: >> Commit id '6e6b4bfc' added the object, but forgot the other end. >> >> Signed-off-by: John Ferlan <jferlan(a)redhat.com&gt; >> --- >> src/qemu/qemu_hotplug.c | 12 +++++++++++- >> 1 file changed, 11 insertions(+), 1 deletion(-) > > ACK if you agree with review on the first patch that the order of > qemuMonitorDelObject and qemuMonitorDetachCharDev should be swapped. > > Pavel > I do agree, but I hadn't fully convinced myself - I needed the extra set of eyes to help with that... Of course, I botched that again here by removing the tlsobj before the chardev. A problem which naturally persists in patch 5, but is easily adjusted.. I looked at and thought about qemuDomainRemoveRNGDevice long enough and compared to qemuDomainRemoveDiskDevice, but couldn't quite make up my mind and really didn't want to be creating "extra" patches for perhaps all the erroneous removals. If my comparison is {Attach|Remove}Disk, which has... Attach: Add secobj, Add encobj, Add Drive, and Add Device where drive can rely on secobj and/or encobj. The encobj and secobj do not rely on each other. The secobj is the possible secret for the iSCSI/RBD server, while encobj is the possible secret for On attach error the removal is Del Drive, Del secobj, Del encobj (order of secobj/encobj doesn't matter). Detach: Del Device and call RemoveDisk which Del secobj, Del encobj, and Del Drive. Then, the RemoveDisk is probably wrong (Del Drive should be first), but I didn't want to continue to bog down the chardev changes if my thoughts in patch 2 were wrong.

Long way of saying, I want to get it right for this path and then if I'm right generate a RemoveDisk patch to swap order. John >> >> diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c >> index e287aaf..f401b48 100644 >> --- a/src/qemu/qemu_hotplug.c >> +++ b/src/qemu/qemu_hotplug.c >> @@ -3608,6 +3608,7 @@ qemuDomainRemoveRNGDevice(virQEMUDriverPtr driver, >> virObjectEventPtr event; >> char *charAlias = NULL; >> char *objAlias = NULL; >> + char *tlsAlias = NULL; >> qemuDomainObjPrivatePtr priv = vm->privateData; >> ssize_t idx; >> int ret = -1; >> @@ -3616,15 +3617,23 @@ qemuDomainRemoveRNGDevice(virQEMUDriverPtr driver, >> VIR_DEBUG("Removing RNG device %s from domain %p %s", >> rng->info.alias, vm, vm->def->name); >> >> + >> if (virAsprintf(&objAlias, "obj%s", rng->info.alias) < 0) >> goto cleanup; >> >> if (!(charAlias = qemuAliasChardevFromDevAlias(rng->info.alias))) >> goto cleanup; >> >> + if (rng->backend == VIR_DOMAIN_RNG_BACKEND_EGD && >> + !(tlsAlias = qemuAliasTLSObjFromChardevAlias(charAlias))) >> + goto cleanup; >> + >> qemuDomainObjEnterMonitor(driver, vm); >> - if (rng->backend == VIR_DOMAIN_RNG_BACKEND_EGD) >> + if (rng->backend == VIR_DOMAIN_RNG_BACKEND_EGD) { >> + if (tlsAlias) >> + ignore_value(qemuMonitorDelObject(priv->mon, tlsAlias)); >> ignore_value(qemuMonitorDetachCharDev(priv->mon, charAlias)); >> + } >> >> rc = qemuMonitorDelObject(priv->mon, objAlias); >> >> @@ -3648,6 +3657,7 @@ qemuDomainRemoveRNGDevice(virQEMUDriverPtr driver, >> cleanup: >> VIR_FREE(charAlias); >> VIR_FREE(objAlias); >> + VIR_FREE(tlsAlias); >> return ret; >> } >> >> -- >> 2.7.4 >> >> -- >> libvir-list mailing list >> libvir-list(a)redhat.com >> https://www.redhat.com/mailman/listinfo/libvir-list -- libvir-list mailing list libvir-list(a)redhat.com https://www.redhat.com/mailman/listinfo/libvir-list