qemu-stable
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH 2/5] qemu-nbd: fix regression with qemu-nbd --fork run over s


From: Denis V. Lunev
Subject: Re: [PATCH 2/5] qemu-nbd: fix regression with qemu-nbd --fork run over ssh
Date: Mon, 17 Jul 2023 22:26:41 +0200
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.13.0

On 7/17/23 21:04, Eric Blake wrote:
On Mon, Jul 17, 2023 at 04:55:41PM +0200, Denis V. Lunev wrote:
Commit e6df58a5578fee7a50bbf36f4a50a2781cff855d
     Author: Hanna Reitz <hreitz@redhat.com>
     Date:   Wed May 8 23:18:18 2019 +0200
     qemu-nbd: Do not close stderr
has introduced an interesting regression. Original behavior of
     ssh somehost qemu-nbd /home/den/tmp/file -f raw --fork
was the following:
  * qemu-nbd was started as a daemon
  * the command execution is done and ssh exited with success

The patch has changed this behavior and 'ssh' command now hangs forever.

According to the normal specification of the daemon() call, we should
endup with STDERR pointing to /dev/null. That should be done at the
very end of the successful startup sequence when the pipe to the
bootstrap process (used for diagnostics) is no longer needed.

This could be achived in the same way as done for 'qemu-nbd -c' case.
That was commit 0eaf453e, also fixing up e6df58a5. STDOUT copying to
STDERR does the trick.

This also leads to proper 'ssh' connection closing which fixes my
original problem.

Signed-off-by: Denis V. Lunev <den@openvz.org>
CC: Eric Blake <eblake@redhat.com>
CC: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
CC: Hanna Reitz <hreitz@redhat.com>
CC: <qemu-stable@nongnu.org>
---
  qemu-nbd.c | 13 ++++---------
  1 file changed, 4 insertions(+), 9 deletions(-)

diff --git a/qemu-nbd.c b/qemu-nbd.c
index 77f98c736b..186ce9474c 100644
--- a/qemu-nbd.c
+++ b/qemu-nbd.c
@@ -274,6 +274,7 @@ static void *show_parts(void *arg)
struct NbdClientOpts {
      char *device;
+    bool fork_process;
  };
static void *nbd_client_thread(void *arg)
@@ -317,7 +318,7 @@ static void *nbd_client_thread(void *arg)
      /* update partition table */
      pthread_create(&show_parts_thread, NULL, show_parts, opts->device);
- if (verbose) {
+    if (verbose && !opts->fork_process) {
It seems a bit odd to use the global 'fork' but the local
'opts->fork_process' in the same conditional.  Perhaps patch 1/5
should be modified to also pass verbose through opts?

Reviewed-by: Eric Blake <eblake@redhat.com>

sent to the thread as [PATCH 6/5] for convenience

Den



reply via email to

[Prev in Thread] Current Thread [Next in Thread]