[Qemu-devel] [PATCH 0/3] file descriptor passing using getfd over QMP

[Qemu-devel] [PATCH 0/3] file descriptor passing using getfd over QMP

[Corey Bryant]

libvirt's sVirt security driver provides SELinux MAC isolation for
Qemu guest processes and their corresponding image files.  In other
words, sVirt uses SELinux to prevent a QEMU process from opening
files that do not belong to it.

sVirt provides this support by labeling guests and resources with
security labels that are stored in file system extended attributes.
Some file systems, such as NFS, do not support the extended
attribute security namespace, and therefore cannot support sVirt
isolation.

A solution to this problem is to provide fd passing support, where
libvirt opens files and passes file descriptors to QEMU.  This,
along with SELinux policy to prevent QEMU from opening files, can
provide image file isolation for NFS files.

This patch series adds the getfd QMP monitor command, which allows
an fd to be passed via SCM_RIGHTS, and returns the received file
descriptor.  Support is also added to the block layer to allow QEMU
to dup() the fd when the filename is of the /dev/fd/X format.  This
is useful if MAC policy prevents QEMU from open()ing specific types
of files.

This solution will enable libvirt to open() an image file and push
the file descriptor down to QEMU.  When QEMU needs to "open" a file,
it will first check if the filename is of the /dev/fd/X format and
dup(X) if so.  Otherwise it will continue and open() the file as it
has in the past.

This series reuses the file_open() function that Anthony Liguori
<aliguori@us.ibm.com> created in a recent fd passing prototype.  It
also reuses the test driver that Stefan Hajnoczi
<stefanha@linux.vnet.ibm.com> created for that prototype, but with
several modifications.

Corey Bryant (3):
  qmp/hmp: Add QMP getfd command that returns fd
  block: Add support to "open" /dev/fd/X filenames
  Sample server that opens image files for QEMU

 block.c           |   15 +++
 block/raw-posix.c |   20 ++--
 block/raw-win32.c |    4 +-
 block/vdi.c       |    5 +-
 block/vmdk.c      |   21 ++--
 block/vpc.c       |    2 +-
 block/vvfat.c     |   21 ++--
 block_int.h       |   13 +++
 hmp-commands.hx   |    2 +-
 monitor.c         |   37 +++++-
 qapi-schema.json  |   13 +++
 qmp-commands.hx   |    6 +-
 test-fd-passing.c |  321 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 13 files changed, 439 insertions(+), 41 deletions(-)
 create mode 100644 test-fd-passing.c

-- 
1.7.10.2

[Qemu-devel] [PATCH 1/3] qmp/hmp: Add QMP getfd command that returns fd

[Corey Bryant]

This patch adds QMP support for the getfd command using the QAPI framework.
Like the HMP getfd command, it is used to pass a file descriptor via
SCM_RIGHTS.  However, the QMP getfd command also returns the received file
descriptor, which is a difference in behavior from the HMP getfd command,
which returns nothing.

Signed-off-by: Corey Bryant <coreyb@linux.vnet.ibm.com>
---
 hmp-commands.hx  |    2 +-
 monitor.c        |   37 ++++++++++++++++++++++++++++++++++++-
 qapi-schema.json |   13 +++++++++++++
 qmp-commands.hx  |    6 ++++--
 4 files changed, 54 insertions(+), 4 deletions(-)

diff --git a/hmp-commands.hx b/hmp-commands.hx
index 18cb415..dfab369 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -1211,7 +1211,7 @@ ETEXI
         .params     = "getfd name",
         .help       = "receive a file descriptor via SCM rights and assign it a name",
         .user_print = monitor_user_noop,
-        .mhandler.cmd_new = do_getfd,
+        .mhandler.cmd_new = hmp_getfd,
     },
 
 STEXI
diff --git a/monitor.c b/monitor.c
index 12a6fe2..6acf5a3 100644
--- a/monitor.c
+++ b/monitor.c
@@ -2199,7 +2199,7 @@ static void do_inject_mce(Monitor *mon, const QDict *qdict)
 }
 #endif
 
-static int do_getfd(Monitor *mon, const QDict *qdict, QObject **ret_data)
+static int hmp_getfd(Monitor *mon, const QDict *qdict, QObject **ret_data)
 {
     const char *fdname = qdict_get_str(qdict, "fdname");
     mon_fd_t *monfd;
@@ -2235,6 +2235,41 @@ static int do_getfd(Monitor *mon, const QDict *qdict, QObject **ret_data)
     return 0;
 }
 
+int64_t qmp_getfd(const char *fdname, Error **errp)
+{
+    mon_fd_t *monfd;
+    int fd;
+
+    fd = qemu_chr_fe_get_msgfd(cur_mon->chr);
+    if (fd == -1) {
+        error_set(errp, QERR_FD_NOT_SUPPLIED);
+        return -1;
+    }
+
+    if (qemu_isdigit(fdname[0])) {
+        error_set(errp, QERR_INVALID_PARAMETER_VALUE, "fdname",
+                  "a name not starting with a digit");
+        return -1;
+    }
+
+    QLIST_FOREACH(monfd, &cur_mon->fds, next) {
+        if (strcmp(monfd->name, fdname) != 0) {
+            continue;
+        }
+
+        close(monfd->fd);
+        monfd->fd = fd;
+        return fd;
+    }
+
+    monfd = g_malloc0(sizeof(mon_fd_t));
+    monfd->name = g_strdup(fdname);
+    monfd->fd = fd;
+
+    QLIST_INSERT_HEAD(&cur_mon->fds, monfd, next);
+    return fd;
+}
+
 static int do_closefd(Monitor *mon, const QDict *qdict, QObject **ret_data)
 {
     const char *fdname = qdict_get_str(qdict, "fdname");
diff --git a/qapi-schema.json b/qapi-schema.json
index 2ca7195..5f26ba2 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -1755,3 +1755,16 @@
 # Since: 0.14.0
 ##
 { 'command': 'device_del', 'data': {'id': 'str'} }
+
+##
+# @getfd:
+#
+# Receive a file descriptor via SCM rights and assign it a name
+#
+# @fdname: file descriptor name
+#
+# Returns: The QEMU @fd that was received
+#
+# Since: 1.2
+##
+{ 'command': 'getfd', 'data': {'fdname': 'str'}, 'returns': 'int' }
diff --git a/qmp-commands.hx b/qmp-commands.hx
index db980fa..e13d583 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -844,7 +844,7 @@ EQMP
         .params     = "getfd name",
         .help       = "receive a file descriptor via SCM rights and assign it a name",
         .user_print = monitor_user_noop,
-        .mhandler.cmd_new = do_getfd,
+        .mhandler.cmd_new = qmp_marshal_input_getfd,
     },
 
 SQMP
@@ -857,10 +857,12 @@ Arguments:
 
 - "fdname": file descriptor name (json-string)
 
+Return a json-int with the QEMU file descriptor that was received.
+
 Example:
 
 -> { "execute": "getfd", "arguments": { "fdname": "fd1" } }
-<- { "return": {} }
+<- { "return": 42 }
 
 EQMP
 
-- 
1.7.10.2

[Qemu-devel] [PATCH 2/3] block: Add support to "open" /dev/fd/X filenames

[Corey Bryant]

The main goal of this patch series is to enable isolation of guest
images that are stored on the same NFS mount.  This can be achieved
if the management application opens files for QEMU, and QEMU is
restricted from opening files.

This patch adds support to the block layer open paths to dup(X) a
pre-opened file descriptor if the filename is of the format
/dev/fd/X.

One nice thing about this approach is that no new SELinux policy is
required to prevent open of NFS files (files with type nfs_t).  The
virt_use_nfs boolean type simply needs to be set to false, and open
will be prevented (yet dup will be allowed).  For example:

  # setsebool virt_use_nfs 0
  # getsebool virt_use_nfs
  virt_use_nfs --> off

Signed-off-by: Corey Bryant <coreyb@linux.vnet.ibm.com>
---
 block.c           |   15 +++++++++++++++
 block/raw-posix.c |   20 ++++++++++----------
 block/raw-win32.c |    4 ++--
 block/vdi.c       |    5 +++--
 block/vmdk.c      |   21 +++++++++------------
 block/vpc.c       |    2 +-
 block/vvfat.c     |   21 +++++++++++----------
 block_int.h       |   13 +++++++++++++
 8 files changed, 64 insertions(+), 37 deletions(-)

diff --git a/block.c b/block.c
index af2ab4f..8416313 100644
--- a/block.c
+++ b/block.c
@@ -102,6 +102,21 @@ static BlockDriverState *bs_snapshots;
 /* If non-zero, use only whitelisted block drivers */
 static int use_bdrv_whitelist;
 
