[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: |
Max Reitz |
Subject: |
Re: [Qemu-block] [PATCH v3 1/3] qemu-nbd: Add --fork option |
Date: |
Tue, 27 Sep 2016 21:41:10 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.3.0 |
On 27.09.2016 11:04, Kevin Wolf wrote:
> 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 <address@hidden>
>> Signed-off-by: Max Reitz <address@hidden>
>> ---
>> 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?
Well, for me the parent closes as it should.
>
>> @@ -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).
Hm, so? It never ran before either, because I'm only just now
introducing it. And the fact that I'm keeping the original stderr FD
open has nothing to do with when the parent process will quit because
the parent process is not connected to that *original* stderr.
Also, when using --connect, the FD is closed in nbd_client_thread().
>
>> state = RUNNING;
>> do {
>> main_loop_wait(false);
>
> The documentation needs an update, too.
Right. I wonder why I forgot this. I guess the answer is "Because I
wrote this in some spare time at KVM Forum to see if it would work at
all"...
Max
signature.asc
Description: OpenPGP digital signature