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

["Daniel P. Berrange" <berrange@redhat.com>]

On Mon, Jan 16, 2012 at 06:09:52PM -0200, Luiz Capitulino wrote:

> +/* Try to find executable file 'file'. If it's found, its absolute path is
> +   returned in 'abs_path' and the function returns true. If it's not found,
> +   the function will return false and 'abs_path' will contain zeros */
> +static bool find_executable_file(const char *file, char *abs_path, size_t len)
> +{
> +    char *path, *saveptr;
> +    const char *token, *delim = ":";
> + 
> +    if (!getenv("PATH")) {
> +        return false;
> +    }
> +
> +    path = g_strdup(getenv("PATH"));
> +    if (!path) {
> +        return false;
> +    }
> +
> +    for (token = strtok_r(path, delim, &saveptr); token;
> +         token = strtok_r(NULL, delim, &saveptr)) {
> +        snprintf(abs_path, len, "%s/%s", token, file);
> +        if (access(abs_path, X_OK) == 0) {
> +            g_free(path);
> +            return true;
> +        }
> +    }
> +
> +    g_free(path);
> +    memset(abs_path, 0, len);
> +
> +    return false;
> +}

I hope you don't hate me for now pointing out...

   g_find_program_in_path()

http://developer.gnome.org/glib/stable/glib-Miscellaneous-Utility-Functions.html#g-find-program-in-path


> +
>  int64_t qmp_guest_sync(int64_t id, Error **errp)
>  {
>      return id;
> @@ -574,6 +623,220 @@ int64_t qmp_guest_fsfreeze_thaw(Error **err)
>  }
>  #endif
>  
> +#define LINUX_SYS_STATE_FILE "/sys/power/state"
> +#define SUS_MODE_SUPPORTED 0
> +#define SUS_MODE_NOT_SUPPORTED 1
> +
> +/**
> + * This function forks twice and the information about the mode support
> + * status is passed to the qemu-ga process via a pipe.
> + *
> + * This approach allows us to keep the way we reap terminated children
> + * in qemu-ga quite simple.
> + */
> +static bool bios_supports_mode(const char *mode, Error **err)
> +{
> +    pid_t pid;
> +    ssize_t ret;
> +    bool has_pmutils;
> +    int status, pipefds[2];
> +    char pmutils_path[PATH_MAX];
> +    const char *pmutils_bin = "pm-is-supported";
> +
> +    if (pipe(pipefds) < 0) {
> +        error_set(err, QERR_UNDEFINED_ERROR);
> +        return false;
> +    }
> +
> +    has_pmutils = find_executable_file(pmutils_bin, pmutils_path,
> +                                       sizeof(pmutils_path));
> +
> +    pid = fork();
> +    if (!pid) {
> +        struct sigaction act;
> +
> +        memset(&act, 0, sizeof(act));
> +        act.sa_handler = SIG_DFL;
> +        sigaction(SIGCHLD, &act, NULL);
> +
> +        setsid();
> +        close(pipefds[0]);
> +        reopen_fd_to_null(0);
> +        reopen_fd_to_null(1);
> +        reopen_fd_to_null(2);
> +
> +        pid = fork();
> +        if (!pid) {
> +            int fd;
> +            char buf[32]; /* hopefully big enough */
> +            const char *arg;
> +
> +            if (strcmp(mode, "hibernate") == 0) {
> +                arg = "--hibernate";
> +            } else if (strcmp(mode, "sleep") == 0) {
> +                arg = "--suspend";
> +            } else if (strcmp(mode, "hybrid") == 0) {
> +                arg = "--suspend-hybrid";
> +            } else {
> +                _exit(SUS_MODE_NOT_SUPPORTED);
> +            }
> +
> +            if (has_pmutils) {
> +                execle(pmutils_path, pmutils_bin, arg, NULL, environ);
> +            }
> +
> +            /*
> +             * If we get here either pm-utils is not installed or  execle() has
> +             * failed. Let's try the manual approach if mode is not hybrid (as
> +             * it's only supported by pm-utils)
> +             */
> +
> +            if (strcmp(mode, "hybrid") == 0) {
> +                _exit(SUS_MODE_NOT_SUPPORTED);
> +            }
> +
> +            fd = open(LINUX_SYS_STATE_FILE, O_RDONLY);
> +            if (fd < 0) {
> +                _exit(SUS_MODE_NOT_SUPPORTED);
> +            }
> +
> +            ret = read(fd, buf, sizeof(buf));

You want "sizeof(buf)-1" here, otherwise if read return 'ret'
set to exactly sizeof(buf)...

> +            if (ret <= 0) {
> +                _exit(SUS_MODE_NOT_SUPPORTED);
> +            }
> +
> +            buf[ret] = '\0';

...this becomes a stack overflow.

> +    has_pmutils = find_executable_file(pmutils_bin, pmutils_path,
> +                                       sizeof(pmutils_path));
> +
> +    pid = fork();
> +    if (pid == 0) {
> +        /* child */
> +        int fd;
> +        const char *cmd;
> +
> +        setsid();
> +        reopen_fd_to_null(0);
> +        reopen_fd_to_null(1);
> +        reopen_fd_to_null(2);
> +
> +        if (has_pmutils) {
> +            execle(pmutils_path, pmutils_bin, NULL, environ);

You could just use  execl()  and drop the trailing 'environ' here,
since that is the default anyway.

> +        }
> +
> +        /*
> +         * If we get here either pm-utils is not installed or  execle() has
> +         * failed. Let's try the manual approach if mode is not hybrid (as
> +         * it's only supported by pm-utils)
> +         */
> +
> +        slog("could not execute %s\n", pmutils_bin);
> +        if (strcmp(mode, "hybrid") == 0) {
> +            _exit(EXIT_FAILURE);
> +        }
> +
> +        slog("trying to suspend using the manual method...\n");
> +
> +        fd = open(LINUX_SYS_STATE_FILE, O_WRONLY);
> +        if (fd < 0) {
> +            slog("can't open file %s: %s\n", LINUX_SYS_STATE_FILE,
> +                    strerror(errno));
> +            _exit(EXIT_FAILURE);
> +        }
> +
> +        cmd = strcmp(mode, "sleep") == 0 ? "mem" : "disk";
> +        if (write(fd, cmd, strlen(cmd)) < 0) {
> +            slog("failued to write to %s\n", LINUX_SYS_STATE_FILE);
> +            _exit(EXIT_FAILURE);
> +        }
> +
> +        _exit(EXIT_SUCCESS);
> +    } else if (pid < 0) {
> +        error_set(err, QERR_UNDEFINED_ERROR);
> +        return;
> +    }
> +}
> +
>  /* register init/cleanup routines for stateful command groups */
>  void ga_command_state_init(GAState *s, GACommandState *cs)
>  {


Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|