Libguestfs - Oct 2019 - [PATCH libnbd 0/2] Change qemu-nbd interop tests to use socket activation.

Now that we have implemented systemd socket activation, we can use
this to run qemu-nbd in tests.

The first patch leaves some dead code around (the -DSERVE_OVER_TCP=1
path).  It's possible we might want to use this to test against a
putative future NBD server that only supports TCP, but on the other
hand maybe we should just remove it.

Tests & valgrind still pass for me.

Rich.

Now that we have implemented systemd socket activation we can use this
as a way to test qemu-nbd instead of having to use a TCP port.
---
 interop/Makefile.am | 12 ++++++------
 interop/interop.c   |  8 +++++++-
 2 files changed, 13 insertions(+), 7 deletions(-)

diff --git a/interop/Makefile.am b/interop/Makefile.am
index d30cdf1..345be7c 100644
--- a/interop/Makefile.am
+++ b/interop/Makefile.am
@@ -85,9 +85,9 @@ endif
 interop_qemu_nbd_SOURCES = interop.c
 interop_qemu_nbd_CPPFLAGS = \
 	-I$(top_srcdir)/include \
-	-DSERVE_OVER_TCP=1 \
+	-DSOCKET_ACTIVATION=1 \
 	-DSERVER=\"$(QEMU_NBD)\" \
-	-DSERVER_PARAMS='"-f", "raw", "-x",
"/", "-p", port_str, tmpfile' \
+	-DSERVER_PARAMS='"-f", "raw", "-x",
"/", tmpfile' \
 	-DEXPORT_NAME='"/"' \
 	$(NULL)
 interop_qemu_nbd_CFLAGS = $(WARNINGS_CFLAGS)
@@ -97,9 +97,9 @@ interop_qemu_nbd_LDADD = $(top_builddir)/lib/libnbd.la
 interop_qemu_nbd_tls_certs_SOURCES = interop.c
 interop_qemu_nbd_tls_certs_CPPFLAGS = \
 	-I$(top_srcdir)/include \
-	-DSERVE_OVER_TCP=1 \
+	-DSOCKET_ACTIVATION=1 \
 	-DSERVER=\"$(QEMU_NBD)\" \
-	-DSERVER_PARAMS='"--object",
"tls-creds-x509,id=tls0,endpoint=server,dir=$(abs_top_builddir)/tests/pki",
"--tls-creds", "tls0", "-f", "raw",
"-x", "/", "-p", port_str, tmpfile' \
+	-DSERVER_PARAMS='"--object",
"tls-creds-x509,id=tls0,endpoint=server,dir=$(abs_top_builddir)/tests/pki",
"--tls-creds", "tls0", "-f", "raw",
"-x", "/", tmpfile' \
 	-DEXPORT_NAME='"/"' \
 	-DCERTS=1 \
 	-DTLS_MODE=LIBNBD_TLS_REQUIRE \
@@ -111,9 +111,9 @@ interop_qemu_nbd_tls_certs_LDADD =
$(top_builddir)/lib/libnbd.la
 interop_qemu_nbd_tls_psk_SOURCES = interop.c
 interop_qemu_nbd_tls_psk_CPPFLAGS = \
 	-I$(top_srcdir)/include \
-	-DSERVE_OVER_TCP=1 \
+	-DSOCKET_ACTIVATION=1 \
 	-DSERVER=\"$(QEMU_NBD)\" \
-	-DSERVER_PARAMS='"--object",
"tls-creds-psk,id=tls0,endpoint=server,dir=$(abs_top_builddir)/tests",
"--tls-creds", "tls0", "-f", "raw",
"-x", "/", "-p", port_str, tmpfile' \
+	-DSERVER_PARAMS='"--object",
"tls-creds-psk,id=tls0,endpoint=server,dir=$(abs_top_builddir)/tests",
"--tls-creds", "tls0", "-f", "raw",
"-x", "/", tmpfile' \
 	-DEXPORT_NAME='"/"' \
 	-DPSK=1 \
 	-DTLS_MODE=LIBNBD_TLS_REQUIRE \
diff --git a/interop/interop.c b/interop/interop.c
index 7ba0d0e..28b53d1 100644
--- a/interop/interop.c
+++ b/interop/interop.c
@@ -139,7 +139,13 @@ main (int argc, char *argv[])
 #else /* !SERVE_OVER_TCP */
 
   char *args[] = { SERVER, SERVER_PARAMS, NULL };
