mirror of
https://gitlab.com/libvirt/libvirt.git
synced 2024-12-22 21:55:25 +00:00
25f87817ab
The old semantics of virFork() violates the priciple of good usability: it requires the caller to check the pid argument after use, *even when virFork returned -1*, in order to properly abort a child process that failed setup done immediately after fork() - that is, the caller must call _exit() in the child. While uses in virfile.c did this correctly, uses in 'virsh lxc-enter-namespace' and 'virt-login-shell' would happily return from the calling function in both the child and the parent, leading to very confusing results. [Thankfully, I found the problem by inspection, and can't actually trigger the double return on error without an LD_PRELOAD library.] It is much better if the semantics of virFork are impossible to abuse. Looking at virFork(), the parent could only ever return -1 with a non-negative pid if it misused pthread_sigmask, but this never happens. Up until this patch series, the child could return -1 with non-negative pid if it fails to set up signals correctly, but we recently fixed that to make the child call _exit() at that point instead of forcing the caller to do it. Thus, the return value and contents of the pid argument are now redundant (a -1 return now happens only for failure to fork, a child 0 return only happens for a successful 0 pid, and a parent 0 return only happens for a successful non-zero pid), so we might as well return the pid directly rather than an integer of whether it succeeded or failed; this is also good from the interface design perspective as users are already familiar with fork() semantics. One last change in this patch: before returning the pid directly, I found cases where using virProcessWait unconditionally on a cleanup path of a virFork's -1 pid return would be nicer if there were a way to avoid it overwriting an earlier message. While such paths are a bit harder to come by with my change to a direct pid return, I decided to keep the virProcessWait change in this patch. * src/util/vircommand.h (virFork): Change signature. * src/util/vircommand.c (virFork): Guarantee that child will only return on success, to simplify callers. Return pid rather than status, now that the situations are always the same. (virExec): Adjust caller, also avoid open-coding process death. * src/util/virprocess.c (virProcessWait): Tweak semantics when pid is -1. (virProcessRunInMountNamespace): Adjust caller. * src/util/virfile.c (virFileAccessibleAs, virFileOpenForked) (virDirCreate): Likewise. * tools/virt-login-shell.c (main): Likewise. * tools/virsh-domain.c (cmdLxcEnterNamespace): Likewise. * tests/commandtest.c (test23): Likewise. Signed-off-by: Eric Blake <eblake@redhat.com> |
||
|---|---|---|
| .. | ||
| wireshark | ||
| libvirt_win_icon_16x16.ico | ||
| libvirt_win_icon_32x32.ico | ||
| libvirt_win_icon_48x48.ico | ||
| libvirt_win_icon_64x64.ico | ||
| libvirt-guests.init.in | ||
| libvirt-guests.service.in | ||
| libvirt-guests.sh.in | ||
| libvirt-guests.sysconf | ||
| Makefile.am | ||
| virsh_win_icon.rc | ||
| virsh-console.c | ||
| virsh-console.h | ||
| virsh-domain-monitor.c | ||
| virsh-domain-monitor.h | ||
| virsh-domain.c | ||
| virsh-domain.h | ||
| virsh-edit.c | ||
| virsh-host.c | ||
| virsh-host.h | ||
| virsh-interface.c | ||
| virsh-interface.h | ||
| virsh-network.c | ||
| virsh-network.h | ||
| virsh-nodedev.c | ||
| virsh-nodedev.h | ||
| virsh-nwfilter.c | ||
| virsh-nwfilter.h | ||
| virsh-pool.c | ||
| virsh-pool.h | ||
| virsh-secret.c | ||
| virsh-secret.h | ||
| virsh-snapshot.c | ||
| virsh-snapshot.h | ||
| virsh-volume.c | ||
| virsh-volume.h | ||
| virsh.c | ||
| virsh.h | ||
| virsh.pod | ||
| virt-host-validate-common.c | ||
| virt-host-validate-common.h | ||
| virt-host-validate-lxc.c | ||
| virt-host-validate-lxc.h | ||
| virt-host-validate-qemu.c | ||
| virt-host-validate-qemu.h | ||
| virt-host-validate.c | ||
| virt-login-shell.c | ||
| virt-login-shell.conf | ||
| virt-login-shell.pod | ||
| virt-pki-validate.in | ||
| virt-sanlock-cleanup.in | ||
| virt-xml-validate.in | ||