Libguestfs - Mar 2017 - [PATCH 0/3] p2v, v2v: Ensure the full version is always available in several places.

After debugging a virt-p2v issue with a customer in the middle of the
night on Tuesday, I felt it would have been helpful to know exactly
which version(s) of virt-p2v and virt-v2v they were using.  That
wasn't very clear from the log file I was provided with, so this
change makes sure the information is included every time.

Rich.

Ensure the full version of virt-v2v and virt-p2v is available in the
conversion log file, the only log file that we reliably get from users
and customers.  "Full version" means the major, minor, release and
extra fields.  The extra field is especially important as it contains
the downstream package release in Fedora, RHEL etc.

This change saves the virt-p2v version as a comment in the
physical.xml file, which is included in full in the conversion log (by
virt-v2v).

It also ensures that the initial virt-v2v debug message contains the
full version number including the 'extra' field.

  $ cat virt-v2v-conversion-log.txt
  virt-v2v: libguestfs 1.37.7local,libvirt (x86_64)
  ...
  <!-- virt-p2v 1.37.7local,libvirt -->

It also adds 'p2v-version' and 'v2v-version' files in the
virt-p2v
debug directory.  These are strictly superfluous but could be useful
for end users.

  $ cat p2v-version
  virt-p2v 1.37.7local,libvirt
  $ cat v2v-version
  virt-v2v 1.37.7local,libvirt
---
 p2v/conversion.c   | 30 ++++++++++++++++++++++++++++++
 p2v/physical-xml.c | 12 ++++++++----
 p2v/virt-p2v.pod   |  8 ++++++++
 v2v/v2v.ml         |  4 ++--
 4 files changed, 48 insertions(+), 6 deletions(-)

diff --git a/p2v/conversion.c b/p2v/conversion.c
index 57b83b9c4..2f1bbac3e 100644
--- a/p2v/conversion.c
+++ b/p2v/conversion.c
@@ -59,6 +59,7 @@ static void cleanup_data_conns (struct data_conn *data_conns,
size_t nr);
 static void generate_name (struct config *, const char *filename);
 static void generate_wrapper_script (struct config *, const char *remote_dir,
const char *filename);
 static void generate_system_data (const char *dmesg_file, const char
*lscpu_file, const char *lspci_file, const char *lsscsi_file, const char
*lsusb_file);
+static void generate_p2v_version_file (const char *p2v_version_file);
 static void print_quoted (FILE *fp, const char *s);
 
 static char *conversion_error;
