| Message ID | 20200930121105.667049-2-eblake@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | Exposing backing-chain allocation over NBD | expand |
30.09.2020 15:11, Eric Blake wrote: > Honoring just SIGTERM on Linux is too weak; we also want to handle > other common signals, and do so even on BSD. Why? Because at least > 'qemu-nbd -B bitmap' needs a chance to clean up the in-use bit on > bitmaps when the server is shut down via a signal. Probably not bad to update a comment [*] if you have a good wording in mind. > > See also: http://bugzilla.redhat.com/1883608 > > Signed-off-by: Eric Blake <eblake@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> > --- > qemu-nbd.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/qemu-nbd.c b/qemu-nbd.c > index bacb69b0898b..e7520261134f 100644 > --- a/qemu-nbd.c > +++ b/qemu-nbd.c > @@ -581,7 +581,7 @@ int main(int argc, char **argv) > const char *pid_file_name = NULL; > BlockExportOptions *export_opts; > > -#if HAVE_NBD_DEVICE > +#ifdef CONFIG_POSIX > /* The client thread uses SIGTERM to interrupt the server. A signal > * handler ensures that "qemu-nbd -v -c" exits with a nice status code. [*] > */ > @@ -589,9 +589,9 @@ int main(int argc, char **argv) > memset(&sa_sigterm, 0, sizeof(sa_sigterm)); > sa_sigterm.sa_handler = termsig_handler; > sigaction(SIGTERM, &sa_sigterm, NULL); > -#endif /* HAVE_NBD_DEVICE */ > + sigaction(SIGINT, &sa_sigterm, NULL); > + sigaction(SIGHUP, &sa_sigterm, NULL); > > -#ifdef CONFIG_POSIX > signal(SIGPIPE, SIG_IGN); > #endif >
On 10/7/20 5:32 AM, Vladimir Sementsov-Ogievskiy wrote: > 30.09.2020 15:11, Eric Blake wrote: >> Honoring just SIGTERM on Linux is too weak; we also want to handle >> other common signals, and do so even on BSD. Why? Because at least >> 'qemu-nbd -B bitmap' needs a chance to clean up the in-use bit on >> bitmaps when the server is shut down via a signal. > > Probably not bad to update a comment [*] if you have a good wording in > mind. > >> >> See also: http://bugzilla.redhat.com/1883608 >> >> Signed-off-by: Eric Blake <eblake@redhat.com> > > Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> > >> --- >> qemu-nbd.c | 6 +++--- >> 1 file changed, 3 insertions(+), 3 deletions(-) >> >> diff --git a/qemu-nbd.c b/qemu-nbd.c >> index bacb69b0898b..e7520261134f 100644 >> --- a/qemu-nbd.c >> +++ b/qemu-nbd.c >> @@ -581,7 +581,7 @@ int main(int argc, char **argv) >> const char *pid_file_name = NULL; >> BlockExportOptions *export_opts; >> >> -#if HAVE_NBD_DEVICE >> +#ifdef CONFIG_POSIX >> /* The client thread uses SIGTERM to interrupt the server. A >> signal >> * handler ensures that "qemu-nbd -v -c" exits with a nice >> status code. > > [*] > Sure, I went with: diff --git i/qemu-nbd.c w/qemu-nbd.c index e7520261134f..c731dda04ec0 100644 --- i/qemu-nbd.c +++ w/qemu-nbd.c @@ -582,8 +582,9 @@ int main(int argc, char **argv) BlockExportOptions *export_opts; #ifdef CONFIG_POSIX - /* The client thread uses SIGTERM to interrupt the server. A signal - * handler ensures that "qemu-nbd -v -c" exits with a nice status code. + /* + * Exit gracefully on various signals, which includes SIGTERM used + * by 'qemu-nbd -v -c'. */ struct sigaction sa_sigterm; memset(&sa_sigterm, 0, sizeof(sa_sigterm));
08.10.2020 00:13, Eric Blake wrote: > On 10/7/20 5:32 AM, Vladimir Sementsov-Ogievskiy wrote: >> 30.09.2020 15:11, Eric Blake wrote: >>> Honoring just SIGTERM on Linux is too weak; we also want to handle >>> other common signals, and do so even on BSD. Why? Because at least >>> 'qemu-nbd -B bitmap' needs a chance to clean up the in-use bit on >>> bitmaps when the server is shut down via a signal. >> >> Probably not bad to update a comment [*] if you have a good wording in >> mind. >> >>> >>> See also: http://bugzilla.redhat.com/1883608 >>> >>> Signed-off-by: Eric Blake <eblake@redhat.com> >> >> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> >> >>> --- >>> qemu-nbd.c | 6 +++--- >>> 1 file changed, 3 insertions(+), 3 deletions(-) >>> >>> diff --git a/qemu-nbd.c b/qemu-nbd.c >>> index bacb69b0898b..e7520261134f 100644 >>> --- a/qemu-nbd.c >>> +++ b/qemu-nbd.c >>> @@ -581,7 +581,7 @@ int main(int argc, char **argv) >>> const char *pid_file_name = NULL; >>> BlockExportOptions *export_opts; >>> >>> -#if HAVE_NBD_DEVICE >>> +#ifdef CONFIG_POSIX >>> /* The client thread uses SIGTERM to interrupt the server. A >>> signal >>> * handler ensures that "qemu-nbd -v -c" exits with a nice >>> status code. >> >> [*] >> > > Sure, I went with: > > diff --git i/qemu-nbd.c w/qemu-nbd.c > index e7520261134f..c731dda04ec0 100644 > --- i/qemu-nbd.c > +++ w/qemu-nbd.c > @@ -582,8 +582,9 @@ int main(int argc, char **argv) > BlockExportOptions *export_opts; > > #ifdef CONFIG_POSIX > - /* The client thread uses SIGTERM to interrupt the server. A signal > - * handler ensures that "qemu-nbd -v -c" exits with a nice status code. > + /* > + * Exit gracefully on various signals, which includes SIGTERM used > + * by 'qemu-nbd -v -c'. > */ > struct sigaction sa_sigterm; > memset(&sa_sigterm, 0, sizeof(sa_sigterm)); > Thanks!
diff --git a/qemu-nbd.c b/qemu-nbd.c index bacb69b0898b..e7520261134f 100644 --- a/qemu-nbd.c +++ b/qemu-nbd.c @@ -581,7 +581,7 @@ int main(int argc, char **argv) const char *pid_file_name = NULL; BlockExportOptions *export_opts; -#if HAVE_NBD_DEVICE +#ifdef CONFIG_POSIX /* The client thread uses SIGTERM to interrupt the server. A signal * handler ensures that "qemu-nbd -v -c" exits with a nice status code. */ @@ -589,9 +589,9 @@ int main(int argc, char **argv) memset(&sa_sigterm, 0, sizeof(sa_sigterm)); sa_sigterm.sa_handler = termsig_handler; sigaction(SIGTERM, &sa_sigterm, NULL); -#endif /* HAVE_NBD_DEVICE */ + sigaction(SIGINT, &sa_sigterm, NULL); + sigaction(SIGHUP, &sa_sigterm, NULL); -#ifdef CONFIG_POSIX signal(SIGPIPE, SIG_IGN); #endif
Honoring just SIGTERM on Linux is too weak; we also want to handle other common signals, and do so even on BSD. Why? Because at least 'qemu-nbd -B bitmap' needs a chance to clean up the in-use bit on bitmaps when the server is shut down via a signal. See also: http://bugzilla.redhat.com/1883608 Signed-off-by: Eric Blake <eblake@redhat.com> --- qemu-nbd.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)