Re: [libvirt] [PATCH v5 6/6] 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. 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) --- block/raw-posix.c | 24 +++++----- block/raw-win32.c | 2 +- block/vmdk.c | 4 +- block/vpc.c | 2 +- block/vvfat.c | 12 ++--- cutils.c | 5 ++ monitor.c | 85 +++++++++++++++++++++++++++++++++ monitor.h | 4 ++ osdep.c | 138 ++++++++++++++++++++++++++++++++++++++++++++++++++++- qemu-common.h | 3 +- qemu-tool.c | 12 +++++ 11 files changed, 267 insertions(+), 24 deletions(-) diff --git a/block/raw-posix.c b/block/raw-posix.c index a172de3..5d0a801 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: - qemu_close(fd); + qemu_close(fd, filename); return -errno; }

@@ -2551,6 +2551,91 @@ static void monitor_fdsets_set_in_use(Monitor *mon, bool in_use) } } +void monitor_fdset_increment_refcount(Monitor *mon, int64_t fdset_id) +{ + mon_fdset_t *mon_fdset; + + if (!mon) { + return; + } + + QLIST_FOREACH(mon_fdset, &mon->fdsets, next) { + if (mon_fdset->id == fdset_id) { + mon_fdset->refcount++; + break; + } + } +} + +void monitor_fdset_decrement_refcount(Monitor *mon, int64_t fdset_id) +{ + mon_fdset_t *mon_fdset; + + if (!mon) { + return; + } + + QLIST_FOREACH(mon_fdset, &mon->fdsets, next) { + if (mon_fdset->id == fdset_id) { + mon_fdset->refcount--; + if (mon_fdset->refcount == 0) { + monitor_fdset_cleanup(mon_fdset); + } + break; + } + } +}

+ +int monitor_fdset_get_fd(Monitor *mon, int64_t fdset_id, int flags) +{ + mon_fdset_t *mon_fdset; + mon_fdset_fd_t *mon_fdset_fd; + int mon_fd_flags; + + if (!mon) { + errno = ENOENT; + return -1; + } + + 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; + } + + switch (flags & O_ACCMODE) { + case O_RDWR: + if ((mon_fd_flags & O_ACCMODE) == O_RDWR) { + return mon_fdset_fd->fd; + } + break; + case O_RDONLY: + if ((mon_fd_flags & O_ACCMODE) == O_RDONLY) { + return mon_fdset_fd->fd; + } + break; + case O_WRONLY: + if ((mon_fd_flags & O_ACCMODE) == O_WRONLY) { + return mon_fdset_fd->fd; + } + break; + }

+ } + errno = EACCES; + return -1; + } + errno = ENOENT; + return -1; +} + /* mon_cmds and info_cmds would be sorted at runtime */ static mon_cmd_t mon_cmds[] = { #include "hmp-commands.h"

@@ -75,6 +76,79 @@ 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 i; + int ret; + int serrno; + int dup_flags; + int setfl_flags[] = { O_APPEND, O_ASYNC, O_DIRECT, O_NOATIME, + O_NONBLOCK, 0 }; + + if (flags & O_CLOEXEC) { + ret = fcntl(fd, F_DUPFD_CLOEXEC, 0); + if (ret == -1 && errno == EINVAL) { + ret = dup(fd); + if (ret != -1 && fcntl_setfl(ret, O_CLOEXEC) == -1) { + goto fail; + } + } + } 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 */ + i = 0; + while (setfl_flags[i] != 0) { + if (flags & setfl_flags[i]) { + dup_flags = (dup_flags | setfl_flags[i]); + } else { + dup_flags = (dup_flags & ~setfl_flags[i]); + } + i++; + }

+ + 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; + } + }

+ + qemu_set_cloexec(ret);