All of lore.kernel.org
 help / color / mirror / Atom feed
From: Kevin Wolf <kwolf@redhat.com>
To: qemu-block@nongnu.org
Cc: kwolf@redhat.com, peter.maydell@linaro.org, qemu-devel@nongnu.org
Subject: [PULL 11/50] qsd: Add pre-init argument parsing pass
Date: Fri,  4 Mar 2022 17:46:32 +0100	[thread overview]
Message-ID: <20220304164711.474713-12-kwolf@redhat.com> (raw)
In-Reply-To: <20220304164711.474713-1-kwolf@redhat.com>

From: Hanna Reitz <hreitz@redhat.com>

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

- --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>
Message-Id: <20220303164814.284974-3-hreitz@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 storage-daemon/qemu-storage-daemon.c | 43 ++++++++++++++++++++++++----
 1 file changed, 38 insertions(+), 5 deletions(-)

diff --git a/storage-daemon/qemu-storage-daemon.c b/storage-daemon/qemu-storage-daemon.c
index 504d33aa91..b798954edb 100644
--- a/storage-daemon/qemu-storage-daemon.c
+++ b/storage-daemon/qemu-storage-daemon.c
@@ -177,7 +177,23 @@ static int getopt_set_loc(int argc, char **argv, const char *optstring,
     return c;
 }
 
-static void process_options(int argc, char *argv[])
+/**
+ * Process QSD command-line arguments.
+ *
+ * This is done in two passes:
+ *
+ * First (@pre_init_pass is true), we do a pass where all global
+ * arguments pertaining to the QSD process (like --help or --daemonize)
+ * are processed.  This pass is done before most of the QEMU-specific
+ * initialization steps (e.g. initializing the block layer or QMP), and
+ * so must only process arguments that are not really QEMU-specific.
+ *
+ * Second (@pre_init_pass is false), we (sequentially) process all
+ * QEMU/QSD-specific arguments.  Many of these arguments are effectively
+ * translated to QMP commands (like --blockdev for blockdev-add, or
+ * --export for block-export-add).
+ */
+static void process_options(int argc, char *argv[], bool pre_init_pass)
 {
     int c;
 
@@ -196,11 +212,26 @@ static void process_options(int argc, char *argv[])
     };
 
     /*
-     * In contrast to the system emulator, options are processed in the order
-     * they are given on the command lines. This means that things must be
-     * defined first before they can be referenced in another option.
+     * In contrast to the system emulator, QEMU-specific options are processed
+     * in the order they are given on the command lines. This means that things
+     * must be defined first before they can be referenced in another option.
      */
