[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
eblake at redhat.com
Tue Aug 27 13:31:26 UTC 2019
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>
More information about the Libguestfs
mailing list