Libguestfs - Oct 2020 - [PATCH libnbd] interop: Add test of qemu-storage-daemon.

This commit adds a simple test of qemu-storage-daemon (QSD).  On the
basis that QSD is just qemu-nbd in new clothes this is only a simple
test, not complete coverage.  Nor does it test the unique features of
QSD like being able to use QMP to create new server instances.

Unfortunately QSD is not yet stable upstream.  This version works with
qemu 5.1.0 but at least two of the command line parameters have
changed name upstream (see comment in patch) so it will break soon.
Also there are quite a few other problems with QSD:

  https://lists.gnu.org/archive/html/qemu-devel/2020-09/threads.html#10952

So this is probably not something we can take upstream in libnbd yet,
but here's the patch to look at.  (I could push a patch with the test
disabled and plan to enable it later.)

Rich.

---
 .gitignore          |  1 +
 configure.ac        |  6 ++-
 interop/Makefile.am | 24 ++++++++++++
 interop/interop.c   | 89 ++++++++++++++++++++++++++++++++++++++++++++-
 4 files changed, 118 insertions(+), 2 deletions(-)

diff --git a/.gitignore b/.gitignore
index aa9e0bb..199e445 100644
--- a/.gitignore
+++ b/.gitignore
@@ -94,6 +94,7 @@ Makefile.in
 /interop/interop-qemu-nbd
 /interop/interop-qemu-nbd-tls-certs
 /interop/interop-qemu-nbd-tls-psk
+/interop/interop-qemu-storage-daemon
 /interop/list-exports-nbd-server
 /interop/list-exports-qemu-nbd
 /interop/socket-activation-nbdkit