@@ -170,6 +171,7 @@ start_conversion (struct config *config,
   char lspci_file[]       = "/tmp/p2v.XXXXXX/lspci";
   char lsscsi_file[]      = "/tmp/p2v.XXXXXX/lsscsi";
   char lsusb_file[]       = "/tmp/p2v.XXXXXX/lsusb";
+  char p2v_version_file[] = "/tmp/p2v.XXXXXX/p2v-version";
   int inhibit_fd = -1;
 
 #if DEBUG_STDERR
@@ -306,6 +308,7 @@ start_conversion (struct config *config,
   memcpy (lspci_file, tmpdir, strlen (tmpdir));
   memcpy (lsscsi_file, tmpdir, strlen (tmpdir));
   memcpy (lsusb_file, tmpdir, strlen (tmpdir));
+  memcpy (p2v_version_file, tmpdir, strlen (tmpdir));
 
   /* Generate the static files. */
   generate_name (config, name_file);
@@ -313,6 +316,7 @@ start_conversion (struct config *config,
   generate_wrapper_script (config, remote_dir, wrapper_script);
   generate_system_data (dmesg_file,
                         lscpu_file, lspci_file, lsscsi_file, lsusb_file);
+  generate_p2v_version_file (p2v_version_file);
 
   /* Open the control connection.  This also creates remote_dir. */
   if (notify_ui)
@@ -347,6 +351,7 @@ start_conversion (struct config *config,
   ignore_value (scp_file (config, lspci_file, remote_dir));
   ignore_value (scp_file (config, lsscsi_file, remote_dir));
   ignore_value (scp_file (config, lsusb_file, remote_dir));
+  ignore_value (scp_file (config, p2v_version_file, remote_dir));
 
   /* Do the conversion.  This runs until virt-v2v exits. */
   if (notify_ui)
@@ -565,6 +570,13 @@ generate_wrapper_script (struct config *config, const char
*remote_dir,
   fprintf (fp, "\n");
 
   fprintf (fp,
+           "# Log the version of virt-v2v (for information
only).\n");
+  if (config->sudo)
+    fprintf (fp, "sudo -n ");
+  fprintf (fp, "virt-v2v --version > v2v-version\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");
@@ -645,3 +657,21 @@ generate_system_data (const char *dmesg_file,
 
   ignore_value (system (cmd));
 }
+
+/**
+ * Generate a file containing the version of virt-p2v.
+ *
+ * The version of virt-v2v is contained in the conversion log.
+ */
+static void
+generate_p2v_version_file (const char *p2v_version_file)
+{
+  FILE *fp = fopen (p2v_version_file, "w");
+  if (fp == NULL) {
+    perror (p2v_version_file);
+    return;                     /* non-fatal */
+  }
+  fprintf (fp, "%s %s\n",
+           getprogname (), PACKAGE_VERSION_FULL);
+  fclose (fp);
+}
diff --git a/p2v/physical-xml.c b/p2v/physical-xml.c
index 6e81c27c1..0ae11fc53 100644
--- a/p2v/physical-xml.c
+++ b/p2v/physical-xml.c
@@ -37,6 +37,8 @@
 
 #include <glib.h>
 
+#include "getprogname.h"
+
 #include "p2v.h"
 
 static const char *map_interface_to_network (struct config *, const char
*interface);
@@ -90,10 +92,10 @@ static const char *map_interface_to_network (struct config
*, const char *interf
   } while (0)
 
 /* An XML comment. */
-#define comment(str)						\
-  do {                                                          \
-    if (xmlTextWriterWriteComment (xo, BAD_CAST (str)) == -1)	\
-      error (EXIT_FAILURE, errno, "xmlTextWriterWriteComment");	\
+#define comment(fs,...)                                                 \
+  do {                                                                  \
+    if (xmlTextWriterWriteFormatComment (xo, fs, ##__VA_ARGS__) == -1)	\
+      error (EXIT_FAILURE, errno, "xmlTextWriterWriteFormatComment");
\
   } while (0)
 
 /**
@@ -123,6 +125,8 @@ generate_physical_xml (struct config *config, struct
data_conn *data_conns,
 
   memkb = config->memory / 1024;
 
+  comment (" %s %s ", getprogname (), PACKAGE_VERSION_FULL);
+
   comment
     (" NOTE!\n"
      "\n"
diff --git a/p2v/virt-p2v.pod b/p2v/virt-p2v.pod
index 4a5f58724..ee870fdd9 100644
--- a/p2v/virt-p2v.pod
+++ b/p2v/virt-p2v.pod
@@ -767,6 +767,14 @@ Note this is not "real" libvirt XML (and must
B<never> be loaded into
 libvirt, which would reject it anyhow).  Also it is not the same as
 the libvirt XML which virt-v2v generates in certain output modes.
 
+=item F<p2v-version>
+
+=item F<v2v-version>
+
+I<(before conversion)>
+
+The versions of virt-p2v and virt-v2v respectively.
+
 =item F<status>
 
 I<(after conversion)>
diff --git a/v2v/v2v.ml b/v2v/v2v.ml
index bd3a413d2..0f14c2189 100644
--- a/v2v/v2v.ml
+++ b/v2v/v2v.ml
@@ -42,8 +42,8 @@ let rec main ()  
   (* Print the version, easier than asking users to tell us. *)
   debug "%s: %s %s (%s)"
-        prog Guestfs_config.package_name
-        Guestfs_config.package_version Guestfs_config.host_cpu;
+        prog Guestfs_config.package_name Guestfs_config.package_version_full
+        Guestfs_config.host_cpu;
 
   (* Print the libvirt version if debugging.  Note that if
    * we're configured --without-libvirt, then this will throw
-- 
2.12.0

Each scp command takes a considerable amount of time -- several
seconds -- because it must set up, authenticate and tear down a new
connection.  Avoid this by combining several copies into a single
command.

We still have to use two scp commands because we want to check that
some files are copied and ignore failures in a second set of
informational files.
---
 p2v/conversion.c | 31 +++++++++++--------------------
 p2v/p2v.h        |  2 +-
 p2v/ssh.c        | 25 +++++++++++++++++++++----
 3 files changed, 33 insertions(+), 25 deletions(-)

diff --git a/p2v/conversion.c b/p2v/conversion.c
index 2f1bbac3e..e55f3d291 100644
--- a/p2v/conversion.c
+++ b/p2v/conversion.c
@@ -330,28 +330,19 @@ start_conversion (struct config *config,
   }
 
   /* Copy the static files to the remote dir. */
-  if (scp_file (config, name_file, remote_dir) == -1) {
-    set_conversion_error ("scp: %s to %s: %s",
-                          name_file, remote_dir, get_ssh_error ());
-    goto out;
-  }
-  if (scp_file (config, physical_xml_file, remote_dir) == -1) {
-    set_conversion_error ("scp: %s to %s: %s",
-                          physical_xml_file, remote_dir, get_ssh_error ());
-    goto out;
-  }
-  if (scp_file (config, wrapper_script, remote_dir) == -1) {
-    set_conversion_error ("scp: %s to %s: %s",
-                          wrapper_script, remote_dir, get_ssh_error ());
+
+  /* These three files must not fail, so check for errors here. */
+  if (scp_file (config, remote_dir,
+                name_file, physical_xml_file, wrapper_script, NULL) == -1) {
+    set_conversion_error ("scp: %s: %s",
+                          remote_dir, get_ssh_error ());
     goto out;
   }
-  /* It's not essential that these files are copied. */
-  ignore_value (scp_file (config, dmesg_file, remote_dir));
-  ignore_value (scp_file (config, lscpu_file, remote_dir));
-  ignore_value (scp_file (config, lspci_file, remote_dir));
-  ignore_value (scp_file (config, lsscsi_file, remote_dir));
-  ignore_value (scp_file (config, lsusb_file, remote_dir));
-  ignore_value (scp_file (config, p2v_version_file, remote_dir));
+
+  /* It's not essential that these files are copied, so ignore errors. */
+  ignore_value (scp_file (config, remote_dir,
+                          dmesg_file, lscpu_file, lspci_file, lsscsi_file,
+                          lsusb_file, p2v_version_file, NULL));
 
   /* Do the conversion.  This runs until virt-v2v exits. */
   if (notify_ui)
diff --git a/p2v/p2v.h b/p2v/p2v.h
index bd4b484ab..f51bdfcf1 100644
--- a/p2v/p2v.h
+++ b/p2v/p2v.h
@@ -161,7 +161,7 @@ extern int test_connection (struct config *);
 extern mexp_h *open_data_connection (struct config *, const char *local_ipaddr,
int local_port, int *remote_port);
 extern mexp_h *start_remote_connection (struct config *, const char
*remote_dir);
 extern const char *get_ssh_error (void);
-extern int scp_file (struct config *config, const char *localfile, const char
*remotefile);
+extern int scp_file (struct config *config, const char *target, const char
*local, ...) __attribute__((sentinel));
 
 /* nbd.c */
 extern void set_nbd_option (const char *opt);
diff --git a/p2v/ssh.c b/p2v/ssh.c
index 8beaf74bb..bfeb80661 100644
--- a/p2v/ssh.c
+++ b/p2v/ssh.c
@@ -572,14 +572,18 @@ start_ssh (unsigned spawn_flags, struct config *config,
 #endif
 
 /**
- * Upload a file to remote using L<scp(1)>.
+ * Upload file(s) to remote using L<scp(1)>.
+ *
+ * Note that the target (directory or file) comes before the list of
+ * local files, because the list of local files is a varargs list.
  *
  * This is a simplified version of L</start_ssh> above.
  */
 int
-scp_file (struct config *config, const char *localfile, const char *remotefile)
+scp_file (struct config *config, const char *target, const char *local, ...)
 {
   size_t i = 0;
+  va_list args;
   const size_t MAX_ARGS = 64;
   const char *argv[MAX_ARGS];
   char port_str[64];
@@ -618,12 +622,25 @@ scp_file (struct config *config, const char *localfile,
const char *remotefile)
     ADD_ARG (argv, i, "-i");
     ADD_ARG (argv, i, config->identity_file);
   }
-  ADD_ARG (argv, i, localfile);
+
+  /* Source files or directories.
+   * Strictly speaking this could abort() if the list of files is
+   * too long, but that never happens in virt-p2v. XXX
+   */
+  va_start (args, local);
+  do ADD_ARG (argv, i, local);
+  while ((local = va_arg (args, const char *)) != NULL);
+  va_end (args);
+
+  /* The target file or directory.  We need to rewrite this as
+   * "username@server:target".
+   */
   if (asprintf (&remote, "%s@%s:%s",
                 config->username ? config->username : "root",
-                config->server, remotefile) == -1)
+                config->server, target) == -1)
     error (EXIT_FAILURE, errno, "asprintf");
   ADD_ARG (argv, i, remote);
+
   ADD_ARG (argv, i, NULL);
 
 #if DEBUG_STDERR
-- 
2.12.0

---
 p2v/virt-p2v.pod | 28 +++++++++++++---------------
 1 file changed, 13 insertions(+), 15 deletions(-)

diff --git a/p2v/virt-p2v.pod b/p2v/virt-p2v.pod
index ee870fdd9..7fd637152 100644
--- a/p2v/virt-p2v.pod
+++ b/p2v/virt-p2v.pod
@@ -724,18 +724,6 @@ Into this directory are written various files which
include:
 
 =item F<dmesg>
 
-I<(before conversion)>
-
-The dmesg output from the physical machine.  Useful for detecting
-problems such as missing device drivers or firmware on the virt-p2v
-ISO.
-
-=item F<environment>
-
-I<(before conversion)>
-
-The content of the environment where L<virt-v2v(1)> will run.
-
 =item F<lscpu>
 
 =item F<lspci>
@@ -746,8 +734,18 @@ The content of the environment where L<virt-v2v(1)>
will run.
 
 I<(before conversion)>
 
-The output of the corresponding commands (ie L<lscpu(1)> etc) on the
-physical machine.  Useful for debugging novel hardware configurations.
+The output of the corresponding commands (ie L<dmesg(1)>,
L<lscpu(1)>
+etc) on the physical machine.
+
+The dmesg output is useful for detecting problems such as missing
+device drivers or firmware on the virt-p2v ISO.  The others are useful
+for debugging novel hardware configurations.
+
+=item F<environment>
+
+I<(before conversion)>
+
+The content of the environment where L<virt-v2v(1)> will run.
 
 =item F<name>
 
@@ -799,7 +797,7 @@ B<unedited> log file in any bug reports or support
tickets.
 
 =item F<virt-v2v-wrapper.sh>
 
-I<(during/after conversion)>
+I<(before conversion)>
 
 This is the wrapper script which is used when running virt-v2v.  For
 interest only, do not attempt to run this script yourself.
-- 
2.12.0

On Thursday, 30 March 2017 15:49:12 CEST Richard W.M. Jones
wrote:
> After debugging a virt-p2v issue with a customer in the middle of the
> night on Tuesday, I felt it would have been helpful to know exactly
> which version(s) of virt-p2v and virt-v2v they were using.  That
> wasn't very clear from the log file I was provided with, so this
> change makes sure the information is included every time.
LGTM.

-- 
Pino Toscano