From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:41525) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SU3hY-0005aG-LH for qemu-devel@nongnu.org; Mon, 14 May 2012 18:25:14 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1SU3hW-0005KI-Ih for qemu-devel@nongnu.org; Mon, 14 May 2012 18:25:12 -0400 Received: from mail-ob0-f173.google.com ([209.85.214.173]:43284) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SU3hW-0005K7-BI for qemu-devel@nongnu.org; Mon, 14 May 2012 18:25:10 -0400 Received: by obbwd20 with SMTP id wd20so9830637obb.4 for ; Mon, 14 May 2012 15:25:08 -0700 (PDT) Sender: fluxion Date: Mon, 14 May 2012 17:25:04 -0500 From: Michael Roth Message-ID: <20120514222504.GG28865@illuin> References: <20120514152520.69ce699a@doriath.home> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20120514152520.69ce699a@doriath.home> Subject: Re: [Qemu-devel] [PATCH v2] qemu-ga: guest-shutdown: use only async-signal-safe functions List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Luiz Capitulino Cc: Eric Blake , qemu-devel 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 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 >