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>
> ---
> 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