[Qemu-devel] Re: [PATCH 18/21] move blocking of signals to qemu_signalfd_init

[Paolo Bonzini]

On 02/21/2011 11:12 AM, Jan Kiszka wrote:
> On 2011-02-21 09:43, Paolo Bonzini wrote:
> > This makes it easier to add a Win32 stub.  The patch does that, too.
> >
> > Signed-off-by: Paolo Bonzini<address@hidden>
> > ---
> >   cpus.c |   87 ++++++++++++++++++++++++++-------------------------------------
> >   1 files changed, 36 insertions(+), 51 deletions(-)
> >
> > diff --git a/cpus.c b/cpus.c
> > index 1b6893d..5bb95d8 100644
> > --- a/cpus.c
> > +++ b/cpus.c
> > @@ -346,11 +346,37 @@ static void sigfd_handler(void *opaque)
> >       }
> >   }
> >
> > -static int qemu_signalfd_init(sigset_t mask)
> > +static int qemu_signalfd_init(void)
> >   {
> >       int sigfd;
> > +    sigset_t set;
> >
> > -    sigfd = qemu_signalfd(&mask);
> > +#ifdef CONFIG_IOTHREAD
> > +    /* SIGUSR2 used by posix-aio-compat.c */
> > +    sigemptyset(&set);
> > +    sigaddset(&set, SIGUSR2);
> > +    pthread_sigmask(SIG_UNBLOCK,&set, NULL);
>
> These lines are actually not related to signalfd setup. So you may want
> to rename this function or move everything that does not block signals
> and then pass the very same set to qemu_signalfd elsewhere.

Ok, I'll just rename qemu_signalfd_init to qemu_init_iothread_signals.

> For consistency reasons, you should also refactor the !IOTHREAD case.

I think I did?

non-iothread:

> >   int qemu_init_main_loop(void)
> >   {
> > -#ifndef _WIN32
> > -    sigset_t blocked_signals;
> >       int ret;
> >
> > -    blocked_signals = block_synchronous_signals();
> > -
> > -    ret = qemu_signalfd_init(blocked_signals);
> > +    ret = qemu_signalfd_init();
> >       if (ret) {
> >           return ret;
> >       }
> > -#endif

iothread:

> >   int qemu_init_main_loop(void)
> >   {
> >       int ret;
> > -    sigset_t blocked_signals;
> >
> >       qemu_init_sigbus();
> >
> > -    blocked_signals = block_io_signals();
> > -
> > -    ret = qemu_signalfd_init(blocked_signals);
> > +    ret = qemu_signalfd_init();
> >       if (ret) {
> >           return ret;
> >       }

Paolo