Re: [PATCH v2 2/4] qsd: Add pre-init argument parsing pass

[Eric Blake]

On Thu, Mar 03, 2022 at 05:48:12PM +0100, Hanna Reitz wrote:
> In contrast to qemu-nbd (where it is called --fork) and the system
> emulator, QSD does not have a --daemonize switch yet.  Just like them,
> QSD allows setting up block devices and exports on the command line.
> When doing so, it is often necessary for whoever invoked the QSD to wait
> until these exports are fully set up.  A --daemonize switch allows
> precisely this, by virtue of the parent process exiting once everything
> is set up.
> 
> Note that there are alternative ways of waiting for all exports to be
> set up, for example:
> - Passing the --pidfile option and waiting until the respective file
>   exists (but I do not know if there is a way of implementing this
>   without a busy wait loop)

Non-portably, you could use inotify or similar, to get a true
event-driven wakeup when the file is created.  And here's the python
glue that libnbd uses, instead of --pidfile:

https://gitlab.com/nbdkit/libnbd/-/blob/master/interop/interop-qemu-storage-daemon.sh#L58

> - Set up some network server (e.g. on a Unix socket) and have the QSD
>   connect to it after all arguments have been processed by appending
>   corresponding --chardev and --monitor options to the command line,
>   and then wait until the QSD connects
> 
> Having a --daemonize option would make this simpler, though, without
> having to rely on additional tools (to set up a network server) or busy
> waiting.
> 
> Implementing a --daemonize switch means having to fork the QSD process.
> Ideally, we should do this as early as possible: All the parent process
> has to do is to wait for the child process to signal completion of its
> set-up phase, and therefore there is basically no initialization that
> needs to be done before the fork.  On the other hand, forking after
> initialization steps means having to consider how those steps (like
> setting up the block layer or QMP) interact with a later fork, which is
> often not trivial.
> 
> In order to fork this early, we must scan the command line for
> --daemonize long before our current process_options() call.  Instead of
> adding custom new code to do so, just reuse process_options() and give
> it a @pre_init_pass argument to distinguish the two passes.  I believe
> there are some other switches but --daemonize that deserve parsing in

s/but/beyond/

> the first pass:
> 
> - --help and --version are supposed to only print some text and then
>   immediately exit (so any initialization we do would be for naught).
>   This changes behavior, because now "--blockdev inv-drv --help" will
>   print a help text instead of complaining about the --blockdev
>   argument.
>   Note that this is similar in behavior to other tools, though: "--help"
>   is generally immediately acted upon when finding it in the argument
>   list, potentially before other arguments (even ones before it) are
>   acted on.  For example, "ls /does-not-exist --help" prints a help text
>   and does not complain about ENOENT.

Well, GNU ls does that, but only if POSIXLY_CORRECT is not set (a
strict POSIX ls must give you two ENOENT).

> 
> - --pidfile does not need initialization, and is already exempted from
>   the sequential order that process_options() claims to strictly follow
>   (the PID file is only created after all arguments are processed, not
>   at the time the --pidfile argument appears), so it makes sense to
>   include it in the same category as --daemonize.
> 
> - Invalid arguments should always be reported as soon as possible.  (The
>   same caveat with --help applies: That means that "--blockdev inv-drv
>   --inv-arg" will now complain about --inv-arg, not inv-drv.)
> 
> This patch does make some references to --daemonize without having
> implemented it yet, but that will happen in the next patch.
> 
> Signed-off-by: Hanna Reitz <hreitz@redhat.com>
> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
>  storage-daemon/qemu-storage-daemon.c | 43 ++++++++++++++++++++++++----
>  1 file changed, 38 insertions(+), 5 deletions(-)
>

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

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org