From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:35826) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SzSnm-0000ZP-FO for qemu-devel@nongnu.org; Thu, 09 Aug 2012 09:29:32 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1SzSng-0000aH-BD for qemu-devel@nongnu.org; Thu, 09 Aug 2012 09:29:26 -0400 Received: from e34.co.us.ibm.com ([32.97.110.152]:34159) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SzSng-0000aC-2k for qemu-devel@nongnu.org; Thu, 09 Aug 2012 09:29:20 -0400 Received: from /spool/local by e34.co.us.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Thu, 9 Aug 2012 07:29:19 -0600 Received: from d03relay05.boulder.ibm.com (d03relay05.boulder.ibm.com [9.17.195.107]) by d03dlp01.boulder.ibm.com (Postfix) with ESMTP id 104961FF0021 for ; Thu, 9 Aug 2012 13:28:56 +0000 (WET) Received: from d03av03.boulder.ibm.com (d03av03.boulder.ibm.com [9.17.195.169]) by d03relay05.boulder.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id q79DSHY5083986 for ; Thu, 9 Aug 2012 07:28:33 -0600 Received: from d03av03.boulder.ibm.com (loopback [127.0.0.1]) by d03av03.boulder.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id q79DS15h004540 for ; Thu, 9 Aug 2012 07:28:02 -0600 Message-ID: <5023BADD.6040508@linux.vnet.ibm.com> Date: Thu, 09 Aug 2012 09:27:57 -0400 From: Corey Bryant MIME-Version: 1.0 References: <1344355108-14786-1-git-send-email-coreyb@linux.vnet.ibm.com> <1344355108-14786-3-git-send-email-coreyb@linux.vnet.ibm.com> <5021472E.3090501@redhat.com> <50238CE5.50105@redhat.com> In-Reply-To: <50238CE5.50105@redhat.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v7 2/6] 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, Eric Blake On 08/09/2012 06:11 AM, Kevin Wolf wrote: > Am 07.08.2012 18:49, schrieb Eric Blake: >> On 08/07/2012 09:58 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. >>> >>> 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 >>> >> >>> + >>> +# @AddfdInfo: >>> +# >>> +# Information about a file descriptor that was added to an fd set. >>> +# >>> +# @fdset_id: The ID of the fd set that @fd was added to. >>> +# >>> +# @fd: The file descriptor that was received via SCM rights and >>> +# added to the fd set. >>> +# >>> +# Since: 1.2.0 >> >> We're not very consistent on '1.2' vs. '1.2.0' in since listings, but >> that's probably worth a global cleanup closer to hard freeze. >> >>> +## >>> +{ 'type': 'AddfdInfo', 'data': {'fdset_id': 'int', 'fd': 'int'} } >> >> This is a new command, so s/fdset_id/fdset-id/ >> >>> + >>> +## >>> +# @add-fd: >>> +# >>> +# Add a file descriptor, that was passed via SCM rights, to an fd set. >>> +# >>> +# @fdset_id: #optional The ID of the fd set to add the file descriptor to. >>> +# >>> +# Returns: @AddfdInfo on success >>> +# If file descriptor was not received, FdNotSupplied >>> +# If @fdset_id does not exist, FdSetNotFound >>> +# >>> +# Notes: The list of fd sets is shared by all monitor connections. >>> +# >>> +# If @fdset_id is not specified, a new fd set will be created. >>> +# >>> +# Since: 1.2.0 >>> +## >>> +{ 'command': 'add-fd', 'data': {'*fdset_id': 'int'}, >> >> Again, s/fdset_id/fdset-id/ >> >>> + 'returns': 'AddfdInfo' } >>> + >>> +## >>> +# @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: #optional The file descriptor that is to be removed. >>> +# >>> +# Returns: Nothing on success >>> +# If @fdset_id or @fd is not found, FdNotFound >>> +# >>> +# Since: 1.2.0 >>> +# >>> +# Notes: The list of fd sets is shared by all monitor connections. >>> +# >>> +# File descriptors that are removed: >>> +# o will not be closed until the reference count corresponding >>> +# to @fdset_id reaches zero. >>> +# o will not be available for use after successful completion >>> +# of the remove-fd command. >>> +# >>> +# If @fd is not specified, all file descriptors in @fdset_id >>> +# will be removed. >>> +## >>> +{ 'command': 'remove-fd', 'data': {'fdset_id': 'int', '*fd': 'int'} } >> >> And again. >> >>> + >>> +## >>> +# @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'} } >> >> Is it worth providing any additional information? For example, knowing >> whether the fd is O_RDONLY, O_WRONLY, or O_RDWR might be beneficial to >> management apps trying to discover what fds are already present after a >> reconnection, in order to decide whether to close them without having to >> resort to /proc/$qemupid/fdinfo/nnn lookups. It might even be worth >> marking such information optional, present only when 'removed':false. > > Why do we even include removed=true file descriptors in query-fdsets? > Shouldn't they appear to be, well, removed from a clients POV? > > The problem with adding flags is the same as with errno numbers: How to > do it in a platform independent way? The management application might > run on a different OS than qemu, so a numeric 'flags' field could have > an entirely different meaning there. > > We could add bools for some flags and an enum for > O_RDONLY/O_WRONLY/O_RDWR, but it's probably better to wait until we know > which of them we really need. Based on the other email thread that's going on in parallel to this, I think these issues are resolved. -- Regards, Corey