From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:35390) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Symyu-0002uc-99 for qemu-devel@nongnu.org; Tue, 07 Aug 2012 12:50:10 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Symyl-0001LO-7L for qemu-devel@nongnu.org; Tue, 07 Aug 2012 12:50:08 -0400 Received: from mx1.redhat.com ([209.132.183.28]:38210) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Symyk-0001LJ-VP for qemu-devel@nongnu.org; Tue, 07 Aug 2012 12:49:59 -0400 Message-ID: <5021472E.3090501@redhat.com> Date: Tue, 07 Aug 2012 10:49:50 -0600 From: Eric Blake 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> In-Reply-To: <1344355108-14786-3-git-send-email-coreyb@linux.vnet.ibm.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="------------enig868FB1F46AAFA410A02389D3" 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: Corey Bryant Cc: kwolf@redhat.com, aliguori@us.ibm.com, stefanha@linux.vnet.ibm.com, libvir-list@redhat.com, qemu-devel@nongnu.org, lcapitulino@redhat.com This is an OpenPGP/MIME signed message (RFC 2440 and 3156) --------------enig868FB1F46AAFA410A02389D3 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable 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. >=20 > 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. >=20 > 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 >=20 > + > +# @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= =2E > +# > +# 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. > + > +## > +# @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 s= et. > +# > +# @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']} } s/fdset_id/fdset-id/; s/in_use/in-use/ > + > +## > +# @query-fdsets: > +# > +# Return information describing all fd sets. > +# > +# Returns: A list of @FdsetInfo > +# > +# Since: 1.2.0 > +# > +# Notes: The list of fd sets is shared by all monitor connections. > +# > +# File descriptors are not closed until @refcount is zero, > +# and either @in_use is false or @removed is true. > +# > +## > +{ 'command': 'query-fdsets', 'returns': ['FdsetInfo'] } > diff --git a/qerror.c b/qerror.c > index 92c4eff..63a0aa1 100644 > --- a/qerror.c > +++ b/qerror.c > @@ -148,6 +148,10 @@ static const QErrorStringTable qerror_table[] =3D = { > .desc =3D "No file descriptor supplied via SCM_RIGHTS", > }, > { > + .error_fmt =3D QERR_FDSET_NOT_FOUND, > + .desc =3D "File descriptor set with ID '%(id)' not found"= , > + }, Conflicts with Luiz's error cleanups. I'm not sure whether libvirt needs to know this specific error, so I'd rather leave it generic for now= =2E > +++ b/qmp-commands.hx > @@ -926,6 +926,137 @@ Example: > =20 > EQMP > =20 > + { > + .name =3D "add-fd", > + .args_type =3D "fdset_id:i?", > + .params =3D "add-fd fdset_id", More fallout from s/_/-/ in the QMP naming, throughout this file. --=20 Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --------------enig868FB1F46AAFA410A02389D3 Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.12 (GNU/Linux) Comment: Public key at http://people.redhat.com/eblake/eblake.gpg Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/ iQEcBAEBCAAGBQJQIUcuAAoJEKeha0olJ0NqiYMH/2FCp52xY0XNDPf4u99yoNje mjMVZ/m4jwuwJp7oAlwcs2mRqFBm9pQv1JPy5Q69qEMIAl5Ump1YtwPNPn/cCdfZ WpNnMWJ9vnlWargXFj8nUhddvqcey69IheT38ftzRZN+dwDzfaSs0qsdOaox6ldb Rh3eDHFU61nm86MxZOuBcWXBcckropm2OZuoS6LLCA+utBhzKIGDMCoQK/SwvHN0 Z3VftUDAQA57s5+6nbOTMb9jVTHENiVhRvtKApZShn0Sik6gvsSO7K43uSIjDCem dLAB6YmntsuNWwsZ9L+td9x/bQ+A6YdJai1UxUT+51hAg0zXcYbmPuG5lW+rL94= =cDuz -----END PGP SIGNATURE----- --------------enig868FB1F46AAFA410A02389D3--