Re: [Qemu-devel] [PATCH v4 3/7] qapi: Add pass-fd QMP command

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 06/22/2012 04:24 PM, Eric Blake wrote:
On 06/22/2012 12:36 PM, Corey Bryant wrote:
This patch adds the pass-fd QMP command using the QAPI framework.
Like the getfd command, it is used to pass a file descriptor via
SCM_RIGHTS and associate it with a name.  However, the pass-fd
command also returns the received file descriptor, which is a
difference in behavior from the getfd command, which returns
nothing.  pass-fd also supports a boolean "force" argument that
can be used to specify that an existing file descriptor is
associated with the specified name, and that it should become a
copy of the newly passed file descriptor.

+        if (replace) {
+            /* the existing fd is replaced with the new fd */
+            close(monfd->fd);
+            monfd->fd = fd;
+            return fd;
+        }
+
+        if (copy) {

Since 'replace' and 'copy' are never both true, should this instead be a
tri-state enum argument instead of two independent bool arguments?


Sure, that makes sense.  I'll do that in v5.

+            /* the existing fd becomes a copy of the new fd */
+            if (dup2(fd, monfd->fd) == -1) {

Broken - you want to use dup3(fd, monfd->fd, O_CLOEXEC) (and fall back
to dup2()+fcntl(F_GETFD/F_SETFD) on platforms where dup3 is not
available; it has been proposed for the next revision of POSIX but right
now is pretty much limited to Linux and Cygwin).  Otherwise, you are
undoing the fact that patch 1/7 just changed the 'getfd' list to always
keep all its fds marked cloexec.


Thanks for catching this. I'll fix this in v5. In terms of platforms that support dup3 vs dup2, I'm assuming the following preprocessor checks will do what we need:

#if defined(__linux__) || defined(__CYGWIN__)
dup3(fd, monfd->fd, O_CLOEXEC)
#else
dup2()+fcntl(F_GETFD/F_SETFD)
#endif

--
Regards,
Corey


--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list


[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]