[libvirt] [PATCH] hotplug: Fix libvirtd crash on qemu-attached guest

[Eric Blake]

On 09/22/2014 02:49 PM, John Ferlan wrote:
> https://bugzilla.redhat.com/show_bug.cgi?id=1141621
> 
> Using qemu-attach to attach to a qemu created process and then
> attempting to virsh detach-interface caused a libvirtd crash
> since the assumption was that device aliases were in place and
> that assumption is not necessarily true for the qemu-attach environment.

Hmm. I'm wondering whether it is better to patch qemu-attach to populate
alias information at attach time rather than having to chase all the
places that will later fail when the assumption is broken.  While your
patch is certainly more appealing for being smaller, it makes me wonder
if it is the only such place bitten by the assumptions.

> 
> Add some more verbiage to the virsh man page.
> 
> Signed-off-by: John Ferlan <jferlan at redhat.com>
> ---
> Notes:
>  * The bz also mentions a failure for the hotplug/virsh attach-interface,
>    but that has been resolved by commit id 'ba7468db'
>  * For the bz, the attempt to attach-interface/hotplug still is not
>    successful, since device aliases are not set for the PCI controller.
>    * I did try adding a call to qemuAssignDeviceAliases() during the qemu-
>      attach code; however, that failed as well (perhaps for a different
>      reason) as the attach to PCI slot 3 function 0 was not available
>      for rtl8139 since it was being used by e1000

That is, I'm wondering if this approach should be used after all, and we
have yet another problem to solve first.

>    * If it's desired to add the alias call in that's fine, then that could
>      be added in qemuDomainQemuAttach prior to the qemuDomainAssignAddresses 
>      call similar to qemuConnectDomainXMLToNative.
> 
>  src/qemu/qemu_hotplug.c | 3 ++-
>  tools/virsh.pod         | 5 +++--
>  2 files changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
> index 7bc19cd..b7514ce 100644
> --- a/src/qemu/qemu_hotplug.c
> +++ b/src/qemu/qemu_hotplug.c
> @@ -3520,7 +3520,8 @@ qemuDomainDetachNetDevice(virConnectPtr conn,
>      qemuDomainMarkDeviceForRemoval(vm, &detach->info);
>  
>      qemuDomainObjEnterMonitor(driver, vm);
> -    if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE)) {
> +    if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE) &&
> +        detach->info.alias) {
>          if (qemuMonitorDelDevice(priv->mon, detach->info.alias) < 0) {
>              qemuDomainObjExitMonitor(driver, vm);
>              virDomainAuditNet(vm, detach, NULL, "detach", false);
> diff --git a/tools/virsh.pod b/tools/virsh.pod
> index 9919f92..947adaf 100644
> --- a/tools/virsh.pod
> +++ b/tools/virsh.pod
> @@ -3686,8 +3686,9 @@ using the UNIX driver. Ideally the process will also have had the
>  
>  Not all functions of libvirt are expected to work reliably after
>  attaching to an externally launched QEMU process. There may be
> -issues with the guest ABI changing upon migration, and hotunplug
> -may not work.
> +issues with the guest ABI changing upon migration and device hotplug
> +or hotunplug may not work. The attached environment should be considered
> +primarily read-only.

This doc hunk is fine to push now, whether or not we have consensus on
the code side.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 539 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20140922/15e041c1/attachment-0001.sig>