Richard W.M. Jones
2012-Oct-18 21:14 UTC
[Libguestfs] [PATCH 0/10] Add a mini-library for running external commands.
Inspired by libvirt's virCommand* internal mini-library, this adds some internal APIs for running commands. The first patch contains the new APIs. The subsequent patches change various parts of the library over to use it. Rich.
Richard W.M. Jones
2012-Oct-18 21:14 UTC
[Libguestfs] [PATCH 01/10] lib: Add a new 'command' mini-library for running external commands.
From: "Richard W.M. Jones" <rjones at 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
Richard W.M. Jones
2012-Oct-18 21:14 UTC
[Libguestfs] [PATCH 02/10] lib: Change guestfs___remove_tmpdir function to use command mini-library.
From: "Richard W.M. Jones" <rjones at redhat.com>
---
src/appliance.c | 20 ++++++++++----------
src/filearch.c | 2 +-
src/guestfs-internal.h | 2 +-
src/guestfs.c | 2 +-
src/launch.c | 29 ++++++++++-------------------
5 files changed, 23 insertions(+), 32 deletions(-)
diff --git a/src/appliance.c b/src/appliance.c
index ecbb7af..9db42cb 100644
--- a/src/appliance.c
+++ b/src/appliance.c
@@ -500,7 +500,7 @@ build_supermin_appliance (guestfs_h *g,
int r = run_supermin_helper (g, supermin_path, tmpcd, len);
if (r == -1) {
- guestfs___remove_tmpdir (tmpcd);
+ guestfs___remove_tmpdir (g, tmpcd);
return -1;
}
@@ -519,7 +519,7 @@ build_supermin_appliance (guestfs_h *g,
int fd = open (filename, O_WRONLY|O_CREAT|O_NOCTTY|O_CLOEXEC, 0755);
if (fd == -1) {
perrorf (g, "open: %s", filename);
- guestfs___remove_tmpdir (tmpcd);
+ guestfs___remove_tmpdir (g, tmpcd);
return -1;
}
struct flock fl;
@@ -533,7 +533,7 @@ build_supermin_appliance (guestfs_h *g,
goto again;
perrorf (g, "fcntl: F_SETLKW: %s", filename);
close (fd);
- guestfs___remove_tmpdir (tmpcd);
+ guestfs___remove_tmpdir (g, tmpcd);
return -1;
}
@@ -545,7 +545,7 @@ build_supermin_appliance (guestfs_h *g,
if (ftruncate (fd, clen) == -1) {
perrorf (g, "ftruncate: %s", filename);
close (fd);
- guestfs___remove_tmpdir (tmpcd);
+ guestfs___remove_tmpdir (g, tmpcd);
return -1;
}
@@ -553,13 +553,13 @@ build_supermin_appliance (guestfs_h *g,
if (rr == -1) {
perrorf (g, "write: %s", filename);
close (fd);
- guestfs___remove_tmpdir (tmpcd);
+ guestfs___remove_tmpdir (g, tmpcd);
return -1;
}
if ((size_t) rr != clen) {
error (g, "partial write: %s", filename);
close (fd);
- guestfs___remove_tmpdir (tmpcd);
+ guestfs___remove_tmpdir (g, tmpcd);
return -1;
}
@@ -569,7 +569,7 @@ build_supermin_appliance (guestfs_h *g,
if (rename (filename, filename2) == -1) {
perrorf (g, "rename: %s %s", filename, filename2);
close (fd);
- guestfs___remove_tmpdir (tmpcd);
+ guestfs___remove_tmpdir (g, tmpcd);
return -1;
}
@@ -579,7 +579,7 @@ build_supermin_appliance (guestfs_h *g,
if (rename (filename, filename2) == -1) {
perrorf (g, "rename: %s %s", filename, filename2);
close (fd);
- guestfs___remove_tmpdir (tmpcd);
+ guestfs___remove_tmpdir (g, tmpcd);
return -1;
}
@@ -589,11 +589,11 @@ build_supermin_appliance (guestfs_h *g,
if (rename (filename, filename2) == -1) {
perrorf (g, "rename: %s %s", filename, filename2);
close (fd);
- guestfs___remove_tmpdir (tmpcd);
+ guestfs___remove_tmpdir (g, tmpcd);
return -1;
}
- guestfs___remove_tmpdir (tmpcd);
+ guestfs___remove_tmpdir (g, tmpcd);
/* Now finish off by linking to the cached appliance and returning it. */
if (hard_link_to_cached_appliance (g, cachedir,
diff --git a/src/filearch.c b/src/filearch.c
index 0b5d37b..10c1d4a 100644
--- a/src/filearch.c
+++ b/src/filearch.c
@@ -214,7 +214,7 @@ cpio_arch (guestfs_h *g, const char *file, const char *path)
error (g, "file_architecture: could not determine architecture of cpio
archive");
out:
- guestfs___remove_tmpdir (dir);
+ guestfs___remove_tmpdir (g, dir);
return ret;
#undef dir_len
diff --git a/src/guestfs-internal.h b/src/guestfs-internal.h
index cf810e1..2cfa35b 100644
--- a/src/guestfs-internal.h
+++ b/src/guestfs-internal.h
@@ -526,7 +526,7 @@ extern int guestfs___build_appliance (guestfs_h *g, char
**kernel, char **initrd
/* launch.c */
extern const char *guestfs___persistent_tmpdir (void);
extern int guestfs___lazy_make_tmpdir (guestfs_h *g);
-extern void guestfs___remove_tmpdir (const char *dir);
+extern void guestfs___remove_tmpdir (guestfs_h *g, const char *dir);
extern int64_t guestfs___timeval_diff (const struct timeval *x, const struct
timeval *y);
extern void guestfs___print_timestamped_message (guestfs_h *g, const char *fs,
...);
extern void guestfs___launch_send_progress (guestfs_h *g, int perdozen);
diff --git a/src/guestfs.c b/src/guestfs.c
index cc645fd..e755ffa 100644
--- a/src/guestfs.c
+++ b/src/guestfs.c
@@ -302,7 +302,7 @@ guestfs_close (guestfs_h *g)
/* Remove whole temporary directory. */
if (g->tmpdir)
- guestfs___remove_tmpdir (g->tmpdir);
+ guestfs___remove_tmpdir (g, g->tmpdir);
/* Test output file used by bindtests. */
if (g->test_fp != NULL)
diff --git a/src/launch.c b/src/launch.c
index cc45a10..4681707 100644
--- a/src/launch.c
+++ b/src/launch.c
@@ -760,29 +760,20 @@ guestfs___lazy_make_tmpdir (guestfs_h *g)
/* Recursively remove a temporary directory. If removal fails, just
* return (it's a temporary directory so it'll eventually be cleaned
* up by a temp cleaner). This is done using "rm -rf" because
that's
- * simpler and safer, but we have to exec to ensure that paths don't
- * need to be quoted.
+ * simpler and safer.
*/
void
-guestfs___remove_tmpdir (const char *dir)
+guestfs___remove_tmpdir (guestfs_h *g, const char *dir)
{
- pid_t pid = fork ();
+ struct command *cmd;
- if (pid == -1) {
- perror ("remove tmpdir: fork");
- return;
- }
- if (pid == 0) {
- execlp ("rm", "rm", "-rf", dir, NULL);
- perror ("remove tmpdir: exec: rm");
- _exit (EXIT_FAILURE);
- }
-
- /* Parent. */
- if (waitpid (pid, NULL, 0) == -1) {
- perror ("remove tmpdir: waitpid");
- return;
- }
+ cmd = guestfs___new_command (g);
+ guestfs___cmd_add_arg (cmd, "rm");
+ guestfs___cmd_add_arg (cmd, "-rf");
+ guestfs___cmd_add_arg (cmd, dir);
+ /* Ignore failures. */
+ guestfs___cmd_run (cmd);
+ guestfs___cmd_close (cmd);
}
int
--
1.7.11.4
Richard W.M. Jones
2012-Oct-18 21:14 UTC
[Libguestfs] [PATCH 03/10] appliance: Use command mini-library to run febootstrap-supermin-helper (RHBZ#713678)
From: "Richard W.M. Jones" <rjones at 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
Richard W.M. Jones
2012-Oct-18 21:14 UTC
[Libguestfs] [PATCH 04/10] launch: appliance: Use command mini-library to parse output of qemu -help etc.
From: "Richard W.M. Jones" <rjones at redhat.com>
---
src/launch-appliance.c | 112 +++++++++++++++++++++----------------------------
1 file changed, 48 insertions(+), 64 deletions(-)
diff --git a/src/launch-appliance.c b/src/launch-appliance.c
index 378f121..17e90d9 100644
--- a/src/launch-appliance.c
+++ b/src/launch-appliance.c
@@ -729,8 +729,7 @@ print_qemu_command_line (guestfs_h *g, char **argv)
}
}
-static int test_qemu_cmd (guestfs_h *g, const char *cmd, char **ret);
-static int read_all (guestfs_h *g, FILE *fp, char **ret);
+static void read_all (guestfs_h *g, void *retv, const char *buf, size_t len);
/* Test qemu binary (or wrapper) runs, and do 'qemu -help' and
* 'qemu -version' so we know what options this qemu supports and
@@ -739,7 +738,8 @@ static int read_all (guestfs_h *g, FILE *fp, char **ret);
static int
test_qemu (guestfs_h *g)
{
- char cmd[1024];
+ struct command *cmd;
+ int r;
free (g->app.qemu_help);
g->app.qemu_help = NULL;
@@ -748,74 +748,58 @@ test_qemu (guestfs_h *g)
free (g->app.qemu_devices);
g->app.qemu_devices = NULL;
- snprintf (cmd, sizeof cmd, "LC_ALL=C '%s' -nographic
-help", g->qemu);
-
- /* If this command doesn't work then it probably indicates that the
- * qemu binary is missing.
- */
- if (test_qemu_cmd (g, cmd, &g->app.qemu_help) == -1) {
- qemu_error:
- error (g, _("command failed: %s\nerrno: %s\n\nIf qemu is located on a
non-standard path, try setting the LIBGUESTFS_QEMU\nenvironment variable. There
may also be errors printed above."),
- cmd, strerror (errno));
- return -1;
- }
-
- snprintf (cmd, sizeof cmd, "LC_ALL=C '%s' -nographic -version
2>/dev/null",
- g->qemu);
-
- if (test_qemu_cmd (g, cmd, &g->app.qemu_version) == -1)
- goto qemu_error;
-
- snprintf (cmd, sizeof cmd,
- "LC_ALL=C '%s' -nographic -machine accel=kvm:tcg
-device '?' 2>&1",
- g->qemu);
-
- if (test_qemu_cmd (g, cmd, &g->app.qemu_devices) == -1)
- goto qemu_error;
+ cmd = guestfs___new_command (g);
+ guestfs___cmd_add_arg (cmd, g->qemu);
+ guestfs___cmd_add_arg (cmd, "-nographic");
+ guestfs___cmd_add_arg (cmd, "-help");
+ guestfs___cmd_set_stdout_callback (cmd, read_all, &g->app.qemu_help,
+ CMD_STDOUT_FLAG_WHOLE_BUFFER);
+ r = guestfs___cmd_run (cmd);
+ if (r == -1 || !WIFEXITED (r) || WEXITSTATUS (r) != 0)
+ goto error;
+ guestfs___cmd_close (cmd);
+
+ cmd = guestfs___new_command (g);
+ guestfs___cmd_add_arg (cmd, g->qemu);
+ guestfs___cmd_add_arg (cmd, "-nographic");
+ guestfs___cmd_add_arg (cmd, "-version");
+ guestfs___cmd_set_stdout_callback (cmd, read_all,
&g->app.qemu_version,
+ CMD_STDOUT_FLAG_WHOLE_BUFFER);
+ r = guestfs___cmd_run (cmd);
+ if (r == -1 || !WIFEXITED (r) || WEXITSTATUS (r) != 0)
+ goto error;
+ guestfs___cmd_close (cmd);
+
+ cmd = guestfs___new_command (g);
+ guestfs___cmd_add_arg (cmd, g->qemu);
+ guestfs___cmd_add_arg (cmd, "-nographic");
+ guestfs___cmd_add_arg (cmd, "-machine");
+ guestfs___cmd_add_arg (cmd, "accel=kvm:tcg");
+ guestfs___cmd_add_arg (cmd, "-device");
+ guestfs___cmd_add_arg (cmd, "?");
+ guestfs___cmd_clear_capture_errors (cmd);
+ guestfs___cmd_set_stderr_to_stdout (cmd);
+ guestfs___cmd_set_stdout_callback (cmd, read_all,
&g->app.qemu_devices,
+ CMD_STDOUT_FLAG_WHOLE_BUFFER);
+ r = guestfs___cmd_run (cmd);
+ if (r == -1 || !WIFEXITED (r) || WEXITSTATUS (r) != 0)
+ goto error;
+ guestfs___cmd_close (cmd);
return 0;
-}
-
-static int
-test_qemu_cmd (guestfs_h *g, const char *cmd, char **ret)
-{
- FILE *fp;
- fp = popen (cmd, "r");
- if (fp == NULL)
- return -1;
-
- if (read_all (g, fp, ret) == -1) {
- pclose (fp);
- return -1;
- }
-
- if (pclose (fp) != 0)
- return -1;
-
- return 0;
+ error:
+ error (g, _("qemu command failed\nIf qemu is located on a non-standard
path, try setting the LIBGUESTFS_QEMU\nenvironment variable. There may also be
errors printed above."));
+ guestfs___cmd_close (cmd);
+ return -1;
}
-static int
-read_all (guestfs_h *g, FILE *fp, char **ret)
+static void
+read_all (guestfs_h *g, void *retv, const char *buf, size_t len)
{
- int r, n = 0;
- char *p;
-
- again:
- if (feof (fp)) {
- *ret = safe_realloc (g, *ret, n + 1);
- (*ret)[n] = '\0';
- return n;
- }
+ char **ret = retv;
- *ret = safe_realloc (g, *ret, n + BUFSIZ);
- p = &(*ret)[n];
- r = fread (p, 1, BUFSIZ, fp);
- if (ferror (fp))
- return -1;
- n += r;
- goto again;
+ *ret = safe_memdup (g, buf, len);
}
/* Test if option is supported by qemu command line (just by grepping
--
1.7.11.4
Richard W.M. Jones
2012-Oct-18 21:14 UTC
[Libguestfs] [PATCH 05/10] launch: libvirt: Use command mini-library to run qemu-img create command.
From: "Richard W.M. Jones" <rjones at 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
Richard W.M. Jones
2012-Oct-18 21:14 UTC
[Libguestfs] [PATCH 06/10] inspect: Change icon code to use command mini-lib instead of system(3).
From: "Richard W.M. Jones" <rjones at redhat.com>
---
src/inspect-icon.c | 119 ++++++++++++++++++++++++++++++++---------------------
1 file changed, 73 insertions(+), 46 deletions(-)
diff --git a/src/inspect-icon.c b/src/inspect-icon.c
index 531ba61..4831ab1 100644
--- a/src/inspect-icon.c
+++ b/src/inspect-icon.c
@@ -364,7 +364,7 @@ icon_cirros (guestfs_h *g, struct inspect_fs *fs, size_t
*size_r)
char *type = NULL;
char *local = NULL;
char *pngfile = NULL;
- char *cmd = NULL;
+ struct command *cmd = NULL;
int r;
r = guestfs_exists (g, CIRROS_LOGO);
@@ -386,13 +386,16 @@ icon_cirros (guestfs_h *g, struct inspect_fs *fs, size_t
*size_r)
/* Use pbmtext to render it. */
pngfile = safe_asprintf (g, "%s/cirros.png", g->tmpdir);
- cmd = safe_asprintf (g, PBMTEXT " < %s | " PNMTOPNG " >
%s",
- local, pngfile);
- r = system (cmd);
- if (r == -1 || WEXITSTATUS (r) != 0) {
- debug (g, "external command failed: %s", cmd);
+ cmd = guestfs___new_command (g);
+ guestfs___cmd_add_string_unquoted (cmd, PBMTEXT " < ");
+ guestfs___cmd_add_string_quoted (cmd, local);
+ guestfs___cmd_add_string_unquoted (cmd, " | " PNMTOPNG " >
");
+ guestfs___cmd_add_string_quoted (cmd, pngfile);
+ r = guestfs___cmd_run (cmd);
+ if (r == -1)
+ goto out;
+ if (!WIFEXITED (r) || WEXITSTATUS (r) != 0)
goto out;
- }
/* Read it into memory. */
if (read_whole_file (g, pngfile, &ret, size_r) == -1) {
@@ -402,9 +405,10 @@ icon_cirros (guestfs_h *g, struct inspect_fs *fs, size_t
*size_r)
out:
free (pngfile);
- free (cmd);
free (local);
free (type);
+ if (cmd)
+ guestfs___cmd_close (cmd);
return ret;
}
@@ -433,34 +437,45 @@ icon_windows_xp (guestfs_h *g, struct inspect_fs *fs,
const char *explorer,
size_t *size_r)
{
char *ret;
- char *pngfile;
- char *cmd;
+ char *pngfile = NULL;
+ struct command *cmd = NULL;
int r;
pngfile = safe_asprintf (g, "%s/windows-xp-icon.png",
g->tmpdir);
- cmd = safe_asprintf (g,
- WRESTOOL " -x --type=2 --name=143 %s | "
- BMPTOPNM " 2>/dev/null | " PNMTOPNG "
> %s",
- explorer, pngfile);
- r = system (cmd);
- if (r == -1 || WEXITSTATUS (r) != 0) {
- debug (g, "external command failed: %s", cmd);
- free (cmd);
- free (pngfile);
- return NOT_FOUND;
- }
+ cmd = guestfs___new_command (g);
+ guestfs___cmd_add_string_unquoted (cmd, WRESTOOL " -x --type=2
--name=143 ");
+ guestfs___cmd_add_string_quoted (cmd, explorer);
+ guestfs___cmd_add_string_unquoted (cmd,
+ " | " BMPTOPNM " | "
PNMTOPNG " > ");
+ guestfs___cmd_add_string_quoted (cmd, pngfile);
+ r = guestfs___cmd_run (cmd);
+ if (r == -1)
+ goto error;
+ if (!WIFEXITED (r) || WEXITSTATUS (r) != 0)
+ goto not_found;
- free (cmd);
+ guestfs___cmd_close (cmd);
+ cmd = NULL;
- if (read_whole_file (g, pngfile, &ret, size_r) == -1) {
- free (pngfile);
- return NULL;
- }
+ if (read_whole_file (g, pngfile, &ret, size_r) == -1)
+ goto error;
free (pngfile);
return ret;
+
+ error:
+ if (cmd)
+ guestfs___cmd_close (cmd);
+ free (pngfile);
+ return NULL;
+
+ not_found:
+ if (cmd)
+ guestfs___cmd_close (cmd);
+ free (pngfile);
+ return NOT_FOUND;
}
static char *
@@ -468,35 +483,47 @@ icon_windows_7 (guestfs_h *g, struct inspect_fs *fs, const
char *explorer,
size_t *size_r)
{
char *ret;
- char *pngfile;
- char *cmd;
+ char *pngfile = NULL;
+ struct command *cmd = NULL;
int r;
pngfile = safe_asprintf (g, "%s/windows-7-icon.png", g->tmpdir);
- cmd = safe_asprintf (g,
- WRESTOOL " -x --type=2 --name=6801 %s | "
- BMPTOPNM " 2>/dev/null | " PAMCUT "
-bottom 54 | "
- PNMTOPNG " > %s",
- explorer, pngfile);
- r = system (cmd);
- if (r == -1 || WEXITSTATUS (r) != 0) {
- debug (g, "external command failed: %s", cmd);
- free (cmd);
- free (pngfile);
- return NOT_FOUND;
- }
+ cmd = guestfs___new_command (g);
+ guestfs___cmd_add_string_unquoted (cmd, WRESTOOL " -x --type=2
--name=6801 ");
+ guestfs___cmd_add_string_quoted (cmd, explorer);
+ guestfs___cmd_add_string_unquoted (cmd,
+ " | " BMPTOPNM " | "
+ PAMCUT " -bottom 54 | "
+ PNMTOPNG " > ");
+ guestfs___cmd_add_string_quoted (cmd, pngfile);
+ r = guestfs___cmd_run (cmd);
+ if (r == -1)
+ goto error;
+ if (!WIFEXITED (r) || WEXITSTATUS (r) != 0)
+ goto not_found;
+
+ guestfs___cmd_close (cmd);
+ cmd = NULL;
+
+ if (read_whole_file (g, pngfile, &ret, size_r) == -1)
+ goto error;
- free (cmd);
+ free (pngfile);
- if (read_whole_file (g, pngfile, &ret, size_r) == -1) {
- free (pngfile);
- return NULL;
- }
+ return ret;
+ error:
+ if (cmd)
+ guestfs___cmd_close (cmd);
free (pngfile);
+ return NULL;
- return ret;
+ not_found:
+ if (cmd)
+ guestfs___cmd_close (cmd);
+ free (pngfile);
+ return NOT_FOUND;
}
static char *
--
1.7.11.4
Richard W.M. Jones
2012-Oct-18 21:14 UTC
[Libguestfs] [PATCH 07/10] inspect: Use command mini-library to parse the output of db_dump command.
From: "Richard W.M. Jones" <rjones at 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
Richard W.M. Jones
2012-Oct-18 21:14 UTC
[Libguestfs] [PATCH 08/10] info: Use command mini-library to run 'qemu-img info' commands.
From: "Richard W.M. Jones" <rjones at redhat.com>
---
src/info.c | 224 +++++++++++++++++++++++++------------------------------------
1 file changed, 91 insertions(+), 133 deletions(-)
diff --git a/src/info.c b/src/info.c
index 958f7b0..156c11f 100644
--- a/src/info.c
+++ b/src/info.c
@@ -33,124 +33,142 @@
#include "guestfs-internal-actions.h"
#include "guestfs_protocol.h"
-static int run_qemu_img_info (guestfs_h *g, const char *filename, int (*fn)
(guestfs_h *g, char *line, void *data), void *data);
-static int check_disk_format (guestfs_h *h, char *line, void *data);
-static int check_disk_virtual_size (guestfs_h *h, char *line, void *data);
-static int check_disk_has_backing_file (guestfs_h *h, char *line, void *data);
+static int run_qemu_img_info (guestfs_h *g, const char *filename,
cmd_stdout_callback cb, void *data);
+
+/* NB: For security reasons, the check_* callbacks MUST bail
+ * after seeing the first line that matches /^backing file: /. See:
+ * https://lists.gnu.org/archive/html/qemu-devel/2012-09/msg00137.html
+ * Eventually we should use the JSON output of qemu-img info.
+ */
+
+struct check_data {
+ int stop, failed;
+ union {
+ char *ret;
+ int reti;
+ int64_t reti64;
+ };
+};
+
+static void check_disk_format (guestfs_h *g, void *data, const char *line,
size_t len);
+static void check_disk_virtual_size (guestfs_h *g, void *data, const char
*line, size_t len);
+static void check_disk_has_backing_file (guestfs_h *g, void *data, const char
*line, size_t len);
char *
guestfs__disk_format (guestfs_h *g, const char *filename)
{
- char *ret = NULL;
+ struct check_data data;
+
+ memset (&data, 0, sizeof data);
- if (run_qemu_img_info (g, filename, check_disk_format, &ret) == -1) {
- free (ret);
+ if (run_qemu_img_info (g, filename, check_disk_format, &data) == -1) {
+ free (data.ret);
return NULL;
}
- if (ret == NULL)
- ret = safe_strdup (g, "unknown");
+ if (data.ret == NULL)
+ data.ret = safe_strdup (g, "unknown");
- return ret;
+ return data.ret;
}
-static int
-check_disk_format (guestfs_h *g, char *line, void *retpv)
+static void
+check_disk_format (guestfs_h *g, void *datav, const char *line, size_t len)
{
- char **retp = retpv;
- char *p;
- size_t n;
+ struct check_data *data = datav;
+ const char *p;
+
+ if (data->stop)
+ return;
+
+ if (STRPREFIX (line, "backing file: ")) {
+ data->stop = 1;
+ return;
+ }
if (STRPREFIX (line, "file format: ")) {
p = &line[13];
- n = strlen (p);
- if (n > 0 && p[n-1] == '\n')
- p[n-1] = '\0';
- *retp = safe_strdup (g, p);
- return 0; /* finish processing */
+ data->ret = safe_strdup (g, p);
+ data->stop = 1;
}
-
- return 1; /* continue processing */
}
int64_t
guestfs__disk_virtual_size (guestfs_h *g, const char *filename)
{
- int64_t ret = -1;
+ struct check_data data;
- if (run_qemu_img_info (g, filename, check_disk_virtual_size, &ret) == -1)
+ memset (&data, 0, sizeof data);
+
+ if (run_qemu_img_info (g, filename, check_disk_virtual_size, &data) ==
-1)
return -1;
- if (ret == -1)
+ if (data.failed)
error (g, _("%s: cannot detect virtual size of disk image"),
filename);
- return ret;
+ return data.reti64;
}
-static int
-check_disk_virtual_size (guestfs_h *g, char *line, void *retpv)
+static void
+check_disk_virtual_size (guestfs_h *g, void *datav,
+ const char *line, size_t len)
{
- int64_t *retp = retpv;
- char *p;
+ struct check_data *data = datav;
+ const char *p;
+
+ if (data->stop)
+ return;
+
+ if (STRPREFIX (line, "backing file: ")) {
+ data->stop = 1;
+ return;
+ }
if (STRPREFIX (line, "virtual size: ")) {
/* "virtual size: 500M (524288000 bytes)\n" */
p = &line[14];
p = strchr (p, ' ');
- if (!p || p[1] != '(' || sscanf (&p[2], "%" SCNi64,
retp) != 1) {
- error (g, _("cannot parse output of qemu-img info:
'%s'"),
- line);
- return -1;
- }
- return 0; /* finish processing */
+ if (!p || p[1] != '(' || sscanf (&p[2], "%" SCNi64,
&data->reti64) != 1)
+ data->failed = 1;
+ data->stop = 1;
}
-
- return 1; /* continue processing */
}
int
guestfs__disk_has_backing_file (guestfs_h *g, const char *filename)
{
- int ret = 0;
+ struct check_data data;
+
+ memset (&data, 0, sizeof data);
- if (run_qemu_img_info (g, filename, check_disk_has_backing_file, &ret) ==
-1)
+ if (run_qemu_img_info (g, filename, check_disk_has_backing_file, &data)
== -1)
return -1;
- return ret;
+ return data.reti;
}
-static int
-check_disk_has_backing_file (guestfs_h *g, char *line, void *retpv)
+static void
+check_disk_has_backing_file (guestfs_h *g, void *datav,
+ const char *line, size_t len)
{
- int *retp = retpv;
+ struct check_data *data = datav;
+
+ if (data->stop)
+ return;
if (STRPREFIX (line, "backing file: ")) {
- *retp = 1;
- return 0; /* finish processing */
+ data->reti = 1;
+ data->stop = 1;
}
-
- return 1; /* continue processing */
}
-/* It's hard to use 'qemu-img info' safely. See:
- * https://lists.gnu.org/archive/html/qemu-devel/2012-09/msg00137.html
- * Eventually we should switch to the JSON output format, when it
- * becomes available. In the meantime: (1) make a symlink to ensure
- * we control the input filename, and (2) bail parsing as soon as
- * /^backing file: / is seen in the input.
- */
static int
run_qemu_img_info (guestfs_h *g, const char *filename,
- int (*fn) (guestfs_h *g, char *line, void *data),
- void *data)
+ cmd_stdout_callback fn, void *data)
{
char *abs_filename = NULL;
char *safe_filename = NULL;
- pid_t pid = 0;
- int fd[2] = { -1, -1 };
- FILE *fp = NULL;
- char *line = NULL;
- size_t len;
+ struct command *cmd = NULL;
int r;
if (guestfs___lazy_make_tmpdir (g) == -1)
@@ -170,90 +188,30 @@ run_qemu_img_info (guestfs_h *g, const char *filename,
goto error;
}
- if (pipe2 (fd, O_CLOEXEC) == -1) {
- perrorf (g, "pipe2");
- goto error;
- }
-
- pid = fork ();
- if (pid == -1) {
- perrorf (g, "fork");
+ cmd = guestfs___new_command (g);
+ guestfs___cmd_add_arg (cmd, "qemu-img");
+ guestfs___cmd_add_arg (cmd, "info");
+ guestfs___cmd_add_arg (cmd, safe_filename);
+ guestfs___cmd_set_stdout_callback (cmd, fn, data, 0);
+ r = guestfs___cmd_run (cmd);
+ if (r == -1)
goto error;
- }
-
- if (pid == 0) { /* child */
- close (fd[0]);
- dup2 (fd[1], 1);
- close (fd[1]);
-
- setenv ("LC_ALL", "C", 1);
-
- /* XXX stderr to event log */
-
- execlp ("qemu-img", "qemu-img", "info",
safe_filename, NULL);
- perror ("could not execute 'qemu-img info' command");
- _exit (EXIT_FAILURE);
- }
-
- close (fd[1]);
- fd[1] = -1;
-
- fp = fdopen (fd[0], "r");
- if (fp == NULL) {
- perrorf (g, "fdopen: qemu-img info");
- goto error;
- }
- fd[0] = -1;
-
- while (getline (&line, &len, fp) != -1) {
- r = fn (g, line, data);
- if (r == -1) /* error */
- goto error;
- if (r == 0) /* finished processing */
- break;
-
- /* This is for security reasons, see comment above. */
- if (STRPREFIX (line, "backing file: "))
- break;
- }
-
- if (fclose (fp) == -1) { /* also closes fd[0] */
- perrorf (g, "fclose");
- fp = NULL;
- goto error;
- }
- fp = NULL;
-
- if (waitpid (pid, &r, 0) == -1) {
- perrorf (g, "waitpid");
- pid = 0;
- goto error;
- }
- pid = 0;
-
if (!WIFEXITED (r) || WEXITSTATUS (r) != 0) {
- error (g, "qemu-img: %s: child process failed", filename);
+ error (g, _("qemu-img: %s: child process failed"), filename);
goto error;
}
+ guestfs___cmd_close (cmd);
free (safe_filename);
free (abs_filename);
- free (line);
return 0;
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);
+ if (cmd)
+ guestfs___cmd_close (cmd);
free (safe_filename);
free (abs_filename);
- free (line);
return -1;
}
--
1.7.11.4
Richard W.M. Jones
2012-Oct-18 21:14 UTC
[Libguestfs] [PATCH 09/10] fuse: Use command mini-library to run the fusermount command.
From: "Richard W.M. Jones" <rjones at 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
Richard W.M. Jones
2012-Oct-18 21:14 UTC
[Libguestfs] [PATCH 10/10] filearch: Use command mini-library to run external cpio command.
From: "Richard W.M. Jones" <rjones at redhat.com>
---
src/filearch.c | 22 ++++++++++++++--------
1 file changed, 14 insertions(+), 8 deletions(-)
diff --git a/src/filearch.c b/src/filearch.c
index 10c1d4a..93a697f 100644
--- a/src/filearch.c
+++ b/src/filearch.c
@@ -131,8 +131,7 @@ cpio_arch (guestfs_h *g, const char *file, const char *path)
#define dir_len (strlen (dir))
#define initrd_len (dir_len + 16)
char initrd[initrd_len];
-#define cmd_len (dir_len + 256)
- char cmd[cmd_len];
+ struct command *cmd = NULL;
#define bin_len (dir_len + 32)
char bin[bin_len];
char *ret = NULL;
@@ -166,11 +165,16 @@ cpio_arch (guestfs_h *g, const char *file, const char
*path)
if (guestfs_download (g, path, initrd) == -1)
goto out;
- snprintf (cmd, cmd_len,
- "cd %s && %s initrd | cpio --quiet -id "
INITRD_BINARIES1,
- dir, method);
- r = system (cmd);
- if (r == -1 || WEXITSTATUS (r) != 0) {
+ cmd = guestfs___new_command (g);
+ guestfs___cmd_add_string_unquoted (cmd, "cd ");
+ guestfs___cmd_add_string_quoted (cmd, dir);
+ guestfs___cmd_add_string_unquoted (cmd, " && ");
+ guestfs___cmd_add_string_unquoted (cmd, method);
+ guestfs___cmd_add_string_unquoted (cmd,
+ " initrd | cpio --quiet -id "
+ INITRD_BINARIES1);
+ r = guestfs___cmd_run (cmd);
+ if (r == -1 || !WIFEXITED (r) || WEXITSTATUS (r) != 0) {
perrorf (g, "cpio command failed");
goto out;
}
@@ -214,12 +218,14 @@ cpio_arch (guestfs_h *g, const char *file, const char
*path)
error (g, "file_architecture: could not determine architecture of cpio
archive");
out:
+ if (cmd)
+ guestfs___cmd_close (cmd);
+
guestfs___remove_tmpdir (g, dir);
return ret;
#undef dir_len
#undef initrd_len
-#undef cmd_len
#undef bin_len
}
--
1.7.11.4
Apparently Analagous Threads
- [PATCH 0/7 v2] Make copy_in & copy_out APIs, and use copy_in in customize
- [PATCH 1/6] cmd: add a way to run (and wait) asynchronously commands
- [PATCH 0/7] copy-in/copy-out: Capture errors from tar subprocess (RHBZ#1267032).
- Remove temporary directories created during appliance building along error paths (RHBZ#769680)
- [PATCH 0/2] Change guestfs__*