Kevin Wolf <kwolf@redhat.com> writes:
Am 19.01.2022 um 14:44 hat Hanna Reitz geschrieben:
On 19.01.22 13:58, Markus Armbruster wrote:
Hanna Reitz <hreitz@redhat.com> writes:
We want to add a --daemonize argument to QSD's command line.
Why?
OK, s/we/I/. I find it useful, because without such an option, I need to
have whoever invokes QSD loop until the PID file exists, before I can be
sure that all exports are set up. I make use of it in the test cases added
in patch 3.
I suppose this could be worked around with a special character device, like
so:
```
ncat --listen -U /tmp/qsd-done.sock </dev/null &
ncat_pid=$!
qemu-storage-daemon \
... \
--chardev socket,id=signal_done,path=/tmp/qsd-done.sock \
--monitor signal_done \
--pidfile /tmp/qsd.pid &
wait $ncat_pid
```
But having to use an extra tool for this is unergonomic. I mean, if there’s
no other way...
I know duplicating this into every program that could server as a daemon
is the Unix tradition. Doesn't make it good. Systemd[*] has tried to
make it superfluous.
The other point is that the system emulator has it, qemu-nbd has it,
so certainly qsd should have it as well. Not the least because it should
be able to replace qemu-nbd (at least for the purpose of exporting NBD.
not necessarily for attaching it to the host).
Point taken, but I think it's a somewhat weak one. qsd could certainly
replace qemu-nbd even without --daemonize; we could use other means to
run it in the background.
This will
require forking the process before we do any complex initialization
steps, like setting up the block layer or QMP. Therefore, we must scan
the command line for it long before our current process_options() call.
Can you explain in a bit more detail why early forking is required?
I have a strong dislike for parsing more than once...
Because I don’t want to set up QMP and block devices, and then fork the
process into two. That sounds like there’d be a lot of stuff to think
about, which just isn’t necessary, because we don’t need to set up any
of this in the parent.
We must fork() before we create threads. Other resources are easy
enough to hand over to the child. Still, having to think about less is
good, I readily grant you that.
The trouble is that forking early creates a new problem: any
configuration errors detected in the child must be propagated to the
parent somehow (output and exit status). I peeked at your PATCH 2, and
I'm not convinced, but that's detail here.
Here we can compare again: Both the system emulator and qemu-nbd behave
the same, they fork before they do anything interesting.
The difference is that they still parse the command line only once
because they don't immediately create things, but just store the options
and later process them in their own magic order. I'd much rather parse
the command line twice than copy that behaviour.
The part I hate is "own magic order". Without that, multiple passes are
just fine with me.
Parsing twice is a bit like having a two pass compiler run the first
pass left to right, and then both passes intertwined left to right. The
pedestrian way to do it is running the first pass left to right, then
the second pass left to right.
We're clearly talking taste here.
Kevin
For example, if I set up a monitor on a Unix socket (server=true),
processing is delayed until the client connects. Say I put --daemonize
afterwards. I connect to the waiting server socket, the child is forked
off, and then... I’m not sure what happens, actually. Do I have a
connection with both the parent and the child listening? I know that in
practice, what happens is that once the parent exits, the connection is
closed, and I get a “qemu: qemu_thread_join: Invalid argument” warning/error
on the QSD side.
There’s a lot of stuff to think about if you allow forking after other
options, so it should be done first. We could just require the user to put
--daemonize before all other options, and so have a single pass; but still,
before options are even parsed, we have already for example called
bdrv_init(), init_qmp_commands(), qemu_init_main_loop(). These are all
things that the parent of a daemonizing process doesn’t need to do, and
where I’d simply rather not think about what impact it has if we fork
afterwards.
Hanna
Care to put a brief version of the rationale for --daemonize and for
forking early in the commit message?