[PATCH 0/4] p2v: Send ping packets, document timeout problems.
Fix and/or document issues raised in this thread: https://www.redhat.com/archives/libguestfs/2017-February/msg00010.html Rich.
This is a simple macro with no dependencies. Allow it to be used from any program. --- daemon/daemon.h | 12 ------------ lib/guestfs-internal-all.h | 17 ++++++++++++++++- 2 files changed, 16 insertions(+), 13 deletions(-) diff --git a/daemon/daemon.h b/daemon/daemon.h index 3af5c0e..8097bfc 100644 --- a/daemon/daemon.h +++ b/daemon/daemon.h @@ -340,18 +340,6 @@ is_zero (const char *buffer, size_t size) return 1; } -/* Helper for building up short lists of arguments. Your code has to - * define MAX_ARGS to a suitable value. - */ -#define ADD_ARG(argv,i,v) \ - do { \ - if ((i) >= MAX_ARGS) { \ - fprintf (stderr, "%s: %d: internal error: exceeded MAX_ARGS (%zu) when constructing the command line\n", __FILE__, __LINE__, (size_t) MAX_ARGS); \ - abort (); \ - } \ - (argv)[(i)++] = (v); \ - } while (0) - /* Helper for functions that need a root filesystem mounted. * NB. Cannot be used for FileIn functions. */ diff --git a/lib/guestfs-internal-all.h b/lib/guestfs-internal-all.h index dbf9735..0c841ff 100644 --- a/lib/guestfs-internal-all.h +++ b/lib/guestfs-internal-all.h @@ -1,5 +1,5 @@ /* libguestfs - * Copyright (C) 2013 Red Hat Inc. + * Copyright (C) 2013-2017 Red Hat Inc. * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -58,6 +58,21 @@ #define STRCASEPREFIX(a,b) (strncasecmp((a),(b),strlen((b))) == 0) #define STRSUFFIX(a,b) (strlen((a)) >= strlen((b)) && STREQ((a)+strlen((a))-strlen((b)),(b))) +/* A simple (indeed, simplistic) way to build up short lists of + * arguments. Your code must define MAX_ARGS to a suitable "larger + * than could ever be needed" value. (If the value is exceeded then + * your code will abort). For more complex needs, use something else + * more suitable. + */ +#define ADD_ARG(argv,i,v) \ + do { \ + if ((i) >= MAX_ARGS) { \ + fprintf (stderr, "%s: %d: internal error: exceeded MAX_ARGS (%zu) when constructing the command line\n", __FILE__, __LINE__, (size_t) MAX_ARGS); \ + abort (); \ + } \ + (argv)[(i)++] = (v); \ + } while (0) + #ifndef SOCK_CLOEXEC #define SOCK_CLOEXEC 0 #endif -- 2.10.2
No functional change. --- p2v/ssh.c | 134 +++++++++++++++++++++++++++++--------------------------------- 1 file changed, 62 insertions(+), 72 deletions(-) diff --git a/p2v/ssh.c b/p2v/ssh.c index 4c1d64c..817048c 100644 --- a/p2v/ssh.c +++ b/p2v/ssh.c @@ -306,6 +306,15 @@ cache_ssh_identity (struct config *config) return 0; } +/* GCC complains about the argv array in the next function which it + * thinks might grow to an unbounded size. Since we control + * extra_args, this is not in fact a problem. + */ +#if defined(__GNUC__) && GUESTFS_GCC_VERSION >= 40800 /* gcc >= 4.8.0 */ +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wstack-usage=" +#endif + /** * Start ssh subprocess with the standard arguments and possibly some * optional arguments. Also handles authentication. @@ -314,15 +323,18 @@ static mexp_h * start_ssh (unsigned spawn_flags, struct config *config, char **extra_args, int wait_prompt) { - size_t i, j, nr_args, count; + size_t i = 0; + const size_t MAX_ARGS = + 64 + (extra_args != NULL ? guestfs_int_count_strings (extra_args) : 0); + const char *argv[MAX_ARGS]; char port_str[64]; char connect_timeout_str[128]; - CLEANUP_FREE /* [sic] */ const char **args = NULL; mexp_h *h; const int ovecsize = 12; int ovector[ovecsize]; int saved_timeout; int using_password_auth; + size_t count; if (cache_ssh_identity (config) == -1) return NULL; @@ -330,63 +342,48 @@ start_ssh (unsigned spawn_flags, struct config *config, /* Are we using password or identity authentication? */ using_password_auth = config->identity_file == NULL; - /* Create the ssh argument array. */ - nr_args = 0; - if (extra_args != NULL) - nr_args = guestfs_int_count_strings (extra_args); - - if (using_password_auth) - nr_args += 13; - else - nr_args += 15; - args = malloc (sizeof (char *) * nr_args); - if (args == NULL) - error (EXIT_FAILURE, errno, "malloc"); - - j = 0; - args[j++] = "ssh"; - args[j++] = "-p"; /* Port. */ + ADD_ARG (argv, i, "ssh"); + ADD_ARG (argv, i, "-p"); /* Port. */ snprintf (port_str, sizeof port_str, "%d", config->port); - args[j++] = port_str; - args[j++] = "-l"; /* Username. */ - args[j++] = config->username ? config->username : "root"; - args[j++] = "-o"; /* Host key will always be novel. */ - args[j++] = "StrictHostKeyChecking=no"; - args[j++] = "-o"; /* ConnectTimeout */ + ADD_ARG (argv, i, port_str); + ADD_ARG (argv, i, "-l"); /* Username. */ + ADD_ARG (argv, i, config->username ? config->username : "root"); + ADD_ARG (argv, i, "-o"); /* Host key will always be novel. */ + ADD_ARG (argv, i, "StrictHostKeyChecking=no"); + ADD_ARG (argv, i, "-o"); /* ConnectTimeout */ snprintf (connect_timeout_str, sizeof connect_timeout_str, "ConnectTimeout=%d", SSH_TIMEOUT); - args[j++] = connect_timeout_str; + ADD_ARG (argv, i, connect_timeout_str); if (using_password_auth) { /* Only use password authentication. */ - args[j++] = "-o"; - args[j++] = "PreferredAuthentications=keyboard-interactive,password"; + ADD_ARG (argv, i, "-o"); + ADD_ARG (argv, i, "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; + ADD_ARG (argv, i, "-o"); + ADD_ARG (argv, i, "PreferredAuthentications=publickey"); + ADD_ARG (argv, i, "-i"); + ADD_ARG (argv, i, config->identity_file); } if (extra_args != NULL) { - for (i = 0; extra_args[i] != NULL; ++i) - args[j++] = extra_args[i]; + for (size_t j = 0; extra_args[j] != NULL; ++j) + ADD_ARG (argv, i, extra_args[j]); } - args[j++] = config->server; /* Conversion server. */ - args[j++] = NULL; - assert (j == nr_args); + ADD_ARG (argv, i, config->server); /* Conversion server. */ + ADD_ARG (argv, i, NULL); #if DEBUG_STDERR fputs ("ssh command: ", stderr); - for (i = 0; i < nr_args - 1; ++i) { + for (i = 0; argv[i] != NULL; ++i) { if (i > 0) fputc (' ', stderr); - fputs (args[i], stderr); + fputs (argv[i], stderr); } fputc ('\n', stderr); #endif /* Create the miniexpect handle. */ - h = mexp_spawnvf (spawn_flags, "ssh", (char **) args); + h = mexp_spawnvf (spawn_flags, "ssh", (char **) argv); if (h == NULL) { set_ssh_internal_error ("ssh: mexp_spawnvf: %m"); return NULL; @@ -566,6 +563,10 @@ start_ssh (unsigned spawn_flags, struct config *config, return h; } +#if defined(__GNUC__) && GUESTFS_GCC_VERSION >= 40800 /* gcc >= 4.8.0 */ +#pragma GCC diagnostic pop +#endif + /** * Upload a file to remote using L<scp(1)>. * @@ -574,16 +575,16 @@ start_ssh (unsigned spawn_flags, struct config *config, int scp_file (struct config *config, const char *localfile, const char *remotefile) { - size_t j, nr_args; + size_t i = 0; + const size_t MAX_ARGS = 64; + const char *argv[MAX_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; - size_t i; if (cache_ssh_identity (config) == -1) return -1; @@ -591,58 +592,47 @@ scp_file (struct config *config, const char *localfile, const char *remotefile) /* 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. */ + ADD_ARG (argv, i, "scp"); + ADD_ARG (argv, i, "-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 */ + ADD_ARG (argv, i, port_str); + ADD_ARG (argv, i, "-o"); /* Host key will always be novel. */ + ADD_ARG (argv, i, "StrictHostKeyChecking=no"); + ADD_ARG (argv, i, "-o"); /* ConnectTimeout */ snprintf (connect_timeout_str, sizeof connect_timeout_str, "ConnectTimeout=%d", SSH_TIMEOUT); - args[j++] = connect_timeout_str; + ADD_ARG (argv, i, connect_timeout_str); if (using_password_auth) { /* Only use password authentication. */ - args[j++] = "-o"; - args[j++] = "PreferredAuthentications=keyboard-interactive,password"; + ADD_ARG (argv, i, "-o"); + ADD_ARG (argv, i, "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; + ADD_ARG (argv, i, "-o"); + ADD_ARG (argv, i, "PreferredAuthentications=publickey"); + ADD_ARG (argv, i, "-i"); + ADD_ARG (argv, i, config->identity_file); } - args[j++] = localfile; + ADD_ARG (argv, i, 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); + ADD_ARG (argv, i, remote); + ADD_ARG (argv, i, NULL); #if DEBUG_STDERR fputs ("scp command: ", stderr); - for (i = 0; i < nr_args - 1; ++i) { + for (i = 0; argv[i] != NULL; ++i) { if (i > 0) fputc (' ', stderr); - fputs (args[i], stderr); + fputs (argv[i], stderr); } fputc ('\n', stderr); #endif /* Create the miniexpect handle. */ - h = mexp_spawnv ("scp", (char **) args); + h = mexp_spawnv ("scp", (char **) argv); if (h == NULL) { set_ssh_internal_error ("scp: mexp_spawnv: %m"); return -1; -- 2.10.2
Also drop the connection if no response at all has been received after 30 minutes. This action should prevent firewall timeouts from causing virt-p2v to fail (see this thread: https://www.redhat.com/archives/libguestfs/2017-February/msg00010.html). Thanks: Tomáš Golembiovský --- p2v/ssh.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/p2v/ssh.c b/p2v/ssh.c index 817048c..4dd4c44 100644 --- a/p2v/ssh.c +++ b/p2v/ssh.c @@ -354,6 +354,10 @@ start_ssh (unsigned spawn_flags, struct config *config, snprintf (connect_timeout_str, sizeof connect_timeout_str, "ConnectTimeout=%d", SSH_TIMEOUT); ADD_ARG (argv, i, connect_timeout_str); + ADD_ARG (argv, i, "-o"); /* Send ping packets every 5 mins to sshd. */ + ADD_ARG (argv, i, "ServerAliveInterval=300"); + ADD_ARG (argv, i, "-o"); + ADD_ARG (argv, i, "ServerAliveCountMax=6"); if (using_password_auth) { /* Only use password authentication. */ ADD_ARG (argv, i, "-o"); -- 2.10.2
--- p2v/virt-p2v.pod | 38 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/p2v/virt-p2v.pod b/p2v/virt-p2v.pod index e04c1f1..98763f6 100644 --- a/p2v/virt-p2v.pod +++ b/p2v/virt-p2v.pod @@ -568,6 +568,44 @@ L<virt-p2v-make-disk(1)/ADDING AN SSH IDENTITY> L<virt-p2v-make-kickstart(1)/ADDING AN SSH IDENTITY> +=head1 COMMON PROBLEMS + +=head2 Timeouts + +As described below (see L</HOW VIRT-P2V WORKS>) virt-p2v makes several +long-lived ssh connections to the conversion server. If these +connections time out then virt-p2v will fail. + +To test if a timeout might be causing problems, open an XTerm on the +virt-p2v machine, C<ssh root@I<conversion-server>>, and leave it for +at least an hour. If the session disconnects without you doing +anything, then there is a timeout which you should turn off. + +Timeouts happen because: + +=over 4 + +=item C<TIMEOUT> or C<TMOUT> environment variable + +Check if one of these environment variables is set in the root shell +on the conversion server. + +=item sshd C<ClientAlive*> setting + +Check for C<ClientAlive*> settings in C</etc/ssh/sshd_config> on the +conversion server. + +=item Firewall or NAT settings + +Check if there is a firewall or NAT box between virt-p2v and the +conversion server, and if this firewall drops idle connections after a +too-short time. + +virt-p2v E<ge> 1.36 attempts to work around firewall timeouts by +sending ssh keepalive messages every 5 minutes. + +=back + =head1 OPTIONS =over 4 -- 2.10.2
participants (1)
-
Richard W.M. Jones