diff --git a/configure.ac b/configure.ac
index 328c6cb..6bb057f 100644
--- a/configure.ac
+++ b/configure.ac
@@ -182,11 +182,15 @@ AS_IF([test "x$NBDKIT" != "x"], [
 AM_CONDITIONAL([HAVE_NBDKIT],
     [test "x$NBDKIT" != "x" && test
"x$have_nbdkit_features" = "xyes"])
 
-dnl nbd-server and qemu-nbd for interop testing.
+dnl nbd-server, qemu-nbd and qemu-storage-daemon for interop testing.
 AC_CHECK_PROG([NBD_SERVER], [nbd-server], [nbd-server])
 AM_CONDITIONAL([HAVE_NBD_SERVER], [test "x$NBD_SERVER" !=
"x"])
 AC_CHECK_PROG([QEMU_NBD], [qemu-nbd], [qemu-nbd])
 AM_CONDITIONAL([HAVE_QEMU_NBD], [test "x$QEMU_NBD" != "x"])
+AC_CHECK_PROG([QEMU_STORAGE_DAEMON],
+               [qemu-storage-daemon], [qemu-storage-daemon])
+AM_CONDITIONAL([HAVE_QEMU_STORAGE_DAEMON],
+               [test "x$QEMU_STORAGE_DAEMON" != "x"])
 
 dnl glib2 main loop for examples that interoperate with the glib main loop.
 PKG_CHECK_MODULES([GLIB], [glib-2.0], [
diff --git a/interop/Makefile.am b/interop/Makefile.am
index 9787c26..7712777 100644
--- a/interop/Makefile.am
+++ b/interop/Makefile.am
@@ -174,6 +174,30 @@ structured_read_LDADD = $(top_builddir)/lib/libnbd.la
 
 endif HAVE_QEMU_NBD
 
+# We treat qemu-storage-daemon as effectively the same as qemu-nbd
+# since they use the same code, but there is a single test to make
+# sure we can use it.
+if HAVE_QEMU_STORAGE_DAEMON
+
+check_PROGRAMS += interop-qemu-storage-daemon
+TESTS += interop-qemu-storage-daemon
+
+# NB for upstream we must change --export parameters:
+#  device= -> node.name+#  name=   ->
id+interop_qemu_storage_daemon_SOURCES = interop.c
+interop_qemu_storage_daemon_CPPFLAGS = \
+	-I$(top_srcdir)/include \
+	-DUNIXSOCKET=\"`mktemp -u /tmp/libnbd-interop-qsd.XXXXXX`\" \
+	-DSERVER=\"$(QEMU_STORAGE_DAEMON)\" \
+	-DSERVER_PARAMS='"--blockdev", SPRINTF
("raw,file.driver=file,file.filename=%s,node-name=disk0", tmpfile),
"--nbd-server", SPRINTF ("addr.type=unix,addr.path=%s",
UNIXSOCKET), "--export", "nbd,device=disk0,name=/"' \
+	-DEXPORT_NAME='"/"' \
+	$(NULL)
+interop_qemu_storage_daemon_CFLAGS = $(WARNINGS_CFLAGS)
+interop_qemu_storage_daemon_LDADD = $(top_builddir)/lib/libnbd.la
+
+endif HAVE_QEMU_STORAGE_DAEMON
+
 if HAVE_NBDKIT
 
 check_PROGRAMS += \
diff --git a/interop/interop.c b/interop/interop.c
index ec3e735..64608fc 100644
--- a/interop/interop.c
+++ b/interop/interop.c
@@ -35,6 +35,27 @@
 
 #define SIZE (1024*1024)
 
+struct free_on_exit {
+  struct free_on_exit *next;
+  void *v;
+};
+
+/* Macro which can be used in SERVER_PARAMS to deal with the case
+ * where a parameter must be embedded inside a parameter.  The return
+ * value is automatically freed.
+ */
+#define SPRINTF(fs,...)                                                 \
+  ({                                                                    \
+    char *ret;                                                          \
+    if (asprintf (&ret, (fs), ##__VA_ARGS__) == -1) abort ();           \
+    struct free_on_exit *f = malloc (sizeof *f);                        \
+    if (!f) abort ();                                                   \
+    f->next = free_on_exit_list;                                        \
+    f->v = ret;                                                         \
+    free_on_exit_list = f;                                              \
+    ret;                                                                \
+  })
+
 #if CERTS || PSK
 #define TLS 1
 #ifndef TLS_MODE
@@ -45,13 +66,17 @@
 int
 main (int argc, char *argv[])
 {
+  struct free_on_exit *free_on_exit_list = NULL;
   struct nbd_handle *nbd;
   char tmpfile[] = "/tmp/nbdXXXXXX";
-  char *args[] = { SERVER, SERVER_PARAMS, NULL };
+  size_t i;
   int fd;
   int64_t actual_size;
   char buf[512];
   int r = -1;
+#ifdef UNIXSOCKET
+  pid_t pid = 0;
+#endif
 
   /* Create a large sparse temporary file. */
   fd = mkstemp (tmpfile);
@@ -62,6 +87,16 @@ main (int argc, char *argv[])
     goto out;
   }
 
+  /* Server name and parameters.  These come from -D options defined
+   * in Makefile.am.
+   */
+  char *args[] = { SERVER, SERVER_PARAMS, NULL };
+  printf ("server:");
+  for (i = 0; args[i] != NULL; ++i)
+    printf (" %s", args[i]);
+  printf ("\n");
+  fflush (stdout);
+
   nbd = nbd_create ();
   if (nbd == NULL) {
     fprintf (stderr, "%s\n", nbd_get_error ());
@@ -103,6 +138,7 @@ main (int argc, char *argv[])
 #endif
 
   /* Start the server. */
+#ifndef UNIXSOCKET
 #if SOCKET_ACTIVATION
 #define NBD_CONNECT nbd_connect_systemd_socket_activation
 #else
@@ -112,6 +148,40 @@ main (int argc, char *argv[])
     fprintf (stderr, "%s\n", nbd_get_error ());
     goto out;
   }
+#else /* UNIXSOCKET */
+  /* This is for servers which don't support either stdin/stdout or
+   * systemd socket activation.  We have to run the command in the
+   * background and connect to the Unix domain socket.
+   */
+  pid = fork ();
+  if (pid == -1) {
+    perror ("fork");
+    goto out;
+  }
+  if (pid == 0) {
+    execvp (args[0], args);
+    perror (args[0]);
+    _exit (EXIT_FAILURE);
+  }
+  /* Unfortunately qemu-storage-daemon doesn't support pidfiles, so we
+   * have to wait for the socket to be created then sleep "a bit".
+   */
+  for (i = 0; i < 60; ++i) {
+    if (access (UNIXSOCKET, R_OK) == 0)
+      break;
+    sleep (1);
+  }
+  sleep (1);
+  /* Connect to the socket. */
+  if (nbd_connect_unix (nbd, UNIXSOCKET) == -1) {
+    fprintf (stderr, "%s\n", nbd_get_error ());
+    goto out;
+  }
+  /* We don't need the socket to exist in the filesystem any more, so
+   * we can remove it early.
+   */
+  unlink (UNIXSOCKET);
+#endif /* UNIXSOCKET */
 
 #if TLS
   if (TLS_MODE == LIBNBD_TLS_REQUIRE) {
@@ -179,5 +249,22 @@ main (int argc, char *argv[])
  out:
   unlink (tmpfile);
 
+#ifdef UNIXSOCKET
+  /* Kill the background server. */
+  if (pid > 0) {
+    printf ("killing %s PID %d ...\n", args[0], (int) pid);
+    fflush (stdout);
+    kill (pid, SIGTERM);
+  }
+#endif
+
+  /* Free strings allocated by SPRINTF macro. */
+  while (free_on_exit_list != NULL) {
+    struct free_on_exit *next = free_on_exit_list->next;
+    free (free_on_exit_list->v);
+    free (free_on_exit_list);
+    free_on_exit_list = next;
+  }
+
   exit (r == 0 ? EXIT_SUCCESS : EXIT_FAILURE);
 }
-- 
2.27.0

On Thu, Oct 01, 2020 at 10:50:01AM +0100, Richard W.M. Jones
wrote:
> This commit adds a simple test of qemu-storage-daemon (QSD).  On the
> basis that QSD is just qemu-nbd in new clothes this is only a simple
> test, not complete coverage.  Nor does it test the unique features of
> QSD like being able to use QMP to create new server instances.
> 
> Unfortunately QSD is not yet stable upstream.  This version works with
> qemu 5.1.0 but at least two of the command line parameters have
> changed name upstream (see comment in patch) so it will break soon.
> Also there are quite a few other problems with QSD:
> 
>   https://lists.gnu.org/archive/html/qemu-devel/2020-09/threads.html#10952
Wrong link, should be:
https://lists.gnu.org/archive/html/qemu-devel/2020-09/threads.html#10807

Rich.
> So this is probably not something we can take upstream in libnbd yet,
> but here's the patch to look at.  (I could push a patch with the test
> disabled and plan to enable it later.)
> 
> Rich.
> 
> 
> _______________________________________________
> Libguestfs mailing list
> Libguestfs@redhat.com
> https://www.redhat.com/mailman/listinfo/libguestfs
-- 
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