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

2011-11-21 Thread Stefan Berger

On 11/16/2011 09:34 PM, Roopa Prabhu wrote:

From: Roopa Prabhuropra...@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 Prabhuropra...@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);



--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


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

2011-11-21 Thread Eric Blake
That's a long subject line; I shortened it to:

qemu: don't release network actual device twice

On 11/16/2011 07:34 PM, Roopa Prabhu wrote:
 From: Roopa Prabhu ropra...@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.

as well as filtering the uncertainty out of the commit message.

 
 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 ropra...@cisco.com
 ---
  src/qemu/qemu_command.c |2 --
  1 files changed, 0 insertions(+), 2 deletions(-)

Given Stefan's testing, ACK and pushed with commit message modified per
above.

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



signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

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

2011-11-16 Thread Roopa Prabhu
From: Roopa Prabhu ropra...@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.

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 ropra...@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);

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list