[libvirt] [PATCH] Bug Fix: Do not release network actual device in qemuBuildCommandLine on error

[Stefan Berger]

On 11/16/2011 09:34 PM, Roopa Prabhu wrote:
> From: Roopa Prabhu<roprabhu at cisco.com>
>
> For direct attach devices, in qemuBuildCommandLine, we seem to be freeing
> actual device on error path (with networkReleaseActualDevice). But the actual
> device is not deleted.
>
> qemuProcessStop eventually deletes the direct attach device and releases actual device. But by the time qemuProcessStop is called qemuBuildCommandLine
> has already freed actual device. Leaving stray macvtap devices behind on error.
> So the simplest fix is to remove the networkReleaseActualDevice in
> qemuBuildCommandLine. This patch does just that.
>
> Does this look right ?. I have only verified this with direct and bridge mode.
You're right and to me your patch looks ok too. When using the interface 
type=network and then network in 'macvtap direction connection' mode [1] 
and an error happens in qemuBuildCommandLine I also see a stray macvtap 
device left behind. We're calling networkReleaseActualDevice two times 
and that seems to not work in this configuration. In the case when 
macvtap is used via interface type='direct' it works fine with or 
without your patch.

[1] file:///root/tmp/libvirt-acl/docs/formatnetwork.html#examplesDirect


    Stefan
> The other option is to do both delMacvtap and networkReleaseActualDevice in
> qemuBuildCommandLine instead of doing only networkReleaseActualDevice.
> I do have a patch for this too.
>
> Signed-off-by: Roopa Prabhu<roprabhu at cisco.com>
> ---
>   src/qemu/qemu_command.c |    2 --
>   1 files changed, 0 insertions(+), 2 deletions(-)
>
>
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index bb12016..ba33a4a 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -5378,8 +5378,6 @@ qemuBuildCommandLine(virConnectPtr conn,
>       virReportOOMError();
>    error:
>       /* free up any resources in the network driver */
> -    for (i = 0 ; i<  def->nnets ; i++)
> -        networkReleaseActualDevice(def->nets[i]);
>       for (i = 0; i<= last_good_net; i++)
>           virDomainConfNWFilterTeardown(def->nets[i]);
>       virCommandFree(cmd);
>