From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=36152 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1PPxdu-0003Az-J4 for qemu-devel@nongnu.org; Tue, 07 Dec 2010 08:31:53 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1PPxdt-0001K5-CC for qemu-devel@nongnu.org; Tue, 07 Dec 2010 08:31:42 -0500 Received: from mx1.redhat.com ([209.132.183.28]:34648) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1PPxdt-0001Jy-2D for qemu-devel@nongnu.org; Tue, 07 Dec 2010 08:31:41 -0500 Message-ID: <4CFE3738.4010506@redhat.com> Date: Tue, 07 Dec 2010 14:31:36 +0100 From: Jes Sorensen MIME-Version: 1.0 References: <1291399402-20366-1-git-send-email-mdroth@linux.vnet.ibm.com> <1291399402-20366-2-git-send-email-mdroth@linux.vnet.ibm.com> In-Reply-To: <1291399402-20366-2-git-send-email-mdroth@linux.vnet.ibm.com> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Subject: [Qemu-devel] Re: [RFC][PATCH v5 01/21] Move code related to fd handlers into utility functions List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Michael Roth Cc: agl@linux.vnet.ibm.com, stefanha@linux.vnet.ibm.com, abeekhof@redhat.com, qemu-devel@nongnu.org, aliguori@linux.vnet.ibm.com, ryanh@us.ibm.com On 12/03/10 19:03, Michael Roth wrote: > This allows us to implement an i/o loop outside of vl.c that can > interact with objects that use qemu_set_fd_handler() > > Signed-off-by: Michael Roth This commit message really tells us nothing. Please be more specific about what is in the commit. > diff --git a/qemu-ioh.c b/qemu-ioh.c > new file mode 100644 > index 0000000..cc71470 > --- /dev/null > +++ b/qemu-ioh.c > @@ -0,0 +1,115 @@ > +/* > + * QEMU System Emulator > + * > + * Copyright (c) 2003-2008 Fabrice Bellard > + * > + * Permission is hereby granted, free of charge, to any person obtaining a copy > + * of this software and associated documentation files (the "Software"), to deal > + * in the Software without restriction, including without limitation the rights > + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell > + * copies of the Software, and to permit persons to whom the Software is > + * furnished to do so, subject to the following conditions: Is this moved or new code? If the former, fine, but if it is new code, you might want to leave your own name on the (c). I presume at least some of the changes are (c) 2010? > +/* XXX: fd_read_poll should be suppressed, but an API change is > + necessary in the character devices to suppress fd_can_read(). */ XXX in the comment isn't really of much use. Please make it more explicit, or put your name in if it is a statement you wish to make. > +int qemu_set_fd_handler3(void *ioh_record_list, > + int fd, > + IOCanReadHandler *fd_read_poll, > + IOHandler *fd_read, > + IOHandler *fd_write, > + void *opaque) I am not happy with this addition of numbers to these functions, it doesn't tell us why we have a 3 and how it differs from 2. If 3 is really the backend for implementing 2, maybe it would be better to name it __qemu_set_fd_handler2() and then have macros/wrappers calling into it. Cheers, Jes