Libguestfs - Sep 2015 - [PATCH 1/2] copy-in: print tar stderr when it fails

Get also the fd for the tar subprocess, and drain and print its content
if the tar invocation fails.
---
 src/copy-in-out.c | 65 +++++++++++++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 61 insertions(+), 4 deletions(-)

diff --git a/src/copy-in-out.c b/src/copy-in-out.c
index dc9e7b7..0dd8cd3 100644
--- a/src/copy-in-out.c
+++ b/src/copy-in-out.c
@@ -34,17 +34,19 @@
 #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);
+static int drain_fd (guestfs_h *g, int fd, char **ret);
 
 int
 guestfs_impl_copy_in (guestfs_h *g, const char *localpath, const char
*remotedir)
 {
   CLEANUP_CMD_CLOSE struct command *cmd = guestfs_int_new_command (g);
-  int fd;
+  int fd, err_fd;
   int r;
   char fdbuf[64];
   size_t buf_len = strlen (localpath) + 1;
   char buf[buf_len];
   const char *dirname, *basename;
+  CLEANUP_FREE char *tar_buf = NULL;
 
   int remote_is_dir = guestfs_is_dir (g, remotedir);
   if (remote_is_dir == -1)
@@ -67,7 +69,7 @@ guestfs_impl_copy_in (guestfs_h *g, const char *localpath,
const char *remotedir
   guestfs_int_cmd_add_arg (cmd, "-");
   guestfs_int_cmd_add_arg (cmd, basename);
 
-  r = guestfs_int_cmd_run_async (cmd, NULL, NULL, &fd, NULL);
+  r = guestfs_int_cmd_run_async (cmd, NULL, NULL, &fd, &err_fd);
   if (r == -1)
     return -1;
 
@@ -81,10 +83,20 @@ guestfs_impl_copy_in (guestfs_h *g, const char *localpath,
const char *remotedir
   }
 
   r = guestfs_int_cmd_wait (cmd);
-  if (r == -1)
+  if (r == -1) {
+    if (drain_fd (g, err_fd, &tar_buf) == 0) {
+      close (err_fd);
+      error (g, _("tar command failed: %s"), tar_buf);
+    }
     return -1;
-  if (!(WIFEXITED (r) && WEXITSTATUS (r) == 0))
+  }
+  if (!(WIFEXITED (r) && WEXITSTATUS (r) == 0)) {
+    if (drain_fd (g, err_fd, &tar_buf) == 0) {
+      close (err_fd);
+      error (g, _("tar command failed with %d: %s"), WEXITSTATUS (r),
tar_buf);
+    }
     return -1;
+  }
 
   return 0;
 }
@@ -248,3 +260,48 @@ split_path (guestfs_h *g, char *buf, size_t buf_size,
 
   return 0;
 }
+
+static int
+drain_fd (guestfs_h *g, int fd, char **ret)
+{
+  char *data = NULL;
+  size_t size = 0;
+  size_t n = 0;
+  size_t left = 0;
+  ssize_t r;
+
+  while (1) {
+    size_t to_read;
+    if (left > 0) {
+      to_read = left;
+    } else {
+      to_read = 1024;
+      size += to_read;
+      data = safe_realloc (g, data, size);
+    }
+    r = read (fd, &data[n], to_read);
+    if (r == -1) {
+      perrorf (g, _("drain_fd: read"));
+      free (data);
+      close (fd);
+      return -1;
+    }
+    if (r == 0)
+      break;
+    n += r;
+    left = 1024 - r;
+  }
+
+  if (close (fd) == -1) {
+    perrorf (g, _("drain_fd: close"));
+    free (data);
+    return -1;
+  }
+
+  data[n] = 0;
+  if (n > 0 && data[n-1] == '\n')
+    data[n-1] = 0;
+
+  *ret = data;
+  return 0;
+}
-- 
2.1.0

---
 src/copy-in-out.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/src/copy-in-out.c b/src/copy-in-out.c
index 0dd8cd3..2b1e4d4 100644
--- a/src/copy-in-out.c
+++ b/src/copy-in-out.c
@@ -47,6 +47,12 @@ guestfs_impl_copy_in (guestfs_h *g, const char *localpath,
const char *remotedir
   char buf[buf_len];
   const char *dirname, *basename;
   CLEANUP_FREE char *tar_buf = NULL;
+  struct stat statbuf;
+
+  if (stat (localpath, &statbuf) == -1) {
+    error (g, _("source '%s' does not exist (or cannot be
read)"), localpath);
+    return -1;
+  }
 
   int remote_is_dir = guestfs_is_dir (g, remotedir);
   if (remote_is_dir == -1)
-- 
2.1.0

On Tue, Sep 29, 2015 at 11:38:52AM +0200, Pino Toscano
wrote:
> ---
>  src/copy-in-out.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/src/copy-in-out.c b/src/copy-in-out.c
> index 0dd8cd3..2b1e4d4 100644
> --- a/src/copy-in-out.c
> +++ b/src/copy-in-out.c
> @@ -47,6 +47,12 @@ guestfs_impl_copy_in (guestfs_h *g, const char
*localpath, const char *remotedir
>    char buf[buf_len];
>    const char *dirname, *basename;
>    CLEANUP_FREE char *tar_buf = NULL;
> +  struct stat statbuf;
> +
> +  if (stat (localpath, &statbuf) == -1) {
> +    error (g, _("source '%s' does not exist (or cannot be
read)"), localpath);
> +    return -1;
> +  }
>  
>    int remote_is_dir = guestfs_is_dir (g, remotedir);
>    if (remote_is_dir == -1)
NACK.  See the patch series I just posted.

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