[libvirt PATCH] tests: don't mix FILE* and UNIX FD I/O on same stream
There is currently a hand in test27 that exhibits itself on FreeBSD 11.4 only. The behaviour is that virCommandProcessIO gets POLLIN on the FD for stdout, but read() blocks. Meanwhile commandtest also blocks in write for stderr because the pipe buffers are full. This fix in commandhelper likely does not really address the root cause just hides it due to the buffering done by FILE *. Mixing UNIX FD I/O and FILE * I/O is bad practice regardles. Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- tests/commandhelper.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/commandhelper.c b/tests/commandhelper.c index 05f577730f..7c260c4e13 100644 --- a/tests/commandhelper.c +++ b/tests/commandhelper.c @@ -221,9 +221,9 @@ int main(int argc, char **argv) { } for (i = 0; i < numpollfds; i++) { - if (write(STDOUT_FILENO, buffers[i], buflen[i]) != buflen[i]) + if (fwrite(buffers[i], 1, buflen[i], stdout) != buflen[i]) goto cleanup; - if (write(STDERR_FILENO, buffers[i], buflen[i]) != buflen[i]) + if (fwrite(buffers[i], 1, buflen[i], stderr) != buflen[i]) goto cleanup; } -- 2.26.2
On 9/21/20 12:36 PM, Daniel P. Berrangé wrote:
There is currently a hand in test27 that exhibits itself on FreeBSD 11.4
s/hand/hang/
only. The behaviour is that virCommandProcessIO gets POLLIN on the FD for stdout, but read() blocks. Meanwhile commandtest also blocks in write for stderr because the pipe buffers are full.
This fix in commandhelper likely does not really address the root cause just hides it due to the buffering done by FILE *. Mixing UNIX FD I/O and FILE * I/O is bad practice regardles.
regardless POSIX has rules for when it is safe (it has a notion of an active handle, and what must be done to a FILE* that is currently the active handle before doing further I/O via an fd that wants to become the active handle https://pubs.opengroup.org/onlinepubs/9699919799/functions/V2_chap02.html#ta...). But you're right that not mixing is the easiest approach.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- tests/commandhelper.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
With typos fixed, Reviewed-by: Eric Blake <eblake@redhat.com> -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
participants (2)
-
Daniel P. Berrangé -
Eric Blake