From 06138651f3347a4ad7527d48b1ccbeae89f9e7f6 Mon Sep 17 00:00:00 2001 From: Corey Bryant Date: Tue, 14 Aug 2012 16:43:42 -0400 Subject: [PATCH 01/10] qemu-char: Add MSG_CMSG_CLOEXEC flag to recvmsg Set the close-on-exec flag for the file descriptor received via SCM_RIGHTS. Signed-off-by: Corey Bryant Signed-off-by: Kevin Wolf --- qemu-char.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/qemu-char.c b/qemu-char.c index 382c71ebcd..10d1504948 100644 --- a/qemu-char.c +++ b/qemu-char.c @@ -2238,6 +2238,9 @@ static void unix_process_msgfd(CharDriverState *chr, struct msghdr *msg) if (fd < 0) continue; +#ifndef MSG_CMSG_CLOEXEC + qemu_set_cloexec(fd); +#endif if (s->msgfd != -1) close(s->msgfd); s->msgfd = fd; @@ -2253,6 +2256,7 @@ static ssize_t tcp_chr_recv(CharDriverState *chr, char *buf, size_t len) struct cmsghdr cmsg; char control[CMSG_SPACE(sizeof(int))]; } msg_control; + int flags = 0; ssize_t ret; iov[0].iov_base = buf; @@ -2263,9 +2267,13 @@ static ssize_t tcp_chr_recv(CharDriverState *chr, char *buf, size_t len) msg.msg_control = &msg_control; msg.msg_controllen = sizeof(msg_control); - ret = recvmsg(s->fd, &msg, 0); - if (ret > 0 && s->is_unix) +#ifdef MSG_CMSG_CLOEXEC + flags |= MSG_CMSG_CLOEXEC; +#endif + ret = recvmsg(s->fd, &msg, flags); + if (ret > 0 && s->is_unix) { unix_process_msgfd(chr, &msg); + } return ret; } From ba1c048a8f9c4a62812a8735ebd4fde0cfd086e8 Mon Sep 17 00:00:00 2001 From: Corey Bryant Date: Tue, 14 Aug 2012 16:43:43 -0400 Subject: [PATCH 02/10] qapi: Introduce add-fd, remove-fd, query-fdsets 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 Signed-off-by: Kevin Wolf --- monitor.c | 189 +++++++++++++++++++++++++++++++++++++++++++++++ qapi-schema.json | 98 ++++++++++++++++++++++++ qmp-commands.hx | 122 ++++++++++++++++++++++++++++++ 3 files changed, 409 insertions(+) diff --git a/monitor.c b/monitor.c index dd63f1d640..8d813d5121 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 MonFdsetFd MonFdsetFd; +struct MonFdsetFd { + int fd; + bool removed; + char *opaque; + QLIST_ENTRY(MonFdsetFd) next; +}; + +/* file descriptor set containing fds passed via SCM_RIGHTS */ +typedef struct MonFdset MonFdset; +struct MonFdset { + int64_t id; + QLIST_HEAD(, MonFdsetFd) fds; + QLIST_ENTRY(MonFdset) next; +}; + typedef struct MonitorControl { QObject *id; JSONMessageParser parser; @@ -181,6 +198,7 @@ struct Monitor { #define QMP_ACCEPT_UNKNOWNS 1 static QLIST_HEAD(mon_list, Monitor) mon_list; +static QLIST_HEAD(mon_fdsets, MonFdset) mon_fdsets; static mon_cmd_t mon_cmds[]; static mon_cmd_t info_cmds[]; @@ -2366,6 +2384,177 @@ int monitor_get_fd(Monitor *mon, const char *fdname) return -1; } +static void monitor_fdset_cleanup(MonFdset *mon_fdset) +{ + MonFdsetFd *mon_fdset_fd; + MonFdsetFd *mon_fdset_fd_next; + + QLIST_FOREACH_SAFE(mon_fdset_fd, &mon_fdset->fds, next, mon_fdset_fd_next) { + if (mon_fdset_fd->removed) { + close(mon_fdset_fd->fd); + g_free(mon_fdset_fd->opaque); + 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, bool has_opaque, + const char *opaque, Error **errp) +{ + int fd; + Monitor *mon = cur_mon; + MonFdset *mon_fdset; + MonFdsetFd *mon_fdset_fd; + AddfdInfo *fdinfo; + + fd = qemu_chr_fe_get_msgfd(mon->chr); + if (fd == -1) { + error_set(errp, QERR_FD_NOT_SUPPLIED); + goto error; + } + + if (has_fdset_id) { + QLIST_FOREACH(mon_fdset, &mon_fdsets, next) { + if (mon_fdset->id == fdset_id) { + break; + } + } + if (mon_fdset == NULL) { + error_set(errp, QERR_INVALID_PARAMETER_VALUE, "fdset-id", + "an existing fdset-id"); + goto error; + } + } else { + int64_t fdset_id_prev = -1; + MonFdset *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; + + /* 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; + if (has_opaque) { + mon_fdset_fd->opaque = g_strdup(opaque); + } + 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; + +error: + if (fd != -1) { + close(fd); + } + return NULL; +} + +void qmp_remove_fd(int64_t fdset_id, bool has_fd, int64_t fd, Error **errp) +{ + MonFdset *mon_fdset; + MonFdsetFd *mon_fdset_fd; + char fd_str[60]; + + 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) { + if (mon_fdset_fd->fd != fd) { + continue; + } + mon_fdset_fd->removed = true; + break; + } else { + mon_fdset_fd->removed = true; + } + } + if (has_fd && !mon_fdset_fd) { + goto error; + } + monitor_fdset_cleanup(mon_fdset); + return; + } + +error: + if (has_fd) { + snprintf(fd_str, sizeof(fd_str), "fdset-id:%" PRId64 ", fd:%" PRId64, + fdset_id, fd); + } else { + snprintf(fd_str, sizeof(fd_str), "fdset-id:%" PRId64, fdset_id); + } + error_set(errp, QERR_FD_NOT_FOUND, fd_str); +} + +FdsetInfoList *qmp_query_fdsets(Error **errp) +{ + MonFdset *mon_fdset; + MonFdsetFd *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; + + QLIST_FOREACH(mon_fdset_fd, &mon_fdset->fds, next) { + FdsetFdInfoList *fdsetfd_info; + + fdsetfd_info = g_malloc0(sizeof(*fdsetfd_info)); + fdsetfd_info->value = g_malloc0(sizeof(*fdsetfd_info->value)); + fdsetfd_info->value->fd = mon_fdset_fd->fd; + if (mon_fdset_fd->opaque) { + fdsetfd_info->value->has_opaque = true; + fdsetfd_info->value->opaque = g_strdup(mon_fdset_fd->opaque); + } else { + fdsetfd_info->value->has_opaque = false; + } + + 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 53bbe46e4d..3d2b2d175a 100644 --- a/qapi-schema.json +++ b/qapi-schema.json @@ -2356,3 +2356,101 @@ # Since: 1.2.0 ## { 'command': 'query-cpu-definitions', 'returns': ['CpuDefinitionInfo'] } + +# @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. +# +# @opaque: #optional A free-form string that can be used to describe the fd. +# +# Returns: @AddfdInfo on success +# If file descriptor was not received, FdNotSupplied +# If @fdset-id does not exist, InvalidParameterValue +# +# 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', '*opaque': 'str'}, + '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. +# +# 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. +# +# @opaque: #optional A free-form string that can be used to describe the fd. +# +# Since: 1.2.0 +## +{ 'type': 'FdsetFdInfo', + 'data': {'fd': 'int', '*opaque': 'str'} } + +## +# @FdsetInfo: +# +# Information about an fd set. +# +# @fdset-id: The ID of the fd set. +# +# @fds: A list of file descriptors that belong to this fd set. +# +# Since: 1.2.0 +## +{ 'type': 'FdsetInfo', + 'data': {'fdset-id': 'int', 'fds': ['FdsetFdInfo']} } + +## +# @query-fdsets: +# +# Return information describing all fd sets. +# +# Returns: A list of @FdsetInfo +# +# Since: 1.2.0 +# +# Note: The list of fd sets is shared by all monitor connections. +# +## +{ 'command': 'query-fdsets', 'returns': ['FdsetInfo'] } diff --git a/qmp-commands.hx b/qmp-commands.hx index 527b9f7c24..2ce4ce6556 100644 --- a/qmp-commands.hx +++ b/qmp-commands.hx @@ -968,6 +968,128 @@ Example: -> { "execute": "closefd", "arguments": { "fdname": "fd1" } } <- { "return": {} } +EQMP + + { + .name = "add-fd", + .args_type = "fdset-id:i?,opaque:s?", + .params = "add-fd fdset-id opaque", + .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) +- "opaque": A free-form string that can be used to describe the fd. + (json-string, 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) 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": [ + { + "fds": [ + { + "fd": 30, + "opaque": "rdonly:/path/to/file" + }, + { + "fd": 24, + "opaque": "rdwr:/path/to/file" + } + ], + "fdset-id": 1 + }, + { + "fds": [ + { + "fd": 28 + }, + { + "fd": 29 + } + ], + "fdset-id": 0 + } + ] + } + +Note: The list of fd sets is shared by all monitor connections. + EQMP { From e17408283562be359f16a7e12ddfee7509d6fe11 Mon Sep 17 00:00:00 2001 From: Corey Bryant Date: Tue, 14 Aug 2012 16:43:44 -0400 Subject: [PATCH 03/10] block: Prevent detection of /dev/fdset/ as floppy Signed-off-by: Corey Bryant Signed-off-by: Kevin Wolf --- block/raw-posix.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/block/raw-posix.c b/block/raw-posix.c index 0dce089be5..f606211760 100644 --- a/block/raw-posix.c +++ b/block/raw-posix.c @@ -1052,8 +1052,10 @@ static int floppy_probe_device(const char *filename) struct floppy_struct fdparam; struct stat st; - if (strstart(filename, "/dev/fd", NULL)) + if (strstart(filename, "/dev/fd", NULL) && + !strstart(filename, "/dev/fdset/", NULL)) { prio = 50; + } fd = open(filename, O_RDONLY | O_NONBLOCK); if (fd < 0) { From 6165f4d85d92e15b6aebdeeb2559dc687b0353c7 Mon Sep 17 00:00:00 2001 From: Corey Bryant Date: Tue, 14 Aug 2012 16:43:45 -0400 Subject: [PATCH 04/10] block: Convert open calls to qemu_open 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 Signed-off-by: Kevin Wolf --- 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 f606211760..08b997e2c0 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; @@ -1057,7 +1057,7 @@ static int floppy_probe_device(const char *filename) prio = 50; } - fd = open(filename, O_RDONLY | O_NONBLOCK); + fd = qemu_open(filename, O_RDONLY | O_NONBLOCK); if (fd < 0) { goto out; } @@ -1110,7 +1110,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"); @@ -1160,7 +1160,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; } @@ -1284,7 +1284,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 e4b0b75b70..8d7838d122 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 57325d65c4..c4f1529db9 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 18e9b4caf6..557dc1b2cc 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 5cd13d17a1..60ebf5a067 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 7b1dcee144..22b586a21b 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); From 2e1e79dae7a7e0a3d698fbd359d75e3a0239bdaa Mon Sep 17 00:00:00 2001 From: Corey Bryant Date: Tue, 14 Aug 2012 16:43:46 -0400 Subject: [PATCH 05/10] block: Convert close calls to qemu_close This patch converts all block layer close calls, that correspond to qemu_open calls, to qemu_close. Signed-off-by: Corey Bryant Signed-off-by: Kevin Wolf --- block/raw-posix.c | 24 ++++++++++++------------ block/raw-win32.c | 2 +- block/vmdk.c | 4 ++-- block/vpc.c | 2 +- block/vvfat.c | 12 ++++++------ osdep.c | 5 +++++ qemu-common.h | 1 + savevm.c | 4 ++-- 8 files changed, 30 insertions(+), 24 deletions(-) diff --git a/block/raw-posix.c b/block/raw-posix.c index 08b997e2c0..6be20b1925 100644 --- a/block/raw-posix.c +++ b/block/raw-posix.c @@ -271,7 +271,7 @@ static int raw_open_common(BlockDriverState *bs, const char *filename, out_free_buf: qemu_vfree(s->aligned_buf); out_close: - close(fd); + qemu_close(fd); return -errno; } @@ -376,7 +376,7 @@ static void raw_close(BlockDriverState *bs) { BDRVRawState *s = bs->opaque; if (s->fd >= 0) { - close(s->fd); + qemu_close(s->fd); s->fd = -1; if (s->aligned_buf != NULL) qemu_vfree(s->aligned_buf); @@ -580,7 +580,7 @@ static int raw_create(const char *filename, QEMUOptionParameter *options) if (ftruncate(fd, total_size * BDRV_SECTOR_SIZE) != 0) { result = -errno; } - if (close(fd) != 0) { + if (qemu_close(fd) != 0) { result = -errno; } } @@ -850,7 +850,7 @@ static int hdev_open(BlockDriverState *bs, const char *filename, int flags) if (fd < 0) { bsdPath[strlen(bsdPath)-1] = '1'; } else { - close(fd); + qemu_close(fd); } filename = bsdPath; } @@ -889,7 +889,7 @@ static int fd_open(BlockDriverState *bs) last_media_present = (s->fd >= 0); if (s->fd >= 0 && (get_clock() - s->fd_open_time) >= FD_OPEN_TIMEOUT) { - close(s->fd); + qemu_close(s->fd); s->fd = -1; #ifdef DEBUG_FLOPPY printf("Floppy closed\n"); @@ -988,7 +988,7 @@ static int hdev_create(const char *filename, QEMUOptionParameter *options) else if (lseek(fd, 0, SEEK_END) < total_size * BDRV_SECTOR_SIZE) ret = -ENOSPC; - close(fd); + qemu_close(fd); return ret; } @@ -1038,7 +1038,7 @@ static int floppy_open(BlockDriverState *bs, const char *filename, int flags) return ret; /* close fd so that we can reopen it as needed */ - close(s->fd); + qemu_close(s->fd); s->fd = -1; s->fd_media_changed = 1; @@ -1072,7 +1072,7 @@ static int floppy_probe_device(const char *filename) prio = 100; outc: - close(fd); + qemu_close(fd); out: return prio; } @@ -1107,14 +1107,14 @@ static void floppy_eject(BlockDriverState *bs, bool eject_flag) int fd; if (s->fd >= 0) { - close(s->fd); + qemu_close(s->fd); s->fd = -1; } fd = qemu_open(bs->filename, s->open_flags | O_NONBLOCK); if (fd >= 0) { if (ioctl(fd, FDEJECT, 0) < 0) perror("FDEJECT"); - close(fd); + qemu_close(fd); } } @@ -1175,7 +1175,7 @@ static int cdrom_probe_device(const char *filename) prio = 100; outc: - close(fd); + qemu_close(fd); out: return prio; } @@ -1283,7 +1283,7 @@ static int cdrom_reopen(BlockDriverState *bs) * FreeBSD seems to not notice sometimes... */ if (s->fd >= 0) - close(s->fd); + qemu_close(s->fd); fd = qemu_open(bs->filename, s->open_flags, 0644); if (fd < 0) { s->fd = -1; diff --git a/block/raw-win32.c b/block/raw-win32.c index 8d7838d122..c56bf83375 100644 --- a/block/raw-win32.c +++ b/block/raw-win32.c @@ -261,7 +261,7 @@ static int raw_create(const char *filename, QEMUOptionParameter *options) return -EIO; set_sparse(fd); ftruncate(fd, total_size * 512); - close(fd); + qemu_close(fd); return 0; } diff --git a/block/vmdk.c b/block/vmdk.c index 557dc1b2cc..daee4268be 100644 --- a/block/vmdk.c +++ b/block/vmdk.c @@ -1258,7 +1258,7 @@ static int vmdk_create_extent(const char *filename, int64_t filesize, ret = 0; exit: - close(fd); + qemu_close(fd); return ret; } @@ -1506,7 +1506,7 @@ static int vmdk_create(const char *filename, QEMUOptionParameter *options) } ret = 0; exit: - close(fd); + qemu_close(fd); return ret; } diff --git a/block/vpc.c b/block/vpc.c index 60ebf5a067..c0b82c4f57 100644 --- a/block/vpc.c +++ b/block/vpc.c @@ -744,7 +744,7 @@ static int vpc_create(const char *filename, QEMUOptionParameter *options) } fail: - close(fd); + qemu_close(fd); return ret; } diff --git a/block/vvfat.c b/block/vvfat.c index 22b586a21b..59d3c5b8ac 100644 --- a/block/vvfat.c +++ b/block/vvfat.c @@ -1105,7 +1105,7 @@ static inline void vvfat_close_current_file(BDRVVVFATState *s) if(s->current_mapping) { s->current_mapping = NULL; if (s->current_fd) { - close(s->current_fd); + qemu_close(s->current_fd); s->current_fd = 0; } } @@ -2230,7 +2230,7 @@ static int commit_one_file(BDRVVVFATState* s, } if (offset > 0) { if (lseek(fd, offset, SEEK_SET) != offset) { - close(fd); + qemu_close(fd); g_free(cluster); return -3; } @@ -2251,13 +2251,13 @@ static int commit_one_file(BDRVVVFATState* s, (uint8_t*)cluster, (rest_size + 0x1ff) / 0x200); if (ret < 0) { - close(fd); + qemu_close(fd); g_free(cluster); return ret; } if (write(fd, cluster, rest_size) < 0) { - close(fd); + qemu_close(fd); g_free(cluster); return -2; } @@ -2268,11 +2268,11 @@ static int commit_one_file(BDRVVVFATState* s, if (ftruncate(fd, size)) { perror("ftruncate()"); - close(fd); + qemu_close(fd); g_free(cluster); return -4; } - close(fd); + qemu_close(fd); g_free(cluster); return commit_mappings(s, first_cluster, dir_index); diff --git a/osdep.c b/osdep.c index c07faf546e..7f876aecd0 100644 --- a/osdep.c +++ b/osdep.c @@ -107,6 +107,11 @@ int qemu_open(const char *name, int flags, ...) return ret; } +int qemu_close(int fd) +{ + return close(fd); +} + /* * A variant of write(2) which handles partial write. * diff --git a/qemu-common.h b/qemu-common.h index 095e28d89a..b388c5cd70 100644 --- a/qemu-common.h +++ b/qemu-common.h @@ -208,6 +208,7 @@ const char *path(const char *pathname); void *qemu_oom_check(void *ptr); int qemu_open(const char *name, int flags, ...); +int qemu_close(int fd); ssize_t qemu_write_full(int fd, const void *buf, size_t count) QEMU_WARN_UNUSED_RESULT; ssize_t qemu_send_full(int fd, const void *buf, size_t count, int flags) diff --git a/savevm.c b/savevm.c index 0ea10c9b66..f002bfc48d 100644 --- a/savevm.c +++ b/savevm.c @@ -513,7 +513,7 @@ static void qemu_fill_buffer(QEMUFile *f) * * Returns f->close() return value, or 0 if close function is not set. */ -static int qemu_close(QEMUFile *f) +static int qemu_fclose_internal(QEMUFile *f) { int ret = 0; if (f->close) { @@ -535,7 +535,7 @@ int qemu_fclose(QEMUFile *f) { int ret; qemu_fflush(f); - ret = qemu_close(f); + ret = qemu_fclose_internal(f); /* If any error was spotted before closing, we should report it * instead of the close() return value. */ From adb696f3d8535119fe0f5363de79a29e6bda83ed Mon Sep 17 00:00:00 2001 From: Corey Bryant Date: Tue, 14 Aug 2012 16:43:47 -0400 Subject: [PATCH 06/10] block: Enable qemu_open/close to work with fd sets 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. Signed-off-by: Corey Bryant Signed-off-by: Kevin Wolf --- Makefile | 6 +-- cutils.c | 5 +++ monitor.c | 86 +++++++++++++++++++++++++++++++++++++- monitor.h | 5 +++ osdep.c | 111 +++++++++++++++++++++++++++++++++++++++++++++++++ qemu-common.h | 1 + qemu-tool.c | 20 +++++++++ qemu-user.c | 20 +++++++++ tests/Makefile | 2 +- 9 files changed, 251 insertions(+), 5 deletions(-) diff --git a/Makefile b/Makefile index d736ea5311..6a65f8f2fa 100644 --- a/Makefile +++ b/Makefile @@ -148,9 +148,6 @@ install-libcacard: libcacard.la $(call quiet-command,$(MAKE) $(SUBDIR_MAKEFLAGS) -C libcacard V="$(V)" TARGET_DIR="$*/" install-libcacard,) endif -vscclient$(EXESUF): $(libcacard-y) $(oslib-obj-y) $(trace-obj-y) qemu-timer-common.o libcacard/vscclient.o - $(call quiet-command,$(CC) $(LDFLAGS) -o $@ $^ $(libcacard_libs) $(LIBS)," LINK $@") - ###################################################################### qemu-img.o: qemu-img-cmds.h @@ -166,6 +163,9 @@ qemu-io$(EXESUF): qemu-io.o cmd.o $(tools-obj-y) $(block-obj-y) qemu-bridge-helper$(EXESUF): qemu-bridge-helper.o +vscclient$(EXESUF): $(libcacard-y) $(oslib-obj-y) $(trace-obj-y) $(tools-obj-y) qemu-timer-common.o libcacard/vscclient.o + $(call quiet-command,$(CC) $(LDFLAGS) -o $@ $^ $(libcacard_libs) $(LIBS)," LINK $@") + fsdev/virtfs-proxy-helper$(EXESUF): fsdev/virtfs-proxy-helper.o fsdev/virtio-9p-marshal.o oslib-posix.o $(trace-obj-y) fsdev/virtfs-proxy-helper$(EXESUF): LIBS += -lcap diff --git a/cutils.c b/cutils.c index ee4614d378..8ef648f4b9 100644 --- a/cutils.c +++ b/cutils.c @@ -383,6 +383,11 @@ int qemu_parse_fd(const char *param) return fd; } +int qemu_parse_fdset(const char *param) +{ + return qemu_parse_fd(param); +} + /* round down to the nearest power of 2*/ int64_t pow2floor(int64_t value) { diff --git a/monitor.c b/monitor.c index 8d813d5121..a4a29b9cd7 100644 --- a/monitor.c +++ b/monitor.c @@ -154,6 +154,7 @@ typedef struct MonFdset MonFdset; struct MonFdset { int64_t id; QLIST_HEAD(, MonFdsetFd) fds; + QLIST_HEAD(, MonFdsetFd) dup_fds; QLIST_ENTRY(MonFdset) next; }; @@ -2398,7 +2399,7 @@ static void monitor_fdset_cleanup(MonFdset *mon_fdset) } } - if (QLIST_EMPTY(&mon_fdset->fds)) { + if (QLIST_EMPTY(&mon_fdset->fds) && QLIST_EMPTY(&mon_fdset->dup_fds)) { QLIST_REMOVE(mon_fdset, next); g_free(mon_fdset); } @@ -2555,6 +2556,89 @@ FdsetInfoList *qmp_query_fdsets(Error **errp) return fdset_list; } +int monitor_fdset_get_fd(int64_t fdset_id, int flags) +{ + MonFdset *mon_fdset; + MonFdsetFd *mon_fdset_fd; + int mon_fd_flags; + +#ifndef _WIN32 + QLIST_FOREACH(mon_fdset, &mon_fdsets, next) { + if (mon_fdset->id != fdset_id) { + continue; + } + QLIST_FOREACH(mon_fdset_fd, &mon_fdset->fds, next) { + 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; + } +#endif + + errno = ENOENT; + return -1; +} + +int monitor_fdset_dup_fd_add(int64_t fdset_id, int dup_fd) +{ + MonFdset *mon_fdset; + MonFdsetFd *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); + return 0; + } + return -1; +} + +static int monitor_fdset_dup_fd_find_remove(int dup_fd, bool remove) +{ + MonFdset *mon_fdset; + MonFdsetFd *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); + if (QLIST_EMPTY(&mon_fdset->dup_fds)) { + 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_remove(dup_fd, false); +} + +int monitor_fdset_dup_fd_remove(int dup_fd) +{ + return monitor_fdset_dup_fd_find_remove(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 4ef9a046f8..47d556b9d1 100644 --- a/monitor.h +++ b/monitor.h @@ -87,4 +87,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 7f876aecd0..5b78ceebef 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,66 @@ int qemu_madvise(void *addr, size_t len, int advice) #endif } +#ifndef _WIN32 +/* + * Dups an fd and sets the flags + */ +static int qemu_dup_flags(int fd, int flags) +{ + int ret; + int serrno; + int dup_flags; + int setfl_flags; + +#ifdef F_DUPFD_CLOEXEC + ret = fcntl(fd, F_DUPFD_CLOEXEC, 0); +#else + ret = dup(fd); + if (ret != -1) { + qemu_set_cloexec(ret); + } +#endif + 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; +} +#endif /* * Opens a file with FD_CLOEXEC set @@ -87,6 +148,41 @@ 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_flags(fd, flags); + if (dupfd == -1) { + return -1; + } + + ret = monitor_fdset_dup_fd_add(fdset_id, dupfd); + if (ret == -1) { + close(dupfd); + errno = EINVAL; + return -1; + } + + return dupfd; + } +#endif + if (flags & O_CREAT) { va_list ap; @@ -109,6 +205,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 b388c5cd70..e5c2bcd204 100644 --- a/qemu-common.h +++ b/qemu-common.h @@ -167,6 +167,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 64b5e88bc7..18205babab 100644 --- a/qemu-tool.c +++ b/qemu-tool.c @@ -62,6 +62,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); diff --git a/qemu-user.c b/qemu-user.c index 08ccb0fe8e..13fb9ae77b 100644 --- a/qemu-user.c +++ b/qemu-user.c @@ -35,3 +35,23 @@ void monitor_vprintf(Monitor *mon, const char *fmt, va_list ap) void monitor_set_error(Monitor *mon, QError *qerror) { } + +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; +} diff --git a/tests/Makefile b/tests/Makefile index f3f4159c25..26a67ce6f5 100644 --- a/tests/Makefile +++ b/tests/Makefile @@ -81,7 +81,7 @@ TARGETS=$(patsubst %-softmmu,%, $(filter %-softmmu,$(TARGET_DIRS))) QTEST_TARGETS=$(foreach TARGET,$(TARGETS), $(if $(check-qtest-$(TARGET)-y), $(TARGET),)) check-qtest-$(CONFIG_POSIX)=$(foreach TARGET,$(TARGETS), $(check-qtest-$(TARGET)-y)) -qtest-obj-y = tests/libqtest.o $(oslib-obj-y) +qtest-obj-y = tests/libqtest.o $(oslib-obj-y) $(tools-obj-y) $(check-qtest-y): $(qtest-obj-y) .PHONY: check-help From efb87c169740e618ec548c45c819a43e0ade2bab Mon Sep 17 00:00:00 2001 From: Corey Bryant Date: Tue, 14 Aug 2012 16:43:48 -0400 Subject: [PATCH 07/10] monitor: Clean up fd sets on monitor disconnect Fd sets are shared by all monitor connections. Fd sets are considered to be in use while at least one monitor is connected. When the last monitor disconnects, all fds that are members of an fd set with no outstanding dup references are closed. This prevents any fd leakage associated with a client disconnect prior to using a passed fd. Signed-off-by: Corey Bryant Signed-off-by: Kevin Wolf --- monitor.c | 23 ++++++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) diff --git a/monitor.c b/monitor.c index a4a29b9cd7..ce42466a63 100644 --- a/monitor.c +++ b/monitor.c @@ -200,6 +200,7 @@ struct Monitor { static QLIST_HEAD(mon_list, Monitor) mon_list; static QLIST_HEAD(mon_fdsets, MonFdset) mon_fdsets; +static int mon_refcount; static mon_cmd_t mon_cmds[]; static mon_cmd_t info_cmds[]; @@ -2391,7 +2392,8 @@ static void monitor_fdset_cleanup(MonFdset *mon_fdset) MonFdsetFd *mon_fdset_fd_next; QLIST_FOREACH_SAFE(mon_fdset_fd, &mon_fdset->fds, next, mon_fdset_fd_next) { - if (mon_fdset_fd->removed) { + if (mon_fdset_fd->removed || + (QLIST_EMPTY(&mon_fdset->dup_fds) && mon_refcount == 0)) { close(mon_fdset_fd->fd); g_free(mon_fdset_fd->opaque); QLIST_REMOVE(mon_fdset_fd, next); @@ -2405,6 +2407,16 @@ static void monitor_fdset_cleanup(MonFdset *mon_fdset) } } +static void monitor_fdsets_cleanup(void) +{ + MonFdset *mon_fdset; + MonFdset *mon_fdset_next; + + QLIST_FOREACH_SAFE(mon_fdset, &mon_fdsets, next, mon_fdset_next) { + monitor_fdset_cleanup(mon_fdset); + } +} + AddfdInfo *qmp_add_fd(bool has_fdset_id, int64_t fdset_id, bool has_opaque, const char *opaque, Error **errp) { @@ -4824,9 +4836,12 @@ static void monitor_control_event(void *opaque, int event) data = get_qmp_greeting(); monitor_json_emitter(mon, data); qobject_decref(data); + mon_refcount++; break; case CHR_EVENT_CLOSED: json_message_parser_destroy(&mon->mc->parser); + mon_refcount--; + monitor_fdsets_cleanup(); break; } } @@ -4867,6 +4882,12 @@ static void monitor_event(void *opaque, int event) readline_show_prompt(mon->rs); } mon->reset_seen = 1; + mon_refcount++; + break; + + case CHR_EVENT_CLOSED: + mon_refcount--; + monitor_fdsets_cleanup(); break; } } From 64e69e80920d82df3fa679bc41b13770d2f99360 Mon Sep 17 00:00:00 2001 From: Stefan Priebe Date: Wed, 15 Aug 2012 09:09:54 +0200 Subject: [PATCH 08/10] iscsi: Fix NULL dereferences / races between task completion and abort Signed-off-by: Stefan Priebe Acked-by: Ronnie Sahlberg Signed-off-by: Kevin Wolf --- block/iscsi.c | 55 +++++++++++++++++++++------------------------------ 1 file changed, 23 insertions(+), 32 deletions(-) diff --git a/block/iscsi.c b/block/iscsi.c index 219f927823..bb9cf82459 100644 --- a/block/iscsi.c +++ b/block/iscsi.c @@ -76,6 +76,10 @@ static void iscsi_abort_task_cb(struct iscsi_context *iscsi, int status, void *command_data, void *private_data) { + IscsiAIOCB *acb = (IscsiAIOCB *)private_data; + + scsi_free_scsi_task(acb->task); + acb->task = NULL; } static void @@ -84,15 +88,15 @@ iscsi_aio_cancel(BlockDriverAIOCB *blockacb) IscsiAIOCB *acb = (IscsiAIOCB *)blockacb; IscsiLun *iscsilun = acb->iscsilun; - acb->common.cb(acb->common.opaque, -ECANCELED); acb->canceled = 1; - /* send a task mgmt call to the target to cancel the task on the target */ - iscsi_task_mgmt_abort_task_async(iscsilun->iscsi, acb->task, - iscsi_abort_task_cb, NULL); + acb->common.cb(acb->common.opaque, -ECANCELED); - /* then also cancel the task locally in libiscsi */ - iscsi_scsi_task_cancel(iscsilun->iscsi, acb->task); + /* send a task mgmt call to the target to cancel the task on the target + * this also cancels the task in libiscsi + */ + iscsi_task_mgmt_abort_task_async(iscsilun->iscsi, acb->task, + iscsi_abort_task_cb, &acb); } static AIOPool iscsi_aio_pool = { @@ -179,11 +183,18 @@ iscsi_readv_writev_bh_cb(void *p) qemu_bh_delete(acb->bh); - if (acb->canceled == 0) { + if (!acb->canceled) { acb->common.cb(acb->common.opaque, acb->status); } qemu_aio_release(acb); + + if (acb->canceled) { + return; + } + + scsi_free_scsi_task(acb->task); + acb->task = NULL; } @@ -197,10 +208,8 @@ iscsi_aio_write16_cb(struct iscsi_context *iscsi, int status, g_free(acb->buf); - if (acb->canceled != 0) { + if (acb->canceled) { qemu_aio_release(acb); - scsi_free_scsi_task(acb->task); - acb->task = NULL; return; } @@ -212,8 +221,6 @@ iscsi_aio_write16_cb(struct iscsi_context *iscsi, int status, } iscsi_schedule_bh(iscsi_readv_writev_bh_cb, acb); - scsi_free_scsi_task(acb->task); - acb->task = NULL; } static int64_t sector_qemu2lun(int64_t sector, IscsiLun *iscsilun) @@ -298,10 +305,8 @@ iscsi_aio_read16_cb(struct iscsi_context *iscsi, int status, trace_iscsi_aio_read16_cb(iscsi, status, acb, acb->canceled); - if (acb->canceled != 0) { + if (acb->canceled) { qemu_aio_release(acb); - scsi_free_scsi_task(acb->task); - acb->task = NULL; return; } @@ -313,8 +318,6 @@ iscsi_aio_read16_cb(struct iscsi_context *iscsi, int status, } iscsi_schedule_bh(iscsi_readv_writev_bh_cb, acb); - scsi_free_scsi_task(acb->task); - acb->task = NULL; } static BlockDriverAIOCB * @@ -414,10 +417,8 @@ iscsi_synccache10_cb(struct iscsi_context *iscsi, int status, { IscsiAIOCB *acb = opaque; - if (acb->canceled != 0) { + if (acb->canceled) { qemu_aio_release(acb); - scsi_free_scsi_task(acb->task); - acb->task = NULL; return; } @@ -429,8 +430,6 @@ iscsi_synccache10_cb(struct iscsi_context *iscsi, int status, } iscsi_schedule_bh(iscsi_readv_writev_bh_cb, acb); - scsi_free_scsi_task(acb->task); - acb->task = NULL; } static BlockDriverAIOCB * @@ -468,10 +467,8 @@ iscsi_unmap_cb(struct iscsi_context *iscsi, int status, { IscsiAIOCB *acb = opaque; - if (acb->canceled != 0) { + if (acb->canceled) { qemu_aio_release(acb); - scsi_free_scsi_task(acb->task); - acb->task = NULL; return; } @@ -483,8 +480,6 @@ iscsi_unmap_cb(struct iscsi_context *iscsi, int status, } iscsi_schedule_bh(iscsi_readv_writev_bh_cb, acb); - scsi_free_scsi_task(acb->task); - acb->task = NULL; } static BlockDriverAIOCB * @@ -528,10 +523,8 @@ iscsi_aio_ioctl_cb(struct iscsi_context *iscsi, int status, { IscsiAIOCB *acb = opaque; - if (acb->canceled != 0) { + if (acb->canceled) { qemu_aio_release(acb); - scsi_free_scsi_task(acb->task); - acb->task = NULL; return; } @@ -560,8 +553,6 @@ iscsi_aio_ioctl_cb(struct iscsi_context *iscsi, int status, } iscsi_schedule_bh(iscsi_readv_writev_bh_cb, acb); - scsi_free_scsi_task(acb->task); - acb->task = NULL; } static BlockDriverAIOCB *iscsi_aio_ioctl(BlockDriverState *bs, From d4c823292336598e2a0c79eb38a640d95748e2a2 Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Wed, 15 Aug 2012 12:52:45 +0200 Subject: [PATCH 09/10] block: Flush parent to OS with cache=unsafe Commit 29cdb251 already added a comment that no unnecessary flushes to disk will occur, this patch makes the code even get to the point of the comment. This is mostly theoretical because in practice we only stack one format on top of one protocol, the former implementing flush_to_os and the latter only flush_to_disk. It starts to matter when drivers that are not on top implement flush_to_os. Signed-off-by: Kevin Wolf Reviewed-by: Stefan Hajnoczi --- block.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/block.c b/block.c index 016858bf8c..470bdcc1f6 100644 --- a/block.c +++ b/block.c @@ -3534,7 +3534,7 @@ int coroutine_fn bdrv_co_flush(BlockDriverState *bs) /* But don't actually force it to the disk with cache=unsafe */ if (bs->open_flags & BDRV_O_NO_FLUSH) { - return 0; + goto flush_parent; } if (bs->drv->bdrv_co_flush_to_disk) { @@ -3573,6 +3573,7 @@ int coroutine_fn bdrv_co_flush(BlockDriverState *bs) /* Now flush the underlying protocol. It will also have BDRV_O_NO_FLUSH * in the case of cache=unsafe, so there are no useless flushes. */ +flush_parent: return bdrv_co_flush(bs->file); } From 58c8cce21c13bddd332590fb1fd9a5c369975d3f Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Wed, 15 Aug 2012 14:08:56 +0200 Subject: [PATCH 10/10] qemu-iotests: Fix 030 after switch to GenericError Signed-off-by: Kevin Wolf Reviewed-by: Stefan Hajnoczi --- tests/qemu-iotests/030 | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/tests/qemu-iotests/030 b/tests/qemu-iotests/030 index cc671dd7aa..f71ab8dd10 100755 --- a/tests/qemu-iotests/030 +++ b/tests/qemu-iotests/030 @@ -225,8 +225,7 @@ class TestSetSpeed(ImageStreamingTestCase): self.assert_no_active_streams() result = self.vm.qmp('block-stream', device='drive0', speed=-1) - self.assert_qmp(result, 'error/class', 'InvalidParameter') - self.assert_qmp(result, 'error/data/name', 'speed') + self.assert_qmp(result, 'error/class', 'GenericError') self.assert_no_active_streams() @@ -234,8 +233,7 @@ class TestSetSpeed(ImageStreamingTestCase): self.assert_qmp(result, 'return', {}) result = self.vm.qmp('block-job-set-speed', device='drive0', speed=-1) - self.assert_qmp(result, 'error/class', 'InvalidParameter') - self.assert_qmp(result, 'error/data/name', 'speed') + self.assert_qmp(result, 'error/class', 'GenericError') self.cancel_and_wait()