+int file_open(const char *filename, int flags, mode_t mode)
+{
+#ifndef _WIN32
+    int fd;
+    const char *p;
+
+    if (strstart(filename, "/dev/fd/", &p)) {
+        fd = atoi(p);
+        return dup(fd);
+    }
+#endif
+
+    return qemu_open(filename, flags, mode);
+}
+
 #ifdef _WIN32
 static int is_windows_drive_prefix(const char *filename)
 {
diff --git a/block/raw-posix.c b/block/raw-posix.c
index 03fcfcc..4f7b40f 100644
--- a/block/raw-posix.c
+++ b/block/raw-posix.c
@@ -208,7 +208,7 @@ static int raw_open_common(BlockDriverState *bs, const char *filename,
         s->open_flags |= O_DSYNC;
 
     s->fd = -1;
-    fd = qemu_open(filename, s->open_flags, 0644);
+    fd = file_open(filename, s->open_flags, 0644);
     if (fd < 0) {
         ret = -errno;
         if (ret == -EROFS)
@@ -568,8 +568,8 @@ static int raw_create(const char *filename, QEMUOptionParameter *options)
         options++;
     }
 
-    fd = open(filename, O_WRONLY | O_CREAT | O_TRUNC | O_BINARY,
-              0644);
+    fd = file_open(filename, O_WRONLY | O_CREAT | O_TRUNC | O_BINARY,
+                   0644);
     if (fd < 0) {
         result = -errno;
     } else {
@@ -741,7 +741,7 @@ static int hdev_open(BlockDriverState *bs, const char *filename, int flags)
         if ( bsdPath[ 0 ] != '\0' ) {
             strcat(bsdPath,"s0");
             /* some CDs don't have a partition 0 */
-            fd = open(bsdPath, O_RDONLY | O_BINARY | O_LARGEFILE);
+            fd = file_open(bsdPath, O_RDONLY | O_BINARY | O_LARGEFILE, 0);
             if (fd < 0) {
                 bsdPath[strlen(bsdPath)-1] = '1';
             } else {
@@ -798,7 +798,7 @@ static int fd_open(BlockDriverState *bs)
 #endif
             return -EIO;
         }
-        s->fd = open(bs->filename, s->open_flags & ~O_NONBLOCK);
+        s->fd = file_open(bs->filename, s->open_flags & ~O_NONBLOCK, 0);
         if (s->fd < 0) {
             s->fd_error_time = get_clock();
             s->fd_got_error = 1;
@@ -872,7 +872,7 @@ static int hdev_create(const char *filename, QEMUOptionParameter *options)
         options++;
     }
 
-    fd = open(filename, O_WRONLY | O_BINARY);
+    fd = file_open(filename, O_WRONLY | O_BINARY, 0);
     if (fd < 0)
         return -errno;
 
@@ -950,7 +950,7 @@ static int floppy_probe_device(const char *filename)
     if (strstart(filename, "/dev/fd", NULL))
         prio = 50;
 
-    fd = open(filename, O_RDONLY | O_NONBLOCK);
+    fd = file_open(filename, O_RDONLY | O_NONBLOCK, 0);
     if (fd < 0) {
         goto out;
     }
@@ -1003,7 +1003,7 @@ static void floppy_eject(BlockDriverState *bs, bool eject_flag)
         close(s->fd);
         s->fd = -1;
     }
-    fd = open(bs->filename, s->open_flags | O_NONBLOCK);
+    fd = file_open(bs->filename, s->open_flags | O_NONBLOCK, 0);
     if (fd >= 0) {
         if (ioctl(fd, FDEJECT, 0) < 0)
             perror("FDEJECT");
@@ -1053,7 +1053,7 @@ static int cdrom_probe_device(const char *filename)
     int prio = 0;
     struct stat st;
 
-    fd = open(filename, O_RDONLY | O_NONBLOCK);
+    fd = file_open(filename, O_RDONLY | O_NONBLOCK, 0);
     if (fd < 0) {
         goto out;
     }
@@ -1177,7 +1177,7 @@ static int cdrom_reopen(BlockDriverState *bs)
      */
     if (s->fd >= 0)
         close(s->fd);
-    fd = open(bs->filename, s->open_flags, 0644);
+    fd = file_open(bs->filename, s->open_flags, 0644);
     if (fd < 0) {
         s->fd = -1;
         return -EIO;
diff --git a/block/raw-win32.c b/block/raw-win32.c
index e4b0b75..ec4ac96 100644
--- a/block/raw-win32.c
+++ b/block/raw-win32.c
@@ -255,8 +255,8 @@ static int raw_create(const char *filename, QEMUOptionParameter *options)
         options++;
     }
 
-    fd = open(filename, O_WRONLY | O_CREAT | O_TRUNC | O_BINARY,
-              0644);
+    fd = file_open(filename, O_WRONLY | O_CREAT | O_TRUNC | O_BINARY,
+                   0644);
     if (fd < 0)
         return -EIO;
     set_sparse(fd);
diff --git a/block/vdi.c b/block/vdi.c
index 119d3c7..bd0f4b5 100644
--- a/block/vdi.c
+++ b/block/vdi.c
@@ -648,8 +648,9 @@ static int vdi_create(const char *filename, QEMUOptionParameter *options)
         options++;
     }
 
-    fd = open(filename, O_WRONLY | O_CREAT | O_TRUNC | O_BINARY | O_LARGEFILE,
-              0644);
+    fd = file_open(filename,
+                   O_WRONLY | O_CREAT | O_TRUNC | O_BINARY | O_LARGEFILE,
+                   0644);
     if (fd < 0) {
         return -errno;
     }
diff --git a/block/vmdk.c b/block/vmdk.c
index 18e9b4c..bda4c1a 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -1161,10 +1161,9 @@ static int vmdk_create_extent(const char *filename, int64_t filesize,
     VMDK4Header header;
     uint32_t tmp, magic, grains, gd_size, gt_size, gt_count;
 
-    fd = open(
-        filename,
-        O_WRONLY | O_CREAT | O_TRUNC | O_BINARY | O_LARGEFILE,
-        0644);
+    fd = file_open(filename,
+                   O_WRONLY | O_CREAT | O_TRUNC | O_BINARY | O_LARGEFILE,
+                   0644);
     if (fd < 0) {
         return -errno;
     }
@@ -1484,15 +1483,13 @@ static int vmdk_create(const char *filename, QEMUOptionParameter *options)
             (flags & BLOCK_FLAG_COMPAT6 ? 6 : 4),
             total_size / (int64_t)(63 * 16 * 512));
     if (split || flat) {
-        fd = open(
-                filename,
-                O_WRONLY | O_CREAT | O_TRUNC | O_BINARY | O_LARGEFILE,
-                0644);
+        fd = file_open(filename,
+                       O_WRONLY | O_CREAT | O_TRUNC | O_BINARY | O_LARGEFILE,
+                       0644);
     } else {
-        fd = open(
-                filename,
-                O_WRONLY | O_BINARY | O_LARGEFILE,
-                0644);
+        fd = file_open(filename,
+                       O_WRONLY | O_BINARY | O_LARGEFILE,
+                       0644);
     }
     if (fd < 0) {
         return -errno;
diff --git a/block/vpc.c b/block/vpc.c
index 5cd13d1..c1efb10 100644
--- a/block/vpc.c
+++ b/block/vpc.c
@@ -678,7 +678,7 @@ static int vpc_create(const char *filename, QEMUOptionParameter *options)
     }
 
     /* Create the file */
-    fd = open(filename, O_WRONLY | O_CREAT | O_TRUNC | O_BINARY, 0644);
+    fd = file_open(filename, O_WRONLY | O_CREAT | O_TRUNC | O_BINARY, 0644);
     if (fd < 0) {
         return -EIO;
     }
diff --git a/block/vvfat.c b/block/vvfat.c
index 2dc9d50..555e295 100644
--- a/block/vvfat.c
+++ b/block/vvfat.c
@@ -1152,16 +1152,17 @@ static inline mapping_t* find_mapping_for_cluster(BDRVVVFATState* s,int cluster_
 static int open_file(BDRVVVFATState* s,mapping_t* mapping)
 {
     if(!mapping)
-	return -1;
+        return -1;
     if(!s->current_mapping ||
-	    strcmp(s->current_mapping->path,mapping->path)) {
-	/* open file */
-	int fd = open(mapping->path, O_RDONLY | O_BINARY | O_LARGEFILE);
-	if(fd<0)
-	    return -1;
-	vvfat_close_current_file(s);
-	s->current_fd = fd;
-	s->current_mapping = mapping;
+        strcmp(s->current_mapping->path, mapping->path)) {
+        /* open file */
+        int fd = file_open(mapping->path, O_RDONLY | O_BINARY | O_LARGEFILE, 0);
+        if (fd < 0) {
+            return -1;
+        }
+        vvfat_close_current_file(s);
+        s->current_fd = fd;
+        s->current_mapping = mapping;
     }
     return 0;
 }
@@ -2215,7 +2216,7 @@ static int commit_one_file(BDRVVVFATState* s,
     for (i = s->cluster_size; i < offset; i += s->cluster_size)
 	c = modified_fat_get(s, c);
 
-    fd = open(mapping->path, O_RDWR | O_CREAT | O_BINARY, 0666);
+    fd = file_open(mapping->path, O_RDWR | O_CREAT | O_BINARY, 0666);
     if (fd < 0) {
 	fprintf(stderr, "Could not open %s... (%s, %d)\n", mapping->path,
 		strerror(errno), errno);
diff --git a/block_int.h b/block_int.h
index b80e66d..2510713 100644
--- a/block_int.h
+++ b/block_int.h
@@ -453,4 +453,17 @@ void stream_start(BlockDriverState *bs, BlockDriverState *base,
                   BlockDriverCompletionFunc *cb,
                   void *opaque, Error **errp);
 
+/**
+ * file_open:
+ * @filename: the filename to attempt to open
+ * @flags: O_ flags that determine how the file is opened
+ * @mode: the mode to create the file with if #O_CREAT is included in @flags
+ *
+ * This function behaves just like the @open libc function.  However, it can
+ * also duplicate the file descriptor of a pre-opened file if the filename
+ * is of the format /dev/fd/X.  This is useful when QEMU is restricted from
+ * opening a specific type of file.
+ */
+int file_open(const char *filename, int flags, mode_t mode);
+
 #endif /* BLOCK_INT_H */
-- 
1.7.10.2

[Qemu-devel] [PATCH 3/3] Sample server that opens image files for QEMU

[Corey Bryant]

This sample server opens image files and passes the fds to QEMU.  The
paths for two image files are passed as parameters, the first being
the boot image, and the second being an image to be hot-attached.  The
server will open the files and pass the fds to QEMU in one of two ways:

  1) Over the command line (using -drive file=/dev/fd/X) or
  2) Via the QMP monitor with the getfd command (using SCM_RIGHTS)
     followed by drive_add (using file=/dev/fd/X) and then
     device_add.

Usage:
  gcc -Wall -o test-fd-passing test-fd-passing.c -L/usr/local/lib -ljson
  ./test-fd-passing /path/hda.img /path/hdb.img

Note: This requires json-c and json-c-devel packages.

Signed-off-by: Corey Bryant <coreyb@linux.vnet.ibm.com>
---
 test-fd-passing.c |  321 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 321 insertions(+)
 create mode 100644 test-fd-passing.c

diff --git a/test-fd-passing.c b/test-fd-passing.c
new file mode 100644
index 0000000..f3f06e6
--- /dev/null
+++ b/test-fd-passing.c
@@ -0,0 +1,321 @@
+/*
+ * QEMU fd passing test server
+ *
+ * Copyright IBM, Corp. 2012
+ *
+ * Authors:
+ *  Corey Bryant      <coreyb@linux.vnet.ibm.com>
+ *  Stefan Hajnoczi   <stefanha@linux.vnet.ibm.com>
+ *
+ * This work is licensed under the terms of the GNU LGPL, version 2 or later.
+ * See the COPYING.LIB file in the top-level directory.
+ *
+ * gcc -Wall -o test-fd-passing test-fd-passing.c -L/usr/local/lib -ljson
+ * ./test-fd-passing /path/hda.img /path/hdb.img
+ *
+ * Note: This requires json-c and json-c-devel packages.
+ *
+ */
+
+#define _GNU_SOURCE
+#include <stdlib.h>
+#include <stdio.h>
+#include <errno.h>
+#include <sys/stat.h>
+#include <sys/socket.h>
+#include <fcntl.h>
+#include <unistd.h>
+#include <spawn.h>
+#include <sys/un.h>
+#include <json/json.h>
+
+static int openMonitor(const char *mon_path)
+{
+    int i;
+    int rc;
+    struct sockaddr_un addr;
+    int fd = 0;
+
+    if ((fd = socket(AF_UNIX, SOCK_STREAM, 0)) < 0) {
+        perror("socket");
+        goto error;
+    }
+
+    memset(&addr, 0, sizeof(addr));
+    addr.sun_family = AF_UNIX;
+    strcpy(addr.sun_path, mon_path);
+
+    for (i = 0; i < 100; i++) {
+        rc = connect(fd, (struct sockaddr *) &addr, sizeof(addr));
+        if (rc == 0) {
+            break;
+        }
+        sleep(1);
+    }
+
+    if (rc != 0) {
+        fprintf(stderr, "unable to connect to monitor socket\n");
+        goto error;
+    }
+
+    return fd;
+
+error:
+    close(fd);
+    return -1;
+}
+
+static int writeQMPfd(int qmpfd, int fd, const char *str)
+{
+    int rc;
+    char *json_str = NULL;
+    struct json_object *json_obj;
+    struct msghdr msg;
+    struct iovec iov[1];
+    char control[CMSG_SPACE(sizeof(int))];
+    struct cmsghdr *cmsg;
+
+    fprintf(stderr, "write (fd=%d) %s\n", fd, str);
+
+    memset(&msg, 0, sizeof(msg));
+
+    json_obj = json_tokener_parse(str);
+    asprintf(&json_str, "%s\r\n", json_object_to_json_string(json_obj));
+    json_object_put(json_obj);
+
+    iov[0].iov_base = (void *)json_str;
+    iov[0].iov_len = strlen(json_str);
+
+    msg.msg_iov = iov;
+    msg.msg_iovlen = 1;
+
+    msg.msg_control = control;
+    msg.msg_controllen = sizeof(control);
+
+    cmsg = CMSG_FIRSTHDR(&msg);
+    cmsg->cmsg_len = CMSG_LEN(sizeof(int));
+    cmsg->cmsg_level = SOL_SOCKET;
+    cmsg->cmsg_type = SCM_RIGHTS;
+    memcpy(CMSG_DATA(cmsg), &fd, sizeof(int));
+
+    do {
+        rc = sendmsg(qmpfd, &msg, 0);
+    } while (rc < 0 && errno == EINTR);
+
+    if (rc < 0) {
+        perror("sendmsg");
+    }
+
+    return rc;
+}
+
+static int writeQMP(int qmpfd, const char *str) {
+    int rc;
+    char *json_str = NULL;
+    struct json_object *json_obj;
+
+    fprintf(stderr, "write %s\n", str);
+
+    json_obj = json_tokener_parse(str);
+    asprintf(&json_str, "%s\r\n", json_object_to_json_string(json_obj));
+    json_object_put(json_obj);
+
+    rc = write(qmpfd, json_str, strlen(json_str));
+    if (rc < 0) {
+        perror("write");
+    }
+
+    return rc;
+}
+
+static char *readQMP(int qmpfd) {
+    int rc;
+    int i;
+    char *json_str;
+
+    json_str = calloc(1024, sizeof(char));
+    if (json_str == NULL) {
+        return NULL;
+    }
+
+    i = 0;
+    do {
+        sleep(1);
+        rc = read(qmpfd, json_str, 1024);
+        if (rc < 0) {
+            perror("read");
+            return NULL;
+        }
+        i++;
+    } while (i < 100 && rc == 0);
+
+    if (rc == 0) {
+        fprintf(stderr, "no data to read");
+        free(json_str);
+        return NULL;
+    }
+
+    fprintf(stderr, "read %s", json_str);
+
+    return json_str;
+}
+
+int main(int argc, char *argv[]) {
+    int rc = -1;
+    int qmpfd, bootfd, hotfd_host, hotfd_guest;
+    int flags = O_RDWR;
+    int mode = S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH;
+    pid_t child_pid;
+    char *drive_str = NULL;;
+    char *drive_add_str = NULL;
+    char *device_add_str = NULL;
+    char *drive_add_qmp = NULL;
+    char *device_add_qmp = NULL;
+    char *json_str;
+    struct json_object *json_obj;
+
+    if (argc != 3) {
+        fprintf(stderr, "usage: %s <boot-image-file> <attach-image-file>\n",
+                argv[0]);
+        goto error;
+    }
+
+    /* open the image files */
+    bootfd = open(argv[1], flags, mode);
+    if (bootfd == -1) {
+        perror("open");
+        goto error;
+    }
+
+    hotfd_host = open(argv[2], flags, mode);
+    if (hotfd_host == -1) {
+        perror("open");
+        goto error;
+    }
+
+    /* use '-drive file=/dev/fd/X' on QEMU command line to open boot image */
+    asprintf(&drive_str, "file=/dev/fd/%d,if=none,id=drive-virtio-disk0",
+             bootfd);
+
+    char *child_argv[] = {
+        "qemu-system-x86_64",
+        "-enable-kvm",
+        "-m", "1024",
+        "-chardev", "socket,id=qmp,path=./qmp-sock,server",
+        "-mon", "chardev=qmp,mode=control,pretty=on",
+        "-drive", drive_str,
+        "-device",
+        "virtio-blk-pci,bus=pci.0,addr=0x4,drive=drive-virtio-disk0,id=virtio-disk0",
+        "-vnc", ":0",
+        NULL,
+    };
+
+    rc = posix_spawn(&child_pid, "/usr/local/bin/qemu-system-x86_64",
+                     NULL, NULL, child_argv, environ);
+    if (rc != 0) {
+        perror("posix_spawn\n");
+        goto error;
+    }
+
+    qmpfd = openMonitor("./qmp-sock");
+    if (qmpfd == -1) {
+        rc = -1;
+        goto error;
+    }
+
+    json_str = readQMP(qmpfd);
+    if (json_str == NULL) {
+        rc = -1;
+        goto error;
+    }
+    free(json_str);
+
+    rc = writeQMP(qmpfd, "{ \"execute\": \"qmp_capabilities\" }");
+    if (rc < 0) {
+        goto error;
+    }
+
+    json_str = readQMP(qmpfd);
+    if (json_str == NULL) {
+        rc = -1;
+        goto error;
+    }
+    free(json_str);
+
+    /* issue 'getfd' monitor command and get QEMU guest's fd in return */
+    rc = writeQMPfd(qmpfd, hotfd_host,
+         "{ \"execute\": \"getfd\", \"arguments\": { \"fdname\": \"fd1\" } }");
+    if (rc < 0) {
+        goto error;
+    }
+
+    json_str = readQMP(qmpfd);
+    if (json_str == NULL) {
+        rc = -1;
+        goto error;
+    }
+    json_obj = json_tokener_parse(json_str);
+    json_obj = json_object_object_get(json_obj, "return");
+    hotfd_guest = json_object_get_int(json_obj);
+    json_object_put(json_obj);
+    free(json_str);
+
+    /* issue 'drive_add' monitor command with guest's /dev/fd/X path */
+    asprintf(&drive_add_str, "drive_add data_drive file=/dev/fd/%d,%s",
+             hotfd_guest,
+             "if=none,id=drive-virtio-disk1,cache=writethrough\r\n");
+    asprintf(&drive_add_qmp,
+             "{ \"%s\": \"%s\", \"%s\": { \"%s\": \"%s\" } }",
+             "execute", "human-monitor-command", "arguments",
+             "command-line", drive_add_str);
+    rc = writeQMP(qmpfd, drive_add_qmp);
+    if (rc < 0) {
+        goto error;
+    }
+
+    json_str = readQMP(qmpfd);
+    if (json_str == NULL) {
+        rc = -1;
+        goto error;
+    }
+    free(json_str);
+
+    /* issue 'device_add' monitor command to add the disk drive */
+    asprintf(&device_add_str, "device_add virtio-blk-pci,bus=pci.0,%s",
+             "addr=0x6,drive=drive-virtio-disk1,id=virtio-disk1\r\n");
+    asprintf(&device_add_qmp,
+             "{ \"%s\": \"%s\", \"%s\": { \"%s\": \"%s\" } }",
+             "execute", "human-monitor-command", "arguments",
+             "command-line", device_add_str);
+    rc = writeQMP(qmpfd, device_add_qmp);
+    if (rc < 0) {
+        goto error;
+    }
+
+    json_str = readQMP(qmpfd);
+    if (json_str == NULL) {
+        rc = -1;
+        goto error;
+    }
+    free(json_str);
+
+    rc = 0;
+
+error:
+    if (drive_str) {
+        free(drive_str);
+    }
+    if (drive_add_str) {
+        free(drive_add_str);
+    }
+    if (device_add_str) {
+        free(device_add_str);
+    }
+    if (drive_add_qmp) {
+        free(drive_add_qmp);
+    }
+    if (device_add_qmp) {
+        free(device_add_qmp);
+    }
+    return rc;
+}
-- 
1.7.10.2

Re: [Qemu-devel] [PATCH 2/3] block: Add support to "open" /dev/fd/X filenames

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1515 bytes --]

On 06/04/2012 07:10 AM, Corey Bryant wrote:
> The main goal of this patch series is to enable isolation of guest
> images that are stored on the same NFS mount.  This can be achieved
> if the management application opens files for QEMU, and QEMU is
> restricted from opening files.
> 
> This patch adds support to the block layer open paths to dup(X) a
> pre-opened file descriptor if the filename is of the format
> /dev/fd/X.
> 
> One nice thing about this approach is that no new SELinux policy is
> required to prevent open of NFS files (files with type nfs_t).  The
> virt_use_nfs boolean type simply needs to be set to false, and open
> will be prevented (yet dup will be allowed).  For example:
> 
>   # setsebool virt_use_nfs 0
>   # getsebool virt_use_nfs
>   virt_use_nfs --> off
> 
> Signed-off-by: Corey Bryant <coreyb@linux.vnet.ibm.com>

>  
> +int file_open(const char *filename, int flags, mode_t mode)
> +{
> +#ifndef _WIN32
> +    int fd;
> +    const char *p;
> +
> +    if (strstart(filename, "/dev/fd/", &p)) {
> +        fd = atoi(p);

atoi() is lousy - it has no error checking, and returns 0 if a mistake
was made.  You really want to be using strtol (or even better, a
sensible wrapper around strtol that takes care of the subtleties of
calling it correctly), so that you don't end up dup'ing stdin when the
user passes a bad /dev/fd/ string.

-- 
Eric Blake   eblake@redhat.com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 620 bytes --]

Re: [Qemu-devel] [PATCH 1/3] qmp/hmp: Add QMP getfd command that returns fd

[Kevin Wolf]

Am 04.06.2012 15:10, schrieb Corey Bryant:
> This patch adds QMP support for the getfd command using the QAPI framework.
> Like the HMP getfd command, it is used to pass a file descriptor via
> SCM_RIGHTS.  However, the QMP getfd command also returns the received file
> descriptor, which is a difference in behavior from the HMP getfd command,
> which returns nothing.
> 
> Signed-off-by: Corey Bryant <coreyb@linux.vnet.ibm.com>
> ---
>  hmp-commands.hx  |    2 +-
>  monitor.c        |   37 ++++++++++++++++++++++++++++++++++++-
>  qapi-schema.json |   13 +++++++++++++
>  qmp-commands.hx  |    6 ++++--
>  4 files changed, 54 insertions(+), 4 deletions(-)
> 
> diff --git a/hmp-commands.hx b/hmp-commands.hx
> index 18cb415..dfab369 100644
> --- a/hmp-commands.hx
> +++ b/hmp-commands.hx
> @@ -1211,7 +1211,7 @@ ETEXI
>          .params     = "getfd name",
>          .help       = "receive a file descriptor via SCM rights and assign it a name",
>          .user_print = monitor_user_noop,
> -        .mhandler.cmd_new = do_getfd,
> +        .mhandler.cmd_new = hmp_getfd,
>      },
>  
>  STEXI
> diff --git a/monitor.c b/monitor.c
> index 12a6fe2..6acf5a3 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -2199,7 +2199,7 @@ static void do_inject_mce(Monitor *mon, const QDict *qdict)
>  }
>  #endif
>  
> -static int do_getfd(Monitor *mon, const QDict *qdict, QObject **ret_data)
> +static int hmp_getfd(Monitor *mon, const QDict *qdict, QObject **ret_data)
>  {
>      const char *fdname = qdict_get_str(qdict, "fdname");
>      mon_fd_t *monfd;

This should become a wrapper around qmp_getfd() instead of duplicating
the logic.

Kevin

Re: [Qemu-devel] [PATCH 2/3] block: Add support to "open" /dev/fd/X filenames

[Kevin Wolf]

Am 04.06.2012 15:10, schrieb Corey Bryant:
> The main goal of this patch series is to enable isolation of guest
> images that are stored on the same NFS mount.  This can be achieved
> if the management application opens files for QEMU, and QEMU is
> restricted from opening files.
> 
> This patch adds support to the block layer open paths to dup(X) a
> pre-opened file descriptor if the filename is of the format
> /dev/fd/X.
> 
> One nice thing about this approach is that no new SELinux policy is
> required to prevent open of NFS files (files with type nfs_t).  The
> virt_use_nfs boolean type simply needs to be set to false, and open
> will be prevented (yet dup will be allowed).  For example:
> 
>   # setsebool virt_use_nfs 0
>   # getsebool virt_use_nfs
>   virt_use_nfs --> off
> 
> Signed-off-by: Corey Bryant <coreyb@linux.vnet.ibm.com>
> ---
>  block.c           |   15 +++++++++++++++
>  block/raw-posix.c |   20 ++++++++++----------
>  block/raw-win32.c |    4 ++--
>  block/vdi.c       |    5 +++--
>  block/vmdk.c      |   21 +++++++++------------
>  block/vpc.c       |    2 +-
>  block/vvfat.c     |   21 +++++++++++----------
>  block_int.h       |   13 +++++++++++++
>  8 files changed, 64 insertions(+), 37 deletions(-)
> 
> diff --git a/block.c b/block.c
> index af2ab4f..8416313 100644
> --- a/block.c
> +++ b/block.c
> @@ -102,6 +102,21 @@ static BlockDriverState *bs_snapshots;
>  /* If non-zero, use only whitelisted block drivers */
>  static int use_bdrv_whitelist;
>  
> +int file_open(const char *filename, int flags, mode_t mode)
> +{
> +#ifndef _WIN32
> +    int fd;
> +    const char *p;
> +
> +    if (strstart(filename, "/dev/fd/", &p)) {
> +        fd = atoi(p);
> +        return dup(fd);
> +    }
> +#endif
> +
> +    return qemu_open(filename, flags, mode);
> +}

What's the reason for having separate file_open() and qemu_open()
functions? Are there places where you don't want allow to use /dev/fd?
Otherwise I would have expected that we just extend qemu_open().

I'd have the remaining open -> qemu_open conversions as a separate patch
then. They do not only add fd support as advertised in the commit
message, but also add O_CLOEXEC. I don't think that's bad, these are
likely bonus bug fixes, but they should be mentioned in the commit message.

Kevin

Re: [Qemu-devel] [PATCH 3/3] Sample server that opens image files for QEMU

[Kevin Wolf]

Am 04.06.2012 15:10, schrieb Corey Bryant:
> This sample server opens image files and passes the fds to QEMU.  The
> paths for two image files are passed as parameters, the first being
> the boot image, and the second being an image to be hot-attached.  The
> server will open the files and pass the fds to QEMU in one of two ways:
> 
>   1) Over the command line (using -drive file=/dev/fd/X) or
>   2) Via the QMP monitor with the getfd command (using SCM_RIGHTS)
>      followed by drive_add (using file=/dev/fd/X) and then
>      device_add.
> 
> Usage:
>   gcc -Wall -o test-fd-passing test-fd-passing.c -L/usr/local/lib -ljson
>   ./test-fd-passing /path/hda.img /path/hdb.img
> 
> Note: This requires json-c and json-c-devel packages.
> 
> Signed-off-by: Corey Bryant <coreyb@linux.vnet.ibm.com>
> ---
>  test-fd-passing.c |  321 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 321 insertions(+)
>  create mode 100644 test-fd-passing.c

Is this meant to be applied or just for reference?

Maybe we can make a proper test case out of it that runs during make
check? Would probably require that the json-c dependency is dropped,
though. Maybe we should rewrite it in Python, as we already have QMP
bindings for that (and there are already tests that use them).

Kevin

Re: [Qemu-devel] [PATCH 2/3] block: Add support to "open" /dev/fd/X filenames

[Corey Bryant]

On 06/04/2012 10:32 AM, Eric Blake wrote:
> On 06/04/2012 07:10 AM, Corey Bryant wrote:
>> The main goal of this patch series is to enable isolation of guest
>> images that are stored on the same NFS mount.  This can be achieved
>> if the management application opens files for QEMU, and QEMU is
>> restricted from opening files.
>>
>> This patch adds support to the block layer open paths to dup(X) a
>> pre-opened file descriptor if the filename is of the format
>> /dev/fd/X.
>>
>> One nice thing about this approach is that no new SELinux policy is
>> required to prevent open of NFS files (files with type nfs_t).  The
>> virt_use_nfs boolean type simply needs to be set to false, and open
>> will be prevented (yet dup will be allowed).  For example:
>>
>>    # setsebool virt_use_nfs 0
>>    # getsebool virt_use_nfs
>>    virt_use_nfs -->  off
>>
>> Signed-off-by: Corey Bryant<coreyb@linux.vnet.ibm.com>
>
>>
>> +int file_open(const char *filename, int flags, mode_t mode)
>> +{
>> +#ifndef _WIN32
>> +    int fd;
>> +    const char *p;
>> +
>> +    if (strstart(filename, "/dev/fd/",&p)) {
>> +        fd = atoi(p);
>
> atoi() is lousy - it has no error checking, and returns 0 if a mistake
> was made.  You really want to be using strtol (or even better, a
> sensible wrapper around strtol that takes care of the subtleties of
> calling it correctly), so that you don't end up dup'ing stdin when the
> user passes a bad /dev/fd/ string.
>

It looks like strtol returns 0 on failure too.  Do we need to support 
stdin/stdout/stderr?

-- 
Regards,
Corey

Re: [Qemu-devel] [PATCH 1/3] qmp/hmp: Add QMP getfd command that returns fd

[Corey Bryant]

On 06/04/2012 10:45 AM, Kevin Wolf wrote:
> Am 04.06.2012 15:10, schrieb Corey Bryant:
>> This patch adds QMP support for the getfd command using the QAPI framework.
>> Like the HMP getfd command, it is used to pass a file descriptor via
>> SCM_RIGHTS.  However, the QMP getfd command also returns the received file
>> descriptor, which is a difference in behavior from the HMP getfd command,
>> which returns nothing.
>>
>> Signed-off-by: Corey Bryant<coreyb@linux.vnet.ibm.com>
>> ---
>>   hmp-commands.hx  |    2 +-
>>   monitor.c        |   37 ++++++++++++++++++++++++++++++++++++-
>>   qapi-schema.json |   13 +++++++++++++
>>   qmp-commands.hx  |    6 ++++--
>>   4 files changed, 54 insertions(+), 4 deletions(-)
>>
>> diff --git a/hmp-commands.hx b/hmp-commands.hx
>> index 18cb415..dfab369 100644
>> --- a/hmp-commands.hx
>> +++ b/hmp-commands.hx
>> @@ -1211,7 +1211,7 @@ ETEXI
>>           .params     = "getfd name",
>>           .help       = "receive a file descriptor via SCM rights and assign it a name",
>>           .user_print = monitor_user_noop,
>> -        .mhandler.cmd_new = do_getfd,
>> +        .mhandler.cmd_new = hmp_getfd,
>>       },
>>
>>   STEXI
>> diff --git a/monitor.c b/monitor.c
>> index 12a6fe2..6acf5a3 100644
>> --- a/monitor.c
>> +++ b/monitor.c
>> @@ -2199,7 +2199,7 @@ static void do_inject_mce(Monitor *mon, const QDict *qdict)
>>   }
>>   #endif
>>
>> -static int do_getfd(Monitor *mon, const QDict *qdict, QObject **ret_data)
>> +static int hmp_getfd(Monitor *mon, const QDict *qdict, QObject **ret_data)
>>   {
>>       const char *fdname = qdict_get_str(qdict, "fdname");
>>       mon_fd_t *monfd;
>
> This should become a wrapper around qmp_getfd() instead of duplicating
> the logic.
>
> Kevin
>

Yes, I was thinking about that.  I'll do this in v2.

-- 
Regards,
Corey

Re: [Qemu-devel] [PATCH 2/3] block: Add support to "open" /dev/fd/X filenames

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1292 bytes --]

On 06/04/2012 09:51 AM, Corey Bryant wrote:

>>> +
>>> +    if (strstart(filename, "/dev/fd/",&p)) {
>>> +        fd = atoi(p);
>>
>> atoi() is lousy - it has no error checking, and returns 0 if a mistake
>> was made.  You really want to be using strtol (or even better, a
>> sensible wrapper around strtol that takes care of the subtleties of
>> calling it correctly), so that you don't end up dup'ing stdin when the
>> user passes a bad /dev/fd/ string.
>>
> 
> It looks like strtol returns 0 on failure too.  Do we need to support
> stdin/stdout/stderr?

But at least strtol lets you detect errors:

char *tmp;
errno = 0;
fd = strtol(p, &tmp, 10);
if (errno || tmp == p) {
    /* raise your error here */
}

and if you get past that point, then someone really did pass in
/dev/fd/0 as the string they meant to be parsed (probably a user bug, as
getfd is unlikely to ever return 0 unless you start with stdin closed,
which itself is something that POSIX discourages, but not something we
need to specifically worry about).  So I would argue that yes, we do
need to support fd 0, if only by not special casing it as compared to
any other valid fd.

-- 
Eric Blake   eblake@redhat.com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 620 bytes --]

Re: [Qemu-devel] [PATCH 2/3] block: Add support to "open" /dev/fd/X filenames

[Corey Bryant]

On 06/04/2012 10:54 AM, Kevin Wolf wrote:
> Am 04.06.2012 15:10, schrieb Corey Bryant:
>> The main goal of this patch series is to enable isolation of guest
>> images that are stored on the same NFS mount.  This can be achieved
>> if the management application opens files for QEMU, and QEMU is
>> restricted from opening files.
>>
>> This patch adds support to the block layer open paths to dup(X) a
>> pre-opened file descriptor if the filename is of the format
>> /dev/fd/X.
>>
>> One nice thing about this approach is that no new SELinux policy is
>> required to prevent open of NFS files (files with type nfs_t).  The
>> virt_use_nfs boolean type simply needs to be set to false, and open
>> will be prevented (yet dup will be allowed).  For example:
>>
>>    # setsebool virt_use_nfs 0
>>    # getsebool virt_use_nfs
>>    virt_use_nfs -->  off
>>
>> Signed-off-by: Corey Bryant<coreyb@linux.vnet.ibm.com>
>> ---
>>   block.c           |   15 +++++++++++++++
>>   block/raw-posix.c |   20 ++++++++++----------
>>   block/raw-win32.c |    4 ++--
>>   block/vdi.c       |    5 +++--
>>   block/vmdk.c      |   21 +++++++++------------
>>   block/vpc.c       |    2 +-
>>   block/vvfat.c     |   21 +++++++++++----------
>>   block_int.h       |   13 +++++++++++++
>>   8 files changed, 64 insertions(+), 37 deletions(-)
>>
>> diff --git a/block.c b/block.c
>> index af2ab4f..8416313 100644
>> --- a/block.c
>> +++ b/block.c
>> @@ -102,6 +102,21 @@ static BlockDriverState *bs_snapshots;
>>   /* If non-zero, use only whitelisted block drivers */
>>   static int use_bdrv_whitelist;
>>
>> +int file_open(const char *filename, int flags, mode_t mode)
>> +{
>> +#ifndef _WIN32
>> +    int fd;
>> +    const char *p;
>> +
>> +    if (strstart(filename, "/dev/fd/",&p)) {
>> +        fd = atoi(p);
>> +        return dup(fd);
>> +    }
>> +#endif
>> +
>> +    return qemu_open(filename, flags, mode);
>> +}
>
> What's the reason for having separate file_open() and qemu_open()
> functions? Are there places where you don't want allow to use /dev/fd?
> Otherwise I would have expected that we just extend qemu_open().

I'm not sure there's any good reason to have a separate file_open() vs 
just adding this code to qemu_open().  I followed the lead of Anthony's 
prototype which used file_open() so he may have a reason.  Otherwise 
I'll move the code to qemu_open() in v2.

>
> I'd have the remaining open ->  qemu_open conversions as a separate patch
> then. They do not only add fd support as advertised in the commit
> message, but also add O_CLOEXEC. I don't think that's bad, these are
> likely bonus bug fixes, but they should be mentioned in the commit message.

Yes good point.  I'll be sure to note the addition of O_CLOEXEC for 
these cases in the commit message.

-- 
Regards,
Corey

Re: [Qemu-devel] [PATCH 3/3] Sample server that opens image files for QEMU

[Corey Bryant]

On 06/04/2012 11:01 AM, Kevin Wolf wrote:
> Am 04.06.2012 15:10, schrieb Corey Bryant:
>> This sample server opens image files and passes the fds to QEMU.  The
>> paths for two image files are passed as parameters, the first being
>> the boot image, and the second being an image to be hot-attached.  The
>> server will open the files and pass the fds to QEMU in one of two ways:
>>
>>    1) Over the command line (using -drive file=/dev/fd/X) or
>>    2) Via the QMP monitor with the getfd command (using SCM_RIGHTS)
>>       followed by drive_add (using file=/dev/fd/X) and then
>>       device_add.
>>
>> Usage:
>>    gcc -Wall -o test-fd-passing test-fd-passing.c -L/usr/local/lib -ljson
>>    ./test-fd-passing /path/hda.img /path/hdb.img
>>
>> Note: This requires json-c and json-c-devel packages.
>>
>> Signed-off-by: Corey Bryant<coreyb@linux.vnet.ibm.com>
>> ---
>>   test-fd-passing.c |  321 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>>   1 file changed, 321 insertions(+)
>>   create mode 100644 test-fd-passing.c
>
> Is this meant to be applied or just for reference?
>

