[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH] qemu: add macvlan delete to qemuDomainAttachNetDevice cleanup

On 07/01/2013 11:04 AM, Viktor Mihajlovski wrote:
> From: Matthew Rosato <mjrosato linux vnet ibm com>
> If an error occurs during qemuDomainAttachNetDevice after the macvtap
> was created in qemuPhysIfaceConnect, the macvtap device gets left behind.
> This patch adds code to the cleanup routine to delete the macvtap.
> Signed-off-by: Matthew Rosato <mjrosato linux vnet ibm com>
> Reviewed-by: Viktor Mihajlovski <mihajlov linux vnet ibm com>
> ---
>  src/qemu/qemu_hotplug.c |   10 ++++++++++
>  1 file changed, 10 insertions(+)
> diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
> index 46875ad..c6045a0 100644
> --- a/src/qemu/qemu_hotplug.c
> +++ b/src/qemu/qemu_hotplug.c
> @@ -965,6 +965,16 @@ cleanup:
>          if (iface_connected) {
>              virDomainConfNWFilterTeardown(net);
> +            if (virDomainNetGetActualType(net) == VIR_DOMAIN_NET_TYPE_DIRECT) {
> +                ignore_value(virNetDevMacVLanDeleteWithVPortProfile(
> +                                 net->ifname, &net->mac,
> +                                 virDomainNetGetActualDirectDev(net),
> +                                 virDomainNetGetActualDirectMode(net),
> +                                 virDomainNetGetActualVirtPortProfile(net),
> +                                 cfg->stateDir));
> +                VIR_FREE(net->ifname);
> +            }
> +

This is a good catch, but incomplete. If you search for other
occurrences of virDomainConfNWFilterTeardown() and
qemuPhysIfaceConnect(), you will find the same problem exists in two
other places in the code:

   qemuBuildInterfaceCommandLine (during error cleanup, needs to be called
                                  for the one interface that was partially
   qemuBuildCommandLine          (during error cleanup, needs to be called
                                  for all interfaces that were completely
                                  created (up to last_good_net))

We really should fix them all in one patch, since they are all the same

(Ideally, *all* guest interface setup for each interface should be
handled in a single function, and that function should be in the network
driver (networkReleaseActualDevice() seems properly situated). That way
it could be put behind an RPC, and the non-privileged libvirtd could
call it too (with proper credentials). That is a larger problem, though.)

[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]