From: "Richard W.M. Jones" <rjones@redhat.com> This is a wrapper or mini-library for running external command, loosely based on libvirt's virCommand interface. Amongst the advantages are: - Can redirect errors into the error log (RHBZ#713678). - Can redirect output into a callback function. - Handles shell quoting properly. - Safely resets signal handlers, closes file descriptors, etc. - Single place where we can implement other improvements in future. --- po/POTFILES | 1 + src/Makefile.am | 1 + src/command.c | 752 +++++++++++++++++++++++++++++++++++++++++++++++++ src/guestfs-internal.h | 18 ++ 4 files changed, 772 insertions(+) create mode 100644 src/command.c diff --git a/po/POTFILES b/po/POTFILES index 1de4fc0..a73377d 100644 --- a/po/POTFILES +++ b/po/POTFILES @@ -218,6 +218,7 @@ ruby/ext/guestfs/_guestfs.c src/actions.c src/appliance.c src/bindtests.c +src/command.c src/dbdump.c src/errnostring-gperf.c src/errnostring.c diff --git a/src/Makefile.am b/src/Makefile.am index b189c4a..5f80bc1 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -125,6 +125,7 @@ libguestfs_la_SOURCES = \ actions.c \ appliance.c \ bindtests.c \ + command.c \ dbdump.c \ events.c \ file.c \ diff --git a/src/command.c b/src/command.c new file mode 100644 index 0000000..01cd2c3 --- /dev/null +++ b/src/command.c @@ -0,0 +1,752 @@ +/* libguestfs + * Copyright (C) 2010-2012 Red Hat Inc. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA + */ + +/* Wrapper for running external command, loosely based on libvirt's + * virCommand interface. Read the comments at the top of each + * function for detailed information on how to use this interface. In + * outline what you have to do is: + * + * (1) Create a new command handle: + * + * struct command *cmd; + * cmd = guestfs___new_command (g); + * + * (2) EITHER add arguments: + * + * guestfs___cmd_add_arg (cmd, "qemu-img"); + * guestfs___cmd_add_arg (cmd, "info"); + * guestfs___cmd_add_arg (cmd, filename); + * + * NB: You don't need to add a NULL argument at the end. + * + * (3) OR construct a command using a mix of quoted and unquoted + * strings. (This is useful for system(3)/popen("r")-style shell + * commands, with the added safety of allowing args to be quoted + * properly). + * + * guestfs___cmd_add_string_unquoted (cmd, "qemu-img info "); + * guestfs___cmd_add_string_quoted (cmd, filename); + * + * (4) Set various flags, such as whether you want to capture + * errors in the regular libguestfs error log. + * + * (5) Run the command. This is what does the fork call, optionally + * loops over the output, and then does a waitpid and returns the + * exit status of the command. + * + * r = guestfs___cmd_run (cmd); + * if (r == -1) + * // error + * // else test r using the WIF* functions + * + * (6) Close the handle: + * + * guestfs___cmd_close (cmd); + */ + +#include <config.h> + +#include <stdio.h> +#include <stdlib.h> +#include <stdbool.h> +#include <string.h> +#include <unistd.h> +#include <fcntl.h> +#include <signal.h> +#include <errno.h> +#include <assert.h> +#include <sys/types.h> +#include <sys/stat.h> +#include <sys/wait.h> +#include <sys/select.h> + +#include "guestfs.h" +#include "guestfs-internal.h" + +enum command_style { + COMMAND_STYLE_NOT_SELECTED = 0, + COMMAND_STYLE_EXECV = 1, + COMMAND_STYLE_SYSTEM = 2 +}; + +struct command; + +static void add_line_buffer (struct command *cmd, const char *buf, size_t len); +static void close_line_buffer (struct command *cmd); +static void add_unbuffered (struct command *cmd, const char *buf, size_t len); +static void add_whole_buffer (struct command *cmd, const char *buf, size_t len); +static void close_whole_buffer (struct command *cmd); + +struct buffering { + char *buffer; + size_t len; + void (*add_data) (struct command *cmd, const char *buf, size_t len); + void (*close_data) (struct command *cmd); +}; + +struct command +{ + guestfs_h *g; + + enum command_style style; + union { + /* COMMAND_STYLE_EXECV */ + struct { + char **args; + size_t len, alloc; + } argv; + /* COMMAND_STYLE_SYSTEM */ + struct { + char *str; + size_t len, alloc; + } string; + }; + + /* Capture errors to the error log (defaults to true). */ + bool capture_errors; + int errorfd; + + /* Supply a callback to receive stdout. */ + cmd_stdout_callback stdout_callback; + void *stdout_data; + int outfd; + struct buffering outbuf; + + /* For programs that send output to stderr. Hello qemu. */ + bool stderr_to_stdout; + + /* PID of subprocess (if > 0). */ + pid_t pid; +}; + +/* Create a new command handle. */ +struct command * +guestfs___new_command (guestfs_h *g) +{ + struct command *cmd; + + cmd = safe_calloc (g, 1, sizeof *cmd); + cmd->g = g; + cmd->capture_errors = true; + cmd->errorfd = -1; + cmd->outfd = -1; + return cmd; +} + +/* Add single arg (for execv-style command execution). */ +static void +add_arg_no_strdup (struct command *cmd, char *arg) +{ + assert (cmd->style != COMMAND_STYLE_SYSTEM); + cmd->style = COMMAND_STYLE_EXECV; + + if (cmd->argv.len >= cmd->argv.alloc) { + if (cmd->argv.alloc == 0) + cmd->argv.alloc = 16; + else + cmd->argv.alloc *= 2; + cmd->argv.args = safe_realloc (cmd->g, cmd->argv.args, + cmd->argv.alloc * sizeof (char *)); + } + cmd->argv.args[cmd->argv.len] = arg; + cmd->argv.len++; +} + +static void +add_arg (struct command *cmd, const char *arg) +{ + assert (arg != NULL); + add_arg_no_strdup (cmd, safe_strdup (cmd->g, arg)); +} + +void +guestfs___cmd_add_arg (struct command *cmd, const char *arg) +{ + add_arg (cmd, arg); +} + +void +guestfs___cmd_add_arg_format (struct command *cmd, const char *fs, ...) +{ + va_list args; + char *arg; + + va_start (args, fs); + int err = vasprintf (&arg, fs, args); + va_end (args); + + if (err < 0) + cmd->g->abort_cb (); + + add_arg_no_strdup (cmd, arg); +} + +/* Add strings (for system(3)-style command execution). */ +static void +add_string (struct command *cmd, const char *str, size_t len) +{ + assert (cmd->style != COMMAND_STYLE_EXECV); + cmd->style = COMMAND_STYLE_SYSTEM; + + if (cmd->string.len >= cmd->string.alloc) { + if (cmd->string.alloc == 0) + cmd->string.alloc = 256; + else + cmd->string.alloc += MAX (cmd->string.alloc, len); + cmd->string.str = safe_realloc (cmd->g, cmd->string.str, cmd->string.alloc); + } + + memcpy (&cmd->string.str[cmd->string.len], str, len); + cmd->string.len += len; +} + +void +guestfs___cmd_add_string_unquoted (struct command *cmd, const char *str) +{ + add_string (cmd, str, strlen (str)); +} + +/* Add a string enclosed in double quotes, with any special characters + * within the string which need escaping done. This is used to add a + * single argument to a system(3)-style command string. + */ +void +guestfs___cmd_add_string_quoted (struct command *cmd, const char *str) +{ + add_string (cmd, "\"", 1); + + for (; *str; str++) { + if (*str == '$' || + *str == '`' || + *str == '\\' || + *str == '"') + add_string (cmd, "\\", 1); + add_string (cmd, str, 1); + } + + add_string (cmd, "\"", 1); +} + +/* Set a callback which will capture stdout. + * + * If flags contains CMD_STDOUT_FLAG_LINE_BUFFER (the default), then + * the callback is called line by line on the output. If there is a + * trailing \n then it is automatically removed before the callback is + * called. The line buffer is \0-terminated. + * + * If flags contains CMD_STDOUT_FLAG_UNBUFFERED, then buffers are + * passed to the callback as it is received from the command. Note in + * this case the buffer is NOT \0-terminated, so you need to may + * attention to the length field in the callback. + * + * If flags contains CMD_STDOUT_FLAG_WHOLE_BUFFER, then the callback + * is called exactly once, with the entire buffer. Note in this case + * the buffer is NOT \0-terminated, so you need to may attention to + * the length field in the callback. + */ +void +guestfs___cmd_set_stdout_callback (struct command *cmd, + cmd_stdout_callback stdout_callback, + void *stdout_data, unsigned flags) +{ + cmd->stdout_callback = stdout_callback; + cmd->stdout_data = stdout_data; + + /* Buffering mode. */ + if ((flags & 3) == CMD_STDOUT_FLAG_LINE_BUFFER) { + cmd->outbuf.add_data = add_line_buffer; + cmd->outbuf.close_data = close_line_buffer; + } + else if ((flags & 3) == CMD_STDOUT_FLAG_UNBUFFERED) { + cmd->outbuf.add_data = add_unbuffered; + cmd->outbuf.close_data = NULL; + } + else if ((flags & 3) == CMD_STDOUT_FLAG_WHOLE_BUFFER) { + cmd->outbuf.add_data = add_whole_buffer; + cmd->outbuf.close_data = close_whole_buffer; + } + else + abort (); +} + +/* Equivalent to adding 2>&1 to the end of the command. This is + * incompatible with the capture_errors flag, because it doesn't make + * sense to combine them. + */ +void +guestfs___cmd_set_stderr_to_stdout (struct command *cmd) +{ + cmd->stderr_to_stdout = true; +} + +/* Clear the capture_errors flag. This means that any errors will go + * to stderr, instead of being captured in the event log, and that is + * usually undesirable. + */ +void +guestfs___cmd_clear_capture_errors (struct command *cmd) +{ + cmd->capture_errors = false; +} + +/* Finish off the command by either NULL-terminating the argv array or + * adding a terminating \0 to the string, or die with an internal + * error if no command has been added. + */ +static void +finish_command (struct command *cmd) +{ + switch (cmd->style) { + case COMMAND_STYLE_EXECV: + add_arg_no_strdup (cmd, NULL); + break; + + case COMMAND_STYLE_SYSTEM: + add_string (cmd, "\0", 1); + break; + + case COMMAND_STYLE_NOT_SELECTED: + default: + abort (); + } +} + +static void +debug_command (struct command *cmd) +{ + size_t i, last; + + switch (cmd->style) { + case COMMAND_STYLE_EXECV: + debug (cmd->g, "command: run: %s", cmd->argv.args[0]); + last = cmd->argv.len-1; /* omit final NULL pointer */ + for (i = 1; i < last; ++i) { + if (i < last-1 && + cmd->argv.args[i][0] == '-' && cmd->argv.args[i+1][0] != '-') { + debug (cmd->g, "command: run: \\ %s %s", + cmd->argv.args[i], cmd->argv.args[i+1]); + i++; + } + else + debug (cmd->g, "command: run: \\ %s", cmd->argv.args[i]); + } + break; + + case COMMAND_STYLE_SYSTEM: + debug (cmd->g, "command: run: %s", cmd->string.str); + break; + + case COMMAND_STYLE_NOT_SELECTED: + default: + abort (); + } +} + +static int +run_command (struct command *cmd) +{ + struct sigaction sa; + int i, fd, max_fd, r; + int errorfd[2] = { -1, -1 }; + int outfd[2] = { -1, -1 }; + + /* Set up a pipe to capture command output and send it to the error log. */ + if (cmd->capture_errors) { + if (pipe2 (errorfd, O_CLOEXEC) == -1) { + perrorf (cmd->g, "pipe2"); + goto error; + } + } + + /* Set up a pipe to capture stdout for the callback. */ + if (cmd->stdout_callback) { + if (pipe2 (outfd, O_CLOEXEC) == -1) { + perrorf (cmd->g, "pipe2"); + goto error; + } + } + + cmd->pid = fork (); + if (cmd->pid == -1) { + perrorf (cmd->g, "fork"); + goto error; + } + + /* In parent, return to caller. */ + if (cmd->pid > 0) { + if (cmd->capture_errors) { + close (errorfd[1]); + errorfd[1] = -1; + cmd->errorfd = errorfd[0]; + errorfd[0] = -1; + } + + if (cmd->stdout_callback) { + close (outfd[1]); + outfd[1] = -1; + cmd->outfd = outfd[0]; + outfd[0] = -1; + } + + return 0; + } + + /* Child process. */ + if (cmd->capture_errors) { + close (errorfd[0]); + if (!cmd->stdout_callback) + dup2 (errorfd[1], 1); + dup2 (errorfd[1], 2); + close (errorfd[1]); + } + + if (cmd->stdout_callback) { + close (outfd[0]); + dup2 (outfd[1], 1); + close (outfd[1]); + } + + if (cmd->stderr_to_stdout) + dup2 (1, 2); + + /* Remove all signal handlers. See the justification here: + * https://www.redhat.com/archives/libvir-list/2008-August/msg00303.html + * We don't mask signal handlers yet, so this isn't completely + * race-free, but better than not doing it at all. + */ + memset (&sa, 0, sizeof sa); + sa.sa_handler = SIG_DFL; + sa.sa_flags = 0; + sigemptyset (&sa.sa_mask); + for (i = 1; i < NSIG; ++i) + sigaction (i, &sa, NULL); + + /* Close all other file descriptors. This ensures that we don't + * hold open (eg) pipes from the parent process. + */ + max_fd = sysconf (_SC_OPEN_MAX); + if (max_fd == -1) + max_fd = 1024; + if (max_fd > 65536) + max_fd = 65536; /* bound the amount of work we do here */ + for (fd = 3; fd < max_fd; ++fd) + close (fd); + + /* Clean up the environment. */ + setenv ("LC_ALL", "C", 1); + + /* Set the umask for all subcommands to something sensible (RHBZ#610880). */ + umask (022); + + /* Run the command. */ + switch (cmd->style) { + case COMMAND_STYLE_EXECV: + execvp (cmd->argv.args[0], cmd->argv.args); + perror (cmd->argv.args[0]); + _exit (EXIT_FAILURE); + + case COMMAND_STYLE_SYSTEM: + r = system (cmd->string.str); + if (r == -1) { + perror ("system"); + _exit (EXIT_FAILURE); + } + if (WIFEXITED (r)) + _exit (WEXITSTATUS (r)); + if (WIFSIGNALED (r)) { + fprintf (stderr, "%s: received signal %d\n", cmd->string.str, + WTERMSIG (r)); + _exit (EXIT_FAILURE); + } + if (WIFSTOPPED (r)) { + fprintf (stderr, "%s: stopped by signal %d\n", cmd->string.str, + WSTOPSIG (r)); + _exit (EXIT_FAILURE); + } + _exit (EXIT_FAILURE); + + case COMMAND_STYLE_NOT_SELECTED: + default: + abort (); + } + /*NOTREACHED*/ + + error: + if (errorfd[0] >= 0) + close (errorfd[0]); + if (errorfd[1] >= 0) + close (errorfd[1]); + if (outfd[0] >= 0) + close (outfd[0]); + if (outfd[1] >= 0) + close (outfd[1]); + + return -1; +} + +/* The loop which reads errors and output and directs it either + * to the log or to the stdout callback as appropriate. + */ +static int +loop (struct command *cmd) +{ + fd_set rset, rset2; + int maxfd = -1, r; + size_t nr_fds = 0; + char buf[BUFSIZ]; + ssize_t n; + + FD_ZERO (&rset); + + if (cmd->errorfd >= 0) { + FD_SET (cmd->errorfd, &rset); + maxfd = MAX (cmd->errorfd, maxfd); + nr_fds++; + } + + if (cmd->outfd >= 0) { + FD_SET (cmd->outfd, &rset); + maxfd = MAX (cmd->outfd, maxfd); + nr_fds++; + } + + while (nr_fds > 0) { + rset2 = rset; + r = select (maxfd+1, &rset2, NULL, NULL, NULL); + if (r == -1) { + if (errno == EINTR || errno == EAGAIN) + continue; + perrorf (cmd->g, "select"); + return -1; + } + + if (cmd->errorfd >= 0 && FD_ISSET (cmd->errorfd, &rset2)) { + /* Read output and send it to the log. */ + n = read (cmd->errorfd, buf, sizeof buf); + if (n > 0) + guestfs___call_callbacks_message (cmd->g, GUESTFS_EVENT_APPLIANCE, + buf, n); + else if (n == 0) { + if (close (cmd->errorfd) == -1) + perrorf (cmd->g, "close: errorfd"); + FD_CLR (cmd->errorfd, &rset); + cmd->errorfd = -1; + nr_fds--; + } + else if (n == -1) { + perrorf (cmd->g, "read: errorfd"); + close (cmd->errorfd); + FD_CLR (cmd->errorfd, &rset); + cmd->errorfd = -1; + nr_fds--; + } + } + + if (cmd->outfd >= 0 && FD_ISSET (cmd->outfd, &rset2)) { + /* Read the output, buffer it up to the end of the line, then + * pass it to the callback. + */ + n = read (cmd->outfd, buf, sizeof buf); + if (n > 0) { + if (cmd->outbuf.add_data) + cmd->outbuf.add_data (cmd, buf, n); + } + else if (n == 0) { + if (cmd->outbuf.close_data) + cmd->outbuf.close_data (cmd); + if (close (cmd->outfd) == -1) + perrorf (cmd->g, "close: outfd"); + FD_CLR (cmd->outfd, &rset); + cmd->outfd = -1; + nr_fds--; + } + else if (n == -1) { + perrorf (cmd->g, "read: outfd"); + close (cmd->outfd); + FD_CLR (cmd->outfd, &rset); + cmd->outfd = -1; + nr_fds--; + } + } + } + + return 0; +} + +static int +wait_command (struct command *cmd) +{ + int status; + + if (waitpid (cmd->pid, &status, 0) == -1) { + perrorf (cmd->g, "waitpid"); + return -1; + } + + cmd->pid = 0; + + return status; +} + +/* Fork, run the command, loop over the output, and waitpid. + * + * Returns the exit status. Test it using WIF* macros. + * + * On error: Calls error(g) and returns -1. + */ +int +guestfs___cmd_run (struct command *cmd) +{ + finish_command (cmd); + + if (cmd->g->verbose) + debug_command (cmd); + + if (run_command (cmd) == -1) + return -1; + + if (loop (cmd) == -1) + return -1; + + return wait_command (cmd); +} + +void +guestfs___cmd_close (struct command *cmd) +{ + size_t i; + + switch (cmd->style) { + case COMMAND_STYLE_NOT_SELECTED: + /* nothing */ + break; + + case COMMAND_STYLE_EXECV: + for (i = 0; i < cmd->argv.len; ++i) + free (cmd->argv.args[i]); + free (cmd->argv.args); + break; + + case COMMAND_STYLE_SYSTEM: + free (cmd->string.str); + break; + + default: + abort (); + } + + if (cmd->errorfd >= 0) + close (cmd->errorfd); + + if (cmd->outfd >= 0) + close (cmd->outfd); + + free (cmd->outbuf.buffer); + + if (cmd->pid > 0) + waitpid (cmd->pid, NULL, 0); + + free (cmd); +} + +/* Deal with buffering stdout for the callback. */ +static void +process_line_buffer (struct command *cmd, int closed) +{ + guestfs_h *g = cmd->g; + char *p; + size_t len, newlen; + + while (cmd->outbuf.len > 0) { + /* Length of the next line. */ + p = strchr (cmd->outbuf.buffer, '\n'); + if (p != NULL) { /* Got a whole line. */ + len = p - cmd->outbuf.buffer; + newlen = cmd->outbuf.len - len - 1; + } + else if (closed) { /* Consume rest of input even if no \n found. */ + len = cmd->outbuf.len; + newlen = 0; + } + else /* Need to wait for more input. */ + break; + + /* Call the callback with the next line. */ + cmd->outbuf.buffer[len] = '\0'; + cmd->stdout_callback (g, cmd->stdout_data, cmd->outbuf.buffer, len); + + /* Remove the consumed line from the buffer. */ + cmd->outbuf.len = newlen; + memmove (cmd->outbuf.buffer, cmd->outbuf.buffer + len + 1, newlen); + + /* Keep the buffer \0 terminated. */ + cmd->outbuf.buffer[newlen] = '\0'; + } +} + +static void +add_line_buffer (struct command *cmd, const char *buf, size_t len) +{ + guestfs_h *g = cmd->g; + size_t oldlen; + + /* Append the new content to the end of the current buffer. Keep + * the buffer \0 terminated to make things simple when processing + * the buffer. + */ + oldlen = cmd->outbuf.len; + cmd->outbuf.len += len; + cmd->outbuf.buffer = safe_realloc (g, cmd->outbuf.buffer, + cmd->outbuf.len + 1 /* for \0 */); + memcpy (cmd->outbuf.buffer + oldlen, buf, len); + cmd->outbuf.buffer[cmd->outbuf.len] = '\0'; + + process_line_buffer (cmd, 0); +} + +static void +close_line_buffer (struct command *cmd) +{ + process_line_buffer (cmd, 1); +} + +static void +add_unbuffered (struct command *cmd, const char *buf, size_t len) +{ + cmd->stdout_callback (cmd->g, cmd->stdout_data, buf, len); +} + +static void +add_whole_buffer (struct command *cmd, const char *buf, size_t len) +{ + guestfs_h *g = cmd->g; + size_t oldlen; + + /* Append the new content to the end of the current buffer. */ + oldlen = cmd->outbuf.len; + cmd->outbuf.len += len; + cmd->outbuf.buffer = safe_realloc (g, cmd->outbuf.buffer, cmd->outbuf.len); + memcpy (cmd->outbuf.buffer + oldlen, buf, len); +} + +static void +close_whole_buffer (struct command *cmd) +{ + cmd->stdout_callback (cmd->g, cmd->stdout_data, + cmd->outbuf.buffer, cmd->outbuf.len); +} diff --git a/src/guestfs-internal.h b/src/guestfs-internal.h index 90954f1..cf810e1 100644 --- a/src/guestfs-internal.h +++ b/src/guestfs-internal.h @@ -583,4 +583,22 @@ extern void guestfs___free_fuse (guestfs_h *g); extern virConnectPtr guestfs___open_libvirt_connection (guestfs_h *g, const char *uri, unsigned int flags); #endif +/* command.c */ +struct command; +typedef void (*cmd_stdout_callback) (guestfs_h *g, void *data, const char *line, size_t len); +extern struct command *guestfs___new_command (guestfs_h *g); +extern void guestfs___cmd_add_arg (struct command *, const char *arg); +extern void guestfs___cmd_add_arg_format (struct command *, const char *fs, ...) + __attribute__((format (printf,2,3))); +extern void guestfs___cmd_add_string_unquoted (struct command *, const char *str); +extern void guestfs___cmd_add_string_quoted (struct command *, const char *str); +extern void guestfs___cmd_set_stdout_callback (struct command *, cmd_stdout_callback stdout_callback, void *data, unsigned flags); +#define CMD_STDOUT_FLAG_LINE_BUFFER 0 +#define CMD_STDOUT_FLAG_UNBUFFERED 1 +#define CMD_STDOUT_FLAG_WHOLE_BUFFER 2 +extern void guestfs___cmd_set_stderr_to_stdout (struct command *); +extern void guestfs___cmd_clear_capture_errors (struct command *); +extern int guestfs___cmd_run (struct command *); +extern void guestfs___cmd_close (struct command *); + #endif /* GUESTFS_INTERNAL_H_ */ -- 1.7.11.4

From: "Richard W.M. Jones" <rjones@redhat.com> --- src/appliance.c | 227 +++++++++++++++++--------------------------------------- 1 file changed, 70 insertions(+), 157 deletions(-) diff --git a/src/appliance.c b/src/appliance.c index 9db42cb..2241e18 100644 --- a/src/appliance.c +++ b/src/appliance.c @@ -54,8 +54,7 @@ static char *calculate_supermin_checksum (guestfs_h *g, const char *supermin_pat static int check_for_cached_appliance (guestfs_h *g, const char *supermin_path, const char *checksum, uid_t uid, char **kernel, char **initrd, char **appliance); static int build_supermin_appliance (guestfs_h *g, const char *supermin_path, const char *checksum, uid_t uid, char **kernel, char **initrd, char **appliance); static int hard_link_to_cached_appliance (guestfs_h *g, const char *cachedir, char **kernel, char **initrd, char **appliance); -static int run_supermin_helper (guestfs_h *g, const char *supermin_path, const char *cachedir, size_t cdlen); -static void print_febootstrap_command_line (guestfs_h *g, const char *argv[]); +static int run_supermin_helper (guestfs_h *g, const char *supermin_path, const char *cachedir); /* RHBZ#790721: It makes no sense to have multiple threads racing to * build the appliance from within a single process, and the code @@ -245,6 +244,18 @@ contains_supermin_appliance (guestfs_h *g, const char *path, void *data) return dir_contains_files (path, "supermin.d", NULL); } +#define MAX_CHECKSUM_LEN 256 + +static void +read_checksum (guestfs_h *g, void *checksumv, const char *line, size_t len) +{ + char *checksum = checksumv; + + if (len > MAX_CHECKSUM_LEN) + return; + strcpy (checksum, line); +} + /* supermin_path is a path which is known to contain a supermin * appliance. Using febootstrap-supermin-helper -f checksum calculate * the checksum so we can see if it is cached. @@ -252,59 +263,44 @@ contains_supermin_appliance (guestfs_h *g, const char *path, void *data) static char * calculate_supermin_checksum (guestfs_h *g, const char *supermin_path) { - size_t len = 2 * strlen (supermin_path) + 256; - char cmd[len]; + size_t len; + struct command *cmd; int pass_u_g_args = getuid () != geteuid () || getgid () != getegid (); + int r; + char checksum[MAX_CHECKSUM_LEN + 1] = { 0 }; - if (!pass_u_g_args) - snprintf (cmd, len, - "febootstrap-supermin-helper%s " - "-f checksum " - "'%s/supermin.d' " - host_cpu, - g->verbose ? " --verbose" : "", - supermin_path); - else - snprintf (cmd, len, - "febootstrap-supermin-helper%s " - "-u %i " - "-g %i " - "-f checksum " - "'%s/supermin.d' " - host_cpu, - g->verbose ? " --verbose" : "", - geteuid (), getegid (), - supermin_path); - + cmd = guestfs___new_command (g); + guestfs___cmd_add_arg (cmd, "febootstrap-supermin-helper"); if (g->verbose) - guestfs___print_timestamped_message (g, "%s", cmd); - + guestfs___cmd_add_arg (cmd, "--verbose"); + if (pass_u_g_args) { + guestfs___cmd_add_arg (cmd, "-u"); + guestfs___cmd_add_arg_format (cmd, "%d", geteuid ()); + guestfs___cmd_add_arg (cmd, "-g"); + guestfs___cmd_add_arg_format (cmd, "%d", getegid ()); + } + guestfs___cmd_add_arg (cmd, "-f"); + guestfs___cmd_add_arg (cmd, "checksum"); + guestfs___cmd_add_arg_format (cmd, "%s/supermin.d", supermin_path); + guestfs___cmd_add_arg (cmd, host_cpu); + guestfs___cmd_set_stdout_callback (cmd, read_checksum, checksum, 0); + + r = guestfs___cmd_run (cmd); /* Errors here are non-fatal, so we don't need to call error(). */ - FILE *pp = popen (cmd, "r"); - if (pp == NULL) - return NULL; - - char checksum[256]; - if (fgets (checksum, sizeof checksum, pp) == NULL) { - pclose (pp); + if (r == -1) { + guestfs___cmd_close (cmd); return NULL; } + guestfs___cmd_close (cmd); - if (pclose (pp) != 0) { - warning (g, "pclose: %m"); - return NULL; - } + debug (g, "checksum of existing appliance: %s", checksum); len = strlen (checksum); - if (len < 16) { /* sanity check */ warning (g, "febootstrap-supermin-helper -f checksum returned a short string"); return NULL; } - if (len > 0 && checksum[len-1] == '\n') - checksum[--len] = '\0'; - return safe_strndup (g, checksum, len); } @@ -498,7 +494,7 @@ build_supermin_appliance (guestfs_h *g, if (g->verbose) guestfs___print_timestamped_message (g, "run febootstrap-supermin-helper"); - int r = run_supermin_helper (g, supermin_path, tmpcd, len); + int r = run_supermin_helper (g, supermin_path, tmpcd); if (r == -1) { guestfs___remove_tmpdir (g, tmpcd); return -1; @@ -672,127 +668,44 @@ hard_link_to_cached_appliance (guestfs_h *g, */ static int run_supermin_helper (guestfs_h *g, const char *supermin_path, - const char *cachedir, size_t cdlen) + const char *cachedir) { - size_t pathlen = strlen (supermin_path); - - const char *argv[30]; - size_t i = 0; - - char uid[32]; - snprintf (uid, sizeof uid, "%i", geteuid ()); - char gid[32]; - snprintf (gid, sizeof gid, "%i", getegid ()); - char supermin_d[pathlen + 32]; - snprintf (supermin_d, pathlen + 32, "%s/supermin.d", supermin_path); - char kernel[cdlen + 32]; - snprintf (kernel, cdlen + 32, "%s/kernel", cachedir); - char initrd[cdlen + 32]; - snprintf (initrd, cdlen + 32, "%s/initrd", cachedir); - char root[cdlen + 32]; - snprintf (root, cdlen + 32, "%s/root", cachedir); - - int pass_u_g_args = getuid () != geteuid () || getgid () != getegid (); - - argv[i++] = "febootstrap-supermin-helper"; + struct command *cmd; + int r; + uid_t uid = getuid (); + uid_t euid = geteuid (); + gid_t gid = getgid (); + gid_t egid = getegid (); + int pass_u_g_args = uid != euid || gid != egid; + + cmd = guestfs___new_command (g); + guestfs___cmd_add_arg (cmd, "febootstrap-supermin-helper"); if (g->verbose) - argv[i++] = "--verbose"; + guestfs___cmd_add_arg (cmd, "--verbose"); if (pass_u_g_args) { - argv[i++] = "-u"; - argv[i++] = uid; - argv[i++] = "-g"; - argv[i++] = gid; - } - argv[i++] = "--copy-kernel"; - argv[i++] = "-f"; - argv[i++] = "ext2"; - argv[i++] = supermin_d; - argv[i++] = host_cpu; - argv[i++] = kernel; - argv[i++] = initrd; - argv[i++] = root; - argv[i++] = NULL; - - if (g->verbose) - print_febootstrap_command_line (g, argv); - - pid_t pid = fork (); - if (pid == -1) { - perrorf (g, "fork"); + guestfs___cmd_add_arg (cmd, "-u"); + guestfs___cmd_add_arg_format (cmd, "%d", euid); + guestfs___cmd_add_arg (cmd, "-g"); + guestfs___cmd_add_arg_format (cmd, "%d", egid); + } + guestfs___cmd_add_arg (cmd, "--copy-kernel"); + guestfs___cmd_add_arg (cmd, "-f"); + guestfs___cmd_add_arg (cmd, "ext2"); + guestfs___cmd_add_arg_format (cmd, "%s/supermin.d", supermin_path); + guestfs___cmd_add_arg (cmd, host_cpu); + guestfs___cmd_add_arg_format (cmd, "%s/kernel", cachedir); + guestfs___cmd_add_arg_format (cmd, "%s/initrd", cachedir); + guestfs___cmd_add_arg_format (cmd, "%s/root", cachedir); + + r = guestfs___cmd_run (cmd); + if (r == -1) + return -1; + if (!WIFEXITED (r) || WEXITSTATUS (r) != 0) { + error (g, _("external command failed, see earlier error messages")); return -1; } - if (pid > 0) { /* Parent. */ - int status; - if (waitpid (pid, &status, 0) == -1) { - perrorf (g, "waitpid"); - return -1; - } - if (!WIFEXITED (status) || WEXITSTATUS (status) != 0) { - error (g, _("external command failed, see earlier error messages")); - return -1; - } - return 0; - } - - /* Child. */ - - /* Set a sensible umask in the subprocess, so kernel and initrd - * output files are world-readable (RHBZ#610880). - */ - umask (0022); - - execvp ("febootstrap-supermin-helper", (char * const *) argv); - perror ("execvp"); - _exit (EXIT_FAILURE); -} - -static void -print_febootstrap_command_line (guestfs_h *g, const char *argv[]) -{ - size_t i; - int needs_quote; - char *buf; - size_t len; - - /* Calculate length of the buffer needed. This is an overestimate. */ - len = 0; - for (i = 0; argv[i] != NULL; ++i) - len += strlen (argv[i]) + 32; - - buf = malloc (len); - if (buf == NULL) { - warning (g, "malloc: %m"); - return; - } - - len = 0; - for (i = 0; argv[i] != NULL; ++i) { - if (i > 0) { - strcpy (&buf[len], " "); - len++; - } - - /* Does it need shell quoting? This only deals with simple cases. */ - needs_quote = strcspn (argv[i], " ") != strlen (argv[i]); - - if (needs_quote) { - strcpy (&buf[len], "'"); - len++; - } - - strcpy (&buf[len], argv[i]); - len += strlen (argv[i]); - - if (needs_quote) { - strcpy (&buf[len], "'"); - len++; - } - } - - guestfs___print_timestamped_message (g, "%s", buf); - - free (buf); + return 0; } /* Search elements of g->path, returning the first path element which -- 1.7.11.4

From: "Richard W.M. Jones" <rjones@redhat.com> --- src/launch-libvirt.c | 104 +++++++++------------------------------------------ 1 file changed, 17 insertions(+), 87 deletions(-) diff --git a/src/launch-libvirt.c b/src/launch-libvirt.c index a18e4d5..8bdf117 100644 --- a/src/launch-libvirt.c +++ b/src/launch-libvirt.c @@ -1153,12 +1153,8 @@ ignore_errors (void *ignore, virErrorPtr ignore2) static char * make_qcow2_overlay (guestfs_h *g, const char *path, const char *format) { - char *tmpfile = NULL; - int fd[2] = { -1, -1 }; - pid_t pid = -1; - FILE *fp = NULL; - char *line = NULL; - size_t len; + char *tmpfile; + struct command *cmd; int r; /* Path must be absolute. */ @@ -1167,98 +1163,32 @@ make_qcow2_overlay (guestfs_h *g, const char *path, const char *format) tmpfile = safe_asprintf (g, "%s/snapshot%d", g->tmpdir, ++g->unique); - /* Because 'qemu-img create' spews junk to stdout and stderr, pass - * all output from it up through the event system. - * XXX Like libvirt, we should create a generic library for running - * commands. - */ - if (pipe2 (fd, O_CLOEXEC) == -1) { - perrorf (g, "pipe2"); - goto error; - } - - pid = fork (); - if (pid == -1) { - perrorf (g, "fork"); - goto error; + cmd = guestfs___new_command (g); + guestfs___cmd_add_arg (cmd, "qemu-img"); + guestfs___cmd_add_arg (cmd, "create"); + guestfs___cmd_add_arg (cmd, "-f"); + guestfs___cmd_add_arg (cmd, "qcow2"); + guestfs___cmd_add_arg (cmd, "-b"); + guestfs___cmd_add_arg (cmd, path); + if (format) { + guestfs___cmd_add_arg (cmd, "-o"); + guestfs___cmd_add_arg_format (cmd, "backing_fmt=%s", format); } - - if (pid == 0) { /* child: qemu-img create command */ - /* Capture stdout and stderr. */ - close (fd[0]); - dup2 (fd[1], 1); - dup2 (fd[1], 2); - close (fd[1]); - - setenv ("LC_ALL", "C", 1); - - if (!format) - execlp ("qemu-img", "qemu-img", "create", "-f", "qcow2", - "-b", path, tmpfile, NULL); - else { - size_t len = strlen (format); - char backing_fmt[len+64]; - - snprintf (backing_fmt, len+64, "backing_fmt=%s", format); - - execlp ("qemu-img", "qemu-img", "create", "-f", "qcow2", - "-b", path, "-o", backing_fmt, tmpfile, NULL); - } - - perror ("could not execute 'qemu-img create' command"); - _exit (EXIT_FAILURE); - } - - close (fd[1]); - fd[1] = -1; - - fp = fdopen (fd[0], "r"); - if (fp == NULL) { - perrorf (g, "fdopen: qemu-img create"); - goto error; - } - fd[0] = -1; - - while (getline (&line, &len, fp) != -1) { - guestfs___call_callbacks_message (g, GUESTFS_EVENT_LIBRARY, line, len); - } - - if (fclose (fp) == -1) { /* also closes fd[0] */ - perrorf (g, "fclose"); - fp = NULL; - goto error; - } - fp = NULL; - - free (line); - line = NULL; - - if (waitpid (pid, &r, 0) == -1) { - perrorf (g, "waitpid"); - pid = 0; + guestfs___cmd_add_arg (cmd, tmpfile); + r = guestfs___cmd_run (cmd); + if (r == -1) goto error; - } - pid = 0; - if (!WIFEXITED (r) || WEXITSTATUS (r) != 0) { error (g, _("qemu-img create: could not create snapshot over %s"), path); goto error; } + guestfs___cmd_close (cmd); return tmpfile; /* caller frees */ error: - if (fd[0] >= 0) - close (fd[0]); - if (fd[1] >= 0) - close (fd[1]); - if (fp != NULL) - fclose (fp); - if (pid > 0) - waitpid (pid, NULL, 0); - + guestfs___cmd_close (cmd); free (tmpfile); - free (line); return NULL; } -- 1.7.11.4

From: "Richard W.M. Jones" <rjones@redhat.com> --- src/dbdump.c | 181 ++++++++++++++++++++++++++++++++++------------------------- 1 file changed, 103 insertions(+), 78 deletions(-) diff --git a/src/dbdump.c b/src/dbdump.c index 201fa6e..6c9d7a7 100644 --- a/src/dbdump.c +++ b/src/dbdump.c @@ -39,111 +39,136 @@ #if defined(DB_DUMP) +static void read_db_dump_line (guestfs_h *g, void *datav, const char *line, size_t len); static unsigned char *convert_hex_to_binary (guestfs_h *g, const char *hex, size_t hexlen, size_t *binlen_rtn); +struct cb_data { + guestfs___db_dump_callback callback; + void *opaque; + enum { reading_header, + reading_key, reading_value, + reading_finished, + reading_failed } state; + unsigned char *key; + size_t keylen; +}; + /* This helper function is specialized to just reading the hash-format * output from db_dump/db4_dump. It's just enough to support the RPM - * database format. Note that the filename must not contain any shell - * characters (this is guaranteed by the caller). + * database format. */ int guestfs___read_db_dump (guestfs_h *g, const char *dumpfile, void *opaque, guestfs___db_dump_callback callback) { -#define cmd_len (strlen (dumpfile) + 64) - char cmd[cmd_len]; - FILE *pp = NULL; - char *line = NULL; - size_t len = 0; - ssize_t linelen; - unsigned char *key = NULL, *value = NULL; - size_t keylen, valuelen; - int ret = -1; - - snprintf (cmd, cmd_len, DB_DUMP " -k '%s'", dumpfile); - - debug (g, "read_db_dump command: %s", cmd); - - pp = popen (cmd, "r"); - if (pp == NULL) { - perrorf (g, "popen: %s", cmd); - goto out; - } + struct cb_data data; + struct command *cmd; + int r; - /* Ignore everything to end-of-header marker. */ - while ((linelen = getline (&line, &len, pp)) != -1) { - if (STRPREFIX (line, "HEADER=END")) - break; - } + data.callback = callback; + data.opaque = opaque; + data.state = reading_header; + data.key = NULL; + + cmd = guestfs___new_command (g); + guestfs___cmd_add_arg (cmd, DB_DUMP); + guestfs___cmd_add_arg (cmd, "-k"); + guestfs___cmd_add_arg (cmd, dumpfile); + guestfs___cmd_set_stdout_callback (cmd, read_db_dump_line, &data, 0); + + r = guestfs___cmd_run (cmd); + guestfs___cmd_close (cmd); + free (data.key); - if (linelen == -1) { - error (g, _("unexpected end of output from db_dump command before end of header")); - goto out; + if (r == -1) + return -1; + if (!WIFEXITED (r) || WEXITSTATUS (r) != 0) { + error (g, _("%s: command failed"), DB_DUMP); + return -1; + } + if (data.state != reading_finished) { + error (g, _("%s: unexpected error or end of output"), DB_DUMP); + return -1; } - /* Now read the key, value pairs. They are prefixed with a space and - * printed as hex strings, so convert those strings to binary. Pass - * the strings up to the callback function. - */ - while ((linelen = getline (&line, &len, pp)) != -1) { - if (STRPREFIX (line, "DATA=END")) - break; - - if (linelen < 1 || line[0] != ' ') { - error (g, _("unexpected line from db_dump command, no space prefix")); - goto out; - } + return 0; +} - if ((key = convert_hex_to_binary (g, &line[1], linelen-1, - &keylen)) == NULL) - goto out; +static void +read_db_dump_line (guestfs_h *g, void *datav, const char *line, size_t len) +{ + struct cb_data *data = datav; + unsigned char *value; + size_t valuelen; - if ((linelen = getline (&line, &len, pp)) == -1) - break; + switch (data->state) { + case reading_finished: + case reading_failed: + return; - if (linelen < 1 || line[0] != ' ') { - error (g, _("unexpected line from db_dump command, no space prefix")); - goto out; + case reading_header: + /* Ignore everything to end-of-header marker. */ + if (STRPREFIX (line, "HEADER=END")) + data->state = reading_key; + return; + + /* Read the key, value pairs using a state machine. They are + * prefixed with a space and printed as hex strings, so convert + * those strings to binary. Pass the strings up to the callback + * function. + */ + case reading_key: + if (STRPREFIX (line, "DATA=END")) { + data->state = reading_finished; + return; } - if ((value = convert_hex_to_binary (g, &line[1], linelen-1, - &valuelen)) == NULL) - goto out; + if (len < 1 || line[0] != ' ') { + debug (g, _("unexpected line from db_dump command, no space prefix")); + data->state = reading_failed; + return; + } - if (callback (g, key, keylen, value, valuelen, opaque) == -1) - goto out; + data->key = convert_hex_to_binary (g, &line[1], len-1, &data->keylen); + if (data->key == NULL) { + data->state = reading_failed; + return; + } - free (key); - free (value); - key = value = NULL; - } + data->state = reading_value; + return; - if (linelen == -1) { - error (g, _("unexpected end of output from db_dump command before end of data")); - goto out; - } + case reading_value: + if (len < 1 || line[0] != ' ') { + debug (g, _("unexpected line from db_dump command, no space prefix")); + data->state = reading_failed; + return; + } - /* Catch errors from the db_dump command. */ - if (pclose (pp) != 0) { - perrorf (g, "pclose: %s", cmd); - pp = NULL; - goto out; - } - pp = NULL; + value = convert_hex_to_binary (g, &line[1], len-1, &valuelen); + if (value == NULL) { + data->state = reading_failed; + return; + } - ret = 0; + if (data->callback (g, data->key, data->keylen, + value, valuelen, data->opaque) == -1) { + data->state = reading_failed; + return; + } - out: - if (pp) - pclose (pp); + free (data->key); + data->key = NULL; + free (value); + value = NULL; - free (line); - free (key); - free (value); + data->state = reading_key; + return; - return ret; -#undef cmd_len + default: + abort (); + } } static int -- 1.7.11.4

From: "Richard W.M. Jones" <rjones@redhat.com> --- src/fuse.c | 89 ++++++++++++++++++++------------------------------------------ 1 file changed, 28 insertions(+), 61 deletions(-) diff --git a/src/fuse.c b/src/fuse.c index 7342e5c..76d008a 100644 --- a/src/fuse.c +++ b/src/fuse.c @@ -1044,17 +1044,19 @@ guestfs___free_fuse (guestfs_h *g) free_dir_caches (g); } -static int do_fusermount (guestfs_h *g, const char *localmountpoint, int error_fd); - int guestfs__umount_local (guestfs_h *g, const struct guestfs_umount_local_argv *optargs) { + int ret = -1; size_t i, tries; char *localmountpoint; char *fusermount_log = NULL; - int error_fd = -1; - int ret = -1; + struct command *cmd = NULL; + int r; + FILE *fp; + char error_message[4096]; + size_t n; /* How many times should we try the fusermount command? */ if (optargs->bitmask & GUESTFS_UMOUNT_LOCAL_RETRY_BITMASK) @@ -1081,22 +1083,22 @@ guestfs__umount_local (guestfs_h *g, * all 'tries' have failed do we print the contents of this file. A * temporary failure when retry == true will not cause any error. */ - fusermount_log = safe_asprintf (g, "%s/fusermount.log", g->tmpdir); - error_fd = open (fusermount_log, - O_RDWR|O_CREAT|O_TRUNC|O_NOCTTY /* not O_CLOEXEC */, - 0600); - if (error_fd == -1) { - perrorf (g, _("open: %s"), fusermount_log); - goto out; - } + fusermount_log = safe_asprintf (g, "%s/fusermount%d", g->tmpdir, ++g->unique); for (i = 0; i < tries; ++i) { - int r; - - r = do_fusermount (g, localmountpoint, error_fd); + cmd = guestfs___new_command (g); + guestfs___cmd_add_string_unquoted (cmd, "fusermount -u "); + guestfs___cmd_add_string_quoted (cmd, localmountpoint); + guestfs___cmd_add_string_unquoted (cmd, " > "); + guestfs___cmd_add_string_quoted (cmd, fusermount_log); + guestfs___cmd_add_string_unquoted (cmd, " 2>&1"); + guestfs___cmd_clear_capture_errors (cmd); + r = guestfs___cmd_run (cmd); + guestfs___cmd_close (cmd); + cmd = NULL; if (r == -1) goto out; - if (r) { + if (WIFEXITED (r) && WEXITSTATUS (r) == EXIT_SUCCESS) { /* External fusermount succeeded. Note that the original thread * is responsible for setting g->localmountpoint to NULL. */ @@ -1108,26 +1110,25 @@ guestfs__umount_local (guestfs_h *g, } if (ret == -1) { /* fusermount failed */ - char error_message[4096]; - ssize_t n; - /* Get the error message from the log file. */ - if (lseek (error_fd, 0, SEEK_SET) >= 0 && - (n = read (error_fd, error_message, sizeof error_message - 1)) > 0) { + fp = fopen (fusermount_log, "r"); + if (fp != NULL) { + n = fread (error_message, 1, sizeof error_message, fp); while (n > 0 && error_message[n-1] == '\n') n--; error_message[n] = '\0'; + fclose (fp); + error (g, _("fusermount failed: %s: %s"), localmountpoint, error_message); } else { - snprintf (error_message, sizeof error_message, - "(fusermount error could not be preserved)"); + perrorf (g, + _("fusermount failed: %s: " + "original error could not be preserved"), localmountpoint); } - - error (g, _("fusermount failed: %s: %s"), localmountpoint, error_message); - goto out; } out: - if (error_fd >= 0) close (error_fd); + if (cmd) + guestfs___cmd_close (cmd); if (fusermount_log) { unlink (fusermount_log); free (fusermount_log); @@ -1136,40 +1137,6 @@ guestfs__umount_local (guestfs_h *g, return ret; } -static int -do_fusermount (guestfs_h *g, const char *localmountpoint, int error_fd) -{ - pid_t pid; - int status; - - pid = fork (); - if (pid == -1) { - perrorf (g, "fork"); - return -1; - } - - if (pid == 0) { /* child */ - /* Ensure stdout and stderr point to the error_fd. */ - dup2 (error_fd, STDOUT_FILENO); - dup2 (error_fd, STDERR_FILENO); - close (error_fd); - execlp ("fusermount", "fusermount", "-u", localmountpoint, NULL); - perror ("exec: fusermount"); - _exit (EXIT_FAILURE); - } - - /* Parent. */ - if (waitpid (pid, &status, 0) == -1) { - perrorf (g, "waitpid"); - return -1; - } - - if (!WIFEXITED (status) || WEXITSTATUS (status) != EXIT_SUCCESS) - return 0; /* it failed to unmount the mountpoint */ - - return 1; /* unmount was successful */ -} - /* Functions handling the directory cache. * * Note on attribute caching: FUSE can cache filesystem attributes for -- 1.7.11.4