This was just for reference.  It was the majority of my code (unit test) 
so I figured I'd share it.  :)

> Maybe we can make a proper test case out of it that runs during make
> check? Would probably require that the json-c dependency is dropped,
> though. Maybe we should rewrite it in Python, as we already have QMP
> bindings for that (and there are already tests that use them).

I won't promise anything but if I get a chance to convert to a Python 
test I will.

-- 
Regards,
Corey

Re: [Qemu-devel] [PATCH 2/3] block: Add support to "open" /dev/fd/X filenames

[Corey Bryant]

On 06/04/2012 12:03 PM, Eric Blake wrote:
> On 06/04/2012 09:51 AM, Corey Bryant wrote:
>
>>>> +
>>>> +    if (strstart(filename, "/dev/fd/",&p)) {
>>>> +        fd = atoi(p);
>>>
>>> atoi() is lousy - it has no error checking, and returns 0 if a mistake
>>> was made.  You really want to be using strtol (or even better, a
>>> sensible wrapper around strtol that takes care of the subtleties of
>>> calling it correctly), so that you don't end up dup'ing stdin when the
>>> user passes a bad /dev/fd/ string.
>>>
>>
>> It looks like strtol returns 0 on failure too.  Do we need to support
>> stdin/stdout/stderr?
>
> But at least strtol lets you detect errors:
>
> char *tmp;
> errno = 0;
> fd = strtol(p,&tmp, 10);
> if (errno || tmp == p) {
>      /* raise your error here */
> }

I don't think this is legitimate.  errno can be set under the covers of 
library calls even if the strtol() call is successful.

I was thinking if strtol returns 0 and errno is 0, perhaps we could 
assume success, but I don't think this is guaranteed either.

Maybe a combination of isdigit() then strtol() will give a better idea 
of success.

>
> and if you get past that point, then someone really did pass in
> /dev/fd/0 as the string they meant to be parsed (probably a user bug, as
> getfd is unlikely to ever return 0 unless you start with stdin closed,
> which itself is something that POSIX discourages, but not something we
> need to specifically worry about).  So I would argue that yes, we do
> need to support fd 0, if only by not special casing it as compared to
> any other valid fd.
>

Ok

-- 
Regards,
Corey

Re: [Qemu-devel] [PATCH 2/3] block: Add support to "open" /dev/fd/X filenames

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1527 bytes --]