+    optind = 1;
     while ((c = getopt_set_loc(argc, argv, "-hT:V", long_options)) != -1) {
+        bool handle_option_pre_init;
+
+        /* Should this argument be processed in the pre-init pass? */
+        handle_option_pre_init =
+            c == '?' ||
+            c == 'h' ||
+            c == 'V' ||
+            c == OPTION_PIDFILE;
+
+        /* Process every option only in its respective pass */
+        if (pre_init_pass != handle_option_pre_init) {
+            continue;
+        }
+
         switch (c) {
         case '?':
             exit(EXIT_FAILURE);
@@ -334,6 +365,8 @@ int main(int argc, char *argv[])
     qemu_init_exec_dir(argv[0]);
     os_setup_signal_handling();
 
+    process_options(argc, argv, true);
+
     module_call_init(MODULE_INIT_QOM);
     module_call_init(MODULE_INIT_TRACE);
     qemu_add_opts(&qemu_trace_opts);
@@ -348,7 +381,7 @@ int main(int argc, char *argv[])
     qemu_set_log(LOG_TRACE);
 
     qemu_init_main_loop(&error_fatal);
-    process_options(argc, argv);
+    process_options(argc, argv, false);
 
     /*
      * Write the pid file after creating chardevs, exports, and NBD servers but
-- 
2.35.1



  parent reply	other threads:[~2022-03-04 16:57 UTC|newest]

Thread overview: 58+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-03-04 16:46 [PULL 00/50] Block layer patches Kevin Wolf
2022-03-04 16:46 ` [PULL 01/50] crypto: perform permission checks under BQL Kevin Wolf
2022-03-04 16:46 ` [PULL 02/50] crypto: distinguish between main loop and I/O in block_crypto_amend_options_generic_luks Kevin Wolf
2022-03-04 16:46 ` [PULL 03/50] block: introduce bdrv_activate Kevin Wolf
2022-03-04 16:46 ` [PULL 04/50] block: rename bdrv_invalidate_cache_all, blk_invalidate_cache and test_sync_op_invalidate_cache Kevin Wolf
2022-03-04 16:46 ` [PULL 05/50] block: move BQL logic of bdrv_co_invalidate_cache in bdrv_activate Kevin Wolf
2022-03-04 16:46 ` [PULL 06/50] tls: add macros for coroutine-safe TLS variables Kevin Wolf
2022-03-04 16:46 ` [PULL 07/50] util/async: replace __thread with QEMU TLS macros Kevin Wolf
2022-03-04 16:46 ` [PULL 08/50] rcu: use coroutine " Kevin Wolf
2022-03-04 16:46 ` [PULL 09/50] cpus: use coroutine TLS macros for iothread_locked Kevin Wolf
2022-03-04 16:46 ` [PULL 10/50] os-posix: Add os_set_daemonize() Kevin Wolf
2022-03-04 16:46 ` Kevin Wolf [this message]
2022-03-04 16:46 ` [PULL 12/50] qsd: Add --daemonize Kevin Wolf
2022-03-04 16:46 ` [PULL 13/50] iotests/185: Add post-READY quit tests Kevin Wolf
2022-03-04 16:46 ` [PULL 14/50] main-loop.h: introduce qemu_in_main_thread() Kevin Wolf
2022-03-04 16:46 ` [PULL 15/50] main loop: macros to mark GS and I/O functions Kevin Wolf
2022-03-04 16:46 ` [PULL 16/50] include/block/block: split header into I/O and global state API Kevin Wolf
2022-03-04 16:46 ` [PULL 17/50] assertions for block " Kevin Wolf
2022-03-04 16:46 ` [PULL 18/50] IO_CODE and IO_OR_GS_CODE for block I/O API Kevin Wolf
2022-03-04 16:46 ` [PULL 19/50] block/export/fuse.c: allow writable exports to take RESIZE permission Kevin Wolf
2022-03-04 16:46 ` [PULL 20/50] include/sysemu/block-backend: split header into I/O and global state (GS) API Kevin Wolf
2022-03-04 16:46 ` [PULL 21/50] block/block-backend.c: assertions for block-backend Kevin Wolf
2022-03-16 12:44   ` Philippe Mathieu-Daudé
2022-03-16 12:53     ` Philippe Mathieu-Daudé
2022-03-16 14:46       ` Emanuele Giuseppe Esposito
2022-03-16 15:25         ` Philippe Mathieu-Daudé
2022-03-16 16:02           ` Kevin Wolf
2022-03-16 12:54     ` Emanuele Giuseppe Esposito
2022-03-04 16:46 ` [PULL 22/50] IO_CODE and IO_OR_GS_CODE for block-backend I/O API Kevin Wolf
2022-03-04 16:46 ` [PULL 23/50] block.c: assertions to the block layer permissions API Kevin Wolf
2022-03-04 16:46 ` [PULL 24/50] include/block/block_int: split header into I/O and global state API Kevin Wolf
2022-03-04 16:46 ` [PULL 25/50] assertions for block_int " Kevin Wolf
2022-03-04 16:46 ` [PULL 26/50] IO_CODE and IO_OR_GS_CODE for block_int I/O API Kevin Wolf
2022-03-04 16:46 ` [PULL 27/50] block: introduce assert_bdrv_graph_writable Kevin Wolf
2022-03-04 16:46 ` [PULL 28/50] include/block/blockjob_int.h: split header into I/O and GS API Kevin Wolf
2022-03-04 16:46 ` [PULL 29/50] GS and IO CODE macros for blockjob_int.h Kevin Wolf
2022-03-04 16:46 ` [PULL 30/50] block.c: add assertions to static functions Kevin Wolf
2022-03-04 16:46 ` [PULL 31/50] include/block/blockjob.h: global state API Kevin Wolf
2022-03-04 16:46 ` [PULL 32/50] assertions for blockjob.h " Kevin Wolf
2022-03-04 16:46 ` [PULL 33/50] include/sysemu/blockdev.h: " Kevin Wolf
2022-03-04 16:46 ` [PULL 34/50] assertions for blockdev.h " Kevin Wolf
2022-03-04 16:46 ` [PULL 35/50] include/block/snapshot: global state API + assertions Kevin Wolf
2022-03-04 16:46 ` [PULL 36/50] block/copy-before-write.h: " Kevin Wolf
2022-03-04 16:46 ` [PULL 37/50] block/coroutines: I/O and "I/O or GS" API Kevin Wolf
2022-03-04 16:46 ` [PULL 38/50] block_int-common.h: split function pointers in BlockDriver Kevin Wolf
2022-03-04 16:47 ` [PULL 39/50] block_int-common.h: assertions in the callers of BlockDriver function pointers Kevin Wolf
2022-03-04 16:47 ` [PULL 40/50] block_int-common.h: split function pointers in BdrvChildClass Kevin Wolf
2022-03-04 16:47 ` [PULL 41/50] block_int-common.h: assertions in the callers of BdrvChildClass function pointers Kevin Wolf
2022-03-04 16:47 ` [PULL 42/50] block-backend-common.h: split function pointers in BlockDevOps Kevin Wolf
2022-03-04 16:47 ` [PULL 43/50] job.h: split function pointers in JobDriver Kevin Wolf
2022-03-04 16:47 ` [PULL 44/50] job.h: assertions in the callers of JobDriver function pointers Kevin Wolf
2022-03-04 16:47 ` [PULL 45/50] block: Make bdrv_refresh_limits() non-recursive Kevin Wolf
2022-03-04 16:47 ` [PULL 46/50] iotests: Allow using QMP with the QSD Kevin Wolf
2022-03-04 16:47 ` [PULL 47/50] iotests/graph-changes-while-io: New test Kevin Wolf
2022-03-04 16:47 ` [PULL 48/50] tests/qemu-iotests: Rework the checks and spots using GNU sed Kevin Wolf
2022-03-04 16:47 ` [PULL 49/50] block/amend: Always call .bdrv_amend_clean() Kevin Wolf
2022-03-04 16:47 ` [PULL 50/50] block/amend: Keep strong reference to BDS Kevin Wolf
2022-03-05 14:43 ` [PULL 00/50] Block layer patches 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=20220304164711.474713-12-kwolf@redhat.com \
    --to=kwolf@redhat.com \
    --cc=peter.maydell@linaro.org \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.