Re: [Qemu-devel] [PATCH v5 2/6] qapi: Introduce add-fd, remove-fd, query-fdsets

[Corey Bryant]

On 07/25/2012 02:16 PM, Eric Blake wrote:
> On 07/23/2012 07:08 AM, Corey Bryant wrote:
> > This patch adds support that enables passing of file descriptors
> > to the QEMU monitor where they will be stored in specified file
> > descriptor sets.
> >
> > +##
> > +# @add-fd:
> > +#
> > +# Add a file descriptor, that was passed via SCM rights, to an fd set.
> > +#
> > +# @fdset_id: The ID of the fd set to add the file descriptor to.
>
> Should this parameter be optional, in which case qemu generates the set
> number as the next available?  The return information would include the
> set number whether or not this parameter was present from the caller,
> and allowing it to be optional makes it slightly easier for callers to
> generate a new set without having to make the caller track which set ids
> are in use.

Yes, I agree, I think @fdset_id should be optional.

> > +##
> > +# @remove-fd:
> > +#
> > +# Remove a file descriptor from an fd set.
> > +#
> > +# @fdset_id: The ID of the fd set that the file descriptor belongs to.
> > +#
> > +# @fd: The file descriptor that is to be removed.
>
> Should 'fd' be optional, as a shorthand for saying that _all_ fds
> belonging to the set be marked as removed at once?

Yes, this makes sense.

> > +++ b/qmp-commands.hx
> > @@ -926,6 +926,127 @@ Example:
> >
> >   EQMP
> >
> > +     {
> > +        .name       = "add-fd",
> > +        .args_type  = "fdset_id:i",
> > +        .params     = "add-fd fdset_id",
> > +        .help       = "Add a file descriptor, that was passed via SCM rights, to an 
> > fd set",
> > +        .mhandler.cmd_new = qmp_marshal_input_add_fd,
> > +    },
> > +
> > +SQMP
> > +add-fd
> > +-------
> > +
> > +Add a file descriptor, that was passed via SCM rights, to an fd set.
> > +
> > +Arguments:
> > +
> > +- "fdset_id": The ID of the fd set to add the file descriptor to.  (json-int)
>
> May need tweaking, based on the answers to the above questions.

Ok.

--
Regards,
Corey