| Message ID | 20190507183610.9848-4-mreitz@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | iotests: Let 233 run concurrently | expand |
On 5/7/19 1:36 PM, Max Reitz wrote: > We kept old_stderr specifically so we could keep emitting error message > on stderr. However, qemu_daemon() closes stderr. Therefore, we need to > dup() stderr to old_stderr before invoking qemu_daemon(). > > Signed-off-by: Max Reitz <mreitz@redhat.com> > --- > As I hinted at in the cover letter, I am not sure whether this is truly > a bug or whether the current behavior is intentional. So if you > disagree with me on this patch, you are welcome to suggest an > alternative. Looks to me like a bug fix, for a problem present since commit c1f8fdc3 added old_stderr in 2011. > > I personally can see two: > (1) Add a --log option for a file to store the server's messages in. > Seems a bit cumbersome to me. > > (2) Add a --keep-stderr option, which specifically enables this behavior > here. Without this option we keep the old behavior. The approach done here is simplest, I wouldn't worry about your two alternatives. Reviewed-by: Eric Blake <eblake@redhat.com>
diff --git a/qemu-nbd.c b/qemu-nbd.c index 7e48154f44..3805324153 100644 --- a/qemu-nbd.c +++ b/qemu-nbd.c @@ -1015,10 +1015,11 @@ int main(int argc, char **argv) exit(EXIT_FAILURE); } else if (pid == 0) { close(stderr_fd[0]); + + old_stderr = dup(STDERR_FILENO); ret = qemu_daemon(1, 0); /* Temporarily redirect stderr to the parent's pipe... */ - old_stderr = dup(STDERR_FILENO); dup2(stderr_fd[1], STDERR_FILENO); if (ret < 0) { error_report("Failed to daemonize: %s", strerror(errno));
We kept old_stderr specifically so we could keep emitting error message on stderr. However, qemu_daemon() closes stderr. Therefore, we need to dup() stderr to old_stderr before invoking qemu_daemon(). Signed-off-by: Max Reitz <mreitz@redhat.com> --- As I hinted at in the cover letter, I am not sure whether this is truly a bug or whether the current behavior is intentional. So if you disagree with me on this patch, you are welcome to suggest an alternative. I personally can see two: (1) Add a --log option for a file to store the server's messages in. Seems a bit cumbersome to me. (2) Add a --keep-stderr option, which specifically enables this behavior here. Without this option we keep the old behavior. --- qemu-nbd.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)