Re: [Qemu-devel] [PATCH v4 4/4] qemu-config: Add new -add-fd command line option

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 1352 bytes --]

On 10/18/2012 01:19 PM, Corey Bryant wrote:
> This option can be used for passing file descriptors on the
> command line.  It mirrors the existing add-fd QMP command which
> allows an fd to be passed to QEMU via SCM_RIGHTS and added to an
> fd set.
> 
> +
> +static int cleanup_add_fd(QemuOpts *opts, void *opaque)
> +{
> +    int fd;
> +
> +    fd = qemu_opt_get_number(opts, "fd", -1);
> +    close(fd);

One other subtle point: Given 'qemu-kvm -add-fd fd=3,set=1 -add-fd
fd=3,set=2', this code will call close(3) twice.  In a single-threaded
scenario, we happen to be safe (because we merely ignore that the second
one fails with EBADF), but in a multi-threaded scenario, it is a recipe
for disaster with a chance for closing an fd opened by another thread.

> @@ -3320,6 +3390,14 @@ int main(int argc, char **argv, char **envp)
>          exit(1);
>      }
>  
> +    if (qemu_opts_foreach(qemu_find_opts("add-fd"), parse_add_fd, NULL, 1)) {
> +        exit(1);
> +    }
> +
> +    if (qemu_opts_foreach(qemu_find_opts("add-fd"), cleanup_add_fd, NULL, 1)) {
> +        exit(1);
> +    }

Thankfully, we only ever call that code in main(), prior to spawning
threads.  So my positive review still stands.

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 617 bytes --]