-  if (nbd_connect_command (nbd, args) == -1) {
+  if (
+#if SOCKET_ACTIVATION
+      nbd_connect_socket_activation (nbd, args)
+#else
+      nbd_connect_command (nbd, args)
+#endif
+      == -1) {
     fprintf (stderr, "%s\n", nbd_get_error ());
     goto out;
   }
-- 
2.23.0

This allows us to greatly simplify these tests.
---
 interop/dirty-bitmap.c     | 10 ++++------
 interop/dirty-bitmap.sh    | 22 +++-------------------
 interop/structured-read.c  |  9 +++------
 interop/structured-read.sh | 21 ++-------------------
 4 files changed, 12 insertions(+), 50 deletions(-)

diff --git a/interop/dirty-bitmap.c b/interop/dirty-bitmap.c
index 8077957..a300388 100644
--- a/interop/dirty-bitmap.c
+++ b/interop/dirty-bitmap.c
@@ -30,7 +30,6 @@
 
 #include <libnbd.h>
 
-static const char *unixsocket;
 static const char *bitmap;
 
 struct data {
@@ -109,12 +108,11 @@ main (int argc, char *argv[])
   struct data data;
   char c;
 
-  if (argc != 3) {
-    fprintf (stderr, "%s unixsocket bitmap\n", argv[0]);
+  if (argc < 3) {
+    fprintf (stderr, "%s bitmap qemu-nbd [args ...]\n", argv[0]);
     exit (EXIT_FAILURE);
   }
-  unixsocket = argv[1];
-  bitmap = argv[2];
+  bitmap = argv[1];
 
   nbd = nbd_create ();
   if (nbd == NULL) {
@@ -125,7 +123,7 @@ main (int argc, char *argv[])
   nbd_add_meta_context (nbd, LIBNBD_CONTEXT_BASE_ALLOCATION);
   nbd_add_meta_context (nbd, bitmap);
 
-  if (nbd_connect_unix (nbd, unixsocket) == -1) {
+  if (nbd_connect_socket_activation (nbd, &argv[2]) == -1) {
     fprintf (stderr, "%s\n", nbd_get_error ());
     exit (EXIT_FAILURE);
   }
diff --git a/interop/dirty-bitmap.sh b/interop/dirty-bitmap.sh
index 4b67965..e6bb40c 100755
--- a/interop/dirty-bitmap.sh
+++ b/interop/dirty-bitmap.sh
@@ -33,7 +33,7 @@ if ! qemu-nbd --help | grep -sq -- -B; then
     exit 77
 fi
 
-files="dirty-bitmap.sock dirty-bitmap.qcow2"
+files="dirty-bitmap.qcow2"
 rm -f $files
 cleanup_fn rm -f $files
 
@@ -48,22 +48,6 @@ cat <<'EOF' | qemu-kvm -nodefaults -nographic
-qmp stdio
 EOF
 qemu-io -f qcow2 -c 'w 64k 64k' -c 'w -z 512k 64k'
dirty-bitmap.qcow2
 
-qemu-nbd -k $PWD/dirty-bitmap.sock -f qcow2 -B bitmap0 dirty-bitmap.qcow2 &
-qemu_pid=$!
-cleanup_fn kill $qemu_pid
-
-# No good way to wait for qemu-nbd to start server, so ...
-for ((i = 0; i < 300; i++)); do
-  if [ -r $PWD/dirty-bitmap.sock ]; then
-    break
-  fi
-  kill -s 0 $qemu_pid 2>/dev/null
-  if test $? != 0; then
-    echo "qemu-nbd unexpectedly quit" 2>&1
-    exit 1
-  fi
-  sleep 0.1
-done
-
 # Run the test.
-$VG ./dirty-bitmap dirty-bitmap.sock qemu:dirty-bitmap:bitmap0
+$VG ./dirty-bitmap qemu:dirty-bitmap:bitmap0 \
+    qemu-nbd -f qcow2 -B bitmap0 dirty-bitmap.qcow2
diff --git a/interop/structured-read.c b/interop/structured-read.c
index 569a56f..6e85b65 100644
--- a/interop/structured-read.c
+++ b/interop/structured-read.c
@@ -30,8 +30,6 @@
 
 #include <libnbd.h>
 
-static const char *unixsocket;
-
 /* Depends on structured-read.sh setting things up so that qemu-nbd
  * exposes an image with a 512-byte hole at offset 2048 followed by a
  * 512-byte data section containing all '1' bytes at offset 2560
@@ -110,11 +108,10 @@ main (int argc, char *argv[])
   struct data data;
   char c;
 
-  if (argc != 2) {
-    fprintf (stderr, "%s unixsocket\n", argv[0]);
+  if (argc < 2) {
+    fprintf (stderr, "%s qemu-nbd [args ...]\n", argv[0]);
     exit (EXIT_FAILURE);
   }
-  unixsocket = argv[1];
 
   nbd = nbd_create ();
   if (nbd == NULL) {
@@ -122,7 +119,7 @@ main (int argc, char *argv[])
     exit (EXIT_FAILURE);
   }
 
-  if (nbd_connect_unix (nbd, unixsocket) == -1) {
+  if (nbd_connect_socket_activation (nbd, &argv[1]) == -1) {
     fprintf (stderr, "%s\n", nbd_get_error ());
     exit (EXIT_FAILURE);
   }
diff --git a/interop/structured-read.sh b/interop/structured-read.sh
index 15a81c0..e43454b 100755
--- a/interop/structured-read.sh
+++ b/interop/structured-read.sh
@@ -26,7 +26,7 @@ requires qemu-img --version
 requires qemu-io --version
 requires qemu-nbd --version
 
-files="structured-read.sock structured-read.qcow2"
+files="structured-read.qcow2"
 rm -f $files
 cleanup_fn rm -f $files
 
@@ -36,22 +36,5 @@ qemu-img create -f qcow2 -o cluster_size=512,compat=v3
structured-read.qcow2 3k
 qemu-io -d unmap -f qcow2 -c 'w -P 1 0 3k' -c 'w -zu 2k 512' \
 	structured-read.qcow2
 
-qemu-nbd -k $PWD/structured-read.sock -f qcow2 structured-read.qcow2 &
-qemu_pid=$!
-cleanup_fn kill $qemu_pid
-
-# qemu-nbd --pid not available before 4.1, so ...
-for ((i = 0; i < 300; i++)); do
-  if [ -r $PWD/structured-read.sock ]; then
-    break
-  fi
-  kill -s 0 $qemu_pid 2>/dev/null
-  if test $? != 0; then
-    echo "qemu-nbd unexpectedly quit" 2>&1
-    exit 1
-  fi
-  sleep 0.1
-done
-
 # Run the test.
-$VG ./structured-read structured-read.sock
+$VG ./structured-read qemu-nbd -f qcow2 structured-read.qcow2
-- 
2.23.0

On 10/1/19 11:15 AM, Richard W.M. Jones wrote:
> Now that we have implemented systemd socket activation, we can use
> this to run qemu-nbd in tests.
> 
> The first patch leaves some dead code around (the -DSERVE_OVER_TCP=1
> path).  It's possible we might want to use this to test against a
> putative future NBD server that only supports TCP, but on the other
> hand maybe we should just remove it.
> 
> Tests & valgrind still pass for me.
Cool - by passing in a socket, we can bypass qemu-nbd's insistence on 
TCP (still on my TODO list to get qemu-nbd to support TLS over Unix 
sockets, but socket activation does bypass that limitation), and avoid 
having to wait for the server to give us some indication that the socket 
is up (because it is already up).  I like it!

ACK series

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org