From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:39105) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SFlgG-0006Hv-Bk for qemu-devel@nongnu.org; Thu, 05 Apr 2012 08:20:58 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1SFlg4-0003O1-E5 for qemu-devel@nongnu.org; Thu, 05 Apr 2012 08:20:46 -0400 Received: from david.siemens.de ([192.35.17.14]:32824) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SFlg4-0003NZ-1X for qemu-devel@nongnu.org; Thu, 05 Apr 2012 08:20:36 -0400 Message-ID: <4F7D8E0F.3040906@siemens.com> Date: Thu, 05 Apr 2012 14:20:31 +0200 From: Jan Kiszka MIME-Version: 1.0 References: <14c3f383cbeedf4bf67a920a8fb6f6ced85d5be2.1333623555.git.jan.kiszka@siemens.com> <4F7D80B0.5070008@redhat.com> In-Reply-To: <4F7D80B0.5070008@redhat.com> Content-Type: text/plain; charset=ISO-8859-15 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v3 05/10] Introduce QemuEvent abstraction List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini Cc: Kevin Wolf , Anthony Liguori , "qemu-devel@nongnu.org" , "Michael S. Tsirkin" On 2012-04-05 13:23, Paolo Bonzini wrote: > Il 05/04/2012 12:59, Jan Kiszka ha scritto: >> Provide generic services for binary events. Blocking wait would be >> feasible but is not included yet as there are no users. >> >> diff --git a/qemu-event-posix.c b/qemu-event-posix.c >> new file mode 100644 >> index 0000000..6138168 >> --- /dev/null >> +++ b/qemu-event-posix.c >> @@ -0,0 +1,110 @@ >> +/* >> + * Posix implementations of event signaling service >> + * >> + * Copyright Red Hat, Inc. 2012 >> + * Copyright Siemens AG 2012 >> + * >> + * Author: >> + * Paolo Bonzini >> + * Jan Kiszka >> + * >> + * This work is licensed under the terms of the GNU GPL, version 2 or later. >> + * See the COPYING file in the top-level directory. >> + * >> + */ >> +#include "qemu-thread.h" >> +#include "qemu-common.h" >> +#include "main-loop.h" >> + >> +#ifdef CONFIG_EVENTFD >> +#include >> +#endif >> + >> +void qemu_event_init(QemuEvent *event, bool signaled) >> +{ >> + int fds[2]; >> + int ret; >> + >> +#ifdef CONFIG_EVENTFD >> + ret = eventfd(signaled, EFD_NONBLOCK | EFD_CLOEXEC); >> + if (ret >= 0) { >> + event->rfd = ret; >> + event->wfd = dup(ret); >> + if (event->wfd < 0) { >> + qemu_error_exit(errno, __func__); >> + } >> + qemu_set_cloexec(event->wfd); >> + return; >> + } >> + if (errno != ENOSYS) { >> + qemu_error_exit(errno, __func__); >> + } >> + /* fall back to pipe-based support */ >> +#endif >> + >> + ret = qemu_pipe(fds); >> + if (ret < 0) { >> + qemu_error_exit(errno, __func__); >> + } >> + event->rfd = fds[0]; >> + event->wfd = fds[1]; >> + if (signaled) { >> + qemu_event_signal(event); >> + } >> +} >> + >> +void qemu_event_destroy(QemuEvent *event) >> +{ >> + close(event->rfd); >> + close(event->wfd); >> +} >> + >> +int qemu_event_get_signal_fd(QemuEvent *event) >> +{ >> + return event->wfd; >> +} > > How would you use it? Since qemu_event_signal ignores EAGAIN, polling > for writeability of the fd is useless. vhost, see patch 9. > > This is really little more than search-and-replace from what gets out of > event-notifier.c after my patches, and the commit message does not > explain the differences in the two APIs. Having separate commits as I > had would make the steps obvious. Is it really worthwhile to do this > and introduce the need for patches 9+10 (plus IIRC conflicts in qemu-kvm)? Will reorganize the patches to morph & split event-notifier.c. The conflict with qemu-kvm is minimal, though. > > (In fact, qemu_event_get_poll_fd is a layering violation too. In a > perfect world, aio.c would simply get a list of EventNotifiers and no > other types of fd). The actual problem is that we do not have an abstraction of a handle that can be registered with a generic polling service. So we have qemu_aio_set_fd_handler, qemu_set_fd_handler2, qemu_add_wait_object etc. That requires the introduction of qemu_event_set_handler - and makes the service dependent on the main loop. I'm not happy about this either, maybe it's worth rethinking this. Jan -- Siemens AG, Corporate Technology, CT T DE IT 1 Corporate Competence Center Embedded Linux