Re: [libvirt] [PATCHv2] command: avoid fd leak on failure
On 07/14/2011 11:04 AM, Eric Blake wrote:
virCommandTransferFD promises that the fd is no longer owned by
the caller. Normally, we want the fd to remain open until the
child runs, but in error situations, we must close it earlier.
To avoid any risks of double-close due to misunderstanding about
this interface, change it to take a pointer which gets set to
-1 in the caller to record that the transfer was successful.
* src/util/command.h (virCommandTransferFD): Alter signature.
* src/util/command.c (virCommandTransferFD): Close fd now if we
can't track it to close later.
(virCommandKeepFD): Adjust helper to make this easier.
(virCommandRequireHandshake): Adjust callers.
* src/qemu/qemu_command.c (qemuBuildCommandLine): Likewise.
* src/uml/uml_conf.c (umlBuildCommandLineChr): Likewise.
* tests/commandtest.c (test3): Likewise.
* docs/internals/command.html.in: Update the docs.
---
v2: alter the signature, and adjust all callers, to make it foolproof
at avoiding a double-close
Another flaw in v2, that v1 did not have:
@@ -3698,7 +3698,7 @@ qemuBuildCommandLine(virConnectPtr conn,
goto error;
last_good_net = i;
- virCommandTransferFD(cmd, tapfd);
+ virCommandTransferFD(cmd,&tapfd);
if (snprintf(tapfd_name, sizeof(tapfd_name), "%d",
tapfd)>= sizeof(tapfd_name))
Oops - this prints tapfd_name as -1 instead of the fd that the child
will be inheriting.
But rearranging the lines, and doing snprintf prior to
virCommandTransferFD, is also problematic - if the snprintf fails, then
we go to error without doing the virCommandTransferFD, then the
virCommandPtr no longer closes the fd and we have a leak.
I'm starting to think that resetting fd to -1 as a safety valve causes
more harm than good, and hope that we can go with v1 after all.
--
Eric Blake eblake(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org