From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:51989) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1T0uy0-0007n6-0s for qemu-devel@nongnu.org; Mon, 13 Aug 2012 09:46:06 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1T0uxy-0004VG-OO for qemu-devel@nongnu.org; Mon, 13 Aug 2012 09:45:59 -0400 Received: from e33.co.us.ibm.com ([32.97.110.151]:60593) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1T0uxy-0004Tk-Hf for qemu-devel@nongnu.org; Mon, 13 Aug 2012 09:45:58 -0400 Received: from /spool/local by e33.co.us.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Mon, 13 Aug 2012 07:45:57 -0600 Received: from d03relay05.boulder.ibm.com (d03relay05.boulder.ibm.com [9.17.195.107]) by d03dlp02.boulder.ibm.com (Postfix) with ESMTP id 2101F3E40059 for ; Mon, 13 Aug 2012 13:45:30 +0000 (WET) Received: from d03av03.boulder.ibm.com (d03av03.boulder.ibm.com [9.17.195.169]) by d03relay05.boulder.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id q7DDiuDR040086 for ; Mon, 13 Aug 2012 07:45:13 -0600 Received: from d03av03.boulder.ibm.com (loopback [127.0.0.1]) by d03av03.boulder.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id q7DDieUo027260 for ; Mon, 13 Aug 2012 07:44:40 -0600 Message-ID: <502904C6.6050903@linux.vnet.ibm.com> Date: Mon, 13 Aug 2012 09:44:38 -0400 From: Corey Bryant MIME-Version: 1.0 References: <1344690878-1555-1-git-send-email-coreyb@linux.vnet.ibm.com> <1344690878-1555-7-git-send-email-coreyb@linux.vnet.ibm.com> <50266C28.6070908@redhat.com> In-Reply-To: <50266C28.6070908@redhat.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v9 6/7] block: Enable qemu_open/close to work with fd sets List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eric Blake Cc: kwolf@redhat.com, aliguori@us.ibm.com, stefanha@linux.vnet.ibm.com, libvir-list@redhat.com, qemu-devel@nongnu.org, lcapitulino@redhat.com, pbonzini@redhat.com I'll send a new version shortly with these updates also. -- Regards, Corey On 08/11/2012 10:28 AM, Eric Blake wrote: > 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@redhat.com) >> -Fix dupfd leak in qemu_dup(). (eblake@redhat.com) >> -Always set O_CLOEXEC in qemu_dup(). (kwolf@redhat.com) >> -Change name of qemu_dup() to qemu_dup_flags(). (kwolf@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. > -- Regards, Corey