Re: [Qemu-devel] [PATCH 2/2] qemu-ga: Add the guest-suspend command

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 1568 bytes --]

On 01/04/2012 12:45 PM, Luiz Capitulino wrote:
> +    if (pid == 0) {
> +        /* child */
> +        int fd;
> +
> +        setsid();
> +        fclose(stdin);
> +        fclose(stdout);
> +        fclose(stderr);
> +
> +        execlp(pmutils_bin, pmutils_bin, NULL);

It's generally a bad idea to exec a child process without fd 0, 1, and 2
open on something, even if that something is /dev/null.  POSIX says that
the system may, but not must, reopen fds on your behalf, and that the
child without open std descriptors is then executing in a non-conforming
environment and may misbehave in unexpected manners.

> +
> +        /* 
> +         * The exec call should not return, if it does something went wrong.
> +         * In this case we try to suspend manually if 'mode' is 'hibernate'
> +         */
> +        slog("could not execute %s: %s\n", pmutils_bin, strerror(errno));
> +        slog("trying to suspend using the manual method...\n");
> +
> +        fd = open(LINUX_SYS_STATE_FILE, O_WRONLY);

Worse, since you _just_ closed stdin above, fd here will most likely be
0, but a O_WRONLY stdin is asking for problems.

> +        if (fd < 0) {
> +            slog("can't open file %s: %s\n", LINUX_SYS_STATE_FILE,
> +                    strerror(errno));

Also, I have no idea where slog() writes to, but since you closed
stderr, if slog() is trying to use stderr, your error messages would be
invisible.

-- 
Eric Blake   eblake@redhat.com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 620 bytes --]