On 06/04/2012 10:28 AM, Corey Bryant wrote:

>> But at least strtol lets you detect errors:
>>
>> char *tmp;
>> errno = 0;
>> fd = strtol(p,&tmp, 10);
>> if (errno || tmp == p) {
>>      /* raise your error here */
>> }
> 
> I don't think this is legitimate.  errno can be set under the covers of
> library calls even if the strtol() call is successful.

Wrong.  POSIX _specifically_ requires that strtol() leave errno
unchanged unless strtol() is reporting a failure, no matter what other
library calls (if any) are made under the covers by strtol().

In other words, pre-setting errno to 0, then calling strtol(), then
checking errno, _is_ the documented way to check for strtol() failures,
and a correct usage of strtol() MUST use this method.  See also commit
6b0e33be88bbccc3bcb987026089aa09f9622de9.  atoi() does not have this
same guarantee, which makes atoi() worthless at detecting errors in
relation to strtol().

> 
> I was thinking if strtol returns 0 and errno is 0, perhaps we could
> assume success, but I don't think this is guaranteed either.

Actually, it _is_ guaranteed - if you pre-set errno to 0, then call
strtol(), then errno is still 0, then the result did not encounter an
error, so a result of 0 at that point means that you indeed parsed a 0.

> 
> Maybe a combination of isdigit() then strtol() will give a better idea
> of success.

