• E
    virFork: simplify semantics · 25f87817
    Eric Blake 提交于
    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: NEric Blake <eblake@redhat.com>
    25f87817
vircommand.h 6.2 KB