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

[Stefan Hajnoczi]

On Tue, Aug 7, 2012 at 4:58 PM, Corey Bryant <address@hidden> wrote:
> +##
> +# @FdsetFdInfo:
> +#
> +# Information about a file descriptor that belongs to an fd set.
> +#
> +# @fd: The file descriptor value.
> +#
> +# @removed: If true, the remove-fd command has been issued for this fd.
> +#
> +# Since: 1.2.0
> +##
> +{ 'type': 'FdsetFdInfo', 'data': {'fd': 'int', 'removed': 'bool'} }

I'm not sure if the removed field is useful, especially since
remove-fd is idempotent (you can keep querying fds and then marking
them removed again until they finally close).  The reason I suggest
minimizing the schema is so that we can change implementation details
later without having to synthesize this state.

> +##
> +# @FdsetInfo:
> +#
> +# Information about an fd set.
> +#
> +# @fdset_id: The ID of the fd set.
> +#
> +# @refcount: A count of the number of outstanding dup() references to
> +#            this fd set.
> +#
> +# @in_use: If true, a monitor is connected and has access to this fd set.
> +#
> +# @fds: A list of file descriptors that belong to this fd set.
> +#
> +# Since: 1.2.0
> +##
> +{ 'type': 'FdsetInfo',
> +  'data': {'fdset_id': 'int', 'refcount': 'int', 'in_use': 'bool',
> +           'fds': ['FdsetFdInfo']} }

Why are refcount and in_use exposed?  How will applications use them?
This seems like internal state to me.

Should add-fd allow the application to associate an opaque string with
the fdset?  This could be used to recover after crash.  Otherwise the
application needs to store the fdset_id -> filename mapping in a file
on disk and hope it was safely stored before crash.  It seems like the
best place to keep this info is with the fdset.

Stefan