Re: [libvirt] [PATCH v9 6/7] block: Enable qemu_open/close to work with fd sets

On 08/11/2012 07:14 AM, Corey Bryant wrote: > 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. > > v9: > -Drop fdset refcount and check dup_fds instead. (eblake(a)redhat.com) > -Fix dupfd leak in qemu_dup(). (eblake(a)redhat.com) > -Always set O_CLOEXEC in qemu_dup(). (kwolf(a)redhat.com) > -Change name of qemu_dup() to qemu_dup_flags(). (kwolf(a)redhat.com) > > @@ -87,6 +146,40 @@ 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 (fd == -1) { Checking the wrong condition: s/fd/dupfd/ > + return -1; > + } > + > + ret = monitor_fdset_dup_fd_add(fdset_id, dupfd); > + if (ret == -1) { > + close(dupfd); > + return -1; This function appears to promise a reasonable errno on failure. However, I don't think monitor_fdset_dup_fd_add guarantees a reasonable errno, and even if it does, close() can corrupt errno. I think that prior to returning here, you either need an explicit errno=ENOMEM, or fix monitor_fdset_dup_fd to guarantee a nice errno, plus a save and restore of errno here. Unless no one cares about errno on failure, in which case your earlier errno=EINVAL can be dropped.