Not necessary.

-- 
Eric Blake   eblake@redhat.com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 620 bytes --]

Re: [Qemu-devel] [PATCH 2/3] block: Add support to "open" /dev/fd/X filenames

[Corey Bryant]

On 06/04/2012 12:36 PM, Eric Blake wrote:
> On 06/04/2012 10:28 AM, Corey Bryant wrote:
>
>>> But at least strtol lets you detect errors:
>>>
>>> char *tmp;
>>> errno = 0;
>>> fd = strtol(p,&tmp, 10);
>>> if (errno || tmp == p) {
>>>       /* raise your error here */
>>> }
>>
>> I don't think this is legitimate.  errno can be set under the covers of
>> library calls even if the strtol() call is successful.
>
> Wrong.  POSIX _specifically_ requires that strtol() leave errno
> unchanged unless strtol() is reporting a failure, no matter what other
> library calls (if any) are made under the covers by strtol().
>
> In other words, pre-setting errno to 0, then calling strtol(), then
> checking errno, _is_ the documented way to check for strtol() failures,
> and a correct usage of strtol() MUST use this method.  See also commit
> 6b0e33be88bbccc3bcb987026089aa09f9622de9.  atoi() does not have this
> same guarantee, which makes atoi() worthless at detecting errors in
> relation to strtol().
>

