[libvirt] [PATCH 2/4] qemu: Remove interface backend only after frontend is gone

[Peter Krempa]

On 05/27/14 16:53, Jiri Denemark wrote:
> [1] reported that we are removing network's backend too early. I didn't
> really get the reproducer but libvirt behaves strangely when a guest
> does not confirm the removal, e.g., it does not support PCI hotplug. In
> such case, detaching a network device leaves its frontend in place but
> removes the backend, which makes the device unusable for the guest.
> Moreover attaching the same device again succeeds and both the guest and
> libvirt will see two network interfaces attached but only one of them is
> actually working.
> 
> I checked with Paolo Bonzini and he confirmed we should only remove a
> backend after seeing DEVICE_DELETED event for a corresponding frontend.
> 
> [1] https://www.redhat.com/archives/libvir-list/2014-March/msg01740.html
> 
> Signed-off-by: Jiri Denemark <jdenemar at redhat.com>
> ---
>  src/qemu/qemu_hotplug.c | 56 +++++++++++++++++++++++++------------------------
>  1 file changed, 29 insertions(+), 27 deletions(-)
> 
> diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
> index b209b04..fd1f002 100644
> --- a/src/qemu/qemu_hotplug.c
> +++ b/src/qemu/qemu_hotplug.c
> @@ -2636,8 +2636,10 @@ qemuDomainRemoveNetDevice(virQEMUDriverPtr driver,
>                            virDomainNetDefPtr net)
>  {
>      virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
> +    qemuDomainObjPrivatePtr priv = vm->privateData;
>      virNetDevVPortProfilePtr vport;
>      virObjectEventPtr event;
> +    char *hostnet_name = NULL;
>      size_t i;
>  
>      if (virDomainNetGetActualType(net) == VIR_DOMAIN_NET_TYPE_HOSTDEV) {
> @@ -2649,6 +2651,32 @@ qemuDomainRemoveNetDevice(virQEMUDriverPtr driver,
>      VIR_DEBUG("Removing network interface %s from domain %p %s",
>                net->info.alias, vm, vm->def->name);
>  
> +    if (virAsprintf(&hostnet_name, "host%s", net->info.alias) < 0)
> +        goto cleanup;
> +
> +    qemuDomainObjEnterMonitor(driver, vm);
> +    if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_NETDEV) &&
> +        virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE)) {
> +        if (qemuMonitorRemoveNetdev(priv->mon, hostnet_name) < 0) {
> +            qemuDomainObjExitMonitor(driver, vm);
> +            virDomainAuditNet(vm, net, NULL, "detach", false);
> +            goto cleanup;
> +        }
> +    } else {
> +        int vlan;
> +        if ((vlan = qemuDomainNetVLAN(net)) < 0 ||
> +            qemuMonitorRemoveHostNetwork(priv->mon, vlan, hostnet_name) < 0) {
> +            if (vlan < 0) {
> +                virReportError(VIR_ERR_OPERATION_FAILED, "%s",
> +                               _("unable to determine original VLAN"));


This function is void, so this error won't get propagated as it was in
the original place. As this function is called from the place the above
code originally was you should probably convert it to return int and
propagate the error.

> +            }
> +            qemuDomainObjExitMonitor(driver, vm);
> +            virDomainAuditNet(vm, net, NULL, "detach", false);
> +            goto cleanup;
> +        }
> +    }
> +    qemuDomainObjExitMonitor(driver, vm);
> +
>      virDomainAuditNet(vm, net, NULL, "detach", true);
>  
>      event = virDomainEventDeviceRemovedNewFromObj(vm, net->info.alias);

Peter


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