From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:46169) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1boyFn-0006ns-7o for qemu-devel@nongnu.org; Tue, 27 Sep 2016 15:41:24 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1boyFm-0005yU-73 for qemu-devel@nongnu.org; Tue, 27 Sep 2016 15:41:23 -0400 References: <1472142645-7370-1-git-send-email-mreitz@redhat.com> <1472142645-7370-2-git-send-email-mreitz@redhat.com> <20160927090438.GG4090@noname.str.redhat.com> From: Max Reitz Message-ID: <010ed8a5-7545-88d7-fc18-171a5d068e98@redhat.com> Date: Tue, 27 Sep 2016 21:41:10 +0200 MIME-Version: 1.0 In-Reply-To: <20160927090438.GG4090@noname.str.redhat.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="htmBPkwcITveWk3MRkSff4tfUJG0wrnwE" Subject: Re: [Qemu-devel] [PATCH v3 1/3] qemu-nbd: Add --fork option List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Kevin Wolf Cc: qemu-block@nongnu.org, qemu-devel@nongnu.org, Stefan Hajnoczi , Sascha Silbe This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --htmBPkwcITveWk3MRkSff4tfUJG0wrnwE From: Max Reitz To: Kevin Wolf Cc: qemu-block@nongnu.org, qemu-devel@nongnu.org, Stefan Hajnoczi , Sascha Silbe Message-ID: <010ed8a5-7545-88d7-fc18-171a5d068e98@redhat.com> Subject: Re: [PATCH v3 1/3] qemu-nbd: Add --fork option References: <1472142645-7370-1-git-send-email-mreitz@redhat.com> <1472142645-7370-2-git-send-email-mreitz@redhat.com> <20160927090438.GG4090@noname.str.redhat.com> In-Reply-To: <20160927090438.GG4090@noname.str.redhat.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: quoted-printable 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= =2E >> The original process will exit on error of the worker or once the work= er >> enters the main loop. >> >> Suggested-by: Sascha Silbe >> Signed-off-by: Max Reitz >> --- >> 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 >> =20 >> #define MBR_SIZE 512 >> =20 >> @@ -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 =3D false; >> bool writethrough =3D true; >> char *trace_file =3D NULL; >> + bool fork_process =3D false; >> + int old_stderr =3D -1; >> =20 >> /* The client thread uses SIGTERM to interrupt the server. A sig= nal >> * 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 =3D trace_opt_parse(optarg); >> break; >> + case QEMU_NBD_OPT_FORK: >> + fork_process =3D true; >> + break; >> } >> } >> =20 >> @@ -773,7 +780,7 @@ int main(int argc, char **argv) >> return 0; >> } >> =20 >> - 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 =3D qemu_daemon(1, 0); >> =20 >> /* Temporarily redirect stderr to the parent's pipe... *= / >> + old_stderr =3D dup(STDERR_FILENO); >> dup2(stderr_fd[1], STDERR_FILENO); >> if (ret < 0) { >> error_report("Failed to daemonize: %s", strerror(errn= o)); >=20 > I don't have a kernel with NBD support, so I can't test this easily, bu= t > 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. >=20 >> @@ -951,6 +959,11 @@ int main(int argc, char **argv) >> exit(EXIT_FAILURE); >> } >> =20 >> + if (fork_process) { >> + dup2(old_stderr, STDERR_FILENO); >> + close(old_stderr); >> + } >=20 > 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(). >=20 >> state =3D RUNNING; >> do { >> main_loop_wait(false); >=20 > 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 --htmBPkwcITveWk3MRkSff4tfUJG0wrnwE Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- iQEvBAEBCAAZBQJX6stWEhxtcmVpdHpAcmVkaGF0LmNvbQAKCRD0B9sAYdXPQHOS B/9Ar7FYeQnHMHZL5gyZCFO0SF4vFTixBmq4MeEQCo1kc7tWfEcZC8DG9DT7EbBX nenBgDKGhm6nGOtLxfeiOYVVJvV2ZBvHmU1WZlOlhyajdmDovqTMoGieeBJGyFUy sEEVtqTI94D4hNu/Di72AGWa9Jqnt5fZ5OVZ1l5AhqbwmvPzv5925dYCHfG3K6Wz 4W+69ngWrFH3/tQMBUtk5e9I/Gl/T1WgMR/mOQOOpDV/ncuKu1Bcutne7DjyiuFj YgdZv0oH1gAw5WacEQ8iGAB8PVd7Sg5Nvrk8Oqk1ccr8YaeYSzMZs1XAb0Paq7g4 EsOKZ9ONqnTa3x5Sv8oPVRMR =lkXE -----END PGP SIGNATURE----- --htmBPkwcITveWk3MRkSff4tfUJG0wrnwE--