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 <coreyb(a)linux.vnet.ibm.com&gt; --- 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(a)redhat.com, kwolf(a)redhat.com, dberrange(a)redhat.com) v6 -Make @fd optional for remove-fd (eblake(a)redhat.com) -Make @fdset-id optional for add-fd (eblake(a)redhat.com) v7: -Share fd sets among all monitor connections (kwolf(a)redhat.com) -Added mon_refcount to keep track of monitor connection count. monitor.c | 167 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ qapi-schema.json | 110 +++++++++++++++++++++++++++++++++++ qerror.c | 4 ++ qerror.h | 3 + qmp-commands.hx | 131 ++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 415 insertions(+) diff --git a/monitor.c b/monitor.c index 49dccfe..04b86b7 100644 --- a/monitor.c +++ b/monitor.c @@ -140,6 +140,23 @@ struct mon_fd_t { QLIST_ENTRY(mon_fd_t) next; }; +/* file descriptor associated with a file descriptor set */ +typedef struct mon_fdset_fd_t mon_fdset_fd_t; +struct mon_fdset_fd_t { + int fd; + bool removed; + QLIST_ENTRY(mon_fdset_fd_t) next; +}; + +/* file descriptor set containing fds passed via SCM_RIGHTS */ +typedef struct mon_fdset_t mon_fdset_t; +struct mon_fdset_t { + int64_t id; + int refcount; + QLIST_HEAD(, mon_fdset_fd_t) fds; + QLIST_ENTRY(mon_fdset_t) next; +}; + typedef struct MonitorControl { QObject *id; JSONMessageParser parser; @@ -211,6 +228,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, mon_fdset_t) mon_fdsets; +static int mon_refcount; static mon_cmd_t mon_cmds[]; static mon_cmd_t info_cmds[]; @@ -2389,6 +2408,154 @@ int monitor_get_fd(Monitor *mon, const char *fdname) return -1; } +static void monitor_fdset_cleanup(mon_fdset_t *mon_fdset) +{ + mon_fdset_fd_t *mon_fdset_fd; + mon_fdset_fd_t *mon_fdset_fd_next; + + if (mon_fdset->refcount != 0) { + return; + } + + QLIST_FOREACH_SAFE(mon_fdset_fd, &mon_fdset->fds, next, mon_fdset_fd_next) { + if (mon_refcount == 0 || mon_fdset_fd->removed) { + close(mon_fdset_fd->fd); + QLIST_REMOVE(mon_fdset_fd, next); + g_free(mon_fdset_fd); + } + } + + if (QLIST_EMPTY(&mon_fdset->fds)) { + QLIST_REMOVE(mon_fdset, next); + g_free(mon_fdset); + } +} + +AddfdInfo *qmp_add_fd(bool has_fdset_id, int64_t fdset_id, Error **errp) +{ + int fd; + Monitor *mon = cur_mon; + mon_fdset_t *mon_fdset; + mon_fdset_fd_t *mon_fdset_fd; + AddfdInfo *fdinfo; + + fd = qemu_chr_fe_get_msgfd(mon->chr); + if (fd == -1) { + qerror_report(QERR_FD_NOT_SUPPLIED); + return NULL; + } + + if (has_fdset_id) { + QLIST_FOREACH(mon_fdset, &mon_fdsets, next) { + if (mon_fdset->id == fdset_id) { + break; + } + } + if (mon_fdset == NULL) { + qerror_report(QERR_FDSET_NOT_FOUND, fdset_id); + return NULL; + } + } else { + int64_t fdset_id_prev = -1; + mon_fdset_t *mon_fdset_cur = QLIST_FIRST(&mon_fdsets); + + /* Use first available fdset ID */ + QLIST_FOREACH(mon_fdset, &mon_fdsets, next) { + mon_fdset_cur = mon_fdset; + if (fdset_id_prev == mon_fdset_cur->id - 1) { + fdset_id_prev = mon_fdset_cur->id; + continue; + } + break; + } + + mon_fdset = g_malloc0(sizeof(*mon_fdset)); + mon_fdset->id = fdset_id_prev + 1; + mon_fdset->refcount = 0; + + /* The fdset list is ordered by fdset ID */ + if (mon_fdset->id == 0) { + QLIST_INSERT_HEAD(&mon_fdsets, mon_fdset, next); + } else if (mon_fdset->id < mon_fdset_cur->id) { + QLIST_INSERT_BEFORE(mon_fdset_cur, mon_fdset, next); + } else { + QLIST_INSERT_AFTER(mon_fdset_cur, mon_fdset, next); + } + } + + mon_fdset_fd = g_malloc0(sizeof(*mon_fdset_fd)); + mon_fdset_fd->fd = fd; + mon_fdset_fd->removed = false; + QLIST_INSERT_HEAD(&mon_fdset->fds, mon_fdset_fd, next); + + fdinfo = g_malloc0(sizeof(*fdinfo)); + fdinfo->fdset_id = mon_fdset->id; + fdinfo->fd = mon_fdset_fd->fd; + + return fdinfo; +} + +void qmp_remove_fd(int64_t fdset_id, bool has_fd, int64_t fd, Error **errp) +{ + mon_fdset_t *mon_fdset; + mon_fdset_fd_t *mon_fdset_fd; + char fd_str[20]; + + QLIST_FOREACH(mon_fdset, &mon_fdsets, next) { + if (mon_fdset->id != fdset_id) { + continue; + } + QLIST_FOREACH(mon_fdset_fd, &mon_fdset->fds, next) { + if (has_fd && mon_fdset_fd->fd != fd) { + continue; + } + mon_fdset_fd->removed = true; + if (has_fd) { + break; + } + } + monitor_fdset_cleanup(mon_fdset); + return; + } + snprintf(fd_str, sizeof(fd_str), "%ld", fd); + qerror_report(QERR_FD_NOT_FOUND, fd_str); +} + +FdsetInfoList *qmp_query_fdsets(Error **errp) +{ + mon_fdset_t *mon_fdset; + mon_fdset_fd_t *mon_fdset_fd; + FdsetInfoList *fdset_list = NULL; + + QLIST_FOREACH(mon_fdset, &mon_fdsets, next) { + FdsetInfoList *fdset_info = g_malloc0(sizeof(*fdset_info)); + FdsetFdInfoList *fdsetfd_list = NULL; + + fdset_info->value = g_malloc0(sizeof(*fdset_info->value)); + fdset_info->value->fdset_id = mon_fdset->id; + fdset_info->value->refcount = mon_fdset->refcount; + fdset_info->value->in_use = mon_refcount > 0 ? true : false; + + QLIST_FOREACH(mon_fdset_fd, &mon_fdset->fds, next) { + FdsetFdInfoList *fdsetfd_info = g_malloc0(sizeof(*fdsetfd_info)); + + fdsetfd_info->value = g_malloc0(sizeof(*fdsetfd_info->value)); + fdsetfd_info->value->fd = mon_fdset_fd->fd; + fdsetfd_info->value->removed = mon_fdset_fd->removed; + + fdsetfd_info->next = fdsetfd_list; + fdsetfd_list = fdsetfd_info; + } + + fdset_info->value->fds = fdsetfd_list; + + fdset_info->next = fdset_list; + fdset_list = fdset_info; + } + + return fdset_list; +} + /* mon_cmds and info_cmds would be sorted at runtime */ static mon_cmd_t mon_cmds[] = { #include "hmp-commands.h" diff --git a/qapi-schema.json b/qapi-schema.json index cddf63a..1a21bf8 100644 --- a/qapi-schema.json +++ b/qapi-schema.json @@ -2200,3 +2200,113 @@ # Since: 0.14.0 ## { 'command': 'closefd', 'data': {'fdname': 'str'} } + +# @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 +## +{ 'type': 'AddfdInfo', 'data': {'fdset_id': 'int', 'fd': 'int'} } + +## +# @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'}, + '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'} } + +## +# @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'} } + +## +# @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 set. +# +# @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']} } + +## +# @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[] = { .desc = "No file descriptor supplied via SCM_RIGHTS", }, { + .error_fmt = QERR_FDSET_NOT_FOUND, + .desc = "File descriptor set with ID '%(id)' not found", + }, + { .error_fmt = QERR_FEATURE_DISABLED, .desc = "The feature '%(name)' is not enabled", }, diff --git a/qerror.h b/qerror.h index b4c8758..b908d3f 100644 --- a/qerror.h +++ b/qerror.h @@ -133,6 +133,9 @@ QError *qobject_to_qerror(const QObject *obj); #define QERR_FD_NOT_SUPPLIED \ "{ 'class': 'FdNotSupplied', 'data': {} }" +#define QERR_FDSET_NOT_FOUND \ + "{ 'class': 'FdSetNotFound', 'data': { 'id': %ld } }" + #define QERR_FEATURE_DISABLED \ "{ 'class': 'FeatureDisabled', 'data': { 'name': %s } }" diff --git a/qmp-commands.hx b/qmp-commands.hx index ac46638..3c243d8 100644 --- a/qmp-commands.hx +++ b/qmp-commands.hx @@ -926,6 +926,137 @@ Example: EQMP + { + .name = "add-fd", + .args_type = "fdset_id:i?", + .params = "add-fd fdset_id", + .help = "Add a file descriptor, that was passed via SCM rights, to an fd set", + .mhandler.cmd_new = qmp_marshal_input_add_fd, + }, + +SQMP +add-fd +------- + +Add a file descriptor, that was passed via SCM rights, to an fd set. + +Arguments: + +- "fdset_id": The ID of the fd set to add the file descriptor to. + (json-int, optional) + +Return a json-object with the following information: + +- "fdset_id": The ID of the fd set that the fd was added to. (json-int) +- "fd": The file descriptor that was received via SCM rights and added to the + fd set. (json-int) + +Example: + +-> { "execute": "add-fd", "arguments": { "fdset_id": 1 } } +<- { "return": { "fdset_id": 1, "fd": 3 } } + +Notes: + +(1) The list of fd sets is shared by all monitor connections. +(2) If "fdset_id" is not specified, a new fd set will be created. + +EQMP + + { + .name = "remove-fd", + .args_type = "fdset_id:i,fd:i?", + .params = "remove-fd fdset_id fd", + .help = "Remove a file descriptor from an fd set", + .mhandler.cmd_new = qmp_marshal_input_remove_fd, + }, + +SQMP +remove-fd +--------- + +Remove a file descriptor from an fd set. + +Arguments: + +- "fdset_id": The ID of the fd set that the file descriptor belongs to. + (json-int) +- "fd": The file descriptor that is to be removed. (json-int, optional) + +Example: + +-> { "execute": "remove-fd", "arguments": { "fdset_id": 1, "fd": 3 } } +<- { "return": {} } + +Notes: + +(1) The list of fd sets is shared by all monitor connections. +(2) File descriptors that are removed: + (a) will not be closed until the reference count corresponding to fdset_id + reaches zero. + (b) will not be available for use after successful completion of the + remove-fd command. +(3) If "fd" is not specified, all file descriptors in "fdset_id" will be + removed. + +EQMP + + { + .name = "query-fdsets", + .args_type = "", + .help = "Return information describing all fd sets", + .mhandler.cmd_new = qmp_marshal_input_query_fdsets, + }, + +SQMP +query-fdsets +------------- + +Return information describing all fd sets. + +Arguments: None + +Example: + +-> { "execute": "query-fdsets" } +<- { "return": [ + { + "fdset_id": 1 + "refcount": 0, + "in_use": true, + "fds": [ + { + "fd": 21, + "removed": false + }, + { + "fd": 23, + "removed": false + } + ], + }, + { + "fdset_id": 2 + "refcount": 0, + "in_use": true, + "fds": [ + { + "fd": 22, + "removed": false + } + ], + } + ] + } + +Notes: + +(1) The list of fd sets is shared by all monitor connections. +(2) File descriptors are not closed until refcount is zero, and + either in_use is false or removed is true. + +EQMP + { .name = "block_passwd", .args_type = "device:B,password:s", -- 1.7.10.4

On 08/07/2012 12:49 PM, Eric Blake wrote: I'll make a note of it. Or does Luiz usually do a cleanup? Ok Ok Ok It makes sense but I'd like to limit the new functionality at this point so that I can get this support into QEMU 1.2. Can this be added as a follow up patch? Ok Ok. I'll try to use something more generic. Ok -- Regards, Corey

This patch converts all block layer open calls to qemu_open. Note that this adds the O_CLOEXEC flag to the changed open paths when the O_CLOEXEC macro is defined. Signed-off-by: Corey Bryant <coreyb(a)linux.vnet.ibm.com&gt; --- v2: -Convert calls to qemu_open instead of file_open (kwolf(a)redhat.com) -Mention introduction of O_CLOEXEC (kwolf(a)redhat.com) v3-v7: -No changes block/raw-posix.c | 18 +++++++++--------- block/raw-win32.c | 4 ++-- block/vdi.c | 5 +++-- block/vmdk.c | 21 +++++++++------------ block/vpc.c | 2 +- block/vvfat.c | 4 ++-- 6 files changed, 26 insertions(+), 28 deletions(-) diff --git a/block/raw-posix.c b/block/raw-posix.c index 0dce089..7408a42 100644 --- a/block/raw-posix.c +++ b/block/raw-posix.c @@ -572,8 +572,8 @@ static int raw_create(const char *filename, QEMUOptionParameter *options) options++; } - fd = open(filename, O_WRONLY | O_CREAT | O_TRUNC | O_BINARY, - 0644); + fd = qemu_open(filename, O_WRONLY | O_CREAT | O_TRUNC | O_BINARY, + 0644); if (fd < 0) { result = -errno; } else { @@ -846,7 +846,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 = qemu_open(bsdPath, O_RDONLY | O_BINARY | O_LARGEFILE); if (fd < 0) { bsdPath[strlen(bsdPath)-1] = '1'; } else { @@ -903,7 +903,7 @@ static int fd_open(BlockDriverState *bs) #endif return -EIO; } - s->fd = open(bs->filename, s->open_flags & ~O_NONBLOCK); + s->fd = qemu_open(bs->filename, s->open_flags & ~O_NONBLOCK); if (s->fd < 0) { s->fd_error_time = get_clock(); s->fd_got_error = 1; @@ -977,7 +977,7 @@ static int hdev_create(const char *filename, QEMUOptionParameter *options) options++; } - fd = open(filename, O_WRONLY | O_BINARY); + fd = qemu_open(filename, O_WRONLY | O_BINARY); if (fd < 0) return -errno; @@ -1055,7 +1055,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 = qemu_open(filename, O_RDONLY | O_NONBLOCK); if (fd < 0) { goto out; } @@ -1108,7 +1108,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 = qemu_open(bs->filename, s->open_flags | O_NONBLOCK); if (fd >= 0) { if (ioctl(fd, FDEJECT, 0) < 0) perror("FDEJECT"); @@ -1158,7 +1158,7 @@ static int cdrom_probe_device(const char *filename) int prio = 0; struct stat st; - fd = open(filename, O_RDONLY | O_NONBLOCK); + fd = qemu_open(filename, O_RDONLY | O_NONBLOCK); if (fd < 0) { goto out; } @@ -1282,7 +1282,7 @@ static int cdrom_reopen(BlockDriverState *bs) */ if (s->fd >= 0) close(s->fd); - fd = open(bs->filename, s->open_flags, 0644); + fd = qemu_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..8d7838d 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 = qemu_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 57325d6..c4f1529 100644 --- a/block/vdi.c +++ b/block/vdi.c @@ -653,8 +653,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 = qemu_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..557dc1b 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 = qemu_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 = qemu_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 = qemu_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..60ebf5a 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 = qemu_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 7b1dcee..22b586a 100644 --- a/block/vvfat.c +++ b/block/vvfat.c @@ -1162,7 +1162,7 @@ static int open_file(BDRVVVFATState* s,mapping_t* mapping) if(!s->current_mapping || strcmp(s->current_mapping->path,mapping->path)) { /* open file */ - int fd = open(mapping->path, O_RDONLY | O_BINARY | O_LARGEFILE); + int fd = qemu_open(mapping->path, O_RDONLY | O_BINARY | O_LARGEFILE); if(fd<0) return -1; vvfat_close_current_file(s); @@ -2221,7 +2221,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 = qemu_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); -- 1.7.10.4

When qemu_open is passed a filename of the "/dev/fdset/nnn" format (where nnn is the fdset ID), an fd with matching access mode flags will be searched for within the specified monitor fd set. If the fd is found, a dup of the fd will be returned from qemu_open. Each fd set has a reference count. The purpose of the reference count is to determine if an fd set contains file descriptors that have open dup() references that have not yet been closed. It is incremented on qemu_open and decremented on qemu_close. It is not until the refcount is zero that file desriptors in an fd set can be closed. If an fd set has dup() references open, then we must keep the other fds in the fd set open in case a reopen of the file occurs that requires an fd with a different access mode. Signed-off-by: Corey Bryant <coreyb(a)linux.vnet.ibm.com&gt; --- v2: -Get rid of file_open and move dup code to qemu_open (kwolf(a)redhat.com) -Use strtol wrapper instead of atoi (kwolf(a)redhat.com) v3: -Add note about fd leakage (eblake(a)redhat.com) v4 -Moved patch to be later in series (lcapitulino(a)redhat.com) -Update qemu_open to check access mode flags and set flags that can be set (eblake(a)redhat.com, kwolf(a)redhat.com) v5: -This patch was overhauled quite a bit in this version, with the addition of fd set and refcount support. -Use qemu_set_cloexec() on dup'd fd (eblake(a)redhat.com) -Modify flags set by fcntl on dup'd fd (eblake(a)redhat.com) -Reduce syscalls when setting flags for dup'd fd (eblake(a)redhat.com) -Fix O_RDWR, O_RDONLY, O_WRONLY checks (eblake(a)redhat.com) v6: -Pass only the fd to qemu_close() and keep track of dup fds per fd set. (kwolf(a)redhat.com, eblake(a)redhat.com) -Handle refcount incr/decr in new dup_fd_add/remove fd functions. -Use qemu_set_cloexec() appropriately in qemu_dup() (kwolf(a)redhat.com) -Simplify setting of setfl_flags in qemu_dup() (kwolf(a)redhat.com) -Add preprocessor checks for F_DUPFD_CLOEXEC (eblake(a)redhat.com) -Simplify flag checking in monitor_fdset_get_fd() (kwolf(a)redhat.com) v7: -Minor updates to reference global mon_fdsets, and to remove default_mon usage in osdep.c. (kwolf(a)redhat.com) cutils.c | 5 +++ monitor.c | 87 ++++++++++++++++++++++++++++++++++++++++++++ monitor.h | 5 +++ osdep.c | 112 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++ qemu-common.h | 1 + qemu-tool.c | 20 +++++++++++ 6 files changed, 230 insertions(+) diff --git a/cutils.c b/cutils.c index 9d4c570..8b0d2bb 100644 --- a/cutils.c +++ b/cutils.c @@ -382,3 +382,8 @@ int qemu_parse_fd(const char *param) } return fd; } + +int qemu_parse_fdset(const char *param) +{ + return qemu_parse_fd(param); +} diff --git a/monitor.c b/monitor.c index 84eade8..a16d48b 100644 --- a/monitor.c +++ b/monitor.c @@ -154,6 +154,7 @@ struct mon_fdset_t { int64_t id; int refcount; QLIST_HEAD(, mon_fdset_fd_t) fds; + QLIST_HEAD(, mon_fdset_fd_t) dup_fds; QLIST_ENTRY(mon_fdset_t) next; }; @@ -2566,6 +2567,92 @@ FdsetInfoList *qmp_query_fdsets(Error **errp) return fdset_list; } +int monitor_fdset_get_fd(int64_t fdset_id, int flags) +{ + mon_fdset_t *mon_fdset; + mon_fdset_fd_t *mon_fdset_fd; + int mon_fd_flags; + + QLIST_FOREACH(mon_fdset, &mon_fdsets, next) { + if (mon_fdset->id != fdset_id) { + continue; + } + QLIST_FOREACH(mon_fdset_fd, &mon_fdset->fds, next) { + if (mon_fdset_fd->removed) { + continue; + } + + mon_fd_flags = fcntl(mon_fdset_fd->fd, F_GETFL); + if (mon_fd_flags == -1) { + return -1; + } + + if ((flags & O_ACCMODE) == (mon_fd_flags & O_ACCMODE)) { + return mon_fdset_fd->fd; + } + } + errno = EACCES; + return -1; + } + errno = ENOENT; + return -1; +} + +int monitor_fdset_dup_fd_add(int64_t fdset_id, int dup_fd) +{ + mon_fdset_t *mon_fdset; + mon_fdset_fd_t *mon_fdset_fd_dup; + + QLIST_FOREACH(mon_fdset, &mon_fdsets, next) { + if (mon_fdset->id != fdset_id) { + continue; + } + QLIST_FOREACH(mon_fdset_fd_dup, &mon_fdset->dup_fds, next) { + if (mon_fdset_fd_dup->fd == dup_fd) { + return -1; + } + } + mon_fdset_fd_dup = g_malloc0(sizeof(*mon_fdset_fd_dup)); + mon_fdset_fd_dup->fd = dup_fd; + QLIST_INSERT_HEAD(&mon_fdset->dup_fds, mon_fdset_fd_dup, next); + mon_fdset->refcount++; + return 0; + } + return -1; +} + +static int _monitor_fdset_dup_fd_find(int dup_fd, bool remove) +{ + mon_fdset_t *mon_fdset; + mon_fdset_fd_t *mon_fdset_fd_dup; + + QLIST_FOREACH(mon_fdset, &mon_fdsets, next) { + QLIST_FOREACH(mon_fdset_fd_dup, &mon_fdset->dup_fds, next) { + if (mon_fdset_fd_dup->fd == dup_fd) { + if (remove) { + QLIST_REMOVE(mon_fdset_fd_dup, next); + mon_fdset->refcount--; + if (mon_fdset->refcount == 0) { + monitor_fdset_cleanup(mon_fdset); + } + } + return mon_fdset->id; + } + } + } + return -1; +} + +int monitor_fdset_dup_fd_find(int dup_fd) +{ + return _monitor_fdset_dup_fd_find(dup_fd, false); +} + +int monitor_fdset_dup_fd_remove(int dup_fd) +{ + return _monitor_fdset_dup_fd_find(dup_fd, true); +} + /* mon_cmds and info_cmds would be sorted at runtime */ static mon_cmd_t mon_cmds[] = { #include "hmp-commands.h" diff --git a/monitor.h b/monitor.h index 5f4de1b..30b660f 100644 --- a/monitor.h +++ b/monitor.h @@ -86,4 +86,9 @@ int qmp_qom_set(Monitor *mon, const QDict *qdict, QObject **ret); int qmp_qom_get(Monitor *mon, const QDict *qdict, QObject **ret); +int monitor_fdset_get_fd(int64_t fdset_id, int flags); +int monitor_fdset_dup_fd_add(int64_t fdset_id, int dup_fd); +int monitor_fdset_dup_fd_remove(int dup_fd); +int monitor_fdset_dup_fd_find(int dup_fd); + #endif /* !MONITOR_H */ diff --git a/osdep.c b/osdep.c index 7f876ae..9ff3561 100644 --- a/osdep.c +++ b/osdep.c @@ -48,6 +48,7 @@ extern int madvise(caddr_t, size_t, int); #include "qemu-common.h" #include "trace.h" #include "qemu_socket.h" +#include "monitor.h" static bool fips_enabled = false; @@ -78,6 +79,69 @@ int qemu_madvise(void *addr, size_t len, int advice) #endif } +/* + * Dups an fd and sets the flags + */ +static int qemu_dup(int fd, int flags) +{ + int ret; + int serrno; + int dup_flags; + int setfl_flags; + + if (flags & O_CLOEXEC) { +#ifdef F_DUPFD_CLOEXEC + ret = fcntl(fd, F_DUPFD_CLOEXEC, 0); +#else + ret = dup(fd); + if (ret != -1) { + qemu_set_cloexec(ret); + } +#endif + } else { + ret = dup(fd); + } + + if (ret == -1) { + goto fail; + } + + dup_flags = fcntl(ret, F_GETFL); + if (dup_flags == -1) { + goto fail; + } + + if ((flags & O_SYNC) != (dup_flags & O_SYNC)) { + errno = EINVAL; + goto fail; + } + + /* Set/unset flags that we can with fcntl */ + setfl_flags = O_APPEND | O_ASYNC | O_DIRECT | O_NOATIME | O_NONBLOCK; + dup_flags &= ~setfl_flags; + dup_flags |= (flags & setfl_flags); + if (fcntl(ret, F_SETFL, dup_flags) == -1) { + goto fail; + } + + /* Truncate the file in the cases that open() would truncate it */ + if (flags & O_TRUNC || + ((flags & (O_CREAT | O_EXCL)) == (O_CREAT | O_EXCL))) { + if (ftruncate(ret, 0) == -1) { + goto fail; + } + } + + return ret; + +fail: + serrno = errno; + if (ret != -1) { + close(ret); + } + errno = serrno; + return -1; +} /* * Opens a file with FD_CLOEXEC set @@ -87,6 +151,39 @@ int qemu_open(const char *name, int flags, ...) int ret; int mode = 0; +#ifndef _WIN32 + const char *fdset_id_str; + + /* Attempt dup of fd from fd set */ + if (strstart(name, "/dev/fdset/", &fdset_id_str)) { + int64_t fdset_id; + int fd, dupfd; + + fdset_id = qemu_parse_fdset(fdset_id_str); + if (fdset_id == -1) { + errno = EINVAL; + return -1; + } + + fd = monitor_fdset_get_fd(fdset_id, flags); + if (fd == -1) { + return -1; + } + + dupfd = qemu_dup(fd, flags); + if (fd == -1) { + return -1; + } + + ret = monitor_fdset_dup_fd_add(fdset_id, dupfd); + if (ret == -1) { + return -1; + } + + return dupfd; + } +#endif + if (flags & O_CREAT) { va_list ap; @@ -109,6 +206,21 @@ int qemu_open(const char *name, int flags, ...) int qemu_close(int fd) { + int64_t fdset_id; + + /* Close fd that was dup'd from an fdset */ + fdset_id = monitor_fdset_dup_fd_find(fd); + if (fdset_id != -1) { + int ret; + + ret = close(fd); + if (ret == 0) { + monitor_fdset_dup_fd_remove(fd); + } + + return ret; + } + return close(fd); } diff --git a/qemu-common.h b/qemu-common.h index e53126d..9becb32 100644 --- a/qemu-common.h +++ b/qemu-common.h @@ -166,6 +166,7 @@ int qemu_fls(int i); int qemu_fdatasync(int fd); int fcntl_setfl(int fd, int flag); int qemu_parse_fd(const char *param); +int qemu_parse_fdset(const char *param); /* * strtosz() suffixes used to specify the default treatment of an diff --git a/qemu-tool.c b/qemu-tool.c index 318c5fc..b7622f5 100644 --- a/qemu-tool.c +++ b/qemu-tool.c @@ -57,6 +57,26 @@ void monitor_protocol_event(MonitorEvent event, QObject *data) { } +int monitor_fdset_get_fd(int64_t fdset_id, int flags) +{ + return -1; +} + +int monitor_fdset_dup_fd_add(int64_t fdset_id, int dup_fd) +{ + return -1; +} + +int monitor_fdset_dup_fd_remove(int dup_fd) +{ + return -1; +} + +int monitor_fdset_dup_fd_find(int dup_fd) +{ + return -1; +} + int64_t cpu_get_clock(void) { return qemu_get_clock_ns(rt_clock); -- 1.7.10.4

On 08/08/2012 09:02 AM, Stefan Hajnoczi wrote: The reason we don't immediately close in remove-fd is so that the client doesn't have to keep track of what fd's are in use by QEMU. Let's say libvirt uses add-fd to add fd=4 (O_RDONLY) and fd=5 (O_RDWR) to fd set 2 and they both refer to /mnt/nfs/data.img. libvirt can then issue a command that uses the fd (e.g. drive_add file=/dev/fdsets/2). QEMU then opens and closes that file as it desires by dup'ing the fd in the fd set that has matching access mode (O_RDONLY or O_RDWR) and closing the dup'd fd. By not closing the fd immediately in remove-fd, we allow the client to issue a command like drive_open and immediately follow it with a remove-fd and not have to worry about determining when QEMU is done using the fd. -- Regards, Corey