[libvirt] [PATCH] qemu_process: Read errors from child

[Michal Privoznik]

On 03.12.2013 19:17, Eric Blake wrote:
> On 12/03/2013 10:52 AM, Michal Privoznik wrote:
>> https://bugzilla.redhat.com/show_bug.cgi?id=1035955
>>
>> There's a window when starting a qemu process between fork() and exec()
>> during which we are doing things that may fail but not tunnelling the
>> error to the daemon. This is basically all within qemuProcessHook().
> 
> Or more precisely, where things fail so early that the child decided not
> to exec() a qemu process in the first place.
> 
>> So whenever we fail in something, e.g. placing a process onto numa node,
>> users ale left with:
> 
> s/ale/are/
> 
>>
>>     error: Child quit during startup handshake: Input/output error
>>
>> while the original error is thrown into the domain log:
>>
>>     libvirt:  error : internal error: NUMA memory tuning in 'preferred'
>>     mode only supports single node
>>
>> Hence, we should read the log file and search for the error message and
>> report it to users.
> 
> An alternative might be to use a pipe back to the parent, marked
> close-on-exec, where we dump any untranslated error messages (the parent
> can then read until EOF, and if it gets anything on the pipe, then we
> know the child raised an error).  Doing so is probably a good idea
> anyways, as printing into the log file in the child process is dangerous
> (if the child uses _("...") to translate an error message, then it is
> calling a function that is not async-signal-safe, and could deadlock).
> But it's more invasive, and is not written, so your approach is find to
> use in the meantime.

Does this mean ACK with grammar fixed? :)

> 
>>  /*
>> + * Read domain log and probably overwrite error if there's one in
>> + * the domain log file. This functions exists to cover the small
> 
> s/functions/function/
> 
>> + * window between fork() and exec() during which child may fail
>> + * by libvirt hand, e.g. placing onto a NUMA node failed.
> 
> s/libvirt/libvirt's/
> 
>> +    if (off > 0) {
>> +        /* Found an error in the log. Report it */
>> +        virResetLastError();
>> +        virReportError(VIR_ERR_INTERNAL_ERROR,
>> +                       _("Process exited prior exec: %s"),
> 
> s/prior/prior to/
> 
> 

Michal