From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:57362) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SyJPT-00083x-2R for qemu-devel@nongnu.org; Mon, 06 Aug 2012 05:15:36 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1SyJPO-0006NY-Ue for qemu-devel@nongnu.org; Mon, 06 Aug 2012 05:15:34 -0400 Received: from mx1.redhat.com ([209.132.183.28]:59805) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SyJPO-0006NU-N2 for qemu-devel@nongnu.org; Mon, 06 Aug 2012 05:15:30 -0400 Message-ID: <501F8B27.2070508@redhat.com> Date: Mon, 06 Aug 2012 11:15:19 +0200 From: Kevin Wolf 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> <500D4E42.8080709@linux.vnet.ibm.com> <501AFD68.8010009@linux.vnet.ibm.com> In-Reply-To: <501AFD68.8010009@linux.vnet.ibm.com> Content-Type: text/plain; charset=ISO-8859-1 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: Corey Bryant Cc: aliguori@us.ibm.com, stefanha@linux.vnet.ibm.com, libvir-list@redhat.com, qemu-devel@nongnu.org, Luiz Capitulino , eblake@redhat.com Am 03.08.2012 00:21, schrieb Corey Bryant: >>> @@ -84,6 +158,36 @@ 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(default_mon, fdset_id, flags); >> >> I know that use of default_mon in this patch is not correct, but I >> wanted to get these patches out for review. I used default_mon for >> testing because cur_mon wasn't pointing to the monitor I'd added fd sets >> to. I need to figure out why. >> > > Does it make sense to use default_mon here? After digging into this > some more, I'm thinking it makes sense, and I'll explain why. > > It looks like cur_mon can't be used. cur_mon will point to the monitor > object for the duration of a command, and be reset to old_mon (NULL in > my case) after the command completes. > > qemu_open() and qemu_close() are frequently called long after a monitor > command has come and gone, so cur_mon won't work. For example, > drive_add will cause qemu_open() to be called, but after the command has > completed, the file will keep getting opened/closed during normal QEMU > operation. I'm not sure why, I've just noticed this behavior. > > Does anyone have any thoughts on this? It would require fd sets to be > added to the default monitor only. I think we have two design options that would make sense: 1. Make the file descriptors global instead of per-monitor. Is there a reason why each monitor has its own set of fds? (Also I'm wondering if they survive a monitor disconnect this way?) 2. Save a monitor reference with the fdset information. Allowing to send file descriptors on every monitor, but making only those of the default monitor actually usable, sounds like a bad choice to me. Kevin