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 <mdr...@linux.vnet.ibm.com>
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
