Re: [Qemu-devel] [PATCH for-2.7] fix qemu exit on memory hotplug when allocation fails at prealloc time

[Markus Armbruster]

Igor Mammedov <address@hidden> writes:

> On Thu, 21 Jul 2016 10:36:40 +0200
> Markus Armbruster <address@hidden> wrote:
>
>> Igor Mammedov <address@hidden> writes:
[...]
>> > @@ -352,15 +352,14 @@ void os_mem_prealloc(int fd, char *area, size_t 
>> > memory)
>> >          for (i = 0; i < numpages; i++) {
>> >              memset(area + (hpagesize * i), 0, 1);
>> >          }
>> > +    }
>> >  
>> > -        ret = sigaction(SIGBUS, &oldact, NULL);
>> > -        if (ret) {
>> > -            perror("os_mem_prealloc: failed to reinstall signal handler");
>> > -            exit(1);
>> > -        }
>> > -
>> > -        pthread_sigmask(SIG_SETMASK, &oldset, NULL);
>> > +    ret = sigaction(SIGBUS, &oldact, NULL);
>> > +    if (ret) {
>> > +        perror("os_mem_prealloc: failed to reinstall signal handler");
>> > +        exit(1);  
>> 
>> I guess you're keeping the exit() here, because you can't recover
>> cleanly from this error.  I should never happen anyway.  Worth a
>> comment, I think.
> I didn't added comment because it's obvious that it's not possible
> to recover and also because it's just the same old code that haven't
> had a comment to begin with (probably also due to its obviousness)

It's obvious enough once you read closely enough to see what the failing
code is doing.

The old code uses perror() + exit() consistently.  The new code mixes
Error with them, which is an anti-pattern.  That's okay, the exit() is
perfectly fine here.  The issue for me is that the anti-pattern triggers
the WTF-detector at a glance, but then I have to read closely to see #1
it's intentional, and #2 it's okay.  That wasn't the case before.

[...]