Re: [libvirt] [PATCH v8 2/7] qapi: Introduce add-fd, remove-fd, query-fdsets

Am 10.08.2012 04:10, schrieb Corey Bryant: > This patch adds support that enables passing of file descriptors > to the QEMU monitor where they will be stored in specified file > descriptor sets. > > A file descriptor set can be used by a client like libvirt to > store file descriptors for the same file. This allows the > client to open a file with different access modes (O_RDWR, > O_WRONLY, O_RDONLY) and add/remove the passed fds to/from an fd > set as needed. This will allow QEMU to (in a later patch in this > series) "open" and "reopen" the same file by dup()ing the fd in > the fd set that corresponds to the file, where the fd has the > matching access mode flag that QEMU requests. > > The new QMP commands are: > add-fd: Add a file descriptor to an fd set > remove-fd: Remove a file descriptor from an fd set > query-fdsets: Return information describing all fd sets > > Note: These commands are not compatible with the existing getfd > and closefd QMP commands. > > Signed-off-by: Corey Bryant <coreyb(a)linux.vnet.ibm.com&gt; > --- > v5: > -This patch is new in v5 and replaces the pass-fd QMP command > from v4. > -By grouping fds in fd sets, we ease managability with an fd > set per file, addressing concerns raised in v4 about handling > "reopens" and preventing fd leakage. (eblake(a)redhat.com, > kwolf(a)redhat.com, dberrange(a)redhat.com) > > v6 > -Make @fd optional for remove-fd (eblake(a)redhat.com) > -Make @fdset-id optional for add-fd (eblake(a)redhat.com) > > v7: > -Share fd sets among all monitor connections (kwolf(a)redhat.com) > -Added mon_refcount to keep track of monitor connection count. > > v8: > -Add opaque string to add-fd/query-fdsets. > (stefanha(a)linux.vnet.ibm.com) > -Use camel case for structures. (stefanha(a)linux.vnet.ibm.com) > -Don't return in-use and refcount from query-fdsets. > (stefanha(a)linux.vnet.ibm.com) > -Don't return removed fd's from query-fdsets. > (stefanha(a)linux.vnet.ibm.com) > -Use fdset-id rather than fdset_id. (eblake(a)redhat.com) > -Fix fd leak in qmp_add_fd(). (stefanha(a)linux.vnet.ibm.com) > -Update QMP errors. (stefanha(a)linux.vnet.ibm.com, eblake(a)redhat.com) > > monitor.c | 188 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ > qapi-schema.json | 98 ++++++++++++++++++++++++++++ > qmp-commands.hx | 117 +++++++++++++++++++++++++++++++++ > 3 files changed, 403 insertions(+) > > diff --git a/monitor.c b/monitor.c > index 49dccfe..f9a0577 100644 > --- a/monitor.c > +++ b/monitor.c > @@ -140,6 +140,24 @@ struct mon_fd_t { > QLIST_ENTRY(mon_fd_t) next; > }; > > +/* file descriptor associated with a file descriptor set */ > +typedef struct MonFdsetFd MonFdsetFd; > +struct MonFdsetFd { > + int fd; > + bool removed; > + char *opaque; > + QLIST_ENTRY(MonFdsetFd) next; > +}; > + > +/* file descriptor set containing fds passed via SCM_RIGHTS */ > +typedef struct MonFdset MonFdset; > +struct MonFdset { > + int64_t id; > + int refcount; > + QLIST_HEAD(, MonFdsetFd) fds; > + QLIST_ENTRY(MonFdset) next; > +}; > + > typedef struct MonitorControl { > QObject *id; > JSONMessageParser parser; > @@ -211,6 +229,8 @@ static inline int mon_print_count_get(const Monitor *mon) { return 0; } > #define QMP_ACCEPT_UNKNOWNS 1 > > static QLIST_HEAD(mon_list, Monitor) mon_list; > +static QLIST_HEAD(mon_fdsets, MonFdset) mon_fdsets; > +static int mon_refcount; You introduce mon_refcount in this patch and check it against 0 in one place, but it never gets changed until patch 3 is applied. Would it make sense to do both in the same patch?