Pino Toscano
2015-Jan-26 16:04 UTC
[Libguestfs] [PATCH 1/6] cmd: add a way to run (and wait) asynchronously commands
--- src/command.c | 64 +++++++++++++++++++++++++++++++++++++++++++------- src/guestfs-internal.h | 3 +++ 2 files changed, 58 insertions(+), 9 deletions(-) diff --git a/src/command.c b/src/command.c index 4bb469b..e26573d 100644 --- a/src/command.c +++ b/src/command.c @@ -360,7 +360,7 @@ debug_command (struct command *cmd) } static int -run_command (struct command *cmd) +run_command (struct command *cmd, bool get_stdout_fd, bool get_stderr_fd) { struct sigaction sa; int i, fd, max_fd, r; @@ -368,8 +368,11 @@ run_command (struct command *cmd) int outfd[2] = { -1, -1 }; char status_string[80]; + get_stdout_fd = get_stdout_fd || cmd->stdout_callback != NULL; + get_stderr_fd = get_stderr_fd || cmd->capture_errors; + /* Set up a pipe to capture command output and send it to the error log. */ - if (cmd->capture_errors) { + if (get_stderr_fd) { if (pipe2 (errorfd, O_CLOEXEC) == -1) { perrorf (cmd->g, "pipe2"); goto error; @@ -377,7 +380,7 @@ run_command (struct command *cmd) } /* Set up a pipe to capture stdout for the callback. */ - if (cmd->stdout_callback) { + if (get_stdout_fd) { if (pipe2 (outfd, O_CLOEXEC) == -1) { perrorf (cmd->g, "pipe2"); goto error; @@ -392,14 +395,14 @@ run_command (struct command *cmd) /* In parent, return to caller. */ if (cmd->pid > 0) { - if (cmd->capture_errors) { + if (get_stderr_fd) { close (errorfd[1]); errorfd[1] = -1; cmd->errorfd = errorfd[0]; errorfd[0] = -1; } - if (cmd->stdout_callback) { + if (get_stdout_fd) { close (outfd[1]); outfd[1] = -1; cmd->outfd = outfd[0]; @@ -410,15 +413,15 @@ run_command (struct command *cmd) } /* Child process. */ - if (cmd->capture_errors) { + if (get_stderr_fd) { close (errorfd[0]); - if (!cmd->stdout_callback) + if (!get_stdout_fd) dup2 (errorfd[1], 1); dup2 (errorfd[1], 2); close (errorfd[1]); } - if (cmd->stdout_callback) { + if (get_stdout_fd) { close (outfd[0]); dup2 (outfd[1], 1); close (outfd[1]); @@ -615,7 +618,7 @@ guestfs___cmd_run (struct command *cmd) if (cmd->g->verbose) debug_command (cmd); - if (run_command (cmd) == -1) + if (run_command (cmd, false, false) == -1) return -1; if (loop (cmd) == -1) @@ -624,6 +627,49 @@ guestfs___cmd_run (struct command *cmd) return wait_command (cmd); } +/* Fork, run the command, and returns the pid of the command, + * and its stdout and stderr file descriptors. + * + * Returns the exit status. Test it using WIF* macros. + * + * On error: Calls error(g) and returns -1. + */ +int +guestfs___cmd_run_async (struct command *cmd, pid_t *pid, + int *stdout_fd, int *stderr_fd) +{ + finish_command (cmd); + + if (cmd->g->verbose) + debug_command (cmd); + + if (run_command (cmd, stdout_fd != NULL, stderr_fd != NULL) == -1) + return -1; + + if (pid) + *pid = cmd->pid; + if (stdout_fd) + *stdout_fd = cmd->outfd; + if (stderr_fd) + *stderr_fd = cmd->errorfd; + + return 0; +} + +/* Wait for the command to finish. + * + * The command MUST have been started with guestfs___cmd_run_async. + * + * Returns the exit status. Test it using WIF* macros. + * + * On error: Calls error(g) and returns -1. + */ +int +guestfs___cmd_wait (struct command *cmd) +{ + return wait_command (cmd); +} + void guestfs___cmd_close (struct command *cmd) { diff --git a/src/guestfs-internal.h b/src/guestfs-internal.h index 573c3da..bd5f675 100644 --- a/src/guestfs-internal.h +++ b/src/guestfs-internal.h @@ -20,6 +20,7 @@ #define GUESTFS_INTERNAL_H_ #include <stdbool.h> +#include <sys/types.h> #include <libintl.h> @@ -870,6 +871,8 @@ extern void guestfs___cmd_set_stderr_to_stdout (struct command *); extern void guestfs___cmd_clear_capture_errors (struct command *); extern void guestfs___cmd_clear_close_files (struct command *); extern int guestfs___cmd_run (struct command *); +extern int guestfs___cmd_run_async (struct command *, pid_t *pid, int *stdout_fd, int *stderr_fd); +extern int guestfs___cmd_wait (struct command *); extern void guestfs___cmd_close (struct command *); #ifdef HAVE_ATTRIBUTE_CLEANUP -- 1.9.3
Pino Toscano
2015-Jan-26 16:04 UTC
[Libguestfs] [PATCH 2/6] cmd: add a child-setup callback
Easy way to do pre-exec setup in the child process. --- src/command.c | 22 ++++++++++++++++++++++ src/guestfs-internal.h | 2 ++ 2 files changed, 24 insertions(+) diff --git a/src/command.c b/src/command.c index e26573d..563d0af 100644 --- a/src/command.c +++ b/src/command.c @@ -134,6 +134,10 @@ struct command /* PID of subprocess (if > 0). */ pid_t pid; + + /* Optional child setup callback. */ + cmd_child_callback child_callback; + void *child_callback_data; }; /* Create a new command handle. */ @@ -308,6 +312,19 @@ guestfs___cmd_clear_close_files (struct command *cmd) cmd->close_files = false; } +/* Set a function to be executed in the child, right before the + * execution. Can be used to setup the child, for example changing + * its current directory. + */ +void +guestfs___cmd_set_child_callback (struct command *cmd, + cmd_child_callback child_callback, + void *data) +{ + cmd->child_callback = child_callback; + cmd->child_callback_data = data; +} + /* 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. @@ -461,6 +478,11 @@ run_command (struct command *cmd, bool get_stdout_fd, bool get_stderr_fd) /* Set the umask for all subcommands to something sensible (RHBZ#610880). */ umask (022); + if (cmd->child_callback) { + if (cmd->child_callback (cmd->g, cmd->child_callback_data) == -1) + _exit (EXIT_FAILURE); + } + /* Run the command. */ switch (cmd->style) { case COMMAND_STYLE_EXECV: diff --git a/src/guestfs-internal.h b/src/guestfs-internal.h index bd5f675..91e3065 100644 --- a/src/guestfs-internal.h +++ b/src/guestfs-internal.h @@ -857,6 +857,7 @@ extern int guestfs___osinfo_map (guestfs_h *g, const struct guestfs_isoinfo *iso /* command.c */ struct command; typedef void (*cmd_stdout_callback) (guestfs_h *g, void *data, const char *line, size_t len); +typedef int (*cmd_child_callback) (guestfs_h *g, void *data); 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, ...) @@ -870,6 +871,7 @@ extern void guestfs___cmd_set_stdout_callback (struct command *, cmd_stdout_call extern void guestfs___cmd_set_stderr_to_stdout (struct command *); extern void guestfs___cmd_clear_capture_errors (struct command *); extern void guestfs___cmd_clear_close_files (struct command *); +extern void guestfs___cmd_set_child_callback (struct command *, cmd_child_callback child_callback, void *data); extern int guestfs___cmd_run (struct command *); extern int guestfs___cmd_run_async (struct command *, pid_t *pid, int *stdout_fd, int *stderr_fd); extern int guestfs___cmd_wait (struct command *); -- 1.9.3
Pino Toscano
2015-Jan-26 16:04 UTC
[Libguestfs] [PATCH 3/6] cmd: add the possibility to get a fd to the process stdin
Add the possibility to forward to the stdin of the process, and get the fd for it; only in async mode for now. --- src/command.c | 43 ++++++++++++++++++++++++++++++++++++++----- src/guestfs-internal.h | 2 +- 2 files changed, 39 insertions(+), 6 deletions(-) diff --git a/src/command.c b/src/command.c index 563d0af..0480cd3 100644 --- a/src/command.c +++ b/src/command.c @@ -138,6 +138,9 @@ struct command /* Optional child setup callback. */ cmd_child_callback child_callback; void *child_callback_data; + + /* Optional stdin forwarding to the child. */ + int infd; }; /* Create a new command handle. */ @@ -152,6 +155,7 @@ guestfs___new_command (guestfs_h *g) cmd->close_files = true; cmd->errorfd = -1; cmd->outfd = -1; + cmd->infd = -1; return cmd; } @@ -377,17 +381,27 @@ debug_command (struct command *cmd) } static int -run_command (struct command *cmd, bool get_stdout_fd, bool get_stderr_fd) +run_command (struct command *cmd, bool get_stdin_fd, bool get_stdout_fd, + bool get_stderr_fd) { struct sigaction sa; int i, fd, max_fd, r; int errorfd[2] = { -1, -1 }; int outfd[2] = { -1, -1 }; + int infd[2] = { -1, -1 }; char status_string[80]; get_stdout_fd = get_stdout_fd || cmd->stdout_callback != NULL; get_stderr_fd = get_stderr_fd || cmd->capture_errors; + /* Set up a pipe to forward the stdin to the command. */ + if (get_stdin_fd) { + if (pipe2 (infd, O_CLOEXEC) == -1) { + perrorf (cmd->g, "pipe2"); + goto error; + } + } + /* Set up a pipe to capture command output and send it to the error log. */ if (get_stderr_fd) { if (pipe2 (errorfd, O_CLOEXEC) == -1) { @@ -426,6 +440,13 @@ run_command (struct command *cmd, bool get_stdout_fd, bool get_stderr_fd) outfd[0] = -1; } + if (get_stdin_fd) { + close (infd[0]); + infd[0] = -1; + cmd->infd = infd[1]; + infd[1] = -1; + } + return 0; } @@ -444,6 +465,12 @@ run_command (struct command *cmd, bool get_stdout_fd, bool get_stderr_fd) close (outfd[1]); } + if (get_stdin_fd) { + close (infd[1]); + dup2 (infd[0], 0); + close (infd[0]); + } + if (cmd->stderr_to_stdout) dup2 (1, 2); @@ -640,7 +667,7 @@ guestfs___cmd_run (struct command *cmd) if (cmd->g->verbose) debug_command (cmd); - if (run_command (cmd, false, false) == -1) + if (run_command (cmd, false, false, false) == -1) return -1; if (loop (cmd) == -1) @@ -650,7 +677,7 @@ guestfs___cmd_run (struct command *cmd) } /* Fork, run the command, and returns the pid of the command, - * and its stdout and stderr file descriptors. + * and its stdin, stdout and stderr file descriptors. * * Returns the exit status. Test it using WIF* macros. * @@ -658,18 +685,21 @@ guestfs___cmd_run (struct command *cmd) */ int guestfs___cmd_run_async (struct command *cmd, pid_t *pid, - int *stdout_fd, int *stderr_fd) + int *stdin_fd, int *stdout_fd, int *stderr_fd) { finish_command (cmd); if (cmd->g->verbose) debug_command (cmd); - if (run_command (cmd, stdout_fd != NULL, stderr_fd != NULL) == -1) + if (run_command (cmd, stdin_fd != NULL, stdout_fd != NULL, + stderr_fd != NULL) == -1) return -1; if (pid) *pid = cmd->pid; + if (stdin_fd) + *stdin_fd = cmd->infd; if (stdout_fd) *stdout_fd = cmd->outfd; if (stderr_fd) @@ -718,6 +748,9 @@ guestfs___cmd_close (struct command *cmd) if (cmd->outfd >= 0) close (cmd->outfd); + if (cmd->infd >= 0) + close (cmd->infd); + free (cmd->outbuf.buffer); if (cmd->pid > 0) diff --git a/src/guestfs-internal.h b/src/guestfs-internal.h index 91e3065..01bee2c 100644 --- a/src/guestfs-internal.h +++ b/src/guestfs-internal.h @@ -873,7 +873,7 @@ extern void guestfs___cmd_clear_capture_errors (struct command *); extern void guestfs___cmd_clear_close_files (struct command *); extern void guestfs___cmd_set_child_callback (struct command *, cmd_child_callback child_callback, void *data); extern int guestfs___cmd_run (struct command *); -extern int guestfs___cmd_run_async (struct command *, pid_t *pid, int *stdout_fd, int *stderr_fd); +extern int guestfs___cmd_run_async (struct command *, pid_t *pid, int *stdin_fd, int *stdout_fd, int *stderr_fd); extern int guestfs___cmd_wait (struct command *); extern void guestfs___cmd_close (struct command *); -- 1.9.3
Pino Toscano
2015-Jan-26 16:04 UTC
[Libguestfs] [PATCH 4/6] generator: add VPublicNoFish visibility type
Usable to have public functions, i.e. like VPublic, but do not showing them in guestfish. --- generator/actions.ml | 6 +++--- generator/c.ml | 2 +- generator/types.ml | 2 ++ 3 files changed, 6 insertions(+), 4 deletions(-) diff --git a/generator/actions.ml b/generator/actions.ml index c0beaae..25f4bb5 100644 --- a/generator/actions.ml +++ b/generator/actions.ml @@ -12744,19 +12744,19 @@ let non_daemon_functions, daemon_functions let all_functions = non_daemon_functions @ daemon_functions let is_external { visibility = v } = match v with - | VPublic | VStateTest | VBindTest | VDebug -> true + | VPublic | VPublicNoFish | VStateTest | VBindTest | VDebug -> true | VInternal -> false let is_internal f = not (is_external f) let is_documented { visibility = v } = match v with - | VPublic | VStateTest -> true + | VPublic | VPublicNoFish | VStateTest -> true | VBindTest | VDebug | VInternal -> false let is_fish { visibility = v; style = (_, args, _) } (* Internal functions are not exported to guestfish. *) match v with - | VStateTest | VBindTest | VInternal -> false + | VPublicNoFish | VStateTest | VBindTest | VInternal -> false | VPublic | VDebug -> (* Functions that take Pointer parameters cannot be used in * guestfish, since there is no way the user could safely diff --git a/generator/c.ml b/generator/c.ml index 1c28853..24c1605 100644 --- a/generator/c.ml +++ b/generator/c.ml @@ -45,7 +45,7 @@ let hash_matches h { name = name } type optarg_proto = Dots | VA | Argv let is_public { visibility = v } = match v with - | VPublic | VStateTest | VDebug -> true + | VPublic | VPublicNoFish | VStateTest | VDebug -> true | VBindTest | VInternal -> false let is_private f = not (is_public f) diff --git a/generator/types.ml b/generator/types.ml index 2be6bae..ad9fa63 100644 --- a/generator/types.ml +++ b/generator/types.ml @@ -322,6 +322,8 @@ and cmd = string list type visibility | VPublic (* Part of the public API *) + | VPublicNoFish (* Like VPublic, but not exported in + guestfish *) | VStateTest (* A function which tests the state of the appliance *) | VBindTest (* Only used for testing language bindings *) -- 1.9.3
Pino Toscano
2015-Jan-26 16:04 UTC
[Libguestfs] [PATCH 5/6] New APIs: copy-in and copy-out
Currently implemented as guestfish commands, provide them instead as single source -> destination functions for the library, so they can be used also in other places. These functions are not added to guestfish, since guestfish has its own implementation (which will soon switch to call copy-in and copy-out for multiple paths). --- generator/actions.ml | 28 ++++++ po/POTFILES | 1 + src/Makefile.am | 1 + src/copy-in-out.c | 244 +++++++++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 274 insertions(+) create mode 100644 src/copy-in-out.c diff --git a/generator/actions.ml b/generator/actions.ml index 25f4bb5..ab97891 100644 --- a/generator/actions.ml +++ b/generator/actions.ml @@ -3310,6 +3310,34 @@ In non-C language bindings, this allows you to retrieve the underlying C pointer to the handle (ie. C<guestfs_h *>). The purpose of this is to allow other libraries to interwork with libguestfs." }; + { defaults with + name = "copy_in"; + style = RErr, [String "localpath"; Pathname "remotedir"], []; + visibility = VPublicNoFish; + shortdesc = "copy local files or directories into an image"; + longdesc = "\ +C<guestfs_copy_in> copies local files or directories recursively into +the disk image, placing them in the directory called C</remotedir> +(which must exist). + +Wildcards cannot be used." }; + + { defaults with + name = "copy_out"; + style = RErr, [Pathname "remotepath"; String "localdir"], []; + visibility = VPublicNoFish; + shortdesc = "copy remote files or directories out of an image"; + longdesc = "\ +C<guestfs_copy_out> copies remote files or directories recursively +out of the disk image, placing them on the host disk in a local +directory called C<localdir> (which must exist). + +To download to the current directory, use C<.> as in: + + C<guestfs_copy_out> /home . + +Wildcards cannot be used." }; + ] (* daemon_functions are any functions which cause some action diff --git a/po/POTFILES b/po/POTFILES index 4194e5f..6e65377 100644 --- a/po/POTFILES +++ b/po/POTFILES @@ -297,6 +297,7 @@ src/canonical-name.c src/cleanup.c src/command.c src/conn-socket.c +src/copy-in-out.c src/create.c src/dbdump.c src/drives.c diff --git a/src/Makefile.am b/src/Makefile.am index a83f257..2496887 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -90,6 +90,7 @@ libguestfs_la_SOURCES = \ canonical-name.c \ command.c \ conn-socket.c \ + copy-in-out.c \ create.c \ dbdump.c \ drives.c \ diff --git a/src/copy-in-out.c b/src/copy-in-out.c new file mode 100644 index 0000000..8e2edd3 --- /dev/null +++ b/src/copy-in-out.c @@ -0,0 +1,244 @@ +/* libguestfs + * Copyright (C) 2015 Red Hat Inc. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program 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 General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. + */ + +#include <config.h> + +#include <stdio.h> +#include <stdlib.h> +#include <stdint.h> +#include <inttypes.h> +#include <unistd.h> +#include <sys/types.h> +#include <sys/stat.h> +#include <sys/wait.h> +#include <errno.h> +#include <string.h> + +#include "guestfs.h" +#include "guestfs-internal.h" +#include "guestfs-internal-actions.h" + +static int split_path (char *buf, size_t buf_size, const char *path, const char **dirname, const char **basename); + +int +guestfs__copy_in (guestfs_h *g, const char *localpath, const char *remotedir) +{ + CLEANUP_CMD_CLOSE struct command *cmd = guestfs___new_command (g); + int fd; + int r; + char fdbuf[64]; + size_t buf_len = strlen (localpath) + 1; + char buf[buf_len]; + const char *dirname, *basename; + + if (guestfs_is_dir (g, remotedir) == -1) + return -1; + + if (split_path (buf, buf_len, localpath, &dirname, &basename) == -1) + return -1; + + guestfs___cmd_add_arg (cmd, "tar"); + if (dirname) { + guestfs___cmd_add_arg (cmd, "-C"); + guestfs___cmd_add_arg (cmd, dirname); + } + guestfs___cmd_add_arg (cmd, "-cf"); + guestfs___cmd_add_arg (cmd, "-"); + guestfs___cmd_add_arg (cmd, basename); + + r = guestfs___cmd_run_async (cmd, NULL, NULL, &fd, NULL); + if (r == -1) + return -1; + + snprintf (fdbuf, sizeof fdbuf, "/dev/fd/%d", fd); + + r = guestfs_tar_in (g, fdbuf, remotedir); + + if (close (fd) == -1) { + perror ("close (tar subprocess)"); + return -1; + } + + r = guestfs___cmd_wait (cmd); + if (r == -1) + return -1; + if (!(WIFEXITED (r) && WEXITSTATUS (r) == 0)) + return -1; + + return 0; +} + +struct copy_out_child_data { + const char *localdir; + const char *basename; +}; + +static int +child_setup (guestfs_h *g, void *data) +{ + struct copy_out_child_data d = *(struct copy_out_child_data *) data; + + if (chdir (d.localdir) == -1) { + perror (d.localdir); + return -1; + } + + if (mkdir (d.basename, 0777) == -1 && errno != EEXIST) { + perror (d.basename); + return -1; + } + + if (chdir (d.basename) == -1) { + perror (d.basename); + return -1; + } + + return 0; +} + +int +guestfs__copy_out (guestfs_h *g, const char *remotepath, const char *localdir) +{ + struct stat statbuf; + int r; + + if (stat (localdir, &statbuf) == -1 || + ! (S_ISDIR (statbuf.st_mode))) { + error (g, _("target '%s' is not a directory"), localdir); + return -1; + } + + /* If the remote is a file, download it. If it's a directory, + * create the directory in localdir first before using tar-out. + */ + r = guestfs_is_file (g, remotepath); + if (r == -1) + return -1; + + if (r == 1) { /* is file */ + CLEANUP_FREE char *filename = NULL; + size_t buf_len = strlen (remotepath) + 1; + char buf[buf_len]; + const char *basename; + + if (split_path (buf, buf_len, remotepath, NULL, &basename) == -1) + return -1; + + if (asprintf (&filename, "%s/%s", localdir, basename) == -1) { + perror ("asprintf"); + return -1; + } + if (guestfs_download (g, remotepath, filename) == -1) + return -1; + } else { /* not a regular file */ + CLEANUP_CMD_CLOSE struct command *cmd = guestfs___new_command (g); + struct copy_out_child_data data; + char fdbuf[64]; + int fd; + + r = guestfs_is_dir (g, remotepath); + if (r == -1) + return -1; + + if (r == 0) { + error (g, _("'%s' is not a file or directory"), remotepath); + return -1; + } + + size_t buf_len = strlen (remotepath) + 1; + char buf[buf_len]; + const char *basename; + if (split_path (buf, buf_len, remotepath, NULL, &basename) == -1) + return -1; + + /* RHBZ#845522: If remotepath == "/" then basename would be an empty + * string. Replace it with "." so that make_tar_output writes + * to "localdir/." + */ + if (STREQ (basename, "")) + basename = "."; + + data.localdir = localdir; + data.basename = basename; + + guestfs___cmd_set_child_callback (cmd, &child_setup, &data); + + guestfs___cmd_add_arg (cmd, "tar"); + guestfs___cmd_add_arg (cmd, "-xf"); + guestfs___cmd_add_arg (cmd, "-"); + + r = guestfs___cmd_run_async (cmd, NULL, &fd, NULL, NULL); + if (r == -1) + return -1; + + snprintf (fdbuf, sizeof fdbuf, "/dev/fd/%d", fd); + + r = guestfs_tar_out (g, remotepath, fdbuf); + + if (close (fd) == -1) { + perror ("close (tar-output subprocess)"); + return -1; + } + + r = guestfs___cmd_wait (cmd); + if (r == -1) + return -1; + if (!(WIFEXITED (r) && WEXITSTATUS (r) == 0)) + return -1; + } + + return 0; +} + +/* Split path into directory name and base name, using the buffer + * provided as a working area. If there is no directory name + * (eg. path == "/") then this can return dirname as NULL. + */ +static int +split_path (char *buf, size_t buf_size, + const char *path, const char **dirname, const char **basename) +{ + size_t len = strlen (path); + if (len == 0 || len > buf_size - 1) { + fprintf (stderr, _("error: argument is zero length or longer than maximum permitted\n")); + return -1; + } + + strcpy (buf, path); + + if (len >= 2 && buf[len-1] == '/') { + buf[len-1] = '\0'; + len--; + } + + char *p = strrchr (buf, '/'); + if (p && p > buf) { /* "foo/bar" */ + *p = '\0'; + p++; + if (dirname) *dirname = buf; + if (basename) *basename = p; + } else if (p && p == buf) { /* "/foo" */ + if (dirname) *dirname = "/"; + if (basename) *basename = buf+1; + } else { + if (dirname) *dirname = NULL; + if (basename) *basename = buf; + } + + return 0; +} -- 1.9.3
Pino Toscano
2015-Jan-26 16:04 UTC
[Libguestfs] [PATCH 6/6] fish: use copy-in and copy-out
Simply the copy-in & copy-out commands of guestfish using the new implementations of copy-in & copy-out in the library. --- fish/copy.c | 279 +----------------------------------------------------------- 1 file changed, 4 insertions(+), 275 deletions(-) diff --git a/fish/copy.c b/fish/copy.c index 8394dba..d3abb2e 100644 --- a/fish/copy.c +++ b/fish/copy.c @@ -22,25 +22,11 @@ #include <stdlib.h> #include <string.h> #include <unistd.h> -#include <limits.h> #include <libintl.h> #include <errno.h> -#include <sys/types.h> -#include <sys/stat.h> -#include <sys/wait.h> -#include <assert.h> #include "fish.h" -struct fd_pid { - int fd; /* -1 == error */ - pid_t pid; -}; - -static struct fd_pid make_tar_from_local (const char *local); -static struct fd_pid make_tar_output (const char *local, const char *basename); -static int split_path (char *buf, size_t buf_size, const char *path, const char **dirname, const char **basename); - int run_copy_in (const char *cmd, size_t argc, char *argv[]) { @@ -61,40 +47,10 @@ run_copy_in (const char *cmd, size_t argc, char *argv[]) int nr_locals = argc-1; - int remote_is_dir = guestfs_is_dir (g, remote); - if (remote_is_dir == -1) - return -1; - - if (!remote_is_dir) { - fprintf (stderr, _("copy-in: target '%s' is not a directory\n"), remote); - return -1; - } - - /* Upload each local one at a time using tar-in. */ + /* Upload each local one at a time using copy-in. */ int i; for (i = 0; i < nr_locals; ++i) { - struct fd_pid fdpid = make_tar_from_local (argv[i]); - if (fdpid.fd == -1) - return -1; - - char fdbuf[64]; - snprintf (fdbuf, sizeof fdbuf, "/dev/fd/%d", fdpid.fd); - - int r = guestfs_tar_in (g, fdbuf, remote); - - if (close (fdpid.fd) == -1) { - perror ("close (tar-from-local subprocess)"); - r = -1; - } - - int status; - if (waitpid (fdpid.pid, &status, 0) == -1) { - perror ("wait (tar-from-local subprocess)"); - return -1; - } - if (!(WIFEXITED (status) && WEXITSTATUS (status) == 0)) - return -1; - + int r = guestfs_copy_in (g, argv[i], remote); if (r == -1) return -1; } @@ -102,98 +58,6 @@ run_copy_in (const char *cmd, size_t argc, char *argv[]) return 0; } -static void tar_create (const char *dir, const char *path) __attribute__((noreturn)); - -/* This creates a subprocess which feeds a tar file back to the - * main guestfish process. - */ -static struct fd_pid -make_tar_from_local (const char *local) -{ - int fd[2]; - struct fd_pid r = { .fd = -1 }; - - if (pipe (fd) == -1) { - perror ("pipe"); - return r; - } - - r.pid = fork (); - if (r.pid == -1) { - perror ("fork"); - return r; - } - - if (r.pid > 0) { /* Parent */ - close (fd[1]); - r.fd = fd[0]; - return r; - } - - /* Child. */ - close (fd[0]); - dup2 (fd[1], 1); - close (fd[1]); - - size_t buf_len = strlen (local) + 1; - char buf[buf_len]; - const char *dirname, *basename; - if (split_path (buf, buf_len, local, &dirname, &basename) == -1) - _exit (EXIT_FAILURE); - - tar_create (dirname, basename); -} - -/* Split path into directory name and base name, using the buffer - * provided as a working area. If there is no directory name - * (eg. path == "/") then this can return dirname as NULL. - */ -static int -split_path (char *buf, size_t buf_size, - const char *path, const char **dirname, const char **basename) -{ - size_t len = strlen (path); - if (len == 0 || len > buf_size - 1) { - fprintf (stderr, _("error: argument is zero length or longer than maximum permitted\n")); - return -1; - } - - strcpy (buf, path); - - if (len >= 2 && buf[len-1] == '/') { - buf[len-1] = '\0'; - len--; - } - - char *p = strrchr (buf, '/'); - if (p && p > buf) { /* "foo/bar" */ - *p = '\0'; - p++; - if (dirname) *dirname = buf; - if (basename) *basename = p; - } else if (p && p == buf) { /* "/foo" */ - if (dirname) *dirname = "/"; - if (basename) *basename = buf+1; - } else { - if (dirname) *dirname = NULL; - if (basename) *basename = buf; - } - - return 0; -} - -static void -tar_create (const char *dir, const char *path) -{ - if (dir) - execlp ("tar", "tar", "-C", dir, "-cf", "-", path, NULL); - else - execlp ("tar", "tar", "-cf", "-", path, NULL); - - perror ("execlp: tar"); - _exit (EXIT_FAILURE); -} - int run_copy_out (const char *cmd, size_t argc, char *argv[]) { @@ -207,14 +71,7 @@ run_copy_out (const char *cmd, size_t argc, char *argv[]) const char *local = argv[argc-1]; int nr_remotes = argc-1; - struct stat statbuf; - if (stat (local, &statbuf) == -1 || - ! (S_ISDIR (statbuf.st_mode))) { - fprintf (stderr, _("copy-out: target '%s' is not a directory\n"), local); - return -1; - } - - /* Download each remote one at a time using tar-out. */ + /* Download each remote one at a time using copy-out. */ int i, r; for (i = 0; i < nr_remotes; ++i) { CLEANUP_FREE char *remote = NULL; @@ -224,138 +81,10 @@ run_copy_out (const char *cmd, size_t argc, char *argv[]) if (remote == NULL) return -1; - /* If the remote is a file, download it. If it's a directory, - * create the directory in local first before using tar-out. - */ - r = guestfs_is_file (g, remote); + r = guestfs_copy_out (g, remote, local); if (r == -1) return -1; - if (r == 1) { /* is file */ - CLEANUP_FREE char *filename = NULL; - size_t buf_len = strlen (remote) + 1; - char buf[buf_len]; - const char *basename; - - if (split_path (buf, buf_len, remote, NULL, &basename) == -1) - return -1; - - if (asprintf (&filename, "%s/%s", local, basename) == -1) { - perror ("asprintf"); - return -1; - } - if (guestfs_download (g, remote, filename) == -1) - return -1; - } - else { /* not a regular file */ - r = guestfs_is_dir (g, remote); - if (r == -1) - return -1; - - if (r == 0) { - fprintf (stderr, _("copy-out: '%s' is not a file or directory\n"), - remote); - return -1; - } - - size_t buf_len = strlen (remote) + 1; - char buf[buf_len]; - const char *basename; - if (split_path (buf, buf_len, remote, NULL, &basename) == -1) - return -1; - - /* RHBZ#845522: If remote == "/" then basename would be an empty - * string. Replace it with "." so that make_tar_output writes - * to "local/." - */ - if (STREQ (basename, "")) - basename = "."; - - struct fd_pid fdpid = make_tar_output (local, basename); - if (fdpid.fd == -1) - return -1; - - char fdbuf[64]; - snprintf (fdbuf, sizeof fdbuf, "/dev/fd/%d", fdpid.fd); - - int r = guestfs_tar_out (g, remote, fdbuf); - - if (close (fdpid.fd) == -1) { - perror ("close (tar-output subprocess)"); - r = -1; - } - - int status; - if (waitpid (fdpid.pid, &status, 0) == -1) { - perror ("wait (tar-output subprocess)"); - return -1; - } - if (!(WIFEXITED (status) && WEXITSTATUS (status) == 0)) - return -1; - - if (r == -1) - return -1; - } } return 0; } - -/* This creates a subprocess which takes a tar file from the main - * guestfish process and unpacks it into the 'local/basename' - * directory. - */ -static struct fd_pid -make_tar_output (const char *local, const char *basename) -{ - int fd[2]; - struct fd_pid r = { .fd = -1 }; - - /* local can't be an empty string because the caller stats it and - * checks it is a directory. - */ - assert (STRNEQ (local, "")); - - /* basename must not be an empty string (see RHBZ#845522). */ - assert (STRNEQ (basename, "")); - - if (pipe (fd) == -1) { - perror ("pipe"); - return r; - } - - r.pid = fork (); - if (r.pid == -1) { - perror ("fork"); - return r; - } - - if (r.pid > 0) { /* Parent */ - close (fd[0]); - r.fd = fd[1]; - return r; - } - - /* Child. */ - close (fd[1]); - dup2 (fd[0], 0); - close (fd[0]); - - if (chdir (local) == -1) { - perror (local); - _exit (EXIT_FAILURE); - } - - if (mkdir (basename, 0777) == -1 && errno != EEXIST) { - perror (basename); - _exit (EXIT_FAILURE); - } - - if (chdir (basename) == -1) { - perror (basename); - _exit (EXIT_FAILURE); - } - - execlp ("tar", "tar", "-xf", "-", NULL); - perror ("execlp: tar"); - _exit (EXIT_FAILURE); -} -- 1.9.3
Richard W.M. Jones
2015-Feb-02 13:40 UTC
Re: [Libguestfs] [PATCH 5/6] New APIs: copy-in and copy-out
On Mon, Jan 26, 2015 at 05:04:10PM +0100, Pino Toscano wrote:> Currently implemented as guestfish commands, provide them instead as > single source -> destination functions for the library, so they can be > used also in other places. > > These functions are not added to guestfish, since guestfish has its own > implementation (which will soon switch to call copy-in and copy-out for > multiple paths). > --- > generator/actions.ml | 28 ++++++ > po/POTFILES | 1 + > src/Makefile.am | 1 + > src/copy-in-out.c | 244 +++++++++++++++++++++++++++++++++++++++++++++++++++ > 4 files changed, 274 insertions(+) > create mode 100644 src/copy-in-out.c > > diff --git a/generator/actions.ml b/generator/actions.ml > index 25f4bb5..ab97891 100644 > --- a/generator/actions.ml > +++ b/generator/actions.ml > @@ -3310,6 +3310,34 @@ In non-C language bindings, this allows you to retrieve the underlying > C pointer to the handle (ie. C<guestfs_h *>). The purpose of this is > to allow other libraries to interwork with libguestfs." }; > > + { defaults with > + name = "copy_in"; > + style = RErr, [String "localpath"; Pathname "remotedir"], []; > + visibility = VPublicNoFish; > + shortdesc = "copy local files or directories into an image"; > + longdesc = "\ > +C<guestfs_copy_in> copies local files or directories recursively into > +the disk image, placing them in the directory called C</remotedir>^ You don't need the / here.> +(which must exist). > + > +Wildcards cannot be used." }; > + > + { defaults with > + name = "copy_out"; > + style = RErr, [Pathname "remotepath"; String "localdir"], []; > + visibility = VPublicNoFish; > + shortdesc = "copy remote files or directories out of an image"; > + longdesc = "\ > +C<guestfs_copy_out> copies remote files or directories recursively > +out of the disk image, placing them on the host disk in a local > +directory called C<localdir> (which must exist). > + > +To download to the current directory, use C<.> as in: > + > + C<guestfs_copy_out> /home . > + > +Wildcards cannot be used." }; > + > ] > > (* daemon_functions are any functions which cause some action > diff --git a/po/POTFILES b/po/POTFILES > index 4194e5f..6e65377 100644 > --- a/po/POTFILES > +++ b/po/POTFILES > @@ -297,6 +297,7 @@ src/canonical-name.c > src/cleanup.c > src/command.c > src/conn-socket.c > +src/copy-in-out.c > src/create.c > src/dbdump.c > src/drives.c > diff --git a/src/Makefile.am b/src/Makefile.am > index a83f257..2496887 100644 > --- a/src/Makefile.am > +++ b/src/Makefile.am > @@ -90,6 +90,7 @@ libguestfs_la_SOURCES = \ > canonical-name.c \ > command.c \ > conn-socket.c \ > + copy-in-out.c \ > create.c \ > dbdump.c \ > drives.c \ > diff --git a/src/copy-in-out.c b/src/copy-in-out.c > new file mode 100644 > index 0000000..8e2edd3 > --- /dev/null > +++ b/src/copy-in-out.c > @@ -0,0 +1,244 @@ > +/* libguestfs > + * Copyright (C) 2015 Red Hat Inc. > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License as published by > + * the Free Software Foundation; either version 2 of the License, or > + * (at your option) any later version. > + * > + * This program 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 General Public License for more details. > + * > + * You should have received a copy of the GNU General Public License > + * along with this program; if not, write to the Free Software > + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. > + */ > + > +#include <config.h> > + > +#include <stdio.h> > +#include <stdlib.h> > +#include <stdint.h> > +#include <inttypes.h> > +#include <unistd.h> > +#include <sys/types.h> > +#include <sys/stat.h> > +#include <sys/wait.h> > +#include <errno.h> > +#include <string.h> > + > +#include "guestfs.h" > +#include "guestfs-internal.h" > +#include "guestfs-internal-actions.h" > + > +static int split_path (char *buf, size_t buf_size, const char *path, const char **dirname, const char **basename); > + > +int > +guestfs__copy_in (guestfs_h *g, const char *localpath, const char *remotedir) > +{ > + CLEANUP_CMD_CLOSE struct command *cmd = guestfs___new_command (g); > + int fd; > + int r; > + char fdbuf[64]; > + size_t buf_len = strlen (localpath) + 1; > + char buf[buf_len]; > + const char *dirname, *basename; > + > + if (guestfs_is_dir (g, remotedir) == -1) > + return -1;I suppose this is here to check that remotedir is a directory, but that's not what the code here does. The original guestfish code is: int remote_is_dir = guestfs_is_dir (g, remote); if (remote_is_dir == -1) return -1; if (!remote_is_dir) { fprintf (stderr, _("copy-in: target '%s' is not a directory\n"), remote); return -1; }> + if (split_path (buf, buf_len, localpath, &dirname, &basename) == -1) > + return -1; > + > + guestfs___cmd_add_arg (cmd, "tar"); > + if (dirname) { > + guestfs___cmd_add_arg (cmd, "-C"); > + guestfs___cmd_add_arg (cmd, dirname); > + } > + guestfs___cmd_add_arg (cmd, "-cf"); > + guestfs___cmd_add_arg (cmd, "-"); > + guestfs___cmd_add_arg (cmd, basename); > + > + r = guestfs___cmd_run_async (cmd, NULL, NULL, &fd, NULL); > + if (r == -1) > + return -1; > + > + snprintf (fdbuf, sizeof fdbuf, "/dev/fd/%d", fd); > + > + r = guestfs_tar_in (g, fdbuf, remotedir); > + > + if (close (fd) == -1) { > + perror ("close (tar subprocess)"); > + return -1; > + } > + > + r = guestfs___cmd_wait (cmd); > + if (r == -1) > + return -1; > + if (!(WIFEXITED (r) && WEXITSTATUS (r) == 0)) > + return -1; > + > + return 0; > +} > + > +struct copy_out_child_data { > + const char *localdir; > + const char *basename; > +}; > + > +static int > +child_setup (guestfs_h *g, void *data) > +{ > + struct copy_out_child_data d = *(struct copy_out_child_data *) data; > + > + if (chdir (d.localdir) == -1) { > + perror (d.localdir); > + return -1; > + } > + > + if (mkdir (d.basename, 0777) == -1 && errno != EEXIST) { > + perror (d.basename); > + return -1; > + } > + > + if (chdir (d.basename) == -1) { > + perror (d.basename); > + return -1; > + } > + > + return 0; > +} > + > +int > +guestfs__copy_out (guestfs_h *g, const char *remotepath, const char *localdir) > +{ > + struct stat statbuf; > + int r; > + > + if (stat (localdir, &statbuf) == -1 || > + ! (S_ISDIR (statbuf.st_mode))) { > + error (g, _("target '%s' is not a directory"), localdir); > + return -1; > + } > + > + /* If the remote is a file, download it. If it's a directory, > + * create the directory in localdir first before using tar-out. > + */ > + r = guestfs_is_file (g, remotepath); > + if (r == -1) > + return -1; > + > + if (r == 1) { /* is file */ > + CLEANUP_FREE char *filename = NULL; > + size_t buf_len = strlen (remotepath) + 1; > + char buf[buf_len]; > + const char *basename; > + > + if (split_path (buf, buf_len, remotepath, NULL, &basename) == -1) > + return -1; > + > + if (asprintf (&filename, "%s/%s", localdir, basename) == -1) { > + perror ("asprintf"); > + return -1; > + } > + if (guestfs_download (g, remotepath, filename) == -1) > + return -1; > + } else { /* not a regular file */ > + CLEANUP_CMD_CLOSE struct command *cmd = guestfs___new_command (g); > + struct copy_out_child_data data; > + char fdbuf[64]; > + int fd; > + > + r = guestfs_is_dir (g, remotepath); > + if (r == -1) > + return -1; > + > + if (r == 0) { > + error (g, _("'%s' is not a file or directory"), remotepath); > + return -1; > + } > + > + size_t buf_len = strlen (remotepath) + 1; > + char buf[buf_len]; > + const char *basename; > + if (split_path (buf, buf_len, remotepath, NULL, &basename) == -1) > + return -1; > + > + /* RHBZ#845522: If remotepath == "/" then basename would be an empty > + * string. Replace it with "." so that make_tar_output writes > + * to "localdir/." > + */ > + if (STREQ (basename, "")) > + basename = "."; > + > + data.localdir = localdir; > + data.basename = basename; > + > + guestfs___cmd_set_child_callback (cmd, &child_setup, &data); > + > + guestfs___cmd_add_arg (cmd, "tar"); > + guestfs___cmd_add_arg (cmd, "-xf"); > + guestfs___cmd_add_arg (cmd, "-"); > + > + r = guestfs___cmd_run_async (cmd, NULL, &fd, NULL, NULL); > + if (r == -1) > + return -1; > + > + snprintf (fdbuf, sizeof fdbuf, "/dev/fd/%d", fd); > + > + r = guestfs_tar_out (g, remotepath, fdbuf); > + > + if (close (fd) == -1) { > + perror ("close (tar-output subprocess)"); > + return -1; > + } > + > + r = guestfs___cmd_wait (cmd); > + if (r == -1) > + return -1; > + if (!(WIFEXITED (r) && WEXITSTATUS (r) == 0)) > + return -1; > + } > + > + return 0; > +} > + > +/* Split path into directory name and base name, using the buffer > + * provided as a working area. If there is no directory name > + * (eg. path == "/") then this can return dirname as NULL. > + */ > +static int > +split_path (char *buf, size_t buf_size, > + const char *path, const char **dirname, const char **basename) > +{ > + size_t len = strlen (path); > + if (len == 0 || len > buf_size - 1) { > + fprintf (stderr, _("error: argument is zero length or longer than maximum permitted\n")); > + return -1; > + } > + > + strcpy (buf, path); > + > + if (len >= 2 && buf[len-1] == '/') { > + buf[len-1] = '\0'; > + len--; > + } > + > + char *p = strrchr (buf, '/'); > + if (p && p > buf) { /* "foo/bar" */ > + *p = '\0'; > + p++; > + if (dirname) *dirname = buf; > + if (basename) *basename = p; > + } else if (p && p == buf) { /* "/foo" */ > + if (dirname) *dirname = "/"; > + if (basename) *basename = buf+1; > + } else { > + if (dirname) *dirname = NULL; > + if (basename) *basename = buf; > + } > + > + return 0; > +} > -- > 1.9.3The guestfish code (which took *ages* to get right) is a lot more complicated ... Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com Fedora Windows cross-compiler. Compile Windows programs, test, and build Windows installers. Over 100 libraries supported. http://fedoraproject.org/wiki/MinGW
Apparently Analagous Threads
- Re: [PATCH 5/6] New APIs: copy-in and copy-out
- [PATCH 1/6] cmd: add a way to run (and wait) asynchronously commands
- [PATCH 1/2] copy-in: print tar stderr when it fails
- [PATCH 0/7 v2] Make copy_in & copy_out APIs, and use copy_in in customize
- [PATCH 06/10] copy-out: new 'excludes' optional argument