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

Attachment: signature.asc
Description: OpenPGP digital signature

_______________________________________________
Libguestfs mailing list
[email protected]
https://www.redhat.com/mailman/listinfo/libguestfs

Reply via email to