Libguestfs - Dec 2014 - Re: [PATCH] p2v: wait for qemu-nbd before starting conversion (RHBZ#1167774)

Wait up to 10 seconds for qemu-nbd to start up and respond to clients.
Otherwise the conversion server may attempt to connect before qemu-nbd
is ready to serve it.
---
 p2v/conversion.c | 72 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 p2v/p2v.h        |  1 +
 2 files changed, 73 insertions(+)

diff --git a/p2v/conversion.c b/p2v/conversion.c
index cb2deed..1aefcc6 100644
--- a/p2v/conversion.c
+++ b/p2v/conversion.c
@@ -31,6 +31,8 @@
 #include <libintl.h>
 #include <sys/types.h>
 #include <sys/wait.h>
+#include <arpa/inet.h>
+#include <netinet/in.h>
 
 #include <glib.h>
 
@@ -49,6 +51,7 @@ struct data_conn {
 
 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 const char *map_interface_to_network (struct config *, const char
*interface);
@@ -156,6 +159,12 @@ start_conversion (struct config *config,
     if (data_conns[i].nbd_pid == 0)
       goto out;
 
+    /* Wait for qemu-nbd to listen */
+    if (wait_qemu_nbd (data_conns[i].nbd_local_port, WAIT_QEMU_NBD_TIMEOUT) ==
-1) {
+      set_conversion_error("unable to connect to qemu-nbd");
+      goto out;
+    }
+
 #if DEBUG_STDERR
     fprintf (stderr,
              "%s: data connection for %s: SSH remote port %d, local port
%d\n",
@@ -371,6 +380,69 @@ start_qemu_nbd (int port, const char *device)
   return pid;
 }
 
+static int
+wait_qemu_nbd (int nbd_local_port, int timeout_seconds)
+{
+  int sockfd;
+  int result = 0;
+  struct sockaddr_in addr;
+  time_t start_t, now_t;
+  struct timeval timeout = { .tv_usec = 0 };
+  char magic[8]; /* NBDMAGIC */
+  size_t bytes_read = 0;
+  ssize_t recvd;
+
+  time (&start_t);
+
+  sockfd = socket (AF_INET, SOCK_STREAM, 0);
+  if (sockfd == -1) {
+    perror ("socket");
+    return -1;
+  }
+
+  memset(&addr, 0, sizeof (struct sockaddr_in));
+  addr.sin_family = AF_INET;
+  addr.sin_port = htons(nbd_local_port);
+  inet_pton(AF_INET, "localhost", &addr.sin_addr);
+
+  for (;;) {
+    time (&now_t);
+
+    if (now_t - start_t >= timeout_seconds) {
+      perror("connect");
+      result = -1;
+      goto cleanup;
+    }
+
+    if (connect (sockfd, (struct sockaddr *) &addr, sizeof addr) == 0)
+      break;
+  }
+
+  time (&now_t);
+  timeout.tv_sec = (start_t + timeout_seconds) - now_t;
+  setsockopt(sockfd, SOL_SOCKET, SO_RCVTIMEO, &timeout, sizeof(struct
timeval));
+
+  do {
+    recvd = recv (sockfd, magic, sizeof(magic)-bytes_read, 0);
+
+    if (recvd == -1) {
+      perror("recv");
+      result = -1;
+      goto cleanup;
+    }
+
+    bytes_read += recvd;
+  } while (bytes_read < sizeof magic);
+
+  if (strncmp(magic, "NBDMAGIC", sizeof magic))
+    result = -1;
+
+cleanup:
+  close(sockfd);
+
+  return result;
+}
+
 static void
 cleanup_data_conns (struct data_conn *data_conns, size_t nr)
 {
diff --git a/p2v/p2v.h b/p2v/p2v.h
index c3ca0f6..7c4c115 100644
--- a/p2v/p2v.h
+++ b/p2v/p2v.h
@@ -98,6 +98,7 @@ extern int start_conversion (struct config *, void
(*notify_ui) (int type, const
 #define NOTIFY_STATUS         3  /* stage in conversion process */
 extern const char *get_conversion_error (void);
 extern void cancel_conversion (void);
+#define WAIT_QEMU_NBD_TIMEOUT 10
 
 /* ssh.c */
 extern int test_connection (struct config *);
-- 
1.9.3

Hi John,

Looks good.  I've attached a slightly modified patch:

 - Don't need to define the WAIT_* timeout in the header file.

 - Call set_conversion_error in the subroutine, so that the
   real error can be propagated back to the user.

 - Use memcmp instead of strncmp.

 - Whitespace changes.

What do you think?

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-p2v converts physical machines to virtual machines.  Boot with a
live CD or over the network (PXE) and turn machines into KVM guests.
http://libguestfs.org/virt-v2v

"Richard W.M. Jones" <rjones@redhat.com>
writes:
> Hi John,
>
> Looks good.  I've attached a slightly modified patch:
>
>  - Don't need to define the WAIT_* timeout in the header file.
>
>  - Call set_conversion_error in the subroutine, so that the
>    real error can be propagated back to the user.
>
>  - Use memcmp instead of strncmp.
>
>  - Whitespace changes.
>
> What do you think?
>
> Rich.
Looks good to me.  Thanks Rich.

eck