From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:37824) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SzsGw-0006wI-II for qemu-devel@nongnu.org; Fri, 10 Aug 2012 12:41:18 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1SzsGv-0005YO-9O for qemu-devel@nongnu.org; Fri, 10 Aug 2012 12:41:14 -0400 Received: from e4.ny.us.ibm.com ([32.97.182.144]:55481) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SzsGv-0005XV-4o for qemu-devel@nongnu.org; Fri, 10 Aug 2012 12:41:13 -0400 Received: from /spool/local by e4.ny.us.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Fri, 10 Aug 2012 12:41:12 -0400 Received: from d01relay06.pok.ibm.com (d01relay06.pok.ibm.com [9.56.227.116]) by d01dlp01.pok.ibm.com (Postfix) with ESMTP id B3D3E38C8056 for ; Fri, 10 Aug 2012 12:41:05 -0400 (EDT) Received: from d01av04.pok.ibm.com (d01av04.pok.ibm.com [9.56.224.64]) by d01relay06.pok.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id q7AGf58I30408810 for ; Fri, 10 Aug 2012 12:41:05 -0400 Received: from d01av04.pok.ibm.com (loopback [127.0.0.1]) by d01av04.pok.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id q7AGf40O006099 for ; Fri, 10 Aug 2012 12:41:05 -0400 Message-ID: <5025399F.3020106@linux.vnet.ibm.com> Date: Fri, 10 Aug 2012 12:41:03 -0400 From: Corey Bryant 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> <5025320C.40206@redhat.com> In-Reply-To: <5025320C.40206@redhat.com> Content-Type: text/plain; charset=ISO-8859-15; format=flowed 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: Kevin Wolf 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 On 08/10/2012 12:08 PM, Kevin Wolf wrote: > 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? Yes, I'll go ahead and move the mon_refcount code from this patch to patch 3. -- Regards, Corey