Libguestfs - Jun 2016 - [PATCH 0/4] p2v: Send ^C to remote end to cancel the conversion.

(I don't have a BZ# for this yet, but I'm expecting it to be
filed as an RFE)

Currently if the user is in the virt-p2v GUI and cancels the
conversion, all that happens is we abruptly close the ssh session to
virt-v2v.

That is bad .. possibly (or maybe not).  But in any case there is an
alternative: we can send a ^C key to the virt-v2v process, which it
could catch and handle gracefully, although it doesn't do that right
now.

Sending ^C turns out to be complex.  It requires changes in miniexpect
(http://git.annexia.org/?p=miniexpect.git;a=commitdiff;h=fb0eeddc671039cabd9c53934e324ae218b21000).
Then we have to make sure the control connection to virt-v2v is opened
in "cooked" mode.

But we cannot open the control connection in cooked mode without also
changing the way we are copying files over to the remote server, so I
also had to implement 'scp' support.

After all that the conversion code can be modified to send ^C when the
connection is cancelled.

Rich.

---
 p2v/ssh.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/p2v/ssh.c b/p2v/ssh.c
index 6d264e2..07fa25e 100644
--- a/p2v/ssh.c
+++ b/p2v/ssh.c
@@ -374,8 +374,10 @@ start_ssh (struct config *config, char **extra_args, int
wait_prompt)
 
   /* Create the miniexpect handle. */
   h = mexp_spawnv ("ssh", (char **) args);
-  if (h == NULL)
+  if (h == NULL) {
+    set_ssh_internal_error ("ssh: mexp_spawnv: %m");
     return NULL;
+  }
 
   /* We want the ssh ConnectTimeout to be less than the miniexpect
    * timeout, so that if the server is completely unresponsive we
-- 
2.7.4

Previously we copied / creates files in the remote dir by running
complex shell commands like:

  cat > file <<'EOF'
    ## the file was copied in here
  EOF

This was a little hairy, but in particular it doesn't allow us to set
the ssh session to cooked mode (so we can send ^C to cancel a
conversion).

A cleaner way to do it is to use 'scp' to copy the files over.
---
 p2v/Makefile.am          |   1 +
 p2v/conversion.c         | 279 ++++++++++++++++++++++++-----------------------
 p2v/p2v.h                |   3 +-
 p2v/ssh.c                | 255 ++++++++++++++++++++++++++++++-------------
 p2v/test-virt-p2v-scp.sh |  58 ++++++++++
 p2v/test-virt-p2v.sh     |   5 +-
 p2v/virt-p2v.pod         |   4 +
 7 files changed, 388 insertions(+), 217 deletions(-)
 create mode 100755 p2v/test-virt-p2v-scp.sh

diff --git a/p2v/Makefile.am b/p2v/Makefile.am
index 525b80c..0067d5a 100644
--- a/p2v/Makefile.am
+++ b/p2v/Makefile.am
@@ -27,6 +27,7 @@ EXTRA_DIST = \
 	p2v.ks.in \
 	p2v.service \
 	test-virt-p2v-pxe.sshd_config.in \
+	test-virt-p2v-scp.sh \
 	test-virt-p2v-ssh.sh \
 	virt-p2v.pod \
 	virt-p2v-make-disk.in \
diff --git a/p2v/conversion.c b/p2v/conversion.c
index 12fd93f..721882f 100644
--- a/p2v/conversion.c
+++ b/p2v/conversion.c
@@ -44,6 +44,7 @@
 #include <locale.h>
 #include <libintl.h>
 #include <netdb.h>
+#include <sys/stat.h>
 #include <sys/types.h>
 #include <sys/wait.h>
 
@@ -95,9 +96,12 @@ struct data_conn {
 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 void generate_name (struct config *, const char *filename);
+static void generate_libvirt_xml (struct config *, struct data_conn *, const
char *filename);
+static void generate_wrapper_script (struct config *, const char *remote_dir,
const char *filename);
+static void generate_dmesg_file (const char *filename);
 static const char *map_interface_to_network (struct config *, const char
*interface);
+static void print_quoted (FILE *fp, const char *s);
 
 static char *conversion_error;
 
@@ -179,15 +183,16 @@ start_conversion (struct config *config,
   int status;
   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,
-    *wrapper_script = NULL;
   time_t now;
   struct tm tm;
   mexp_h *control_h = NULL;
-  char dmesg_cmd[] = "dmesg > /tmp/dmesg.XXXXXX", *dmesg_file =
&dmesg_cmd[8];
-  CLEANUP_FREE char *dmesg = NULL;
-  int fd, r;
+  CLEANUP_FREE struct data_conn *data_conns = NULL;
+  CLEANUP_FREE char *remote_dir = NULL;
+  char tmpdir[]           = "/tmp/p2v.XXXXXX";
+  char name_file[]        = "/tmp/p2v.XXXXXX/name";
+  char libvirt_xml_file[] = "/tmp/p2v.XXXXXX/physical.xml";
+  char wrapper_script[]   = "/tmp/p2v.XXXXXX/virt-v2v-wrapper.sh";
+  char dmesg_file[]       = "/tmp/p2v.XXXXXX/dmesg";
 
 #if DEBUG_STDERR
   print_config (config, stderr);
@@ -285,59 +290,53 @@ start_conversion (struct config *config,
   if (notify_ui)
     notify_ui (NOTIFY_LOG_DIR, remote_dir);
 
-  /* Generate the libvirt XML. */
-  libvirt_xml = generate_libvirt_xml (config, data_conns);
-  if (libvirt_xml == NULL)
-    goto out;
-
-#if DEBUG_STDERR && 0
-  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 && 0
-  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
-   * on the remote server.
-   */
-  fd = mkstemp (dmesg_file);
-  if (fd == -1) {
-    perror ("mkstemp");
-    goto skip_dmesg;
-  }
-  close (fd);
-  r = system (dmesg_cmd);
-  if (r == -1) {
-    perror ("system");
-    goto skip_dmesg;
-  }
-  if (!WIFEXITED (r) || WEXITSTATUS (r) != 0) {
-    fprintf (stderr, "'dmesg' failed (ignored)\n");
-    goto skip_dmesg;
+  /* Generate the local temporary directory. */
+  if (mkdtemp (tmpdir) == NULL) {
+    perror ("mkdtemp");
+    cleanup_data_conns (data_conns, nr_disks);
+    exit (EXIT_FAILURE);
   }
+  memcpy (name_file, tmpdir, strlen (tmpdir));
+  memcpy (libvirt_xml_file, tmpdir, strlen (tmpdir));
+  memcpy (wrapper_script, tmpdir, strlen (tmpdir));
+  memcpy (dmesg_file, tmpdir, strlen (tmpdir));
 
-  ignore_value (read_whole_file (dmesg_file, &dmesg, NULL));
- skip_dmesg:
+  /* Generate the static files. */
+  generate_name (config, name_file);
+  generate_libvirt_xml (config, data_conns, libvirt_xml_file);
+  generate_wrapper_script (config, remote_dir, wrapper_script);
+  generate_dmesg_file (dmesg_file);
 
-  /* Open the control connection and start conversion */
+  /* Open the control connection.  This also creates remote_dir. */
   if (notify_ui)
     notify_ui (NOTIFY_STATUS, _("Setting up the control connection
..."));
 
-  control_h = start_remote_connection (config,
-                                       remote_dir, libvirt_xml,
-                                       wrapper_script, dmesg);
+  control_h = start_remote_connection (config, remote_dir);
   if (control_h == NULL) {
-    const char *err = get_ssh_error ();
+    set_conversion_error ("could not open control connection over SSH to
the conversion server: %s",
+                          get_ssh_error ());
+    goto out;
+  }
 
-    set_conversion_error ("could not open control connection over SSH to
the conversion server: %s", err);
+  /* 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, libvirt_xml_file, remote_dir) == -1) {
+    set_conversion_error ("scp: %s to %s: %s",
+                          libvirt_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 ());
+    goto out;
+  }
+  if (scp_file (config, dmesg_file, remote_dir) == -1) {
+    set_conversion_error ("scp: %s to %s: %s",
+                          dmesg_file, remote_dir, get_ssh_error ());
     goto out;
   }
 
@@ -689,20 +688,16 @@ cleanup_data_conns (struct data_conn *data_conns, size_t
nr)
 /* Macros "inspired" by src/launch-libvirt.c */
 /* <element */
 #define start_element(element)						\
-  if (xmlTextWriterStartElement (xo, BAD_CAST (element)) == -1) {	\
-    set_conversion_error ("xmlTextWriterStartElement: %m");		\
-    return NULL;							\
-  }									\
+  if (xmlTextWriterStartElement (xo, BAD_CAST (element)) == -1)         \
+    error (EXIT_FAILURE, errno, "xmlTextWriterStartElement");		\
   do
 
 /* finish current </element> */
 #define end_element()						\
   while (0);							\
   do {								\
-    if (xmlTextWriterEndElement (xo) == -1) {			\
-      set_conversion_error ("xmlTextWriterEndElement: %m");	\
-      return NULL;						\
-    }								\
+    if (xmlTextWriterEndElement (xo) == -1)			\
+      error (EXIT_FAILURE, errno, "xmlTextWriterEndElement");	\
   } while (0)
 
 /* <element/> */
@@ -711,39 +706,39 @@ cleanup_data_conns (struct data_conn *data_conns, size_t
nr)
 
 /* key=value attribute of the current element. */
 #define attribute(key,value)                                            \
-  if (xmlTextWriterWriteAttribute (xo, BAD_CAST (key), BAD_CAST (value)) == -1)
{ \
-    set_conversion_error ("xmlTextWriterWriteAttribute: %m");        
\
-    return NULL;                                                        \
-  }
+  do {                                                                  \
+    if (xmlTextWriterWriteAttribute (xo, BAD_CAST (key), BAD_CAST (value)) ==
-1) \
+    error (EXIT_FAILURE, errno, "xmlTextWriterWriteAttribute");      
\
+  } while (0)
 
 /* key=value, but value is a printf-style format string. */
 #define attribute_format(key,fs,...)                                    \
-  if (xmlTextWriterWriteFormatAttribute (xo, BAD_CAST (key),            \
-					 fs, ##__VA_ARGS__) == -1) {	\
-    set_conversion_error ("xmlTextWriterWriteFormatAttribute: %m");  
\
-    return NULL;                                                        \
-  }
+  do {                                                                  \
+    if (xmlTextWriterWriteFormatAttribute (xo, BAD_CAST (key),          \
+                                           fs, ##__VA_ARGS__) == -1)	\
+      error (EXIT_FAILURE, errno,
"xmlTextWriterWriteFormatAttribute"); \
+  } while (0)
 
 /* A string, eg. within an element. */
-#define string(str)						\
-  if (xmlTextWriterWriteString (xo, BAD_CAST (str)) == -1) {	\
-    set_conversion_error ("xmlTextWriterWriteString: %m");	\
-    return NULL;						\
-  }
+#define string(str)                                             \
+  do {                                                          \
+    if (xmlTextWriterWriteString (xo, BAD_CAST (str)) == -1)	\
+      error (EXIT_FAILURE, errno, "xmlTextWriterWriteString");	\
+  } while (0)
 
 /* A string, using printf-style formatting. */
 #define string_format(fs,...)                                           \
-  if (xmlTextWriterWriteFormatString (xo, fs, ##__VA_ARGS__) == -1) {   \
-    set_conversion_error ("xmlTextWriterWriteFormatString: %m");     
\
-    return NULL;                                                        \
-  }
+  do {                                                                  \
+    if (xmlTextWriterWriteFormatString (xo, fs, ##__VA_ARGS__) == -1)   \
+      error (EXIT_FAILURE, errno, "xmlTextWriterWriteFormatString"); 
\
+  } while (0)
 
 /* An XML comment. */
 #define comment(str)						\
-  if (xmlTextWriterWriteComment (xo, BAD_CAST (str)) == -1) {	\
-    set_conversion_error ("xmlTextWriterWriteComment: %m");	\
-    return NULL;						\
-  }
+  do {                                                          \
+    if (xmlTextWriterWriteComment (xo, BAD_CAST (str)) == -1)	\
+      error (EXIT_FAILURE, errno, "xmlTextWriterWriteComment");	\
+  } while (0)
 
 /**
  * Write the libvirt XML for this physical machine.
@@ -752,41 +747,23 @@ cleanup_data_conns (struct data_conn *data_conns, size_t
nr)
  * virt-v2v on the conversion server.  Virt-v2v will (if necessary)
  * generate the final libvirt XML.
  */
-static char *
-generate_libvirt_xml (struct config *config, struct data_conn *data_conns)
+static void
+generate_libvirt_xml (struct config *config, struct data_conn *data_conns,
+                      const char *filename)
 {
   uint64_t memkb;
-  char *ret;
-  CLEANUP_XMLBUFFERFREE xmlBufferPtr xb = NULL;
-  xmlOutputBufferPtr ob;
   CLEANUP_XMLFREETEXTWRITER xmlTextWriterPtr xo = NULL;
   size_t i;
 
-  xb = xmlBufferCreate ();
-  if (xb == NULL) {
-    set_conversion_error ("xmlBufferCreate: %m");
-    return NULL;
-  }
-  ob = xmlOutputBufferCreateBuffer (xb, NULL);
-  if (ob == NULL) {
-    set_conversion_error ("xmlOutputBufferCreateBuffer: %m");
-    return NULL;
-  }
-  xo = xmlNewTextWriter (ob);
-  if (xo == NULL) {
-    set_conversion_error ("xmlNewTextWriter: %m");
-    return NULL;
-  }
+  xo = xmlNewTextWriterFilename (filename, 0);
+  if (xo == NULL)
+    error (EXIT_FAILURE, errno, "xmlNewTextWriterFilename");
 
   if (xmlTextWriterSetIndent (xo, 1) == -1 ||
-      xmlTextWriterSetIndentString (xo, BAD_CAST "  ") == -1) {
-    set_conversion_error ("could not set XML indent: %m");
-    return NULL;
-  }
-  if (xmlTextWriterStartDocument (xo, NULL, NULL, NULL) == -1) {
-    set_conversion_error ("xmlTextWriterStartDocument: %m");
-    return NULL;
-  }
+      xmlTextWriterSetIndentString (xo, BAD_CAST "  ") == -1)
+    error (EXIT_FAILURE, errno, "could not set XML indent");
+  if (xmlTextWriterStartDocument (xo, NULL, NULL, NULL) == -1)
+    error (EXIT_FAILURE, errno, "xmlTextWriterStartDocument");
 
   memkb = config->memory / 1024;
 
@@ -929,17 +906,8 @@ generate_libvirt_xml (struct config *config, struct
data_conn *data_conns)
 
   } end_element (); /* </domain> */
 
-  if (xmlTextWriterEndDocument (xo) == -1) {
-    set_conversion_error ("xmlTextWriterEndDocument: %m");
-    return NULL;
-  }
-  ret = (char *) xmlBufferDetach (xb); /* caller frees */
-  if (ret == NULL) {
-    set_conversion_error ("xmlBufferDetach: %m");
-    return NULL;
-  }
-
-  return ret;
+  if (xmlTextWriterEndDocument (xo) == -1)
+    error (EXIT_FAILURE, errno, "xmlTextWriterEndDocument");
 }
 
 /**
@@ -976,19 +944,18 @@ map_interface_to_network (struct config *config, const
char *interface)
 }
 
 /**
- * Print a shell-quoted string on C<fp>.
+ * Write the guest name into C<filename>.
  */
 static void
-print_quoted (FILE *fp, const char *s)
+generate_name (struct config *config, const char *filename)
 {
-  fprintf (fp, "\"");
-  while (*s) {
-    if (*s == '$' || *s == '`' || *s == '\\' || *s ==
'"')
-      fprintf (fp, "\\");
-    fprintf (fp, "%c", *s);
-    ++s;
-  }
-  fprintf (fp, "\"");
+  FILE *fp;
+
+  fp = fopen (filename, "w");
+  if (fp == NULL)
+    error (EXIT_FAILURE, errno, "fopen: %s", filename);
+  fprintf (fp, "%s\n", config->guestname);
+  fclose (fp);
 }
 
 /**
@@ -998,16 +965,15 @@ print_quoted (FILE *fp, const char *s)
  * 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)
+static void
+generate_wrapper_script (struct config *config, const char *remote_dir,
+                         const char *filename)
 {
   FILE *fp;
-  char *output = NULL;
-  size_t output_len = 0;
 
-  fp = open_memstream (&output, &output_len);
+  fp = fopen (filename, "w");
   if (fp == NULL)
-    error (EXIT_FAILURE, errno, "open_memstream");
+    error (EXIT_FAILURE, errno, "fopen: %s", filename);
 
   fprintf (fp, "#!/bin/bash -\n");
   fprintf (fp, "\n");
@@ -1106,7 +1072,42 @@ generate_wrapper_script (struct config *config, const
char *remote_dir)
            "fi\n",
            remote_dir);
 
+  fprintf (fp, "\n");
+  fprintf (fp, "# EOF\n");
   fclose (fp);
 
-  return output;                /* caller frees */
+  if (chmod (filename, 0755) == -1)
+    error (EXIT_FAILURE, errno, "chmod: %s", filename);
+}
+
+/**
+ * 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, "\"");
+}
+
+/**
+ * Put the output of the C<dmesg> command into C<filename>.
+ *
+ * If the command fails, this is non-fatal.
+ */
+static void
+generate_dmesg_file (const char *filename)
+{
+  CLEANUP_FREE char *cmd = NULL;
+
+  if (asprintf (&cmd, "dmesg >%s 2>&1", filename) ==
-1)
+    error (EXIT_FAILURE, errno, "asprintf");
+
+  ignore_value (system (cmd));
 }
diff --git a/p2v/p2v.h b/p2v/p2v.h
index 52d5ccb..da30340 100644
--- a/p2v/p2v.h
+++ b/p2v/p2v.h
@@ -130,8 +130,9 @@ 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 *wrapper_script, const char
*dmesg);
+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);
 
 /* utils.c */
 extern uint64_t get_blockdev_size (const char *dev);
diff --git a/p2v/ssh.c b/p2v/ssh.c
index 07fa25e..0d0874a 100644
--- a/p2v/ssh.c
+++ b/p2v/ssh.c
@@ -553,6 +553,182 @@ start_ssh (struct config *config, char **extra_args, int
wait_prompt)
   return h;
 }
 
+/**
+ * Upload a file to remote using L<scp(1)>.
+ *
+ * This is a simplified version of L</start_ssh> above.
+ */
+int
+scp_file (struct config *config, const char *localfile, const char *remotefile)
+{
+  size_t j, nr_args;
+  char port_str[64];
+  char connect_timeout_str[128];
+  CLEANUP_FREE char *remote = NULL;
+  CLEANUP_FREE /* [sic] */ const char **args = NULL;
+  mexp_h *h;
+  const int ovecsize = 12;
+  int ovector[ovecsize];
+  int using_password_auth;
+
+  if (cache_ssh_identity (config) == -1)
+    return -1;
+
+  /* Are we using password or identity authentication? */
+  using_password_auth = config->identity_file == NULL;
+
+  /* Create the scp argument array. */
+  if (using_password_auth)
+    nr_args = 12;
+  else
+    nr_args = 14;
+  args = malloc (sizeof (char *) * nr_args);
+  if (args == NULL)
+    error (EXIT_FAILURE, errno, "malloc");
+
+  j = 0;
+  args[j++] = "scp";
+  args[j++] = "-P";             /* Port. */
+  snprintf (port_str, sizeof port_str, "%d", config->port);
+  args[j++] = port_str;
+  args[j++] = "-o";             /* Host key will always be novel. */
+  args[j++] = "StrictHostKeyChecking=no";
+  args[j++] = "-o";             /* ConnectTimeout */
+  snprintf (connect_timeout_str, sizeof connect_timeout_str,
+            "ConnectTimeout=%d", SSH_TIMEOUT);
+  args[j++] = connect_timeout_str;
+  if (using_password_auth) {
+    /* Only use password authentication. */
+    args[j++] = "-o";
+    args[j++] =
"PreferredAuthentications=keyboard-interactive,password";
+  }
+  else {
+    /* Use identity file (private key). */
+    args[j++] = "-o";
+    args[j++] = "PreferredAuthentications=publickey";
+    args[j++] = "-i";
+    args[j++] = config->identity_file;
+  }
+  args[j++] = localfile;
+  if (asprintf (&remote, "%s@%s:%s",
+                config->username ? config->username : "root",
+                config->server, remotefile) == -1)
+    error (EXIT_FAILURE, errno, "asprintf");
+  args[j++] = remote;
+  args[j++] = NULL;
+  assert (j == nr_args);
+
+#if DEBUG_STDERR && 0
+  size_t i;
+
+  fputs ("scp command: ", stderr);
+  for (i = 0; i < nr_args - 1; ++i) {
+    if (i > 0) fputc (' ', stderr);
+    fputs (args[i], stderr);
+  }
+  fputc ('\n', stderr);
+#endif
+
+  /* Create the miniexpect handle. */
+  h = mexp_spawnv ("scp", (char **) args);
+  if (h == NULL) {
+    set_ssh_internal_error ("scp: mexp_spawnv: %m");
+    return -1;
+  }
+
+  /* We want the ssh ConnectTimeout to be less than the miniexpect
+   * timeout, so that if the server is completely unresponsive we
+   * still see the error from ssh, not a timeout from miniexpect.  The
+   * obvious solution to this is to set ConnectTimeout (above) and to
+   * set the miniexpect timeout to be a little bit larger.
+   */
+  mexp_set_timeout (h, SSH_TIMEOUT + 20);
+
+  if (using_password_auth &&
+      config->password && strlen (config->password) > 0) {
+    CLEANUP_FREE char *ssh_message = NULL;
+
+    /* Wait for the password prompt. */
+  wait_password_again:
+    switch (mexp_expect (h,
+                         (mexp_regexp[]) {
+                           { 100, .re = password_re },
+                           { 101, .re = ssh_message_re },
+                           { 0 }
+                         }, ovector, ovecsize)) {
+    case 100:                   /* Got password prompt. */
+      if (mexp_printf (h, "%s\n", config->password) == -1) {
+        set_ssh_mexp_error ("mexp_printf");
+        mexp_close (h);
+        return -1;
+      }
+      break;
+
+    case 101:
+      free (ssh_message);
+      ssh_message = strndup (&h->buffer[ovector[2]],
ovector[3]-ovector[2]);
+      goto wait_password_again;
+
+    case MEXP_EOF:
+      /* This is where we get to if the user enters an incorrect or
+       * impossible hostname or port number.  Hopefully scp printed an
+       * error message, and we picked it up and put it in
+       * 'ssh_message' in case 101 above.  If not we have to print a
+       * generic error instead.
+       */
+      if (ssh_message)
+        set_ssh_error ("%s", ssh_message);
+      else
+        set_ssh_error ("scp closed the connection without printing an
error.");
+      mexp_close (h);
+      return -1;
+
+    case MEXP_TIMEOUT:
+      set_ssh_unexpected_timeout ("password prompt");
+      mexp_close (h);
+      return -1;
+
+    case MEXP_ERROR:
+      set_ssh_mexp_error ("mexp_expect");
+      mexp_close (h);
+      return -1;
+
+    case MEXP_PCRE_ERROR:
+      set_ssh_pcre_error ();
+      mexp_close (h);
+      return -1;
+    }
+  }
+
+  /* Wait for the scp subprocess to finish. */
+  switch (mexp_expect (h, NULL, NULL, 0)) {
+  case MEXP_EOF:
+    break;
+
+  case MEXP_TIMEOUT:
+    set_ssh_unexpected_timeout ("copying (scp) file");
+    mexp_close (h);
+    return -1;
+
+  case MEXP_ERROR:
+    set_ssh_mexp_error ("mexp_expect");
+    mexp_close (h);
+    return -1;
+
+  case MEXP_PCRE_ERROR:
+    set_ssh_pcre_error ();
+    mexp_close (h);
+    return -1;
+  }
+
+  if (mexp_close (h) == -1) {
+    set_ssh_internal_error ("scp: mexp_close: %m");
+    return -1;
+  }
+
+  return 0;
+}
+
 static void add_input_driver (const char *name, size_t len);
 static void add_output_driver (const char *name, size_t len);
 static int compatible_version (const char *v2v_version);
@@ -955,17 +1131,9 @@ wait_for_prompt (mexp_h *h)
 }
 
 mexp_h *
-start_remote_connection (struct config *config,
-                         const char *remote_dir, const char *libvirt_xml,
-                         const char *wrapper_script, const char *dmesg)
+start_remote_connection (struct config *config, const char *remote_dir)
 {
   mexp_h *h;
-  char magic[9];
-
-  if (guestfs_int_random_string (magic, 8) == -1) {
-    perror ("random_string");
-    return NULL;
-  }
 
   h = start_ssh (config, NULL, 1);
   if (h == NULL)
@@ -980,16 +1148,9 @@ start_remote_connection (struct config *config,
   if (wait_for_prompt (h) == -1)
     goto error;
 
-  /* Write some useful config information to files in the remote directory. */
-  if (mexp_printf (h, "echo '%s' > %s/name\n",
-                   config->guestname, remote_dir) == -1) {
-    set_ssh_mexp_error ("mexp_printf");
-    goto error;
-  }
-
-  if (wait_for_prompt (h) == -1)
-    goto error;
-
+  /* It's simplest to create the remote 'time' file by running the
date
+   * command, as that won't send any special control characters.
+   */
   if (mexp_printf (h, "date > %s/time\n", remote_dir) == -1) {
     set_ssh_mexp_error ("mexp_printf");
     goto error;
@@ -998,62 +1159,6 @@ start_remote_connection (struct config *config,
   if (wait_for_prompt (h) == -1)
     goto error;
 
-  /* Upload the guest libvirt XML to the remote directory. */
-  if (mexp_printf (h,
-                   "cat > '%s/physical.xml' <<
'__%s__'\n"
-                   "%s"
-                   "__%s__\n",
-                   remote_dir, magic,
-                   libvirt_xml,
-                   magic) == -1) {
-    set_ssh_mexp_error ("mexp_printf");
-    goto error;
-  }
-
-  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_mexp_error ("mexp_printf");
-    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_mexp_error ("mexp_printf");
-    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,
-                     "cat > '%s/dmesg' <<
'__%s__'\n"
-                     "%s"
-                     "\n"
-                     "__%s__\n",
-                     remote_dir, magic,
-                     dmesg,
-                     magic) == -1) {
-      set_ssh_mexp_error ("mexp_printf");
-      goto error;
-    }
-
-    if (wait_for_prompt (h) == -1)
-      goto error;
-  }
-
   return h;
 
  error:
diff --git a/p2v/test-virt-p2v-scp.sh b/p2v/test-virt-p2v-scp.sh
new file mode 100755
index 0000000..be37a03
--- /dev/null
+++ b/p2v/test-virt-p2v-scp.sh
@@ -0,0 +1,58 @@
+#!/bin/bash -
+# Copyright (C) 2014-2016 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.
+
+# This is an scp substitute used by test-virt-p2v.sh.
+
+TEMP=`getopt \
+        -o 'o:P:' \
+        -- "$@"`
+if [ $? != 0 ]; then
+    echo "$0: problem parsing the command line arguments"
+    exit 1
+fi
+eval set -- "$TEMP"
+
+while true ; do
+    case "$1" in
+        # Regular arguments that we can just ignore.
+        -o|-P)
+            shift 2
+            ;;
+
+        --)
+            shift
+	    break
+            ;;
+        *)
+            echo "$0: internal error ($1)"
+            exit 1
+            ;;
+    esac
+done
+
+# Hopefully there are two arguments left, the source (local) file
+# and a remote file of the form user@server:remote.
+if [ $# -ne 2 ]; then
+    echo "$0: incorrect number of arguments found:" "$@"
+    exit 1
+fi
+
+local="$1"
+remote="$(echo $2 | awk -F: '{print $2}')"
+
+# Use the copy command.
+exec cp "$local" "$remote"
diff --git a/p2v/test-virt-p2v.sh b/p2v/test-virt-p2v.sh
index 692894b..e232d42 100755
--- a/p2v/test-virt-p2v.sh
+++ b/p2v/test-virt-p2v.sh
@@ -50,10 +50,11 @@ d=test-virt-p2v.d
 rm -rf $d
 mkdir $d
 
-# We don't want the program under test to run real 'ssh'.  It's
-# unlikely to work.  Therefore create a dummy 'ssh' binary.
+# We don't want the program under test to run real 'ssh' or
'scp'.
+# They won't work.  Therefore create dummy 'ssh' and 'scp'
binaries.
 pushd $d
 ln -sf ../test-virt-p2v-ssh.sh ssh
+ln -sf ../test-virt-p2v-scp.sh scp
 popd
 export PATH=$d:$PATH
 
diff --git a/p2v/virt-p2v.pod b/p2v/virt-p2v.pod
index 664ac20..70d4748 100644
--- a/p2v/virt-p2v.pod
+++ b/p2v/virt-p2v.pod
@@ -56,6 +56,10 @@ by virt-p2v, and it will not work if this is disabled on the
 conversion server.  (C<AllowTcpForwarding> must be C<yes> in the
 L<sshd_config(5)> file on the conversion server).
 
+The scp (secure copy) feature of ssh is required by virt-p2v so it can
+send over small files (this is I<not> the method by which disks are
+copied).
+
 The conversion server does not need to be a physical machine.  It
 could be a virtual machine, as long as it has sufficient memory and
 disk space to do the conversion, and as long as the physical machine
-- 
2.7.4

---
 p2v/ssh.c | 18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/p2v/ssh.c b/p2v/ssh.c
index 0d0874a..9621c17 100644
--- a/p2v/ssh.c
+++ b/p2v/ssh.c
@@ -299,7 +299,8 @@ cache_ssh_identity (struct config *config)
  * optional arguments.  Also handles authentication.
  */
 static mexp_h *
-start_ssh (struct config *config, char **extra_args, int wait_prompt)
+start_ssh (unsigned spawn_flags, struct config *config,
+           char **extra_args, int wait_prompt)
 {
   size_t i, j, nr_args, count;
   char port_str[64];
@@ -373,9 +374,9 @@ start_ssh (struct config *config, char **extra_args, int
wait_prompt)
 #endif
 
   /* Create the miniexpect handle. */
-  h = mexp_spawnv ("ssh", (char **) args);
+  h = mexp_spawnvf (spawn_flags, "ssh", (char **) args);
   if (h == NULL) {
-    set_ssh_internal_error ("ssh: mexp_spawnv: %m");
+    set_ssh_internal_error ("ssh: mexp_spawnvf: %m");
     return NULL;
   }
 
@@ -744,7 +745,7 @@ test_connection (struct config *config)
   const int ovecsize = 12;
   int ovector[ovecsize];
 
-  h = start_ssh (config, NULL, 1);
+  h = start_ssh (0, config, NULL, 1);
   if (h == NULL)
     return -1;
 
@@ -1047,7 +1048,7 @@ open_data_connection (struct config *config, int
*local_port, int *remote_port)
   *local_port = nbd_local_port;
   nbd_local_port++;
 
-  h = start_ssh (config, (char **) extra_args, 0);
+  h = start_ssh (0, config, (char **) extra_args, 0);
   if (h == NULL)
     return NULL;
 
@@ -1135,7 +1136,12 @@ start_remote_connection (struct config *config, const
char *remote_dir)
 {
   mexp_h *h;
 
-  h = start_ssh (config, NULL, 1);
+  /* This connection is opened in cooked mode so that we can send ^C
+   * if the conversion needs to be cancelled.  However that also means
+   * we must be careful not to accidentally send any control
+   * characters over this connection at other times.
+   */
+  h = start_ssh (MEXP_SPAWN_COOKED_MODE, config, NULL, 1);
   if (h == NULL)
     return NULL;
 
-- 
2.7.4

We are now able to cancel the conversion instantly by sending ^C to
the remote virt-v2v process.

Also, this reverts:
"p2v: Poll to make Cancel Conversion button more responsive."
(commit 6da4941db7f8a85997d6281b9b4c5165768e6489)
---
 p2v/conversion.c | 60 +++++++++++++++++++++++++++-----------------------------
 1 file changed, 29 insertions(+), 31 deletions(-)

diff --git a/p2v/conversion.c b/p2v/conversion.c
index 721882f..3021d6c 100644
--- a/p2v/conversion.c
+++ b/p2v/conversion.c
@@ -37,7 +37,6 @@
 #include <fcntl.h>
 #include <inttypes.h>
 #include <unistd.h>
-#include <poll.h>
 #include <time.h>
 #include <errno.h>
 #include <error.h>
@@ -137,6 +136,7 @@ static pthread_mutex_t running_mutex =
PTHREAD_MUTEX_INITIALIZER;
 static int running = 0;
 static pthread_mutex_t cancel_requested_mutex = PTHREAD_MUTEX_INITIALIZER;
 static int cancel_requested = 0;
+static mexp_h *control_h = NULL;
 
 static int
 is_running (void)
@@ -171,6 +171,21 @@ set_cancel_requested (int r)
 {
   pthread_mutex_lock (&cancel_requested_mutex);
   cancel_requested = r;
+
+  /* Send ^C to the remote so that virt-v2v "knows" the connection
has
+   * been cancelled.  mexp_send_interrupt is a single write(2) call.
+   */
+  if (r && control_h)
+    ignore_value (mexp_send_interrupt (control_h));
+
+  pthread_mutex_unlock (&cancel_requested_mutex);
+}
+
+static void
+set_control_h (mexp_h *new_h)
+{
+  pthread_mutex_lock (&cancel_requested_mutex);
+  control_h = new_h;
   pthread_mutex_unlock (&cancel_requested_mutex);
 }
 
@@ -185,7 +200,6 @@ start_conversion (struct config *config,
   size_t nr_disks = guestfs_int_count_strings (config->disks);
   time_t now;
   struct tm tm;
-  mexp_h *control_h = NULL;
   CLEANUP_FREE struct data_conn *data_conns = NULL;
   CLEANUP_FREE char *remote_dir = NULL;
   char tmpdir[]           = "/tmp/p2v.XXXXXX";
@@ -199,6 +213,7 @@ start_conversion (struct config *config,
   fprintf (stderr, "\n");
 #endif
 
+  set_control_h (NULL);
   set_running (1);
   set_cancel_requested (0);
 
@@ -311,7 +326,7 @@ 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);
+  set_control_h (start_remote_connection (config, remote_dir));
   if (control_h == NULL) {
     set_conversion_error ("could not open control connection over SSH to
the conversion server: %s",
                           get_ssh_error ());
@@ -358,35 +373,13 @@ start_conversion (struct config *config,
   }
 
   /* Read output from the virt-v2v process and echo it through the
-   * notify function, until virt-v2v closes the connection.  We
-   * actually poll in this loop (albeit it only every 2 seconds) so
-   * that the user won't have to wait too long between pressing the
-   * cancel button and having the conversion cancelled.
+   * notify function, until virt-v2v closes the connection.
    */
   while (!is_cancel_requested ()) {
-    int fd = mexp_get_fd (control_h);
-    struct pollfd fds[1];
-    int rp;
     char buf[257];
     ssize_t r;
 
-    fds[0].fd = fd;
-    fds[0].events = POLLIN;
-    fds[0].revents = 0;
-    rp = poll (fds, 1, 2000 /* ms */);
-    if (rp == -1) {
-      /* See comment about this in miniexpect.c. */
-      if (errno == EIO)
-        break;
-      set_conversion_error ("poll: %m");
-      goto out;
-    }
-    else if (rp == 0)
-      /* Timeout. */
-      continue;
-    /* ... else rp == 1, ignore revents and just do the read. */
-
-    r = read (fd, buf, sizeof buf - 1);
+    r = read (mexp_get_fd (control_h), buf, sizeof buf - 1);
     if (r == -1) {
       /* See comment about this in miniexpect.c. */
       if (errno == EIO)
@@ -414,12 +407,17 @@ start_conversion (struct config *config,
   ret = 0;
  out:
   if (control_h) {
-    if ((status = mexp_close (control_h)) == -1) {
+    mexp_h *h = control_h;
+    set_control_h (NULL);
+    status = mexp_close (h);
+
+    if (status == -1) {
       set_conversion_error ("mexp_close: %m");
       ret = -1;
-    } else if (ret == 0 &&
-               WIFEXITED (status) &&
-               WEXITSTATUS (status) != 0) {
+    }
+    else if (ret == 0 &&
+             WIFEXITED (status) &&
+             WEXITSTATUS (status) != 0) {
       set_conversion_error ("virt-v2v exited with status %d",
                             WEXITSTATUS (status));
       ret = -1;
-- 
2.7.4