Great!  I see it now (excerpt below is from the opengroup 
specification).  This is definitely an exception from normal behavior of 
C APIs.

"The strtol() function will not change the setting of errno if successful."

>>
>> I was thinking if strtol returns 0 and errno is 0, perhaps we could
>> assume success, but I don't think this is guaranteed either.
>
> Actually, it _is_ guaranteed - if you pre-set errno to 0, then call
> strtol(), then errno is still 0, then the result did not encounter an
> error, so a result of 0 at that point means that you indeed parsed a 0.
>
>>
>> Maybe a combination of isdigit() then strtol() will give a better idea
>> of success.
>
> Not necessary.
>

-- 
Regards,
Corey

Re: [Qemu-devel] [PATCH 1/3] qmp/hmp: Add QMP getfd command that returns fd

[Luiz Capitulino]

On Mon,  4 Jun 2012 09:10:08 -0400
Corey Bryant <coreyb@linux.vnet.ibm.com> wrote:

> This patch adds QMP support for the getfd command using the QAPI framework.
> Like the HMP getfd command, it is used to pass a file descriptor via
> SCM_RIGHTS.  However, the QMP getfd command also returns the received file
> descriptor, which is a difference in behavior from the HMP getfd command,
> which returns nothing.

I have a few comments regarding the qapi conversion below, but something
important to discuss is that returning an int the way you're doing it is
certainly incompatible.

Today, we return a dict on success:

 { "return": {} }

But this patch changes it to:

 { "return": 42 }

There are two ways to do this without breaking compatibility:

 1. Add a new command (say get-file-descriptor)

 2. Return a type instead, like:

     { "return": { "file-descriptor": 42 } }

I think I prefer item 1, as we could also take the opportunity to fix the
argument type and improve its name. Besides, we don't have a schema to do 2.

Also, please split the getfd conversion to the qapi from changing its
return type in two patches (even if we go for a new command, please do
convert getfd too, for completeness); and also convert closefd, please.

More comments below.

> Signed-off-by: Corey Bryant <coreyb@linux.vnet.ibm.com>
> ---
>  hmp-commands.hx  |    2 +-
>  monitor.c        |   37 ++++++++++++++++++++++++++++++++++++-
>  qapi-schema.json |   13 +++++++++++++
>  qmp-commands.hx  |    6 ++++--
>  4 files changed, 54 insertions(+), 4 deletions(-)
> 
> diff --git a/hmp-commands.hx b/hmp-commands.hx
> index 18cb415..dfab369 100644
> --- a/hmp-commands.hx
> +++ b/hmp-commands.hx
> @@ -1211,7 +1211,7 @@ ETEXI
>          .params     = "getfd name",
>          .help       = "receive a file descriptor via SCM rights and assign it a name",
>          .user_print = monitor_user_noop,

The .user_print line should be dropped.

> -        .mhandler.cmd_new = do_getfd,
> +        .mhandler.cmd_new = hmp_getfd,

You should use 'cmd' (instead of 'cmd_new'). Please, look for other qapi
conversions for examples (like a15fef21c).

