qemu-block
[Top][All Lists]
Advanced

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

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


From: Sascha Silbe
Subject: Re: [Qemu-block] [PATCH v3 1/3] qemu-nbd: Add --fork option
Date: Mon, 29 Aug 2016 18:59:22 +0200
User-agent: Notmuch/0.22.1~rc0 (http://notmuchmail.org) Emacs/24.3.1 (x86_64-pc-linux-gnu)

Dear Max,


thanks for taking the time to fix the race condition!


Max Reitz <address@hidden> writes:

> 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.

> @@ -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;

Looking at the surrounding (unchanged) code I see that qemu-nbd already
implemented a daemon mode. It's just that it's completely undocumented
and hinges on both the --device and the --verbose option. Yuck.

It seems there are two things --verbose does (from a user point of
view):

1. Print "NBD device %s is now connected to %s" and keep stderr open.

   Debug messages are always printed to stderr, but in non-verbose
   daemon mode they end up at /dev/null.

   This is more or less what one usually expects from an option named
   --verbose. Except that it only affects daemon mode and messages are
   always printed (but end up at /dev/null).

2. Disable daemon mode.

   I might expect this for an option named --debug, but certainly not
   for --verbose...


A clean way forward would be something like this:

1. Introduce --foreground / --daemon, --quiet

   Default to daemon mode with silent output if --connect is given,
   foreground mode with visible output otherwise. Set non-daemon mode
   with visible output if --verbose is given. Let --foreground /
   --daemon / --quiet any default or implicit value. Document that
   --verbose implicitly enables daemon mode for compatibility with
   previous versions and that future versions may stop doing so
   (i.e. users should use either --verbose --foreground or --verbose
   --daemon).

3. At some point in the future (qemu 3.0?) we can stop having --verbose
   imply --foreground.


I can give it a try if it's out of scope for your current task.


Sascha
-- 
Softwareentwicklung Sascha Silbe, Niederhofenstraße 5/1, 71229 Leonberg
https://se-silbe.de/
USt-IdNr. DE281696641




reply via email to

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