On 12/07/10 15:48, Michael Roth wrote: > On 12/07/2010 07:31 AM, Jes Sorensen wrote: >> 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. >> > > Currently, in qemu, the virtagent client/server functionality is driven > by vl.c:main_loop_wait(), which implements a select() loop that kicks > off handlers registered for various FDs/events via qemu_set_fd_handler(). > > To share the code with the agent, qemu-va.c, I re-implemented this i/o > loop to do same thing, along with vl.c:qemu_set_fd_handler*() and > friends. It was big nasty copy/paste job and I think most of the > reviewers agreed that the i/o loop code should be shared. > > This commit moves the shared code into back-end utility functions that > get called by vl.c:qemu_set_fd_handler()/qemu_process_fd_handlers() and > friends for qemu, and by > qemu-tools.c:qemu_set_fd_handler()/qemu_process_fd_handlers() for tools. > > So now to interact with code that uses qemu_set_fd_handler() you can > built a select() loop around these utility functions.
Please put some of this in the commit message. >> 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. > > That was the initial plan, but qemu_set_fd_handler2() is a back-end of > sorts for qemu_set_fd_handler(), so I was just trying to be consistent > with the naming. Personally I don't have any objections one way or the > other. Anything to avoid qemu_set_fd_handler17() at some point. I think using __qemu_set_fd_handler() encourages people to modify that code rather than copy it. Cheers, Jes
