From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:44784) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Szrli-0001ZH-IE for qemu-devel@nongnu.org; Fri, 10 Aug 2012 12:08:59 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Szrlg-0000Pk-86 for qemu-devel@nongnu.org; Fri, 10 Aug 2012 12:08:58 -0400 Received: from mx1.redhat.com ([209.132.183.28]:49582) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Szrlg-0000Pg-0d for qemu-devel@nongnu.org; Fri, 10 Aug 2012 12:08:56 -0400 Message-ID: <5025320C.40206@redhat.com> Date: Fri, 10 Aug 2012 18:08:44 +0200 From: Kevin Wolf MIME-Version: 1.0 References: <1344564649-6272-1-git-send-email-coreyb@linux.vnet.ibm.com> <1344564649-6272-3-git-send-email-coreyb@linux.vnet.ibm.com> In-Reply-To: <1344564649-6272-3-git-send-email-coreyb@linux.vnet.ibm.com> Content-Type: text/plain; charset=ISO-8859-15 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v8 2/7] qapi: Introduce add-fd, remove-fd, query-fdsets List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Corey Bryant Cc: aliguori@us.ibm.com, stefanha@linux.vnet.ibm.com, libvir-list@redhat.com, qemu-devel@nongnu.org, lcapitulino@redhat.com, pbonzini@redhat.com, eblake@redhat.com 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 > --- > 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@redhat.com, > kwolf@redhat.com, dberrange@redhat.com) > > v6 > -Make @fd optional for remove-fd (eblake@redhat.com) > -Make @fdset-id optional for add-fd (eblake@redhat.com) > > v7: > -Share fd sets among all monitor connections (kwolf@redhat.com) > -Added mon_refcount to keep track of monitor connection count. > > v8: > -Add opaque string to add-fd/query-fdsets. > (stefanha@linux.vnet.ibm.com) > -Use camel case for structures. (stefanha@linux.vnet.ibm.com) > -Don't return in-use and refcount from query-fdsets. > (stefanha@linux.vnet.ibm.com) > -Don't return removed fd's from query-fdsets. > (stefanha@linux.vnet.ibm.com) > -Use fdset-id rather than fdset_id. (eblake@redhat.com) > -Fix fd leak in qmp_add_fd(). (stefanha@linux.vnet.ibm.com) > -Update QMP errors. (stefanha@linux.vnet.ibm.com, eblake@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? Kevin