On 05/14/2012 11:40 AM, 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 >
> # @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 guaruntee of successful shutdown.
As long as you are changing docs, fix the typo:
s/guaruntee/guarantee/
> +++ b/qga/commands-posix.c
> @@ -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);
I prefer the POSIX-mandated macros STDIN_FILENO, STDOUT_FILENO, and
STDERR_FILENO, but don't know if qemu intends to rely on them (according
to gnulib, at least older mingw lacked those macro names from
<unistd.h>). So I won't make you change this.
> +
> + ret = execle("/sbin/shutdown", "shutdown", shutdown_flag, "+0",
> + "hypervisor initiated shutdown", (char*)NULL, environ);
Where was 'environ' declared? POSIX says that environ must exist, but
that it is the one variable where you must declare it yourself rather
than getting it from a public header. (For convenience, glibc declares
environ in <unistd.h> when using _GNU_SOURCE, but when you are asking
for strict standards namespace compliance, it disappears.)
> + _exit(!!ret);
Why are we even bothering with ret? If execle() returns, we _know_ we
had a failure, and !!ret will always be 1.
--
Eric Blake ebl...@redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
