Message ID | 20120514152520.69ce699a@doriath.home |
---|---|
State | New |
Headers | show |
On 05/14/2012 12:25 PM, Luiz Capitulino wrote: > POSIX mandates[1] that a child process of a multi-thread program uses > only async-signal-safe functions before exec(). We consider qemu-ga > to be multi-thread, because it uses glib. > > However, qmp_guest_shutdown() uses functions that are not > async-signal-safe. Fix it the following way: > > - fclose() -> reopen_fd_to_null() > - execl() -> execle() > - exit() -> _exit() > - drop slog() usage (which is not safe) > > [1] http://pubs.opengroup.org/onlinepubs/009695399/functions/fork.html > > Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com> > --- > > o v2 > > - fix doc typo > - drop 'ret' and use EXIT_FAILURE instead Not addressed was the 'environ' declaration issue, but I'm okay if you save it for another day when we actually get a compile failure for using an undeclared variable (since what you have _does_ work on glibc). Reviewed-by: Eric Blake <eblake@redhat.com>
On Mon, May 14, 2012 at 03:25:20PM -0300, Luiz Capitulino wrote: > POSIX mandates[1] that a child process of a multi-thread program uses > only async-signal-safe functions before exec(). We consider qemu-ga > to be multi-thread, because it uses glib. > > However, qmp_guest_shutdown() uses functions that are not > async-signal-safe. Fix it the following way: > > - fclose() -> reopen_fd_to_null() > - execl() -> execle() > - exit() -> _exit() > - drop slog() usage (which is not safe) > > [1] http://pubs.opengroup.org/onlinepubs/009695399/functions/fork.html > > Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com> Thanks, applied to qga branch. > --- > > o v2 > > - fix doc typo > - drop 'ret' and use EXIT_FAILURE instead > > qapi-schema-guest.json | 3 +-- > qga/commands-posix.c | 19 ++++++++----------- > 2 files changed, 9 insertions(+), 13 deletions(-) > > diff --git a/qapi-schema-guest.json b/qapi-schema-guest.json > index 1c949ff..bd2256d 100644 > --- a/qapi-schema-guest.json > +++ b/qapi-schema-guest.json > @@ -126,8 +126,7 @@ > # @guest-shutdown: > # > # Initiate guest-activated shutdown. Note: this is an asynchronous > -# shutdown request, with no guaruntee of successful shutdown. Errors > -# will be logged to guest's syslog. > +# shutdown request, with no guarantee of successful shutdown. > # > # @mode: #optional "halt", "powerdown" (default), or "reboot" > # > diff --git a/qga/commands-posix.c b/qga/commands-posix.c > index 9a59276..15ce928 100644 > --- a/qga/commands-posix.c > +++ b/qga/commands-posix.c > @@ -37,8 +37,8 @@ > void qmp_guest_shutdown(bool has_mode, const char *mode, Error **err) > { > const char *shutdown_flag; > - int ret, status; > pid_t rpid, pid; > + int status; > > slog("guest-shutdown called, mode: %s", mode); > if (!has_mode || strcmp(mode, "powerdown") == 0) { > @@ -57,16 +57,13 @@ void qmp_guest_shutdown(bool has_mode, const char *mode, Error **err) > if (pid == 0) { > /* child, start the shutdown */ > setsid(); > - fclose(stdin); > - fclose(stdout); > - fclose(stderr); > - > - ret = execl("/sbin/shutdown", "shutdown", shutdown_flag, "+0", > - "hypervisor initiated shutdown", (char*)NULL); > - if (ret) { > - slog("guest-shutdown failed: %s", strerror(errno)); > - } > - exit(!!ret); > + reopen_fd_to_null(0); > + reopen_fd_to_null(1); > + reopen_fd_to_null(2); > + > + execle("/sbin/shutdown", "shutdown", shutdown_flag, "+0", > + "hypervisor initiated shutdown", (char*)NULL, environ); > + _exit(EXIT_FAILURE); > } else if (pid < 0) { > goto exit_err; > } > -- > 1.7.9.2.384.g4a92a >
diff --git a/qapi-schema-guest.json b/qapi-schema-guest.json index 1c949ff..bd2256d 100644 --- a/qapi-schema-guest.json +++ b/qapi-schema-guest.json @@ -126,8 +126,7 @@ # @guest-shutdown: # # Initiate guest-activated shutdown. Note: this is an asynchronous -# shutdown request, with no guaruntee of successful shutdown. Errors -# will be logged to guest's syslog. +# shutdown request, with no guarantee of successful shutdown. # # @mode: #optional "halt", "powerdown" (default), or "reboot" # diff --git a/qga/commands-posix.c b/qga/commands-posix.c index 9a59276..15ce928 100644 --- a/qga/commands-posix.c +++ b/qga/commands-posix.c @@ -37,8 +37,8 @@ void qmp_guest_shutdown(bool has_mode, const char *mode, Error **err) { const char *shutdown_flag; - int ret, status; pid_t rpid, pid; + int status; slog("guest-shutdown called, mode: %s", mode); if (!has_mode || strcmp(mode, "powerdown") == 0) { @@ -57,16 +57,13 @@ void qmp_guest_shutdown(bool has_mode, const char *mode, Error **err) if (pid == 0) { /* child, start the shutdown */ setsid(); - fclose(stdin); - fclose(stdout); - fclose(stderr); - - ret = execl("/sbin/shutdown", "shutdown", shutdown_flag, "+0", - "hypervisor initiated shutdown", (char*)NULL); - if (ret) { - slog("guest-shutdown failed: %s", strerror(errno)); - } - exit(!!ret); + reopen_fd_to_null(0); + reopen_fd_to_null(1); + reopen_fd_to_null(2); + + execle("/sbin/shutdown", "shutdown", shutdown_flag, "+0", + "hypervisor initiated shutdown", (char*)NULL, environ); + _exit(EXIT_FAILURE); } else if (pid < 0) { goto exit_err; }
POSIX mandates[1] that a child process of a multi-thread program uses only async-signal-safe functions before exec(). We consider qemu-ga to be multi-thread, because it uses glib. However, qmp_guest_shutdown() uses functions that are not async-signal-safe. Fix it the following way: - fclose() -> reopen_fd_to_null() - execl() -> execle() - exit() -> _exit() - drop slog() usage (which is not safe) [1] http://pubs.opengroup.org/onlinepubs/009695399/functions/fork.html Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com> --- o v2 - fix doc typo - drop 'ret' and use EXIT_FAILURE instead qapi-schema-guest.json | 3 +-- qga/commands-posix.c | 19 ++++++++----------- 2 files changed, 9 insertions(+), 13 deletions(-)