>      },
>  
>  STEXI
> diff --git a/monitor.c b/monitor.c
> index 12a6fe2..6acf5a3 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -2199,7 +2199,7 @@ static void do_inject_mce(Monitor *mon, const QDict *qdict)
>  }
>  #endif
>  
> -static int do_getfd(Monitor *mon, const QDict *qdict, QObject **ret_data)
> +static int hmp_getfd(Monitor *mon, const QDict *qdict, QObject **ret_data)

This should be moved to hmp.c and the function signature should be different,
looking at other qapi conversions will help you.

>  {
>      const char *fdname = qdict_get_str(qdict, "fdname");
>      mon_fd_t *monfd;
> @@ -2235,6 +2235,41 @@ static int do_getfd(Monitor *mon, const QDict *qdict, QObject **ret_data)
>      return 0;
>  }
>  
> +int64_t qmp_getfd(const char *fdname, Error **errp)
> +{
> +    mon_fd_t *monfd;
> +    int fd;
> +
> +    fd = qemu_chr_fe_get_msgfd(cur_mon->chr);
> +    if (fd == -1) {
> +        error_set(errp, QERR_FD_NOT_SUPPLIED);
> +        return -1;
> +    }
> +
> +    if (qemu_isdigit(fdname[0])) {
> +        error_set(errp, QERR_INVALID_PARAMETER_VALUE, "fdname",
> +                  "a name not starting with a digit");
> +        return -1;
> +    }
> +
> +    QLIST_FOREACH(monfd, &cur_mon->fds, next) {
> +        if (strcmp(monfd->name, fdname) != 0) {
> +            continue;
> +        }
> +
> +        close(monfd->fd);
> +        monfd->fd = fd;
> +        return fd;
> +    }
> +
> +    monfd = g_malloc0(sizeof(mon_fd_t));
> +    monfd->name = g_strdup(fdname);
> +    monfd->fd = fd;
> +
> +    QLIST_INSERT_HEAD(&cur_mon->fds, monfd, next);
> +    return fd;
> +}
> +
>  static int do_closefd(Monitor *mon, const QDict *qdict, QObject **ret_data)
>  {
>      const char *fdname = qdict_get_str(qdict, "fdname");
> diff --git a/qapi-schema.json b/qapi-schema.json
> index 2ca7195..5f26ba2 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -1755,3 +1755,16 @@
>  # Since: 0.14.0
>  ##
>  { 'command': 'device_del', 'data': {'id': 'str'} }
> +
> +##
> +# @getfd:
> +#
> +# Receive a file descriptor via SCM rights and assign it a name
> +#
> +# @fdname: file descriptor name
> +#
> +# Returns: The QEMU @fd that was received
> +#
> +# Since: 1.2

This command exists since 0.14 if I'm not mistaken, only the return type
is new.

> +##
> +{ 'command': 'getfd', 'data': {'fdname': 'str'}, 'returns': 'int' }
> diff --git a/qmp-commands.hx b/qmp-commands.hx
> index db980fa..e13d583 100644
> --- a/qmp-commands.hx
> +++ b/qmp-commands.hx
> @@ -844,7 +844,7 @@ EQMP
>          .params     = "getfd name",
>          .help       = "receive a file descriptor via SCM rights and assign it a name",
>          .user_print = monitor_user_noop,
> -        .mhandler.cmd_new = do_getfd,
> +        .mhandler.cmd_new = qmp_marshal_input_getfd,
>      },
>  
>  SQMP
> @@ -857,10 +857,12 @@ Arguments:
>  
>  - "fdname": file descriptor name (json-string)
>  
> +Return a json-int with the QEMU file descriptor that was received.
> +
>  Example:
>  
>  -> { "execute": "getfd", "arguments": { "fdname": "fd1" } }
> -<- { "return": {} }
> +<- { "return": 42 }
>  
>  EQMP
>  

Re: [Qemu-devel] [PATCH 1/3] qmp/hmp: Add QMP getfd command that returns fd

[Corey Bryant]

On 06/05/2012 02:30 PM, Luiz Capitulino wrote:
> On Mon,  4 Jun 2012 09:10:08 -0400
> Corey Bryant<coreyb@linux.vnet.ibm.com>  wrote:
>
>> This patch adds QMP support for the getfd command using the QAPI framework.
>> Like the HMP getfd command, it is used to pass a file descriptor via
>> SCM_RIGHTS.  However, the QMP getfd command also returns the received file
>> descriptor, which is a difference in behavior from the HMP getfd command,
>> which returns nothing.
>
> I have a few comments regarding the qapi conversion below, but something
> important to discuss is that returning an int the way you're doing it is
> certainly incompatible.

Thanks for your feedback.

>
> Today, we return a dict on success:
>
>   { "return": {} }
>
> But this patch changes it to:
>
>   { "return": 42 }
>
> There are two ways to do this without breaking compatibility:
>
>   1. Add a new command (say get-file-descriptor)

What do you think about using getfd2 for the command name?  I'm thinking 
getfd2 may be more obvious that it corresponds to closefd.  That assumes 
we'll use the same array internally to store fds and closefd can be used 
to close the fd opened by get-file-descriptor/getfd2.

I assume this approach would still return an int:  { "return": 42 }

>
>   2. Return a type instead, like:
>
>       { "return": { "file-descriptor": 42 } }
>
> I think I prefer item 1, as we could also take the opportunity to fix the
> argument type and improve its name. Besides, we don't have a schema to do 2.

Is it fdname that you think could be improved?  fdname seems pretty 
straight forward to me.

>
> Also, please split the getfd conversion to the qapi from changing its
> return type in two patches (even if we go for a new command, please do
> convert getfd too, for completeness); and also convert closefd, please.

Ok

>
> More comments below.
>
>> Signed-off-by: Corey Bryant<coreyb@linux.vnet.ibm.com>
>> ---
>>   hmp-commands.hx  |    2 +-
>>   monitor.c        |   37 ++++++++++++++++++++++++++++++++++++-
>>   qapi-schema.json |   13 +++++++++++++
>>   qmp-commands.hx  |    6 ++++--
>>   4 files changed, 54 insertions(+), 4 deletions(-)
>>
>> diff --git a/hmp-commands.hx b/hmp-commands.hx
>> index 18cb415..dfab369 100644
>> --- a/hmp-commands.hx
>> +++ b/hmp-commands.hx
>> @@ -1211,7 +1211,7 @@ ETEXI
>>           .params     = "getfd name",
>>           .help       = "receive a file descriptor via SCM rights and assign it a name",
>>           .user_print = monitor_user_noop,
>
> The .user_print line should be dropped.

Ok

>
>> -        .mhandler.cmd_new = do_getfd,
>> +        .mhandler.cmd_new = hmp_getfd,
>
> You should use 'cmd' (instead of 'cmd_new'). Please, look for other qapi
> conversions for examples (like a15fef21c).

Ok

>
>>       },
>>
>>   STEXI
>> diff --git a/monitor.c b/monitor.c
>> index 12a6fe2..6acf5a3 100644
>> --- a/monitor.c
>> +++ b/monitor.c
>> @@ -2199,7 +2199,7 @@ static void do_inject_mce(Monitor *mon, const QDict *qdict)
>>   }
>>   #endif
>>
>> -static int do_getfd(Monitor *mon, const QDict *qdict, QObject **ret_data)
>> +static int hmp_getfd(Monitor *mon, const QDict *qdict, QObject **ret_data)
>
> This should be moved to hmp.c and the function signature should be different,
> looking at other qapi conversions will help you.

Ok

>
>>   {
>>       const char *fdname = qdict_get_str(qdict, "fdname");
>>       mon_fd_t *monfd;
>> @@ -2235,6 +2235,41 @@ static int do_getfd(Monitor *mon, const QDict *qdict, QObject **ret_data)
>>       return 0;
>>   }
>>
>> +int64_t qmp_getfd(const char *fdname, Error **errp)
>> +{
>> +    mon_fd_t *monfd;
>> +    int fd;
>> +
>> +    fd = qemu_chr_fe_get_msgfd(cur_mon->chr);
>> +    if (fd == -1) {
>> +        error_set(errp, QERR_FD_NOT_SUPPLIED);
>> +        return -1;
>> +    }
>> +
>> +    if (qemu_isdigit(fdname[0])) {
>> +        error_set(errp, QERR_INVALID_PARAMETER_VALUE, "fdname",
>> +                  "a name not starting with a digit");
>> +        return -1;
>> +    }
>> +
>> +    QLIST_FOREACH(monfd,&cur_mon->fds, next) {
>> +        if (strcmp(monfd->name, fdname) != 0) {
>> +            continue;
>> +        }
>> +
>> +        close(monfd->fd);
>> +        monfd->fd = fd;
>> +        return fd;
>> +    }
>> +
>> +    monfd = g_malloc0(sizeof(mon_fd_t));
>> +    monfd->name = g_strdup(fdname);
>> +    monfd->fd = fd;
>> +
>> +    QLIST_INSERT_HEAD(&cur_mon->fds, monfd, next);
>> +    return fd;
>> +}
>> +
>>   static int do_closefd(Monitor *mon, const QDict *qdict, QObject **ret_data)
>>   {
>>       const char *fdname = qdict_get_str(qdict, "fdname");
>> diff --git a/qapi-schema.json b/qapi-schema.json
>> index 2ca7195..5f26ba2 100644
>> --- a/qapi-schema.json
>> +++ b/qapi-schema.json
>> @@ -1755,3 +1755,16 @@
>>   # Since: 0.14.0
>>   ##
>>   { 'command': 'device_del', 'data': {'id': 'str'} }
>> +
>> +##
>> +# @getfd:
>> +#
>> +# Receive a file descriptor via SCM rights and assign it a name
>> +#
>> +# @fdname: file descriptor name
>> +#
>> +# Returns: The QEMU @fd that was received
>> +#
>> +# Since: 1.2
>
> This command exists since 0.14 if I'm not mistaken, only the return type
> is new.

Ok

>
>> +##
>> +{ 'command': 'getfd', 'data': {'fdname': 'str'}, 'returns': 'int' }
>> diff --git a/qmp-commands.hx b/qmp-commands.hx
>> index db980fa..e13d583 100644
>> --- a/qmp-commands.hx
>> +++ b/qmp-commands.hx
>> @@ -844,7 +844,7 @@ EQMP
>>           .params     = "getfd name",
>>           .help       = "receive a file descriptor via SCM rights and assign it a name",
>>           .user_print = monitor_user_noop,
>> -        .mhandler.cmd_new = do_getfd,
>> +        .mhandler.cmd_new = qmp_marshal_input_getfd,
>>       },
>>
>>   SQMP
>> @@ -857,10 +857,12 @@ Arguments:
>>
>>   - "fdname": file descriptor name (json-string)
>>
>> +Return a json-int with the QEMU file descriptor that was received.
>> +
>>   Example:
>>
>>   ->  { "execute": "getfd", "arguments": { "fdname": "fd1" } }
>> -<- { "return": {} }
>> +<- { "return": 42 }
>>
>>   EQMP
>>
>
>

