Libguestfs - May 2010 - [PATCH] Improve errors from tar-in/tgz-in commands (RHBZ#591155 RHBZ#591250).

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming blog: http://rwmj.wordpress.com
Fedora now supports 80 OCaml packages (the OPEN alternative to F#)
http://cocan.org/getting_started_with_ocaml_on_red_hat_and_fedora
-------------- next part --------------
>From 29f37dd36c836974d5db53d6f26571f7708b3172 Mon Sep 17 00:00:00 2001
From: Richard Jones <rjones at redhat.com>
Date: Thu, 13 May 2010 09:56:27 +0100
Subject: [PATCH 2/2] Improve errors from tar-in/tgz-in commands (RHBZ#591155
RHBZ#591250).

This commit improves the error messages from the tar-in, tgz-in (etc)
commands by capturing the stderr from the tar command in a file and
sending that back in the error message.

The method used for the error file is primitive, and there is a case
for a more generic error file mechanism, but this will do for now.

Sample error messages after this change:

$ virt-tar -u /tmp/test1.img /tmp/not.tar /
tar_in: tar subcommand failed on directory: /: tar: This does not look like a
tar archive
tar: Skipping to next header
tar: Exiting with failure status due to previous errors at
/home/rjones/d/libguestfs/tools/virt-tar line 247.

$ virt-tar -u /tmp/test1.img /tmp/test.tar /
tar_in: tar subcommand failed on directory: /: tar: access.log: Cannot open:
Read-only file system
tar: Exiting with failure status due to previous errors at
/home/rjones/d/libguestfs/tools/virt-tar line 247.
---
 daemon/.gitignore         |    1 +
 daemon/m4/gnulib-cache.m4 |    3 +-
 daemon/tar.c              |   49 +++++++++++++++++++++++++++++++++++++++-----
 3 files changed, 46 insertions(+), 7 deletions(-)

diff --git a/daemon/.gitignore b/daemon/.gitignore
index 130acb1..440470e 100644
--- a/daemon/.gitignore
+++ b/daemon/.gitignore
@@ -100,6 +100,7 @@ m4/printf.m4
 m4/priv-set.m4
 m4/rawmemchr.m4
 m4/readlink.m4
+m4/read-file.m4
 m4/realloc.m4
 m4/rmdir.m4
 m4/safe-read.m4
diff --git a/daemon/m4/gnulib-cache.m4 b/daemon/m4/gnulib-cache.m4
index 91c6ac2..f026cb3 100644
--- a/daemon/m4/gnulib-cache.m4
+++ b/daemon/m4/gnulib-cache.m4
@@ -15,7 +15,7 @@
 
 
 # Specification in the form of a command-line invocation:
-#   gnulib-tool --import --dir=. --lib=libgnu --source-base=lib --m4-base=m4
--doc-base=doc --tests-base=tests --aux-dir=build-aux --with-tests --no-libtool
--macro-prefix=gl byteswap c-ctype connect fsusage futimens getaddrinfo getline
glob hash ignore-value manywarnings mkdtemp netdb openat perror pread readlink
select sleep socket strchrnul strndup symlinkat sys_select sys_wait vasprintf
warnings
+#   gnulib-tool --import --dir=. --lib=libgnu --source-base=lib --m4-base=m4
--doc-base=doc --tests-base=tests --aux-dir=build-aux --with-tests --no-libtool
--macro-prefix=gl byteswap c-ctype connect error fsusage futimens getaddrinfo
getline glob hash ignore-value manywarnings mkdtemp netdb openat perror pread
read-file readlink select sleep socket strchrnul strndup symlinkat sys_select
sys_wait vasprintf warnings
 
 # Specification in the form of a few gnulib-tool.m4 macro invocations:
 gl_LOCAL_DIR([])
@@ -37,6 +37,7 @@ gl_MODULES([
   openat
   perror
   pread
+  read-file
   readlink
   select
   sleep
diff --git a/daemon/tar.c b/daemon/tar.c
index 1e9698c..4c691bd 100644
--- a/daemon/tar.c
+++ b/daemon/tar.c
@@ -23,10 +23,40 @@
 #include <string.h>
 #include <fcntl.h>
 
+#include "read-file.h"
+
 #include "../src/guestfs_protocol.h"
 #include "daemon.h"
 #include "actions.h"
 
+/* Redirect errors from the tar command to the error file, then
+ * provide functions for reading it in.  We overwrite the file each
+ * time, and since it's small and stored on the appliance we don't
+ * bother to delete it.
+ */
+static const char *error_file = "/tmp/error";
+
+static char *
+read_error_file (void)
+{
+  size_t len;
+  char *str = read_file (error_file, &len);
+  if (str == NULL) {
+    str = strdup ("(no error)");
+    if (str == NULL) {
+      perror ("strdup");
+      exit (EXIT_FAILURE);
+    }
+    len = strlen (str);
+  }
+
+  /* Remove trailing \n character if any. */
+  if (len > 0 && str[len-1] == '\n')
+    str[--len] = '\0';
+
+  return str;                   /* caller frees */
+}
+
 static int
 write_cb (void *fd_ptr, const void *buf, size_t len)
 {
@@ -43,8 +73,8 @@ do_tXz_in (const char *dir, const char *filter)
   char *cmd;
 
   /* "tar -C /sysroot%s -xf -" but we have to quote the dir. */
-  if (asprintf_nowarn (&cmd, "tar -C %R -%sxf -",
-                       dir, filter) == -1) {
+  if (asprintf_nowarn (&cmd, "tar -C %R -xf - 2> %s",
+                       dir, filter, error_file) == -1) {
     err = errno;
     r = cancel_receive ();
     errno = err;
@@ -73,8 +103,11 @@ do_tXz_in (const char *dir, const char *filter)
 
   r = receive_file (write_cb, &fd);
   if (r == -1) {		/* write error */
-    if (cancel_receive () != -2)
-      reply_with_error ("write error on directory: %s", dir);
+    if (cancel_receive () != -2) {
+      char *errstr = read_error_file ();
+      reply_with_error ("write error on directory: %s: %s", dir,
errstr);
+      free (errstr);
+    }
     pclose (fp);
     return -1;
   }
@@ -87,8 +120,12 @@ do_tXz_in (const char *dir, const char *filter)
   if (pclose (fp) != 0) {
     if (r == -1)                /* if r == 0, file transfer ended already */
       r = cancel_receive ();
-    if (r != -2)
-      reply_with_error ("tar subcommand failed on directory: %s",
dir);
+    if (r != -2) {
+      char *errstr = read_error_file ();
+      reply_with_error ("tar subcommand failed on directory: %s: %s",
+                        dir, errstr);
+      free (errstr);
+    }
     return -1;
   }
 
-- 
1.6.6.1

On Thu, May 13, 2010 at 10:26:19AM +0100, Richard W.M. Jones
wrote:
> -  if (asprintf_nowarn (&cmd, "tar -C %R -%sxf -",
> -                       dir, filter) == -1) {
> +  if (asprintf_nowarn (&cmd, "tar -C %R -xf - 2> %s",
> +                       dir, filter, error_file) == -1) {
The patch got mangled here somehow.  This should read:

-  if (asprintf_nowarn (&cmd, "tar -C %R -%sxf -",
-                       dir, filter) == -1) {
+  if (asprintf_nowarn (&cmd, "tar -C %R -%sxf - 2> %s",
+                       dir, filter, error_file) == -1) {

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
New in Fedora 11: Fedora Windows cross-compiler. Compile Windows
programs, test, and build Windows installers. Over 70 libraries supprt'd
http://fedoraproject.org/wiki/MinGW http://www.annexia.org/fedora_mingw