Libguestfs - Jun 2016 - [PATCH 0/7] p2v: Multiple improvements to the look of virt-p2v.

In the run dialog, I have implemented an ANSI colour escape sequence
interpreter, so that colours displayed by the remote virt-v2v are now
shown to the user.
(https://bugzilla.redhat.com/show_bug.cgi?id=1314244)

This requires virt-v2v to send colours.  It wasn't doing that because
the output was a pipe (as we capture the output into the log file).
So I added a global --colours option to make the tools show ANSI
escape sequences even if the output is not a tty.

We don't want to show debug messages to the user during conversion.
This was tricky: I solved it by only sending back stdout from virt-v2v
to virt-p2v.  However that doesn't show error messages, so if an error
is encountered, a virt-v2v wrapper script shows the last few lines of
the complete log file (in which the error appears).

Various font fixes.

Rich.

---
 mllib/common_utils.ml | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/mllib/common_utils.ml b/mllib/common_utils.ml
index 64bf3d3..e54272c 100644
--- a/mllib/common_utils.ml
+++ b/mllib/common_utils.ml
@@ -366,11 +366,11 @@ let error ?(exit_code = 1) fs  
 let warning fs    let display str -    let chan = stderr in
+    let chan = stdout in
     ansi_blue ~chan ();
     wrap ~chan (sprintf (f_"%s: warning: %s") prog str);
     ansi_restore ~chan ();
-    prerr_newline ();
+    print_newline ();
   in
   ksprintf display fs
 
-- 
2.7.4

This option (alternately spelled: --color, --colour, --colors, or
--colours) enables ANSI colour sequences output even if that would be
disabled becaues the output is not a TTY.
---
 builder/virt-builder.pod       |  9 +++++++++
 customize/virt-customize.pod   |  9 +++++++++
 dib/virt-dib.pod               |  9 +++++++++
 get-kernel/virt-get-kernel.pod |  9 +++++++++
 mllib/common_utils.ml          | 42 +++++++++++++++++++++++++-----------------
 resize/virt-resize.pod         |  9 +++++++++
 sparsify/virt-sparsify.pod     |  9 +++++++++
 sysprep/virt-sysprep.pod       |  9 +++++++++
 v2v/cmdline.ml                 |  1 +
 v2v/virt-v2v.pod               |  9 +++++++++
 10 files changed, 98 insertions(+), 17 deletions(-)

diff --git a/builder/virt-builder.pod b/builder/virt-builder.pod
index cd3be9d..91c1114 100644
--- a/builder/virt-builder.pod
+++ b/builder/virt-builder.pod
@@ -241,6 +241,15 @@ Using I<--no-check-signature> bypasses this check.
 
 See also I<--fingerprint>.
 
+=item B<--colors>
+
+=item B<--colours>
+
+Use ANSI colour sequences to colourize messages.  This is the default
+when the output is a tty.  If the output of the program is redirected
+to a file, ANSI colour sequences are disabled unless you use this
+option.
+
 =item B<--curl> CURL
 
 Specify an alternate L<curl(1)> binary.  You can also use this to add
diff --git a/customize/virt-customize.pod b/customize/virt-customize.pod
index 7654fee..e594f61 100644
--- a/customize/virt-customize.pod
+++ b/customize/virt-customize.pod
@@ -74,6 +74,15 @@ disk format (not just an ISO).
 Specify the disk format for the next I<--attach> option.  The
 C<FORMAT> is usually C<raw> or C<qcow2>.  Use C<raw>
for ISOs.
 
+=item B<--colors>
+
+=item B<--colours>
+
+Use ANSI colour sequences to colourize messages.  This is the default
+when the output is a tty.  If the output of the program is redirected
+to a file, ANSI colour sequences are disabled unless you use this
+option.
+
 =item B<-c> URI
 
 =item B<--connect> URI
diff --git a/dib/virt-dib.pod b/dib/virt-dib.pod
index 9baae23..8ccb9f5 100644
--- a/dib/virt-dib.pod
+++ b/dib/virt-dib.pod
@@ -72,6 +72,15 @@ Right now this option does nothing more than setting the
C<ARCH>
 environment variable for the elements, and it's up to them to
 produce an image for the requested architecture.
 
+=item B<--colors>
+
+=item B<--colours>
+
+Use ANSI colour sequences to colourize messages.  This is the default
+when the output is a tty.  If the output of the program is redirected
+to a file, ANSI colour sequences are disabled unless you use this
+option.
+
 =item B<--debug> LEVEL
 
 Set the debug level to C<LEVEL>, which is a non-negative integer
diff --git a/get-kernel/virt-get-kernel.pod b/get-kernel/virt-get-kernel.pod
index 92d6cb6..97a159c 100644
--- a/get-kernel/virt-get-kernel.pod
+++ b/get-kernel/virt-get-kernel.pod
@@ -44,6 +44,15 @@ force a particular format use the I<--format> option.
 Add a remote disk.  The URI format is compatible with guestfish.
 See L<guestfish(1)/ADDING REMOTE STORAGE>.
 
+=item B<--colors>
+
+=item B<--colours>
+
+Use ANSI colour sequences to colourize messages.  This is the default
+when the output is a tty.  If the output of the program is redirected
+to a file, ANSI colour sequences are disabled unless you use this
+option.
+
 =item B<-c> URI
 
 =item B<--connect> URI
diff --git a/mllib/common_utils.ml b/mllib/common_utils.ml
index e54272c..a663027 100644
--- a/mllib/common_utils.ml
+++ b/mllib/common_utils.ml
@@ -295,27 +295,16 @@ let protect ~f ~finally    finally ();
   match r with Either ret -> ret | Or exn -> raise exn
 
-let istty chan -  Unix.isatty (Unix.descr_of_out_channel chan)
-
-(* ANSI terminal colours. *)
-let ansi_green ?(chan = stdout) () -  if istty chan then output_string chan
"\x1b[0;32m"
-let ansi_red ?(chan = stdout) () -  if istty chan then output_string chan
"\x1b[1;31m"
-let ansi_blue ?(chan = stdout) () -  if istty chan then output_string chan
"\x1b[1;34m"
-let ansi_magenta ?(chan = stdout) () -  if istty chan then output_string chan
"\x1b[1;35m"
-let ansi_restore ?(chan = stdout) () -  if istty chan then output_string chan
"\x1b[0m"
-
 (* Program name. *)
 let prog = Filename.basename Sys.executable_name
 
-(* Stores the quiet (--quiet), trace (-x) and verbose (-v) flags in a
- * global variable.
+(* Stores the colours (--colours), quiet (--quiet), trace (-x) and
+ * verbose (-v) flags in a global variable.
  *)
+let colours = ref false
+let set_colours () = colours := true
+let colours () = !colours
+
 let quiet = ref false
 let set_quiet () = quiet := true
 let quiet () = !quiet
@@ -328,6 +317,21 @@ let verbose = ref false
 let set_verbose () = verbose := true
 let verbose () = !verbose
 
+(* ANSI terminal colours. *)
+let istty chan +  Unix.isatty (Unix.descr_of_out_channel chan)
+
+let ansi_green ?(chan = stdout) () +  if colours () || istty chan then
output_string chan "\x1b[0;32m"
+let ansi_red ?(chan = stdout) () +  if colours () || istty chan then
output_string chan "\x1b[1;31m"
+let ansi_blue ?(chan = stdout) () +  if colours () || istty chan then
output_string chan "\x1b[1;34m"
+let ansi_magenta ?(chan = stdout) () +  if colours () || istty chan then
output_string chan "\x1b[1;35m"
+let ansi_restore ?(chan = stdout) () +  if colours () || istty chan then
output_string chan "\x1b[0m"
+
 (* Timestamped progress messages, used for ordinary messages when not
  * --quiet.
  *)
@@ -592,6 +596,10 @@ let set_standard_options argspec     
"--debug-gc",   Arg.Unit set_debug_gc,     " " ^
s_"Debug GC and memory allocations (internal)";
     "-q",           Arg.Unit set_quiet,        " " ^
s_"Don't print progress messages";
     "--quiet",      Arg.Unit set_quiet,        " " ^
s_"Don't print progress messages";
+    "--color",      Arg.Unit set_colours,      " " ^
s_"Use ANSI colour sequences even if not tty";
+    "--colors",     Arg.Unit set_colours,      " " ^
s_"Use ANSI colour sequences even if not tty";
+    "--colour",     Arg.Unit set_colours,      " " ^
s_"Use ANSI colour sequences even if not tty";
+    "--colours",    Arg.Unit set_colours,      " " ^
s_"Use ANSI colour sequences even if not tty";
   ] @ argspec in
   let argspec      let cmp (arg1, _, _) (arg2, _, _) =
compare_command_line_args arg1 arg2 in
diff --git a/resize/virt-resize.pod b/resize/virt-resize.pod
index a0ab459..2b2a485 100644
--- a/resize/virt-resize.pod
+++ b/resize/virt-resize.pod
@@ -327,6 +327,15 @@ since around 2008.
 
 =back
 
+=item B<--colors>
+
+=item B<--colours>
+
+Use ANSI colour sequences to colourize messages.  This is the default
+when the output is a tty.  If the output of the program is redirected
+to a file, ANSI colour sequences are disabled unless you use this
+option.
+
 =item B<-d>
 
 =item B<--debug>
diff --git a/sparsify/virt-sparsify.pod b/sparsify/virt-sparsify.pod
index b841251..44f7d4e 100644
--- a/sparsify/virt-sparsify.pod
+++ b/sparsify/virt-sparsify.pod
@@ -155,6 +155,15 @@ B<fail> and exit.
 
 You cannot use this option and I<--in-place> together.
 
+=item B<--colors>
+
+=item B<--colours>
+
+Use ANSI colour sequences to colourize messages.  This is the default
+when the output is a tty.  If the output of the program is redirected
+to a file, ANSI colour sequences are disabled unless you use this
+option.
+
 =item B<--compress>
 
 Compress the output file.  This I<only> works if the output format is
diff --git a/sysprep/virt-sysprep.pod b/sysprep/virt-sysprep.pod
index 0c1c7c6..bdb4580 100644
--- a/sysprep/virt-sysprep.pod
+++ b/sysprep/virt-sysprep.pod
@@ -59,6 +59,15 @@ force a particular format use the I<--format> option.
 Add a remote disk.  The URI format is compatible with guestfish.
 See L<guestfish(1)/ADDING REMOTE STORAGE>.
 
+=item B<--colors>
+
+=item B<--colours>
+
+Use ANSI colour sequences to colourize messages.  This is the default
+when the output is a tty.  If the output of the program is redirected
+to a file, ANSI colour sequences are disabled unless you use this
+option.
+
 =item B<-c> URI
 
 =item B<--connect> URI
diff --git a/v2v/cmdline.ml b/v2v/cmdline.ml
index c05dbe3..1064987 100644
--- a/v2v/cmdline.ml
+++ b/v2v/cmdline.ml
@@ -278,6 +278,7 @@ read the man page virt-v2v(1).
   if args = [] && machine_readable then (
     printf "virt-v2v\n";
     printf "libguestfs-rewrite\n";
+    printf "colours-option\n";
     List.iter (printf "input:%s\n") (Modules_list.input_modules ());
     List.iter (printf "output:%s\n") (Modules_list.output_modules
());
     List.iter (printf "convert:%s\n") (Modules_list.convert_modules
());
diff --git a/v2v/virt-v2v.pod b/v2v/virt-v2v.pod
index 292c5a3..a781eb3 100644
--- a/v2v/virt-v2v.pod
+++ b/v2v/virt-v2v.pod
@@ -257,6 +257,15 @@ Display help.
 
 See I<--network> below.
 
+=item B<--colors>
+
+=item B<--colours>
+
+Use ANSI colour sequences to colourize messages.  This is the default
+when the output is a tty.  If the output of the program is redirected
+to a file, ANSI colour sequences are disabled unless you use this
+option.
+
 =item B<--compressed>
 
 Write a compressed output file.  This is only allowed if the output
-- 
2.7.4

---
 p2v/conversion.c |   5 +-
 p2v/gui.c        | 184 +++++++++++++++++++++++++++++++++++++++++++++----------
 p2v/main.c       |   1 +
 p2v/p2v.h        |   3 +
 p2v/ssh.c        |  24 ++++++--
 5 files changed, 175 insertions(+), 42 deletions(-)

diff --git a/p2v/conversion.c b/p2v/conversion.c
index 90e1fbb..dfee0fe 100644
--- a/p2v/conversion.c
+++ b/p2v/conversion.c
@@ -333,9 +333,10 @@ start_conversion (struct config *config,
   /* Build the virt-v2v command up in pieces to make the quoting
    * slightly more sane.
    */
-  if (mexp_printf (control_h, "( %s virt-v2v%s -i libvirtxml",
+  if (mexp_printf (control_h, "( %s virt-v2v%s%s -i libvirtxml",
                    config->sudo ? "sudo -n " : "",
-                   config->verbose ? " -v -x" : "") ==
-1) {
+                   config->verbose ? " -v -x" : "",
+                   feature_colours_option ? " --colours" :
"") == -1) {
   printf_fail:
     set_conversion_error ("mexp_printf: virt-v2v command: %m");
     goto out;
diff --git a/p2v/gui.c b/p2v/gui.c
index 4188ba6..c34bc6c 100644
--- a/p2v/gui.c
+++ b/p2v/gui.c
@@ -208,6 +208,9 @@ static GtkWidget *run_dlg,
   *v2v_output_sw, *v2v_output, *log_label, *status_label,
   *cancel_button, *reboot_button;
 
+/* Colour tags used in the v2v_output GtkTextBuffer. */
+static GtkTextTag *v2v_output_tags[16];
+
 /**
  * The entry point from the main program.
  *
@@ -1649,6 +1652,12 @@ static gboolean close_running_dialog (GtkWidget *w,
GdkEvent *event, gpointer da
 static void
 create_running_dialog (void)
 {
+  size_t i;
+  static const char *tags[16] +    { "black", "maroon",
"green", "olive", "navy", "purple",
"teal", "silver",
+      "gray", "red", "lime", "yellow",
"blue", "fuchsia", "cyan", "white" };
+  GtkTextBuffer *buf;
+
   run_dlg = gtk_dialog_new ();
   gtk_window_set_title (GTK_WINDOW (run_dlg), guestfs_int_program_name);
   gtk_window_set_resizable (GTK_WINDOW (run_dlg), FALSE);
@@ -1662,6 +1671,17 @@ create_running_dialog (void)
   gtk_text_view_set_wrap_mode (GTK_TEXT_VIEW (v2v_output), GTK_WRAP_CHAR);
   /* XXX Gtk3 ignores this, why? */
   gtk_widget_set_size_request (v2v_output, 700, 400);
+
+  buf = gtk_text_view_get_buffer (GTK_TEXT_VIEW (v2v_output));
+  for (i = 0; i < 16; ++i) {
+    CLEANUP_FREE char *tag_name;
+
+    if (asprintf (&tag_name, "tag_%s", tags[i]) == -1)
+      error (EXIT_FAILURE, errno, "asprintf");
+    v2v_output_tags[i] +      gtk_text_buffer_create_tag (buf, tag_name,
"foreground", tags[i], NULL);
+  }
+
   log_label = gtk_label_new (NULL);
   set_alignment (log_label, 0., 0.5);
   set_padding (log_label, 10, 10);
@@ -1767,12 +1787,12 @@ set_status (gpointer user_data)
   return FALSE;
 }
 
-static void add_v2v_output_helper (const char *msg, size_t len);
-
 /**
  * Append output from the virt-v2v process to the buffer, and scroll
  * to ensure it is visible.
  *
+ * This function is able to parse ANSI colour sequences and more.
+ *
  * If this isn't called from the main thread, then you must only
  * call it via an idle task (C<g_idle_add>).
  *
@@ -1783,44 +1803,140 @@ static gboolean
 add_v2v_output (gpointer user_data)
 {
   CLEANUP_FREE const char *msg = user_data;
+  const char *p;
   static size_t linelen = 0;
-  const char *p0, *p;
-
-  /* Gtk2 (in ~ Fedora 23) has a regression where it takes much longer
-   * to display long lines, to the point where the virt-p2v UI would
-   * still be slowly displaying kernel modules while the conversion
-   * had finished.  For this reason, arbitrarily break long lines.
-   */
-  for (p0 = p = msg; *p; ++p) {
-    linelen++;
-    if (*p == '\n' || linelen > 1024) {
-      add_v2v_output_helper (p0, p-p0+1);
-      if (*p != '\n')
-        add_v2v_output_helper ("\n", 1);
-      linelen = 0;
-      p0 = p+1;
-    }
-  }
-  add_v2v_output_helper (p0, p-p0);
-
-  return FALSE;
-}
-
-static void
-add_v2v_output_helper (const char *msg, size_t len)
-{
-  GtkTextBuffer *buf;
-  GtkTextIter iter;
-
-  /* Insert it at the end. */
-  buf = gtk_text_view_get_buffer (GTK_TEXT_VIEW (v2v_output));
-  gtk_text_buffer_get_end_iter (buf, &iter);
-  gtk_text_buffer_insert (buf, &iter, msg, len);
+  static enum {
+    state_normal,
+    state_escape1,       /* seen ESC, expecting [ */
+    state_escape2,       /* seen ESC [, expecting 0 or 1 */
+    state_escape3,       /* seen ESC [ 0/1, expecting ; or m */
+    state_escape4,       /* seen ESC [ 0/1 ;, expecting 3 */
+    state_escape5,       /* seen ESC [ 0/1 ; 3, expecting 1/2/4/5 */
+    state_escape6,       /* seen ESC [ 0/1 ; 3 1/2/5/5, expecting m */
+    state_cr,            /* seen CR */
+    state_truncating,    /* truncating line until next \n */
+  } state = state_normal;
+  static int colour = 0;
+  GtkTextTag *tag = NULL;
+  GtkTextBuffer *buf = gtk_text_view_get_buffer (GTK_TEXT_VIEW (v2v_output));
+  GtkTextIter iter, iter2;
+  const char *dots = " [...]";
+
+  for (p = msg; *p != '\0'; ++p) {
+    char c = *p;
+
+    switch (state) {
+    case state_normal:
+      if (c == '\r')            /* Start of possible CRLF sequence. */
+        state = state_cr;
+      else if (c == '\x1b') {   /* Start of an escape sequence. */
+        state = state_escape1;
+        colour = 0;
+      }
+      else if (c != '\n' && linelen >= 256) {
+        /* Gtk2 (in ~ Fedora 23) has a regression where it takes much
+         * longer to display long lines, to the point where the
+         * virt-p2v UI would still be slowly displaying kernel modules
+         * while the conversion had finished.  For this reason,
+         * arbitrarily truncate very long lines.
+         */
+        gtk_text_buffer_get_end_iter (buf, &iter);
+        gtk_text_buffer_insert_with_tags (buf, &iter,
+                                          dots, strlen (dots), tag, NULL);
+        state = state_truncating;
+        colour = 0;
+        tag = NULL;
+      }
+      else {             /* Treat everything else as a normal char. */
+        if (c != '\n') linelen++; else linelen = 0;
+        gtk_text_buffer_get_end_iter (buf, &iter);
+        gtk_text_buffer_insert_with_tags (buf, &iter, &c, 1, tag,
NULL);
+      }
+      break;
+
+    case state_escape1:
+      if (c == '[')
+        state = state_escape2;
+      else
+        state = state_normal;
+      break;
+
+    case state_escape2:
+      if (c == '0')
+        state = state_escape3;
+      else if (c == '1') {
+        state = state_escape3;
+        colour += 8;
+      }
+      else
+        state = state_normal;
+      break;
+
+    case state_escape3:
+      if (c == ';')
+        state = state_escape4;
+      else if (c == 'm') {
+        tag = NULL;             /* restore text colour */
+        state = state_normal;
+      }
+      else
+        state = state_normal;
+      break;
+
+    case state_escape4:
+      if (c == '3')
+        state = state_escape5;
+      else
+        state = state_normal;
+      break;
+
+    case state_escape5:
+      if (c >= '0' && c <= '7') {
+        state = state_escape6;
+        colour += c - '0';
+      }
+      else
+        state = state_normal;
+      break;
+
+    case state_escape6:
+      if (c == 'm') {
+        assert (colour >= 0 && colour <= 15);
+        tag = v2v_output_tags[colour]; /* set colour tag */
+      }
+      state = state_normal;
+      break;
+
+    case state_cr:
+      if (c == '\n')
+        /* Process CRLF as single a newline character. */
+        p--;
+      else {                    /* Delete current (== last) line. */
+        linelen = 0;
+        gtk_text_buffer_get_end_iter (buf, &iter);
+        iter2 = iter;
+        gtk_text_iter_set_line_offset (&iter, 0);
+        /* Delete from iter..iter2 */
+        gtk_text_buffer_delete (buf, &iter, &iter2);
+      }
+      state = state_normal;
+      break;
+
+    case state_truncating:
+      if (c == '\n') {
+        p--;
+        state = state_normal;
+      }
+      break;
+    } /* switch (state) */
+  } /* for */
 
   /* Scroll to the end of the buffer. */
   gtk_text_buffer_get_end_iter (buf, &iter);
   gtk_text_view_scroll_to_iter (GTK_TEXT_VIEW (v2v_output), &iter,
                                 0, FALSE, 0., 1.);
+
+  return FALSE;
 }
 
 /**
diff --git a/p2v/main.c b/p2v/main.c
index c2f0860..40d8097 100644
--- a/p2v/main.c
+++ b/p2v/main.c
@@ -43,6 +43,7 @@ char **all_disks;
 char **all_removable;
 char **all_interfaces;
 int is_iso_environment = 0;
+int feature_colours_option = 0;
 
 static const char *test_disk = NULL;
 
diff --git a/p2v/p2v.h b/p2v/p2v.h
index 9ccaf0f..12dd210 100644
--- a/p2v/p2v.h
+++ b/p2v/p2v.h
@@ -58,6 +58,9 @@ extern char **all_interfaces;
  */
 extern int is_iso_environment;
 
+/* True if virt-v2v supports the --colours option. */
+extern int feature_colours_option;
+
 /* config.c */
 struct config {
   int verbose;
diff --git a/p2v/ssh.c b/p2v/ssh.c
index 4ed6f98..e916ca3 100644
--- a/p2v/ssh.c
+++ b/p2v/ssh.c
@@ -100,6 +100,7 @@ static pcre *ssh_message_re;
 static pcre *prompt_re;
 static pcre *version_re;
 static pcre *feature_libguestfs_rewrite_re;
+static pcre *feature_colours_option_re;
 static pcre *feature_input_re;
 static pcre *feature_output_re;
 static pcre *portfwd_re;
@@ -148,6 +149,7 @@ compile_regexps (void)
            "virt-v2v ([1-9].*)",
 	   0);
   COMPILE (feature_libguestfs_rewrite_re, "libguestfs-rewrite", 0);
+  COMPILE (feature_colours_option_re, "colours-option", 0);
   COMPILE (feature_input_re, "input:((?:\\w)*)", 0);
   COMPILE (feature_output_re, "output:((?:\\w)*)", 0);
   COMPILE (portfwd_re, "Allocated port ((?:\\d)+) for remote
forward", 0);
@@ -161,6 +163,7 @@ free_regexps (void)
   pcre_free (prompt_re);
   pcre_free (version_re);
   pcre_free (feature_libguestfs_rewrite_re);
+  pcre_free (feature_colours_option_re);
   pcre_free (feature_input_re);
   pcre_free (feature_output_re);
   pcre_free (portfwd_re);
@@ -599,28 +602,37 @@ test_connection (struct config *config)
     switch (mexp_expect (h,
                          (mexp_regexp[]) {
                            { 100, .re = feature_libguestfs_rewrite_re },
-                           { 101, .re = feature_input_re },
-                           { 102, .re = feature_output_re },
-                           { 103, .re = prompt_re },
+                           { 101, .re = feature_colours_option_re },
+                           { 102, .re = feature_input_re },
+                           { 103, .re = feature_output_re },
+                           { 104, .re = prompt_re },
                            { 0 }
                          }, ovector, ovecsize)) {
     case 100:                   /* libguestfs-rewrite. */
       feature_libguestfs_rewrite = 1;
       break;
 
-    case 101:
+    case 101:                   /* virt-v2v supports --colours option */
+#if DEBUG_STDERR
+  fprintf (stderr, "%s: remote virt-v2v supports --colours option\n",
+           guestfs_int_program_name);
+#endif
+      feature_colours_option = 1;
+      break;
+
+    case 102:
       /* input:<driver-name> corresponds to an -i option in virt-v2v. */
       add_input_driver (&h->buffer[ovector[2]],
                         (size_t) (ovector[3]-ovector[2]));
       break;
 
-    case 102:
+    case 103:
       /* output:<driver-name> corresponds to an -o option in virt-v2v. */
       add_output_driver (&h->buffer[ovector[2]],
                          (size_t) (ovector[3]-ovector[2]));
       break;
 
-    case 103:                   /* Got prompt, so end of output. */
+    case 104:                   /* Got prompt, so end of output. */
       goto end_of_machine_readable;
 
     case MEXP_EOF:
-- 
2.7.4

Instead of constructing and directly executing a long virt-v2v command
line, build a wrapper script with the same command line and send it to
the remote server (stored in /<remote_dir>/virt-v2v-wrapper.sh).

This will make it a bit easier to construct more complex virt-v2v
wrappers.

Note this commit on its own is a simple refactoring and does not
change any functionality.
---
 p2v/conversion.c | 204 ++++++++++++++++++++++++++++++++-----------------------
 p2v/p2v.h        |   2 +-
 p2v/ssh.c        |  25 ++++++-
 p2v/virt-p2v.pod |  17 +++--
 4 files changed, 158 insertions(+), 90 deletions(-)

diff --git a/p2v/conversion.c b/p2v/conversion.c
index dfee0fe..c948bf8 100644
--- a/p2v/conversion.c
+++ b/p2v/conversion.c
@@ -91,11 +91,11 @@ struct data_conn {
   int nbd_remote_port;      /* remote NBD port on conversion server */
 };
 
-static int send_quoted (mexp_h *, const char *s);
 static pid_t start_qemu_nbd (int nbd_local_port, const char *device);
 static int wait_qemu_nbd (int nbd_local_port, int timeout_seconds);
 static void cleanup_data_conns (struct data_conn *data_conns, size_t nr);
 static char *generate_libvirt_xml (struct config *, struct data_conn *);
+static char *generate_wrapper_script (struct config *, const char *remote_dir);
 static const char *map_interface_to_network (struct config *, const char
*interface);
 
 static char *conversion_error;
@@ -179,7 +179,8 @@ start_conversion (struct config *config,
   size_t i, len;
   size_t nr_disks = guestfs_int_count_strings (config->disks);
   CLEANUP_FREE struct data_conn *data_conns = NULL;
-  CLEANUP_FREE char *remote_dir = NULL, *libvirt_xml = NULL;
+  CLEANUP_FREE char *remote_dir = NULL, *libvirt_xml = NULL,
+    *wrapper_script = NULL;
   time_t now;
   struct tm tm;
   mexp_h *control_h = NULL;
@@ -289,7 +290,18 @@ start_conversion (struct config *config,
     goto out;
 
 #if DEBUG_STDERR
-  fprintf (stderr, "%s: libvirt XML:\n%s", guestfs_int_program_name,
libvirt_xml);
+  fprintf (stderr, "%s: libvirt XML:\n%s",
+           guestfs_int_program_name, libvirt_xml);
+#endif
+
+  /* Generate the virt-v2v wrapper script. */
+  wrapper_script = generate_wrapper_script (config, remote_dir);
+  if (wrapper_script == NULL)
+    goto out;
+
+#if DEBUG_STDERR
+  fprintf (stderr, "%s: wrapper script:\n%s",
+           guestfs_int_program_name, wrapper_script);
 #endif
 
   /* Get the output from the 'dmesg' command.  We will store this
@@ -318,7 +330,9 @@ start_conversion (struct config *config,
   if (notify_ui)
     notify_ui (NOTIFY_STATUS, _("Setting up the control connection
..."));
 
-  control_h = start_remote_connection (config, remote_dir, libvirt_xml, dmesg);
+  control_h = start_remote_connection (config,
+                                       remote_dir, libvirt_xml,
+                                       wrapper_script, dmesg);
   if (control_h == NULL) {
     const char *err = get_ssh_error ();
 
@@ -330,66 +344,18 @@ start_conversion (struct config *config,
   if (notify_ui)
     notify_ui (NOTIFY_STATUS, _("Doing conversion ..."));
 
-  /* Build the virt-v2v command up in pieces to make the quoting
-   * slightly more sane.
-   */
-  if (mexp_printf (control_h, "( %s virt-v2v%s%s -i libvirtxml",
-                   config->sudo ? "sudo -n " : "",
-                   config->verbose ? " -v -x" : "",
-                   feature_colours_option ? " --colours" :
"") == -1) {
-  printf_fail:
-    set_conversion_error ("mexp_printf: virt-v2v command: %m");
+  if (mexp_printf (control_h,
+                   /* To simplify things in the wrapper script, it
+                    * writes virt-v2v's exit status to
+                    * /remote_dir/status, and here we read that and
+                    * exit the ssh shell with the same status.
+                    */
+                   "%s/virt-v2v-wrapper.sh; "
+                   "exit $(< %s/status)\n",
+                   remote_dir, remote_dir) == -1) {
+    set_conversion_error ("mexp_printf: virt-v2v: %m");
     goto out;
   }
-  if (config->output) {         /* -o */
-    if (mexp_printf (control_h, " -o ") == -1)
-      goto printf_fail;
-    if (send_quoted (control_h, config->output) == -1)
-      goto printf_fail;
-  }
-  switch (config->output_allocation) { /* -oa */
-  case OUTPUT_ALLOCATION_NONE:
-    /* nothing */
-    break;
-  case OUTPUT_ALLOCATION_SPARSE:
-    if (mexp_printf (control_h, " -oa sparse") == -1)
-      goto printf_fail;
-    break;
-  case OUTPUT_ALLOCATION_PREALLOCATED:
-    if (mexp_printf (control_h, " -oa preallocated") == -1)
-      goto printf_fail;
-    break;
-  default:
-    abort ();
-  }
-  if (config->output_format) {  /* -of */
-    if (mexp_printf (control_h, " -of ") == -1)
-      goto printf_fail;
-    if (send_quoted (control_h, config->output_format) == -1)
-      goto printf_fail;
-  }
-  if (config->output_storage) { /* -os */
-    if (mexp_printf (control_h, " -os ") == -1)
-      goto printf_fail;
-    if (send_quoted (control_h, config->output_storage) == -1)
-      goto printf_fail;
-  }
-  if (mexp_printf (control_h, " --root first") == -1)
-    goto printf_fail;
-  if (mexp_printf (control_h, " %s/physical.xml", remote_dir) == -1)
-    goto printf_fail;
-  /* no stdin, and send stdout and stderr to the same place */
-  if (mexp_printf (control_h, " </dev/null 2>&1") == -1)
-    goto printf_fail;
-  if (mexp_printf (control_h, " ; echo $? > %s/status",
remote_dir) == -1)
-    goto printf_fail;
-  if (mexp_printf (control_h, " ) | tee
%s/virt-v2v-conversion-log.txt",
-                   remote_dir) == -1)
-    goto printf_fail;
-  if (mexp_printf (control_h, "; exit $(< %s/status)", remote_dir)
== -1)
-    goto printf_fail;
-  if (mexp_printf (control_h, "\n") == -1)
-    goto printf_fail;
 
   /* Read output from the virt-v2v process and echo it through the
    * notify function, until virt-v2v closes the connection.
@@ -457,28 +423,6 @@ cancel_conversion (void)
 }
 
 /**
- * Send a shell-quoted string to remote.
- */
-static int
-send_quoted (mexp_h *h, const char *s)
-{
-  if (mexp_printf (h, "\"") == -1)
-    return -1;
-  while (*s) {
-    if (*s == '$' || *s == '`' || *s == '\\' || *s ==
'"') {
-      if (mexp_printf (h, "\\") == -1)
-        return -1;
-    }
-    if (mexp_printf (h, "%c", *s) == -1)
-      return -1;
-    ++s;
-  }
-  if (mexp_printf (h, "\"") == -1)
-    return -1;
-  return 0;
-}
-
-/**
  * Start a local L<qemu-nbd(1)> process.
  *
  * Returns the process ID (E<gt> 0) or C<0> if there is an error.
@@ -1007,3 +951,95 @@ map_interface_to_network (struct config *config, const
char *interface)
   /* No mapping found. */
   return "default";
 }
+
+/**
+ * Print a shell-quoted string on C<fp>.
+ */
+static void
+print_quoted (FILE *fp, const char *s)
+{
+  fprintf (fp, "\"");
+  while (*s) {
+    if (*s == '$' || *s == '`' || *s == '\\' || *s ==
'"')
+      fprintf (fp, "\\");
+    fprintf (fp, "%c", *s);
+    ++s;
+  }
+  fprintf (fp, "\"");
+}
+
+/**
+ * Construct the virt-v2v wrapper script.
+ *
+ * This will be sent to the remote server, and is easier than trying
+ * to "type" a long and complex single command line into the ssh
+ * connection when we start the conversion.
+ */
+static char *
+generate_wrapper_script (struct config *config, const char *remote_dir)
+{
+  FILE *fp;
+  char *output = NULL;
+  size_t output_len = 0;
+
+  fp = open_memstream (&output, &output_len);
+  if (fp == NULL)
+    error (EXIT_FAILURE, errno, "open_memstream");
+
+  fprintf (fp, "#!/bin/sh -\n");
+  fprintf (fp, "\n");
+
+  /* The virt-v2v command. */
+  fprintf (fp, "(\n");
+  if (config->sudo)
+    fprintf (fp, "sudo -n ");
+  fprintf (fp, "virt-v2v");
+  if (config->verbose)
+    fprintf (fp, " -v -x");
+  if (feature_colours_option)
+    fprintf (fp, " --colours");
+  fprintf (fp, " -i libvirtxml");
+
+  if (config->output) {         /* -o */
+    fprintf (fp, " -o ");
+    print_quoted (fp, config->output);
+  }
+
+  switch (config->output_allocation) { /* -oa */
+  case OUTPUT_ALLOCATION_NONE:
+    /* nothing */
+    break;
+  case OUTPUT_ALLOCATION_SPARSE:
+    fprintf (fp, " -oa sparse");
+    break;
+  case OUTPUT_ALLOCATION_PREALLOCATED:
+    fprintf (fp, " -oa preallocated");
+    break;
+  default:
+    abort ();
+  }
+
+  if (config->output_format) {  /* -of */
+    fprintf (fp, " -of ");
+    print_quoted (fp, config->output_format);
+  }
+
+  if (config->output_storage) { /* -os */
+    fprintf (fp, " -os ");
+    print_quoted (fp, config->output_storage);
+  }
+
+  fprintf (fp, " --root first");
+  fprintf (fp, " %s/physical.xml", remote_dir);
+  /* no stdin, and send stdout and stderr to the same place */
+  fprintf (fp, " </dev/null 2>&1");
+  fprintf (fp, "\n");
+  fprintf (fp, "echo $? > %s/status", remote_dir);
+  fprintf (fp, "\n");
+  fprintf (fp, " ) | tee %s/virt-v2v-conversion-log.txt",
remote_dir);
+  fprintf (fp, "\n");
+
+  fclose (fp);
+
+  return output;                /* caller frees */
+}
diff --git a/p2v/p2v.h b/p2v/p2v.h
index 12dd210..49c97b6 100644
--- a/p2v/p2v.h
+++ b/p2v/p2v.h
@@ -127,7 +127,7 @@ extern int conversion_is_running (void);
 /* ssh.c */
 extern int test_connection (struct config *);
 extern mexp_h *open_data_connection (struct config *, int *local_port, int
*remote_port);
-extern mexp_h *start_remote_connection (struct config *, const char
*remote_dir, const char *libvirt_xml, const char *dmesg);
+extern mexp_h *start_remote_connection (struct config *, const char
*remote_dir, const char *libvirt_xml, const char *wrapper_script, const char
*dmesg);
 extern const char *get_ssh_error (void);
 
 /* utils.c */
diff --git a/p2v/ssh.c b/p2v/ssh.c
index e916ca3..0a3ea62 100644
--- a/p2v/ssh.c
+++ b/p2v/ssh.c
@@ -886,7 +886,7 @@ wait_for_prompt (mexp_h *h)
 mexp_h *
 start_remote_connection (struct config *config,
                          const char *remote_dir, const char *libvirt_xml,
-                         const char *dmesg)
+                         const char *wrapper_script, const char *dmesg)
 {
   mexp_h *h;
   char magic[9];
@@ -942,6 +942,29 @@ start_remote_connection (struct config *config,
   if (wait_for_prompt (h) == -1)
     goto error;
 
+  /* Upload the wrapper script to the remote directory. */
+  if (mexp_printf (h,
+                   "cat > '%s/virt-v2v-wrapper.sh' <<
'__%s__'\n"
+                   "%s"
+                   "__%s__\n",
+                   remote_dir, magic,
+                   wrapper_script,
+                   magic) == -1) {
+    set_ssh_error ("mexp_printf: %m");
+    goto error;
+  }
+
+  if (wait_for_prompt (h) == -1)
+    goto error;
+
+  if (mexp_printf (h, "chmod +x %s/virt-v2v-wrapper.sh\n",
remote_dir) == -1) {
+    set_ssh_error ("mexp_printf: %m");
+    goto error;
+  }
+
+  if (wait_for_prompt (h) == -1)
+    goto error;
+
   if (dmesg != NULL) {
     /* Upload the physical host dmesg to the remote directory. */
     if (mexp_printf (h,
diff --git a/p2v/virt-p2v.pod b/p2v/virt-p2v.pod
index daf9034..923aed0 100644
--- a/p2v/virt-p2v.pod
+++ b/p2v/virt-p2v.pod
@@ -698,6 +698,13 @@ on the conversion server.  If conversion fails, you should
examine
 this log file, and you may be asked to supply the B<complete>,
 B<unedited> log file in any bug reports or support tickets.
 
+=item F<virt-v2v-wrapper.sh>
+
+I<(during/after conversion)>
+
+This is the wrapper script which is used when running virt-v2v.  For
+interest only, do not attempt to run this script yourself.
+
 =back
 
 Before conversion actually begins, virt-p2v then makes one or more
@@ -734,10 +741,12 @@ used.  Secondly libguestfs creates an overlay on top of
the NBD
 connection which stores writes in a temporary file on the conversion
 file.
 
-The final step is to send the S<C<virt-v2v -i libvirtxml physical.xml
...>>
-command to the conversion server over the control connection.  This
-references the F<physical.xml> file (see above), which in turn
-references the NBD listening port(s) of the data connection(s).
+The long S<C<virt-v2v -i libvirtxml physical.xml ...>> command is
+wrapped inside a wrapper script and uploaded to the conversion server.
+The final step is to run this wrapper script, in turn running the
+virt-v2v command.  The virt-v2v command references the F<physical.xml>
+file (see above), which in turn references the NBD listening port(s)
+of the data connection(s).
 
 Output from the virt-v2v command (messages, debugging etc) is saved
 both in the log file on the conversion server, and sent over the
-- 
2.7.4

Previously we displayed the complete output of virt-v2v in the run
dialog.  This output included all the debugging messages, and was very
long and confusing for users (especially we had false bug reports
about "errors" appearing in the debug output).

Only display stdout in the run dialog.  However make sure everything
(stdout and stderr) is still logged to the conversion log.
---
 p2v/conversion.c | 46 +++++++++++++++++++++++++++++++++++++++-------
 p2v/virt-p2v.pod |  7 ++++---
 2 files changed, 43 insertions(+), 10 deletions(-)

diff --git a/p2v/conversion.c b/p2v/conversion.c
index c948bf8..7538fea 100644
--- a/p2v/conversion.c
+++ b/p2v/conversion.c
@@ -986,11 +986,12 @@ generate_wrapper_script (struct config *config, const char
*remote_dir)
   if (fp == NULL)
     error (EXIT_FAILURE, errno, "open_memstream");
 
-  fprintf (fp, "#!/bin/sh -\n");
+  fprintf (fp, "#!/bin/bash -\n");
   fprintf (fp, "\n");
 
-  /* The virt-v2v command. */
-  fprintf (fp, "(\n");
+  /* The virt-v2v command, as a shell function called "v2v". */
+  fprintf (fp, "v2v ()\n");
+  fprintf (fp, "{\n");
   if (config->sudo)
     fprintf (fp, "sudo -n ");
   fprintf (fp, "virt-v2v");
@@ -1031,14 +1032,45 @@ generate_wrapper_script (struct config *config, const
char *remote_dir)
 
   fprintf (fp, " --root first");
   fprintf (fp, " %s/physical.xml", remote_dir);
-  /* no stdin, and send stdout and stderr to the same place */
-  fprintf (fp, " </dev/null 2>&1");
+  fprintf (fp, " </dev/null");  /* no stdin */
   fprintf (fp, "\n");
-  fprintf (fp, "echo $? > %s/status", remote_dir);
+  fprintf (fp,
+           "# Save the exit code of virt-v2v into the 'status'
file.\n");
+  fprintf (fp, "echo $? > $status\n");
+  fprintf (fp, "}\n");
   fprintf (fp, "\n");
-  fprintf (fp, " ) | tee %s/virt-v2v-conversion-log.txt",
remote_dir);
+
+  fprintf (fp,
+           "# Write a pre-emptive error status, in case the
virt-v2v\n"
+           "# command doesn't get to run at all.  This will be\n"
+           "# overwritten with the true exit code when virt-v2v
runs.\n");
+  fprintf (fp, "status=%s/status\n", remote_dir);
+  fprintf (fp, "echo 99 > $status\n");
   fprintf (fp, "\n");
 
+  fprintf (fp, "log=%s/virt-v2v-conversion-log.txt\n", remote_dir);
+  fprintf (fp, "rm -f $log\n");
+  fprintf (fp, "\n");
+
+  fprintf (fp,
+           "# Run virt-v2v.  Send stdout back to virt-p2v.  Send
stdout\n"
+           "# and stderr (debugging info) to the log file.\n");
+  fprintf (fp, "v2v 2>> $log | tee -a $log\n");
+  fprintf (fp, "\n");
+
+  fprintf (fp,
+           "# If virt-v2v failed then the error message (sent to
stderr)\n"
+           "# will not be seen in virt-p2v.  Send the last few lines
of\n"
+           "# the log back to virt-p2v in this case.\n");
+  fprintf (fp,
+           "if [ \"$(< $status)\" -ne 0 ]; then\n"
+           "    echo\n"
+           "    echo\n"
+           "    echo\n"
+           "    echo '*** virt-v2v command failed ***'\n"
+           "    tail -30 $log\n"
+           "fi\n");
+
   fclose (fp);
 
   return output;                /* caller frees */
diff --git a/p2v/virt-p2v.pod b/p2v/virt-p2v.pod
index 923aed0..945d8c2 100644
--- a/p2v/virt-p2v.pod
+++ b/p2v/virt-p2v.pod
@@ -267,7 +267,7 @@ When conversion is running you will see this dialog:
  │                                                        │
  └────────────────────────────────────────────────────────┘
 
-In the main scrolling area you will see log messages from the virt-v2v
+In the main scrolling area you will see messages from the virt-v2v
 process.
 
 Below the main area, virt-p2v shows you the location of the directory
@@ -749,8 +749,9 @@ file (see above), which in turn references the NBD listening
port(s)
 of the data connection(s).
 
 Output from the virt-v2v command (messages, debugging etc) is saved
-both in the log file on the conversion server, and sent over the
-control connection to be displayed in the graphical UI.
+both in the log file on the conversion server.  Only informational
+messages are sent back over the control connection to be displayed in
+the graphical UI.
 
 =head1 SEE ALSO
 
-- 
2.7.4

Improves the general look of virt-p2v.
---
 p2v/dependencies.m4 | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/p2v/dependencies.m4 b/p2v/dependencies.m4
index ae4a0f5..635e253 100644
--- a/p2v/dependencies.m4
+++ b/p2v/dependencies.m4
@@ -47,6 +47,8 @@ ifelse(REDHAT,1,
   /usr/bin/Xorg
   xorg-x11-drivers
   xorg-x11-fonts-Type1
+  dejavu-sans-fonts
+  dejavu-sans-mono-fonts
   mesa-dri-drivers
   metacity
 
@@ -78,6 +80,7 @@ ifelse(DEBIAN,1,
   xterm
   xorg
   xserver-xorg-video-all
+  fonts-dejavu
   metacity
   network-manager
   network-manager-gnome
@@ -103,6 +106,7 @@ ifelse(ARCHLINUX,1,
   xorg-xinit
   xorg-server
   xf86-video-*
+  ttf-dejavu
   metacity
   NetworkManager
   nm-connection-editor
@@ -128,6 +132,7 @@ ifelse(SUSE,1,
   /usr/bin/xinit
   /usr/bin/Xorg
   xf86-video-*
+  dejavu-fonts
   metacity
   NetworkManager
   nm-connection-editor
-- 
2.7.4

---
 p2v/gui.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/p2v/gui.c b/p2v/gui.c
index c34bc6c..98ce60c 100644
--- a/p2v/gui.c
+++ b/p2v/gui.c
@@ -1657,6 +1657,7 @@ create_running_dialog (void)
     { "black", "maroon", "green",
"olive", "navy", "purple", "teal",
"silver",
       "gray", "red", "lime", "yellow",
"blue", "fuchsia", "cyan", "white" };
   GtkTextBuffer *buf;
+  PangoFontDescription *font;
 
   run_dlg = gtk_dialog_new ();
   gtk_window_set_title (GTK_WINDOW (run_dlg), guestfs_int_program_name);
@@ -1682,6 +1683,10 @@ create_running_dialog (void)
       gtk_text_buffer_create_tag (buf, tag_name, "foreground",
tags[i], NULL);
   }
 
+  font = pango_font_description_from_string ("Monospace 11");
+  gtk_widget_modify_font (v2v_output, font);
+  pango_font_description_free (font);
+
   log_label = gtk_label_new (NULL);
   set_alignment (log_label, 0., 0.5);
   set_padding (log_label, 10, 10);
-- 
2.7.4

Attached is a screenshot of what the new running dialog looks like.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-builder quickly builds VMs from scratch
http://libguestfs.org/virt-builder.1.html