[libvirt] [PATCH] Bug Fix: Do not release network actual device in qemuBuildCommandLine on error
From: Roopa Prabhu 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 --- 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
On 11/16/2011 09:34 PM, Roopa Prabhu wrote: From: Roopa Prabhu 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 --- 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
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 > > 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 > --- > 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