[Libguestfs] [PATCH nbdkit] sh: Remove assert and replace with smarter file descriptor duplication. (was: Re: [nbdkit PATCH v2 14/17] sh: Use pipe2 with CLOEXEC when possible)

[Eric Blake]

On 8/27/19 7:55 AM, Eric Blake wrote:
> On 8/27/19 6:47 AM, Richard W.M. Jones wrote:
>> On Fri, Aug 02, 2019 at 02:26:15PM -0500, Eric Blake wrote:
>>> +  /* Ensure that stdin/out/err of the current process were not empty
>>> +   * before we started creating pipes (otherwise, the close and dup2
>>> +   * calls below become more complex to juggle fds around correctly).
>>> +  */
>>> +  assert (in_fd[0] > STDERR_FILENO && in_fd[1] > STDERR_FILENO &&
>>> +          out_fd[0] > STDERR_FILENO && out_fd[1] > STDERR_FILENO &&
>>> +          err_fd[0] > STDERR_FILENO && err_fd[1] > STDERR_FILENO);
>>
>> This assert is now being thrown whenever we use nbdkit + sh plugin +
>> nbdkit -s option.  It's easy to demonstrate using a libnbd one-liner:
>>

> Hmm, so we've closed stdin/out because the client connection is no
> longer around, but still give the shell script one more callback.  Yeah,
> the failure is definitely explainable, and worth fixing.
> 
>> We can easily paper over this by ignoring the assert on the close
>> path, but I foresee two problems:
>>
>> (1) I don't believe the assert is generally correct.  While it's not
>> ideal for nbdkit to be run with stdin/out/err closed (they obviously
>> should be connected to /dev/null) it's also not impossible for that to
>> happen.  We don't cope well with this situation.

Alternative fix: instead of closing stdin/out for -s, open /dev/null and
dup2() it over stdin/out.  That has the same effect of ending the client
connection, but leaves the fds allocated so that this assert() still
works as-is, then we don't have to do any fd shuffling.  That's
certainly a smaller patch to write.

> But it's close; so I'll use it as the starting point and push the
> corrected version soon.
> 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libguestfs/attachments/20190827/ffaa5bd5/attachment.sig>