From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=55223 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1PgKmi-0007zW-D0 for qemu-devel@nongnu.org; Fri, 21 Jan 2011 12:28:33 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1PgKmS-00069R-GN for qemu-devel@nongnu.org; Fri, 21 Jan 2011 12:28:13 -0500 Received: from e9.ny.us.ibm.com ([32.97.182.139]:60908) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1PgKmS-000683-Dv for qemu-devel@nongnu.org; Fri, 21 Jan 2011 12:28:12 -0500 Received: from d01dlp01.pok.ibm.com (d01dlp01.pok.ibm.com [9.56.224.56]) by e9.ny.us.ibm.com (8.14.4/8.13.1) with ESMTP id p0LGwgDv008496 for ; Fri, 21 Jan 2011 12:03:36 -0500 Received: from d01relay05.pok.ibm.com (d01relay05.pok.ibm.com [9.56.227.237]) by d01dlp01.pok.ibm.com (Postfix) with ESMTP id 3D53C7281F8 for ; Fri, 21 Jan 2011 12:26:49 -0500 (EST) Received: from d01av04.pok.ibm.com (d01av04.pok.ibm.com [9.56.224.64]) by d01relay05.pok.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id p0LHQnns163104 for ; Fri, 21 Jan 2011 12:26:49 -0500 Received: from d01av04.pok.ibm.com (loopback [127.0.0.1]) by d01av04.pok.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id p0LHQmRh027465 for ; Fri, 21 Jan 2011 12:26:48 -0500 Message-ID: <4D39C1D4.9030805@linux.vnet.ibm.com> Date: Fri, 21 Jan 2011 11:26:44 -0600 From: Michael Roth MIME-Version: 1.0 References: <1295270117-24760-1-git-send-email-mdroth@linux.vnet.ibm.com> <1295270117-24760-4-git-send-email-mdroth@linux.vnet.ibm.com> <4D39B4B0.50207@redhat.com> In-Reply-To: <4D39B4B0.50207@redhat.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: [Qemu-devel] Re: [RFC][PATCH v6 03/23] Make qemu timers available for tools List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Jes Sorensen Cc: agl@linux.vnet.ibm.com, stefanha@linux.vnet.ibm.com, abeekhof@redhat.com, marcel.mittelstaedt@de.ibm.com, qemu-devel@nongnu.org, aliguori@linux.vnet.ibm.com, ryanh@us.ibm.com, markus_mueller@de.ibm.com On 01/21/2011 10:30 AM, Jes Sorensen wrote: > On 01/17/11 14:14, Michael Roth wrote: >> diff --git a/qemu-ioh.c b/qemu-ioh.c >> index cc71470..001e7a2 100644 >> --- a/qemu-ioh.c >> +++ b/qemu-ioh.c >> @@ -22,7 +22,11 @@ >> * THE SOFTWARE. >> */ >> #include "qemu-ioh.h" >> +#include "qemu-char.h" >> #include "qlist.h" >> +#ifdef CONFIG_EVENTFD >> +#include >> +#endif >> >> /* XXX: fd_read_poll should be suppressed, but an API change is >> necessary in the character devices to suppress fd_can_read(). */ >> @@ -113,3 +117,92 @@ void qemu_process_fd_handlers2(void *ioh_record_list, const fd_set *rfds, >> } >> } >> } >> + >> +#ifndef _WIN32 >> +void iothread_event_increment(int *io_thread_fd) > > Please split the WIN32 stuff into it's own file, similar to oslib-posix > and oslib-win32.c etc. Will look into this, but qemu-ioh.c has common code too so we'd end up with qemu-ioh/qemu-ioh-posix/qemu-ioh-win2.c. We could alternatively have a "#ifndef _WIN32" around functions in qemu-ioh.c that would be replaced by win32-specific ones from qemu-ioh-win32. No strong preference either way, but sometimes I find navigating across too many files more annoying that #ifdefs, and there's not a whole lot in these. > >> diff --git a/qemu-ioh.h b/qemu-ioh.h >> index 7c6e833..2c714a9 100644 >> --- a/qemu-ioh.h >> +++ b/qemu-ioh.h >> @@ -31,4 +31,13 @@ void qemu_get_fdset2(void *ioh_record_list, int *nfds, fd_set *rfds, >> void qemu_process_fd_handlers2(void *ioh_record_list, const fd_set *rfds, >> const fd_set *wfds, const fd_set *xfds); >> >> + >> +#ifndef _WIN32 >> +void iothread_event_increment(int *io_thread_fd); >> +int iothread_event_init(int *io_thread_fd); >> +#else >> +int win32_event_init(HANDLE *qemu_event_handle); >> +void win32_event_increment(HANDLE *qemu_event_handle); >> +#endif > > Can you not do something slightly nicer that allows for those to be the > same prototype for all users? Like define a event_handle_t? > Don't see why not. >> + >> +#ifndef _WIN32 >> +static int io_thread_fd = -1; > > Needs splitting into separate files too. > >> diff --git a/qemu-tool.h b/qemu-tool.h >> new file mode 100644 >> index 0000000..fd693cf >> --- /dev/null >> +++ b/qemu-tool.h >> @@ -0,0 +1,26 @@ >> +#ifndef QEMU_TOOL_H >> +#define QEMU_TOOL_H >> + >> +#include "qemu-common.h" >> + >> +#ifdef CONFIG_EVENTFD >> +#include >> +#endif >> + >> +typedef void VMStateDescription; >> +typedef int VMStateInfo; >> + >> +#ifndef _WIN32 >> +void qemu_event_increment(void); >> +int qemu_event_init(void); >> +#else >> +int qemu_event_init(void); >> +void qemu_event_increment(void); >> +#endif > > No matter how long I stare at those prototypes, I fail to see the > difference between the win32 and the posix version :) Heh, the ordering of course! :) Not sure how I missed this one. The patch is pretty rough in general, I'll see what I can do about cleaning things up a bit. > > Cheers, > Jes