[PATCH 1/1] vircommand.c: write child pidfile before process tuning in virExec()

[Michal Prívozník]

On 10/1/20 4:35 PM, Daniel Henrique Barboza wrote:
> When VIR_EXEC_DAEMON is true and cmd->pidfile exists, the parent
> will expect the pidfile to be written before exiting, sitting
> tight in a saferead() call waiting.
> 
> The child then does process tuning (via virProcessSet* functions)
> before writing the pidfile. Problem is that these tunings can
> fail, and trigger a 'fork_error' jump, before cmd->pidfile is
> written. The result is that the process was aborted in the
> child, but the parent is still hang in the saferead() call.
> 
> This behavior can be reproduced by trying to create and execute
> a QEMU guest in user mode (e.g. using qemu:///session as non-root).
> virProcessSetMaxMemLock() will fail if the spawned libvirtd user
> process does not have CAP_SYS_RESOURCE capability. setrlimit() will
> fail, and a 'fork_error' jump is triggered before cmd->pidfile
> is written. The parent will hung in saferead() indefinitely. From
> the user perspective, 'virsh start <guest>' will hang up
> indefinitely. CTRL+C can be used to retrieve the terminal, but
> any subsequent 'virsh' call will also hang because the previous
> libvirtd user process is still there.
> 
> We can fix this by moving all virProcessSet*() tuning functions
> to be executed after cmd->pidfile is taken care of. In the case
> mentioned above, this would be the result of 'virsh start'
> after this patch:
> 
> error: Failed to start domain vm1
> error: internal error: Process exited prior to exec: libvirt:  error :
> cannot limit locked memory to 79691776: Operation not permitted
> 
> Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1882093
> 
> Signed-off-by: Daniel Henrique Barboza <danielhb413 at gmail.com>
> ---
>   src/util/vircommand.c | 19 ++++++++++---------
>   1 file changed, 10 insertions(+), 9 deletions(-)

Reviewed-by: Michal Privoznik <mprivozn at redhat.com>

And congratz onto gaining commit rights :-)

Michal