| Message ID | 20190508211820.17851-2-mreitz@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | iotests: Let 233 run concurrently | expand |
On 5/8/19 4:18 PM, Max Reitz wrote: > --fork is a bit boring if there is no way to get the child's PID. This > option helps. > > Signed-off-by: Max Reitz <mreitz@redhat.com> > --- > qemu-nbd.c | 11 +++++++++++ > qemu-nbd.texi | 2 ++ > 2 files changed, 13 insertions(+) > > diff --git a/qemu-nbd.c b/qemu-nbd.c > index dca9e72cee..edb5195208 100644 > --- a/qemu-nbd.c > +++ b/qemu-nbd.c > @@ -59,6 +59,7 @@ > #define QEMU_NBD_OPT_IMAGE_OPTS 262 > #define QEMU_NBD_OPT_FORK 263 > #define QEMU_NBD_OPT_TLSAUTHZ 264 > +#define QEMU_NBD_OPT_PID_FILE 265 > > #define MBR_SIZE 512 > > @@ -111,6 +112,7 @@ static void usage(const char *name) > " specify tracing options\n" > " --fork fork off the server process and exit the parent\n" > " once the server is running\n" > +" --pid-file=PATH store the server's process ID in the given file\n" Are we guaranteed that the pid file does not appear until after the socket has been created? /me rereads full file... > @@ -876,6 +880,9 @@ int main(int argc, char **argv) > case 'L': > list = true; > break; > + case QEMU_NBD_OPT_PID_FILE: > + pid_file_name = optarg; > + break; > } > } > ... socket_activation = check_socket_activation(); if (socket_activation == 0) { setup_address_and_port(&bindto, &port); } else { /* Using socket activation - check user didn't use -p etc. */ ... > @@ -1196,6 +1203,10 @@ int main(int argc, char **argv) > > nbd_update_server_watch(); > > + if (pid_file_name) { > + qemu_write_pidfile(pid_file_name, &error_fatal); > + } Yes, we are. So, > + > /* now when the initialization is (almost) complete, chdir("/") > * to free any busy filesystems */ > if (chdir("/") < 0) { > diff --git a/qemu-nbd.texi b/qemu-nbd.texi > index de342c76b8..7f55657722 100644 > --- a/qemu-nbd.texi > +++ b/qemu-nbd.texi > @@ -117,6 +117,8 @@ option; or provide the credentials needed for connecting as a client > in list mode. > @item --fork > Fork off the server process and exit the parent once the server is running. > +@item --pid-file=PATH > +Store the server's process ID in the given file. since that is a useful trick to know (the pid file intentionally does NOT appear until after the socket is ready to go), we should mention it in the documentation. I can make that tweak while queueing, if you'd like. Reviewed-by: Eric Blake <eblake@redhat.com>
On 24.05.19 17:07, Eric Blake wrote: > On 5/8/19 4:18 PM, Max Reitz wrote: >> --fork is a bit boring if there is no way to get the child's PID. This >> option helps. >> >> Signed-off-by: Max Reitz <mreitz@redhat.com> >> --- >> qemu-nbd.c | 11 +++++++++++ >> qemu-nbd.texi | 2 ++ >> 2 files changed, 13 insertions(+) >> >> diff --git a/qemu-nbd.c b/qemu-nbd.c >> index dca9e72cee..edb5195208 100644 >> --- a/qemu-nbd.c >> +++ b/qemu-nbd.c >> @@ -59,6 +59,7 @@ >> #define QEMU_NBD_OPT_IMAGE_OPTS 262 >> #define QEMU_NBD_OPT_FORK 263 >> #define QEMU_NBD_OPT_TLSAUTHZ 264 >> +#define QEMU_NBD_OPT_PID_FILE 265 >> >> #define MBR_SIZE 512 >> >> @@ -111,6 +112,7 @@ static void usage(const char *name) >> " specify tracing options\n" >> " --fork fork off the server process and exit the parent\n" >> " once the server is running\n" >> +" --pid-file=PATH store the server's process ID in the given file\n" > > Are we guaranteed that the pid file does not appear until after the > socket has been created? > > /me rereads full file... > >> @@ -876,6 +880,9 @@ int main(int argc, char **argv) >> case 'L': >> list = true; >> break; >> + case QEMU_NBD_OPT_PID_FILE: >> + pid_file_name = optarg; >> + break; >> } >> } >> > ... > socket_activation = check_socket_activation(); > if (socket_activation == 0) { > setup_address_and_port(&bindto, &port); > } else { > /* Using socket activation - check user didn't use -p etc. */ > ... > >> @@ -1196,6 +1203,10 @@ int main(int argc, char **argv) >> >> nbd_update_server_watch(); >> >> + if (pid_file_name) { >> + qemu_write_pidfile(pid_file_name, &error_fatal); >> + } > > Yes, we are. So, > >> + >> /* now when the initialization is (almost) complete, chdir("/") >> * to free any busy filesystems */ >> if (chdir("/") < 0) { >> diff --git a/qemu-nbd.texi b/qemu-nbd.texi >> index de342c76b8..7f55657722 100644 >> --- a/qemu-nbd.texi >> +++ b/qemu-nbd.texi >> @@ -117,6 +117,8 @@ option; or provide the credentials needed for connecting as a client >> in list mode. >> @item --fork >> Fork off the server process and exit the parent once the server is running. >> +@item --pid-file=PATH >> +Store the server's process ID in the given file. > > since that is a useful trick to know (the pid file intentionally does > NOT appear until after the socket is ready to go), we should mention it > in the documentation. > > I can make that tweak while queueing, if you'd like. I don’t mind, but I personally find it more useful to just use --fork and wait for the main process to exit. Max
diff --git a/qemu-nbd.c b/qemu-nbd.c index dca9e72cee..edb5195208 100644 --- a/qemu-nbd.c +++ b/qemu-nbd.c @@ -59,6 +59,7 @@ #define QEMU_NBD_OPT_IMAGE_OPTS 262 #define QEMU_NBD_OPT_FORK 263 #define QEMU_NBD_OPT_TLSAUTHZ 264 +#define QEMU_NBD_OPT_PID_FILE 265 #define MBR_SIZE 512 @@ -111,6 +112,7 @@ static void usage(const char *name) " specify tracing options\n" " --fork fork off the server process and exit the parent\n" " once the server is running\n" +" --pid-file=PATH store the server's process ID in the given file\n" #if HAVE_NBD_DEVICE "\n" "Kernel NBD client support:\n" @@ -651,6 +653,7 @@ int main(int argc, char **argv) { "image-opts", no_argument, NULL, QEMU_NBD_OPT_IMAGE_OPTS }, { "trace", required_argument, NULL, 'T' }, { "fork", no_argument, NULL, QEMU_NBD_OPT_FORK }, + { "pid-file", required_argument, NULL, QEMU_NBD_OPT_PID_FILE }, { NULL, 0, NULL, 0 } }; int ch; @@ -677,6 +680,7 @@ int main(int argc, char **argv) bool list = false; int old_stderr = -1; unsigned socket_activation; + const char *pid_file_name = NULL; /* The client thread uses SIGTERM to interrupt the server. A signal * handler ensures that "qemu-nbd -v -c" exits with a nice status code. @@ -876,6 +880,9 @@ int main(int argc, char **argv) case 'L': list = true; break; + case QEMU_NBD_OPT_PID_FILE: + pid_file_name = optarg; + break; } } @@ -1196,6 +1203,10 @@ int main(int argc, char **argv) nbd_update_server_watch(); + if (pid_file_name) { + qemu_write_pidfile(pid_file_name, &error_fatal); + } + /* now when the initialization is (almost) complete, chdir("/") * to free any busy filesystems */ if (chdir("/") < 0) { diff --git a/qemu-nbd.texi b/qemu-nbd.texi index de342c76b8..7f55657722 100644 --- a/qemu-nbd.texi +++ b/qemu-nbd.texi @@ -117,6 +117,8 @@ option; or provide the credentials needed for connecting as a client in list mode. @item --fork Fork off the server process and exit the parent once the server is running. +@item --pid-file=PATH +Store the server's process ID in the given file. @item --tls-authz=ID Specify the ID of a qauthz object previously created with the --object option. This will be used to authorize connecting users
--fork is a bit boring if there is no way to get the child's PID. This option helps. Signed-off-by: Max Reitz <mreitz@redhat.com> --- qemu-nbd.c | 11 +++++++++++ qemu-nbd.texi | 2 ++ 2 files changed, 13 insertions(+)