On 10/25/2016 12:04 PM, Pavel Hrdina wrote:
On Tue, Oct 25, 2016 at 09:19:58AM -0400, John Ferlan wrote:
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@redhat.com> --- 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.
I think that the RemoveDisk is wrong and that the Del Drive should be first. As you've wrote, the drive can rely on secobj and/or encobj so we should not remove the object while they can be still used.
RemoveDisk is a different/separate issue - I sent a separate patch. As for the rest of this - I think in my latest response to 2/5 - the pencil and paper exercise shows the delete TLS object goes after the Detach chardev below. That then just leaves fixing up the order for the secret object in patch 5 of this series. John
Pavel
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@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list