Re: [Qemu-devel] [PATCH v3 1/3] qemu-nbd: Add --fork option

[Kevin Wolf <kwolf@redhat.com>]

Am 25.08.2016 um 18:30 hat Max Reitz geschrieben:
> Using the --fork option, one can make qemu-nbd fork the worker process.
> The original process will exit on error of the worker or once the worker
> enters the main loop.
> 
> Suggested-by: Sascha Silbe <silbe@linux.vnet.ibm.com>
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
>  qemu-nbd.c | 15 ++++++++++++++-
>  1 file changed, 14 insertions(+), 1 deletion(-)
> 
> diff --git a/qemu-nbd.c b/qemu-nbd.c
> index e3571c2..8c2d582 100644
> --- a/qemu-nbd.c
> +++ b/qemu-nbd.c
> @@ -48,6 +48,7 @@
>  #define QEMU_NBD_OPT_OBJECT        260
>  #define QEMU_NBD_OPT_TLSCREDS      261
>  #define QEMU_NBD_OPT_IMAGE_OPTS    262
> +#define QEMU_NBD_OPT_FORK          263
>  
>  #define MBR_SIZE 512
>  
> @@ -503,6 +504,7 @@ int main(int argc, char **argv)
>          { "tls-creds", required_argument, NULL, QEMU_NBD_OPT_TLSCREDS },
>          { "image-opts", no_argument, NULL, QEMU_NBD_OPT_IMAGE_OPTS },
>          { "trace", required_argument, NULL, 'T' },
> +        { "fork", no_argument, NULL, QEMU_NBD_OPT_FORK },
>          { NULL, 0, NULL, 0 }
>      };
>      int ch;
> @@ -524,6 +526,8 @@ int main(int argc, char **argv)
>      bool imageOpts = false;
>      bool writethrough = true;
>      char *trace_file = NULL;
> +    bool fork_process = false;
> +    int old_stderr = -1;
>  
>      /* The client thread uses SIGTERM to interrupt the server.  A signal
>       * handler ensures that "qemu-nbd -v -c" exits with a nice status code.
> @@ -714,6 +718,9 @@ int main(int argc, char **argv)
>              g_free(trace_file);
>              trace_file = trace_opt_parse(optarg);
>              break;
> +        case QEMU_NBD_OPT_FORK:
> +            fork_process = true;
> +            break;
>          }
>      }
>  
> @@ -773,7 +780,7 @@ int main(int argc, char **argv)
>          return 0;
>      }
>  
> -    if (device && !verbose) {
> +    if ((device && !verbose) || fork_process) {
>          int stderr_fd[2];
>          pid_t pid;
>          int ret;
> @@ -796,6 +803,7 @@ int main(int argc, char **argv)
>              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));

I don't have a kernel with NBD support, so I can't test this easily, but
doesn't this break the daemon mode for --connect? I mean, it will still
fork, but won't the parent be alive until the child exits?

> @@ -951,6 +959,11 @@ int main(int argc, char **argv)
>          exit(EXIT_FAILURE);
>      }
>  
> +    if (fork_process) {
> +        dup2(old_stderr, STDERR_FILENO);
> +        close(old_stderr);
> +    }

Because this code doesn't run for --connect (unless --fork is given,
too).

>      state = RUNNING;
>      do {
>          main_loop_wait(false);

The documentation needs an update, too.

Kevin