Libguestfs - Feb 2015 - [PATCH 0/7 v2] Make copy_in & copy_out APIs, and use copy_in in customize

Hi,

attached there is the second version of the patch series adding
copy_in and copy_out in the library, mostly moving them from guestfish.

It also adds the copy_in usage in virt-customize, as aid in a new image
building.

Thanks,

Pino Toscano (7):
  cmd: add a way to run (and wait) asynchronously commands
  cmd: add a child-setup callback
  cmd: add the possibility to get a fd to the process stdin
  generator: add VPublicNoFish visibility type
  New APIs: copy-in and copy-out
  fish: use copy-in and copy-out
  customize: add copy-in operation (RHBZ#1135585).

 builder/cmdline.ml         |   2 +-
 customize/customize_run.ml |   4 +
 fish/copy.c                | 279 +--------------------------------------------
 generator/actions.ml       |  34 +++++-
 generator/c.ml             |   2 +-
 generator/customize.ml     |  13 +++
 generator/types.ml         |   2 +
 po/POTFILES                |   1 +
 src/Makefile.am            |   1 +
 src/command.c              | 119 +++++++++++++++++--
 src/copy-in-out.c          | 250 ++++++++++++++++++++++++++++++++++++++++
 src/guestfs-internal.h     |   5 +
 12 files changed, 423 insertions(+), 289 deletions(-)
 create mode 100644 src/copy-in-out.c

-- 
1.9.3

---
 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

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

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

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

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    | 250 +++++++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 280 insertions(+)
 create mode 100644 src/copy-in-out.c

diff --git a/generator/actions.ml b/generator/actions.ml
index 25f4bb5..985bb81 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..b6f1522
--- /dev/null
+++ b/src/copy-in-out.c
@@ -0,0 +1,250 @@
+/* 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 (guestfs_h *g, 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;
+
+  int remote_is_dir = guestfs_is_dir (g, remotedir);
+  if (remote_is_dir == -1)
+    return -1;
+
+  if (!remote_is_dir) {
+    error (g, _("target '%s' is not a directory"),
remotedir);
+    return -1;
+  }
+
+  if (split_path (g, 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) {
+    perrorf (g, "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 (g, buf, buf_len, remotepath, NULL, &basename) == -1)
+      return -1;
+
+    if (asprintf (&filename, "%s/%s", localdir, basename) == -1)
{
+      perrorf (g, "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 (g, 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) {
+      perrorf (g, "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 (guestfs_h *g, 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) {
+    error (g, _("error: argument is zero length or longer than maximum
permitted"));
+    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

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

Add the copy-in operation to virt-customize/builder/sysprep, so it is
possible to easily copy directories at once in the guest.
---
 builder/cmdline.ml         |  2 +-
 customize/customize_run.ml |  4 ++++
 generator/customize.ml     | 13 +++++++++++++
 3 files changed, 18 insertions(+), 1 deletion(-)

diff --git a/builder/cmdline.ml b/builder/cmdline.ml
index 1c6ab98..9dad141 100644
--- a/builder/cmdline.ml
+++ b/builder/cmdline.ml
@@ -309,7 +309,7 @@ read the man page virt-builder(1).
           | `FirstbootScript _ | `Hostname _ | `Link _ | `Mkdir _
           | `Password _ | `RootPassword _ | `Scrub _ | `SSHInject _
           | `Timezone _ | `Upload _ | `Write _ | `Chmod _
-          | `CommandsFromFile _ -> false
+          | `CommandsFromFile _ | `CopyIn _ -> false
         ) ops.ops in
         if requires_execute_on_guest then
           error (f_"sorry, cannot run commands on a guest with a different
architecture");
diff --git a/customize/customize_run.ml b/customize/customize_run.ml
index fed905b..3c82cb4 100644
--- a/customize/customize_run.ml
+++ b/customize/customize_run.ml
@@ -175,6 +175,10 @@ exec >>%s 2>&1
        * read when their arguments are met. *)
       ()
 
+    | `CopyIn (localpath, remotedir) ->
+      msg (f_"Copying: %s to %s") localpath remotedir;
+      g#copy_in localpath remotedir
+
     | `Delete path ->
       msg (f_"Deleting: %s") path;
       g#rm_rf path
diff --git a/generator/customize.ml b/generator/customize.ml
index c041f82..1c9092b 100644
--- a/generator/customize.ml
+++ b/generator/customize.ml
@@ -85,6 +85,19 @@ as if they were specified as I<--delete /some/file> on
the command
 line.";
   };
 
+  { op_name = "copy-in";
+    op_type = StringPair "LOCALPATH:REMOTEDIR";
+    op_discrim = "`CopyIn";
+    op_shortdesc = "Copy local files or directories into image";
+    op_pod_longdesc = "\
+Copy local files or directories recursively into the disk image,
+placing them in the directory C<REMOTEDIR> (which must exist).
+
+Wildcards cannot be used.
+
+See also: I<--copy-out>.";
+  };
+
   { op_name = "delete";
     op_type = String "PATH";
     op_discrim = "`Delete";
-- 
1.9.3

On Mon, Feb 02, 2015 at 03:44:43PM +0100, Pino Toscano
wrote:
> +  { op_name = "copy-in";
> +    op_type = StringPair "LOCALPATH:REMOTEDIR";
> +    op_discrim = "`CopyIn";
> +    op_shortdesc = "Copy local files or directories into image";
> +    op_pod_longdesc = "\
> +Copy local files or directories recursively into the disk image,
> +placing them in the directory C<REMOTEDIR> (which must exist).
> +
> +Wildcards cannot be used.
> +
> +See also: I<--copy-out>.";
--copy-out doesn't exist ..?

I removed this line and pushed the rest.

Thanks,

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
libguestfs lets you edit virtual machines.  Supports shell scripting,
bindings from many languages.  http://libguestfs.org