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

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 redhat com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org

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