From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:56130) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1T0uwd-0003jx-PZ for qemu-devel@nongnu.org; Mon, 13 Aug 2012 09:44:36 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1T0uwc-0003ah-CE for qemu-devel@nongnu.org; Mon, 13 Aug 2012 09:44:35 -0400 Received: from e8.ny.us.ibm.com ([32.97.182.138]:40480) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1T0uwc-0003ad-7n for qemu-devel@nongnu.org; Mon, 13 Aug 2012 09:44:34 -0400 Received: from /spool/local by e8.ny.us.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Mon, 13 Aug 2012 09:44:33 -0400 Received: from d01relay03.pok.ibm.com (d01relay03.pok.ibm.com [9.56.227.235]) by d01dlp03.pok.ibm.com (Postfix) with ESMTP id DB7B4C9006A for ; Mon, 13 Aug 2012 09:44:30 -0400 (EDT) Received: from d03av02.boulder.ibm.com (d03av02.boulder.ibm.com [9.17.195.168]) by d01relay03.pok.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id q7DDiUv5086866 for ; Mon, 13 Aug 2012 09:44:30 -0400 Received: from d03av02.boulder.ibm.com (loopback [127.0.0.1]) by d03av02.boulder.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id q7DDiI2H001589 for ; Mon, 13 Aug 2012 07:44:20 -0600 Message-ID: <502904A5.7060807@linux.vnet.ibm.com> Date: Mon, 13 Aug 2012 09:44:05 -0400 From: Corey Bryant MIME-Version: 1.0 References: <1344690878-1555-1-git-send-email-coreyb@linux.vnet.ibm.com> <1344690878-1555-3-git-send-email-coreyb@linux.vnet.ibm.com> <50266938.7000003@redhat.com> In-Reply-To: <50266938.7000003@redhat.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v9 2/7] qapi: Introduce add-fd, remove-fd, query-fdsets 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. -- Regards, Corey On 08/11/2012 10:16 AM, Eric Blake wrote: > On 08/11/2012 07:14 AM, Corey Bryant wrote: >> This patch adds support that enables passing of file descriptors >> to the QEMU monitor where they will be stored in specified file >> descriptor sets. >> > >> >> v9: >> -Use fdset-id rather than fdset_id. (eblake@redhat.com) >> -Update example for query-fdsets. (eblake@redhat.com) >> -Close fd immediately on remove-fd. >> (kwolf@redhat.com, eblake@redhat.com) >> -Drop fdset refcount, and check dup_fds instead (in a later patch). >> (eblake@redhat.com) >> -Move mon_refcount code to a later patch. (kwolf@redhat.com) >> > >> +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 or no fdset-id"); > > The 'no fdset-id' portion of this error message doesn't make sense - it > can only be reached if has_fdset_id was true. > >> + >> +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) { > ... > >> + } >> + >> +error: >> + snprintf(fd_str, sizeof(fd_str), >> + "fdset-id:%" PRId64 ", fd:%" PRId64, fdset_id, fd); > > Oops - fd is uninitialized if has_fd is false and the outer loop failed > to find fdset_id. You need two separate error messages here, based on > whether fd was provided. > >> +-> { "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 >> + }, > > No trailing comma here. >