From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:60621) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Submr-00072t-FH for qemu-devel@nongnu.org; Fri, 27 Jul 2012 00:04:26 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Submq-0000Ia-Et for qemu-devel@nongnu.org; Fri, 27 Jul 2012 00:04:25 -0400 Received: from e1.ny.us.ibm.com ([32.97.182.141]:39751) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Submq-0000IW-9y for qemu-devel@nongnu.org; Fri, 27 Jul 2012 00:04:24 -0400 Received: from /spool/local by e1.ny.us.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Fri, 27 Jul 2012 00:04:03 -0400 Received: from d01relay01.pok.ibm.com (d01relay01.pok.ibm.com [9.56.227.233]) by d01dlp01.pok.ibm.com (Postfix) with ESMTP id 4D8EB38C8042 for ; Fri, 27 Jul 2012 00:03:53 -0400 (EDT) Received: from d01av02.pok.ibm.com (d01av02.pok.ibm.com [9.56.224.216]) by d01relay01.pok.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id q6R43rlY395050 for ; Fri, 27 Jul 2012 00:03:53 -0400 Received: from d01av02.pok.ibm.com (loopback [127.0.0.1]) by d01av02.pok.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id q6R43pFN001192 for ; Fri, 27 Jul 2012 01:03:52 -0300 Message-ID: <50121325.1040201@linux.vnet.ibm.com> Date: Fri, 27 Jul 2012 00:03:49 -0400 From: Corey Bryant MIME-Version: 1.0 References: <1343048885-1701-1-git-send-email-coreyb@linux.vnet.ibm.com> <1343048885-1701-7-git-send-email-coreyb@linux.vnet.ibm.com> <50104C44.1050206@redhat.com> <5010C031.9040602@linux.vnet.ibm.com> <501108BB.6070204@redhat.com> In-Reply-To: <501108BB.6070204@redhat.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v5 6/6] block: Enable qemu_open/close to work with fd sets List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Kevin Wolf Cc: aliguori@us.ibm.com, stefanha@linux.vnet.ibm.com, libvir-list@redhat.com, qemu-devel@nongnu.org, lcapitulino@redhat.com, Eric Blake On 07/26/2012 05:07 AM, Kevin Wolf wrote: > Am 26.07.2012 05:57, schrieb Corey Bryant: >> On 07/25/2012 03:43 PM, Eric Blake wrote: >>> On 07/23/2012 07:08 AM, Corey Bryant wrote: >>>> +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; >>> >>> This says we fail on the first fcntl() failure, instead of trying other >>> fds in the set. Granted, an fcntl() failure is probably the sign of a >>> bigger bug (such as closing an fd at the wrong point in time), so I >>> guess trying to go on doesn't make much sense once we already know we >>> are hosed. >>> >> >> I think I'll stick with it the way it is. If fcntl() fails we might >> have a tainted fd set so I think we should fail. > > The alternative would be s/return 1/continue/, right? I think either way > is acceptable. > Yes, we'd continue the loop instead of returning -1. I prefer to return on the first failure, but if anyone feels strongly about continuing the loop, please let me know. -- Regards, Corey