qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Eric Blake <eblake@redhat.com>
To: qemu-devel@nongnu.org
Cc: "Denis V. Lunev" <den@openvz.org>,
	Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>,
	Hanna Reitz <hreitz@redhat.com>,
	qemu-stable@nongnu.org,
	qemu-block@nongnu.org (open list:Network Block Dev...)
Subject: [PULL 02/14] qemu-nbd: fix regression with qemu-nbd --fork run over ssh
Date: Wed, 19 Jul 2023 15:27:39 -0500	[thread overview]
Message-ID: <20230719202736.2675295-18-eblake@redhat.com> (raw)
In-Reply-To: <20230719202736.2675295-16-eblake@redhat.com>

From: "Denis V. Lunev" <den@openvz.org>

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>
Message-ID: <20230717145544.194786-3-den@openvz.org>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
---
 qemu-nbd.c | 13 ++++---------
 1 file changed, 4 insertions(+), 9 deletions(-)

diff --git a/qemu-nbd.c b/qemu-nbd.c
index 77f98c736bb..186ce9474c3 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) {
         fprintf(stderr, "NBD device %s is now connected to %s\n",
                 opts->device, srcpath);
     } else {
@@ -579,7 +580,6 @@ int main(int argc, char **argv)
     bool writethrough = false; /* Client will flush as needed. */
     bool fork_process = false;
     bool list = false;
-    int old_stderr = -1;
     unsigned socket_activation;
     const char *pid_file_name = NULL;
     const char *selinux_label = NULL;
@@ -934,11 +934,6 @@ int main(int argc, char **argv)
         } else if (pid == 0) {
             close(stderr_fd[0]);

-            /* Remember parent's stderr if we will be restoring it. */
-            if (fork_process) {
-                old_stderr = dup(STDERR_FILENO);
-            }
-
             ret = qemu_daemon(1, 0);

             /* Temporarily redirect stderr to the parent's pipe...  */
@@ -1131,6 +1126,7 @@ int main(int argc, char **argv)
         int ret;
         struct NbdClientOpts opts = {
             .device = device,
+            .fork_process = fork_process,
         };

         ret = pthread_create(&client_thread, NULL, nbd_client_thread, &opts);
@@ -1159,8 +1155,7 @@ int main(int argc, char **argv)
     }

     if (fork_process) {
-        dup2(old_stderr, STDERR_FILENO);
-        close(old_stderr);
+        dup2(STDOUT_FILENO, STDERR_FILENO);
     }

     state = RUNNING;
-- 
2.41.0



  parent reply	other threads:[~2023-07-19 20:38 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-07-19 20:27 [PULL 00/14] NBD patches for 2023-07-19 Eric Blake
2023-07-19 20:27 ` [PULL 01/14] qemu-nbd: pass structure into nbd_client_thread instead of plain char* Eric Blake
2023-07-19 20:27 ` Eric Blake [this message]
2023-07-19 20:27 ` [PULL 03/14] qemu-nbd: properly report error if qemu_daemon() is failed Eric Blake
2023-07-19 20:27 ` [PULL 04/14] qemu-nbd: properly report error on error in dup2() after qemu_daemon() Eric Blake
2023-07-19 20:27 ` [PULL 05/14] qemu-nbd: handle dup2() error when qemu-nbd finished setup process Eric Blake
2023-07-19 20:27 ` [PULL 06/14] qemu-nbd: make verbose bool and local variable in main() Eric Blake
2023-07-19 20:27 ` [PULL 07/14] nbd/client: Use smarter assert Eric Blake
2023-07-19 20:27 ` [PULL 08/14] nbd: Consistent typedef usage in header Eric Blake
2023-07-19 20:27 ` [PULL 09/14] nbd/server: Prepare for alternate-size headers Eric Blake
2023-07-19 20:27 ` [PULL 10/14] nbd/server: Refactor to pass full request around Eric Blake
2023-07-19 20:27 ` [PULL 11/14] nbd: s/handle/cookie/ to match NBD spec Eric Blake
2023-07-19 20:27 ` [PULL 12/14] nbd/client: Simplify cookie vs. index computation Eric Blake
2023-07-19 20:27 ` [PULL 13/14] nbd/client: Add safety check on chunk payload length Eric Blake
2023-07-19 20:27 ` [PULL 14/14] nbd: Use enum for various negotiation modes Eric Blake
2023-07-21  9:52 ` [PULL 00/14] NBD patches for 2023-07-19 Peter Maydell

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230719202736.2675295-18-eblake@redhat.com \
    --to=eblake@redhat.com \
    --cc=den@openvz.org \
    --cc=hreitz@redhat.com \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    --cc=qemu-stable@nongnu.org \
    --cc=vsementsov@yandex-team.ru \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).