Re: [Qemu-devel] [PATCH 08/12] Threadlet: Add aio_signal_handler threadlet API

[Stefan Hajnoczi]

On Tue, Jan 18, 2011 at 6:46 AM, Arun R Bharadwaj
<address@hidden> wrote:
> * Stefan Hajnoczi <address@hidden> [2011-01-18 06:31:34]:
>
>> On Tue, Jan 18, 2011 at 4:43 AM, Arun R Bharadwaj
>> <address@hidden> wrote:
>> > * Stefan Hajnoczi <address@hidden> [2011-01-17 09:56:58]:
>> >
>> >> On Thu, Jan 13, 2011 at 12:14 PM, Arun R Bharadwaj
>> >> <address@hidden> wrote:
>> >> > +static void threadlet_io_completion_signal_handler(int signum)
>> >> > +{
>> >> > +    qemu_service_io();
>> >> > +}
>> >> > +
>> >> > +static void threadlet_register_signal_handler(void)
>> >> > +{
>> >> > +    struct sigaction act;
>> >> > +    sigfillset(&act.sa_mask);
>> >> > +    act.sa_flags = 0; /* do not restart syscalls to interrupt select() 
>> >> > */
>> >> > +    act.sa_handler = threadlet_io_completion_signal_handler;
>> >> > +    sigaction(SIGUSR2, &act, NULL);
>> >> > +}
>> >> > +
>> >> > +void threadlet_init(void)
>> >> > +{
>> >> > +    threadlet_register_signal_handler();
>> >> > +}
>> >>
>> >> This would be the right place to create qemu-threadlet.c, instead of
>> >> adding the thread_init() prototype to qemu-thread.h and then including
>> >> that in vl.c.
>> >>
>> >> Stefan
>> >
>> > I did not follow your comment here. How can we avoid including
>> > threadler_init() in vl.c?
>>
>> Instead of adding threadlet_init() and related functions to
>> posix-aio-compat.c and adding the prototype to qemu-thread.h, why not
>> just create qemu-threadlet.c/qemu-threadlet.h and put these functions
>> there instead?
>>
>> Stefan
>
> Got it. So you mean I merge patch 8 and patch 10 into a single patch.
> But wouldn't this mean we are moving code and adding new API in the
> same patch? Anthony did not want this from what I recall. But I can do
> it if you feel it makes things simple.

I don't think you need to merge the patches.  It's odd to add
functions to posix-aio-compat.c but put the prototype in qemu-thread.h
(and then include and call from vl.c).  So for these three functions
(threadlet_init, threadlet_register_signal_handler, and
threadlet_io_completion_signal_handler) only I think it makes sense to
move them to qemu-threadlet.[ch] straight away.

It's not the end of the world if you don't want to do that.  It just
jumped out at me when reading and I had to remember that it'll get
fixed up later.

Stefan