[libvirt] [PATCHv2] command: avoid fd leak on failure

[Eric Blake]

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