-- 
Regards,
Corey

Re: [Qemu-devel] [PATCH 1/3] qmp/hmp: Add QMP getfd command that returns fd

[Luiz Capitulino]

On Wed, 06 Jun 2012 10:04:23 -0400
Corey Bryant <coreyb@linux.vnet.ibm.com> wrote:

> > Today, we return a dict on success:
> >
> >   { "return": {} }
> >
> > But this patch changes it to:
> >
> >   { "return": 42 }
> >
> > There are two ways to do this without breaking compatibility:
> >
> >   1. Add a new command (say get-file-descriptor)
> 
> What do you think about using getfd2 for the command name?  I'm thinking 
> getfd2 may be more obvious that it corresponds to closefd. 

We're going for more descriptive names in QMP. I don't have strong objections
against get-fd2 if there's consensus that 'fd' is better than 'file-descriptor',
although 'fd2' is a bit confusing.

> That assumes 
> we'll use the same array internally to store fds and closefd can be used 
> to close the fd opened by get-file-descriptor/getfd2.

You mean using the same array for getfd and get-file-descriptor? Yes, the
descriptor list is global.

> I assume this approach would still return an int:  { "return": 42 }

The new command? Yes.

> >   2. Return a type instead, like:
> >
> >       { "return": { "file-descriptor": 42 } }
> >
> > I think I prefer item 1, as we could also take the opportunity to fix the
> > argument type and improve its name. Besides, we don't have a schema to do 2.
> 
> Is it fdname that you think could be improved?  fdname seems pretty 
> straight forward to me.

What I'm trying to avoid is having too short names when that's not necessary.
I think I'd just use 'name' or 'file-descriptor-name' for the verbose option,
but I don't have strong objections against 'fdname'.

Re: [Qemu-devel] [PATCH 1/3] qmp/hmp: Add QMP getfd command that returns fd

[Corey Bryant]

On 06/06/2012 01:50 PM, Luiz Capitulino wrote:
> On Wed, 06 Jun 2012 10:04:23 -0400
> Corey Bryant<coreyb@linux.vnet.ibm.com>  wrote:
>
>>> Today, we return a dict on success:
>>>
>>>    { "return": {} }
>>>
>>> But this patch changes it to:
>>>
>>>    { "return": 42 }
>>>
>>> There are two ways to do this without breaking compatibility:
>>>
>>>    1. Add a new command (say get-file-descriptor)
>>
>> What do you think about using getfd2 for the command name?  I'm thinking
>> getfd2 may be more obvious that it corresponds to closefd.
>
> We're going for more descriptive names in QMP. I don't have strong objections
> against get-fd2 if there's consensus that 'fd' is better than 'file-descriptor',
> although 'fd2' is a bit confusing.

I really don't have a strong opinion either so I'll do whatever the 
consensus wants.  Does anyone else have an opinion?

>
>> That assumes
>> we'll use the same array internally to store fds and closefd can be used
>> to close the fd opened by get-file-descriptor/getfd2.
>
> You mean using the same array for getfd and get-file-descriptor? Yes, the
> descriptor list is global.
>

Yes, that's what I meant.

>> I assume this approach would still return an int:  { "return": 42 }
>
> The new command? Yes.
>

Ok

>>>    2. Return a type instead, like:
>>>
>>>        { "return": { "file-descriptor": 42 } }
>>>
>>> I think I prefer item 1, as we could also take the opportunity to fix the
>>> argument type and improve its name. Besides, we don't have a schema to do 2.
>>
>> Is it fdname that you think could be improved?  fdname seems pretty
>> straight forward to me.
>
> What I'm trying to avoid is having too short names when that's not necessary.
> I think I'd just use 'name' or 'file-descriptor-name' for the verbose option,
> but I don't have strong objections against 'fdname'.
>

Unless anyone objects I'll plan on going with more descriptive names, 
since that is the desired direction for QMP.  So we'll have 
get-file-descriptor for the new command name and file-descriptor-name 
for the argument name.

-- 
Regards,
Corey

Re: [Qemu-devel] [PATCH 1/3] qmp/hmp: Add QMP getfd command that returns fd

[Daniel P. Berrange]

On Wed, Jun 06, 2012 at 10:04:23AM -0400, Corey Bryant wrote:
> 
> 
> On 06/05/2012 02:30 PM, Luiz Capitulino wrote:
> >On Mon,  4 Jun 2012 09:10:08 -0400
> >Corey Bryant<coreyb@linux.vnet.ibm.com>  wrote:
> >
> >>This patch adds QMP support for the getfd command using the QAPI framework.
> >>Like the HMP getfd command, it is used to pass a file descriptor via
> >>SCM_RIGHTS.  However, the QMP getfd command also returns the received file
> >>descriptor, which is a difference in behavior from the HMP getfd command,
> >>which returns nothing.
> >
> >I have a few comments regarding the qapi conversion below, but something
> >important to discuss is that returning an int the way you're doing it is
> >certainly incompatible.
> 
> Thanks for your feedback.
> 
> >
> >Today, we return a dict on success:
> >
> >  { "return": {} }
> >
> >But this patch changes it to:
> >
> >  { "return": 42 }
> >
> >There are two ways to do this without breaking compatibility:
> >
> >  1. Add a new command (say get-file-descriptor)
> 
> What do you think about using getfd2 for the command name?  I'm
> thinking getfd2 may be more obvious that it corresponds to closefd.
> That assumes we'll use the same array internally to store fds and
> closefd can be used to close the fd opened by
> get-file-descriptor/getfd2.

How about calling it 'passfd' instead ?  I have always thought
the name 'getfd' to be a little wierd really, since we're not
getting an FD from QEMU, we're passing it one that we have.

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|

Re: [Qemu-devel] [PATCH 1/3] qmp/hmp: Add QMP getfd command that returns fd

[Corey Bryant]

On 06/08/2012 06:46 AM, Daniel P. Berrange wrote:
> On Wed, Jun 06, 2012 at 10:04:23AM -0400, Corey Bryant wrote:
>>
>>
>> On 06/05/2012 02:30 PM, Luiz Capitulino wrote:
>>> On Mon,  4 Jun 2012 09:10:08 -0400
>>> Corey Bryant<coreyb@linux.vnet.ibm.com>   wrote:
>>>
>>>> This patch adds QMP support for the getfd command using the QAPI framework.
>>>> Like the HMP getfd command, it is used to pass a file descriptor via
>>>> SCM_RIGHTS.  However, the QMP getfd command also returns the received file
>>>> descriptor, which is a difference in behavior from the HMP getfd command,
>>>> which returns nothing.
>>>
>>> I have a few comments regarding the qapi conversion below, but something
>>> important to discuss is that returning an int the way you're doing it is
>>> certainly incompatible.
>>
>> Thanks for your feedback.
>>
>>>
>>> Today, we return a dict on success:
>>>
>>>   { "return": {} }
>>>
>>> But this patch changes it to:
>>>
>>>   { "return": 42 }
>>>
>>> There are two ways to do this without breaking compatibility:
>>>
>>>   1. Add a new command (say get-file-descriptor)
>>
>> What do you think about using getfd2 for the command name?  I'm
>> thinking getfd2 may be more obvious that it corresponds to closefd.
>> That assumes we'll use the same array internally to store fds and
>> closefd can be used to close the fd opened by
>> get-file-descriptor/getfd2.
>
> How about calling it 'passfd' instead ?  I have always thought
> the name 'getfd' to be a little wierd really, since we're not
> getting an FD from QEMU, we're passing it one that we have.

I agree, thanks for the suggestion.  I know Luiz suggested detailed 
command names, and I think this will be ok and self documents itself 
fairly well with the compact 'passfd'.

I have v2 patches that are just about ready, so I'll update them to use 
'passfd'.

-- 
Regards,
Corey