[PATCH v2 00/12] block/export: vhost-user-blk server tests and input validation

[PATCH v2 00/12] block/export: vhost-user-blk server tests and input validation

[Stefan Hajnoczi]

v2:
 * Add abrt handler that terminates qemu-storage-daemon to
   vhost-user-blk-test. No more orphaned processes on test failure. [Peter]
 * Fix sector number calculation in vhost-user-blk-server.c
 * Introduce VIRTIO_BLK_SECTOR_BITS/SIZE to make code clearer [Max]
 * Fix vhost-user-blk-server.c blk_size double byteswap
 * Fix vhost-user-blk blkcfg->num_queues endianness [Peter]
 * Squashed cleanups into Coiby vhost-user-blk-test commit so the code is
   easier to review

The vhost-user-blk server test was already in Michael Tsirkin's recent vhost
pull request, but was dropped because it exposed vhost-user regressions
(b7c1bd9d7848 and the Based-on tag below). Now that the vhost-user regressions
are fixed we can re-introduce the test case.

This series adds missing input validation that led to a Coverity report. The
virtio-blk read, write, discard, and write zeroes commands need to check
sector/byte ranges and other inputs. This solves the issue Peter Maydell raised
in "[PATCH for-5.2] block/export/vhost-user-blk-server.c: Avoid potential
integer overflow".

Merging just the input validation patches would be possible too, but I prefer
to merge the corresponding tests so the code is exercised by the CI.

Coiby Xu (1):
  test: new qTest case to test the vhost-user-blk-server

Stefan Hajnoczi (11):
  vhost-user-blk: fix blkcfg->num_queues endianness
  libqtest: add qtest_socket_server()
  libqtest: add qtest_kill_qemu()
  libqtest: add qtest_remove_abrt_handler()
  tests/qtest: add multi-queue test case to vhost-user-blk-test
  block/export: fix blk_size double byteswap
  block/export: use VIRTIO_BLK_SECTOR_BITS
  block/export: fix vhost-user-blk export sector number calculation
  block/export: port virtio-blk discard/write zeroes input validation
  vhost-user-blk-test: test discard/write zeroes invalid inputs
  block/export: port virtio-blk read/write range check

 MAINTAINERS                          |   2 +
 tests/qtest/libqos/libqtest.h        |  37 +
 tests/qtest/libqos/vhost-user-blk.h  |  48 ++
 block/export/vhost-user-blk-server.c | 150 +++-
 hw/block/vhost-user-blk.c            |   7 +-
 tests/qtest/libqos/vhost-user-blk.c  | 130 ++++
 tests/qtest/libqtest.c               |  82 ++-
 tests/qtest/vhost-user-blk-test.c    | 983 +++++++++++++++++++++++++++
 tests/qtest/libqos/meson.build       |   1 +
 tests/qtest/meson.build              |   4 +
 10 files changed, 1385 insertions(+), 59 deletions(-)
 create mode 100644 tests/qtest/libqos/vhost-user-blk.h
 create mode 100644 tests/qtest/libqos/vhost-user-blk.c
 create mode 100644 tests/qtest/vhost-user-blk-test.c

-- 
2.28.0

[PATCH v2 01/12] vhost-user-blk: fix blkcfg->num_queues endianness

[Stefan Hajnoczi]

Treat the num_queues field as virtio-endian. On big-endian hosts the
vhost-user-blk num_queues field was in the wrong endianness.

Move the blkcfg.num_queues store operation from realize to
vhost_user_blk_update_config() so feature negotiation has finished and
we know the endianness of the device. VIRTIO 1.0 devices are
little-endian, but in case someone wants to use legacy VIRTIO we support
all endianness cases.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 hw/block/vhost-user-blk.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c
index 2dd3d93ca0..d9d9dc8a89 100644
--- a/hw/block/vhost-user-blk.c
+++ b/hw/block/vhost-user-blk.c
@@ -53,6 +53,9 @@ static void vhost_user_blk_update_config(VirtIODevice *vdev, uint8_t *config)
 {
     VHostUserBlk *s = VHOST_USER_BLK(vdev);
 
+    /* Our num_queues overrides the device backend */
+    virtio_stw_p(vdev, &s->blkcfg.num_queues, s->num_queues);
+
     memcpy(config, &s->blkcfg, sizeof(struct virtio_blk_config));
 }
 
@@ -490,10 +493,6 @@ reconnect:
         goto reconnect;
     }
 
-    if (s->blkcfg.num_queues != s->num_queues) {
-        s->blkcfg.num_queues = s->num_queues;
-    }
-
     return;
 
 virtio_err:
-- 
2.28.0

[PATCH v2 02/12] libqtest: add qtest_socket_server()

[Stefan Hajnoczi]

Add an API that returns a new UNIX domain socket in the listen state.
The code for this was already there but only used internally in
init_socket().

This new API will be used by vhost-user-blk-test.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 tests/qtest/libqos/libqtest.h |  8 +++++++
 tests/qtest/libqtest.c        | 40 ++++++++++++++++++++---------------
 2 files changed, 31 insertions(+), 17 deletions(-)

diff --git a/tests/qtest/libqos/libqtest.h b/tests/qtest/libqos/libqtest.h
index 724f65aa94..e5f1ec590c 100644
--- a/tests/qtest/libqos/libqtest.h
+++ b/tests/qtest/libqos/libqtest.h
@@ -132,6 +132,14 @@ void qtest_qmp_send(QTestState *s, const char *fmt, ...)
 void qtest_qmp_send_raw(QTestState *s, const char *fmt, ...)
     GCC_FMT_ATTR(2, 3);
 
+/**
+ * qtest_socket_server:
+ * @socket_path: the UNIX domain socket path
+ *
+ * Create and return a listen socket file descriptor, or abort on failure.
+ */
+int qtest_socket_server(const char *socket_path);
+
 /**
  * qtest_vqmp_fds:
  * @s: #QTestState instance to operate on.
diff --git a/tests/qtest/libqtest.c b/tests/qtest/libqtest.c
index e49f3a1e45..bc389d422b 100644
--- a/tests/qtest/libqtest.c
+++ b/tests/qtest/libqtest.c
@@ -81,24 +81,8 @@ static void qtest_client_set_rx_handler(QTestState *s, QTestRecvFn recv);
 
 static int init_socket(const char *socket_path)
 {
-    struct sockaddr_un addr;
-    int sock;
-    int ret;
-
-    sock = socket(PF_UNIX, SOCK_STREAM, 0);
-    g_assert_cmpint(sock, !=, -1);
-
-    addr.sun_family = AF_UNIX;
-    snprintf(addr.sun_path, sizeof(addr.sun_path), "%s", socket_path);
+    int sock = qtest_socket_server(socket_path);
     qemu_set_cloexec(sock);
-
-    do {
-        ret = bind(sock, (struct sockaddr *)&addr, sizeof(addr));
-    } while (ret == -1 && errno == EINTR);
-    g_assert_cmpint(ret, !=, -1);
-    ret = listen(sock, 1);
-    g_assert_cmpint(ret, !=, -1);
-
     return sock;
 }
 
@@ -636,6 +620,28 @@ QDict *qtest_qmp_receive_dict(QTestState *s)
     return qmp_fd_receive(s->qmp_fd);
 }
 
+int qtest_socket_server(const char *socket_path)
+{
+    struct sockaddr_un addr;
+    int sock;
+    int ret;
+
+    sock = socket(PF_UNIX, SOCK_STREAM, 0);
+    g_assert_cmpint(sock, !=, -1);
+
+    addr.sun_family = AF_UNIX;
+    snprintf(addr.sun_path, sizeof(addr.sun_path), "%s", socket_path);
+
+    do {
+        ret = bind(sock, (struct sockaddr *)&addr, sizeof(addr));
+    } while (ret == -1 && errno == EINTR);
+    g_assert_cmpint(ret, !=, -1);
+    ret = listen(sock, 1);
+    g_assert_cmpint(ret, !=, -1);
+
+    return sock;
+}
+
 /**
  * Allow users to send a message without waiting for the reply,
  * in the case that they choose to discard all replies up until
-- 
2.28.0

[PATCH v2 03/12] libqtest: add qtest_kill_qemu()

[Stefan Hajnoczi]

Tests that manage multiple processes may wish to kill QEMU before
destroying the QTestState. Expose a function to do that.

The vhost-user-blk-test testcase will need this.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 tests/qtest/libqos/libqtest.h | 11 +++++++++++
 tests/qtest/libqtest.c        |  7 ++++---
 2 files changed, 15 insertions(+), 3 deletions(-)

diff --git a/tests/qtest/libqos/libqtest.h b/tests/qtest/libqos/libqtest.h
index e5f1ec590c..51287b9276 100644
--- a/tests/qtest/libqos/libqtest.h
+++ b/tests/qtest/libqos/libqtest.h
@@ -74,6 +74,17 @@ QTestState *qtest_init_without_qmp_handshake(const char *extra_args);
  */
 QTestState *qtest_init_with_serial(const char *extra_args, int *sock_fd);
 
+/**
+ * qtest_kill_qemu:
+ * @s: #QTestState instance to operate on.
+ *
+ * Kill the QEMU process and wait for it to terminate. It is safe to call this
+ * function multiple times. Normally qtest_quit() is used instead because it
+ * also frees QTestState. Use qtest_kill_qemu() when you just want to kill QEMU
+ * and qtest_quit() will be called later.
+ */
+void qtest_kill_qemu(QTestState *s);
+
 /**
  * qtest_quit:
  * @s: #QTestState instance to operate on.
diff --git a/tests/qtest/libqtest.c b/tests/qtest/libqtest.c
index bc389d422b..cc2cec4a35 100644
--- a/tests/qtest/libqtest.c
+++ b/tests/qtest/libqtest.c
@@ -133,7 +133,7 @@ void qtest_set_expected_status(QTestState *s, int status)
     s->expected_status = status;
 }
 
-static void kill_qemu(QTestState *s)
+void qtest_kill_qemu(QTestState *s)
 {
     pid_t pid = s->qemu_pid;
     int wstatus;
@@ -143,6 +143,7 @@ static void kill_qemu(QTestState *s)
         kill(pid, SIGTERM);
         TFR(pid = waitpid(s->qemu_pid, &s->wstatus, 0));
         assert(pid == s->qemu_pid);
+        s->qemu_pid = -1;
     }
 
     /*
@@ -169,7 +170,7 @@ static void kill_qemu(QTestState *s)
 
 static void kill_qemu_hook_func(void *s)
 {
-    kill_qemu(s);
+    qtest_kill_qemu(s);
 }
 
 static void sigabrt_handler(int signo)
@@ -373,7 +374,7 @@ void qtest_quit(QTestState *s)
     /* Uninstall SIGABRT handler on last instance */
     cleanup_sigabrt_handler();
 
-    kill_qemu(s);
+    qtest_kill_qemu(s);
     close(s->fd);
     close(s->qmp_fd);
     g_string_free(s->rx, true);
-- 
2.28.0

[PATCH v2 04/12] libqtest: add qtest_remove_abrt_handler()

[Stefan Hajnoczi]

Add a function to remove previously-added abrt handler functions.

Now that a symmetric pair of add/remove functions exists we can also
balance the SIGABRT handler installation. The signal handler was
installed each time qtest_add_abrt_handler() was called. Now it is
installed when the abrt handler list becomes non-empty and removed again
when the list becomes empty.

The qtest_remove_abrt_handler() function will be used by
vhost-user-blk-test.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 tests/qtest/libqos/libqtest.h | 18 ++++++++++++++++++
 tests/qtest/libqtest.c        | 35 +++++++++++++++++++++++++++++------
 2 files changed, 47 insertions(+), 6 deletions(-)

diff --git a/tests/qtest/libqos/libqtest.h b/tests/qtest/libqos/libqtest.h
index 51287b9276..a68dcd79d4 100644
--- a/tests/qtest/libqos/libqtest.h
+++ b/tests/qtest/libqos/libqtest.h
@@ -649,8 +649,26 @@ void qtest_add_data_func_full(const char *str, void *data,
         g_free(path); \
     } while (0)
 
+/**
+ * qtest_add_abrt_handler:
+ * @fn: Handler function
+ * @data: Argument that is passed to the handler
+ *
+ * Add a handler function that is invoked on SIGABRT. This can be used to
+ * terminate processes and perform other cleanup. The handler can be removed
+ * with qtest_remove_abrt_handler().
+ */
 void qtest_add_abrt_handler(GHookFunc fn, const void *data);
 
+/**
+ * qtest_remove_abrt_handler:
+ * @data: Argument previously passed to qtest_add_abrt_handler()
+ *
+ * Remove an abrt handler that was previously added with
+ * qtest_add_abrt_handler().
+ */
+void qtest_remove_abrt_handler(void *data);
+
 /**
  * qtest_qmp_assert_success:
  * @qts: QTestState instance to operate on
diff --git a/tests/qtest/libqtest.c b/tests/qtest/libqtest.c
index cc2cec4a35..7cf247baf0 100644
--- a/tests/qtest/libqtest.c
+++ b/tests/qtest/libqtest.c
@@ -196,15 +196,30 @@ static void cleanup_sigabrt_handler(void)
     sigaction(SIGABRT, &sigact_old, NULL);
 }
 
+static bool hook_list_is_empty(GHookList *hook_list)
+{
+    GHook *hook = g_hook_first_valid(hook_list, TRUE);
+
+    if (!hook) {
+        return false;
+    }
+
+    g_hook_unref(hook_list, hook);
+    return true;
+}
+
 void qtest_add_abrt_handler(GHookFunc fn, const void *data)
 {
     GHook *hook;
 
-    /* Only install SIGABRT handler once */
     if (!abrt_hooks.is_setup) {
         g_hook_list_init(&abrt_hooks, sizeof(GHook));
     }
-    setup_sigabrt_handler();
+
+    /* Only install SIGABRT handler once */
+    if (hook_list_is_empty(&abrt_hooks)) {
+        setup_sigabrt_handler();
+    }
 
     hook = g_hook_alloc(&abrt_hooks);
     hook->func = fn;
@@ -213,6 +228,17 @@ void qtest_add_abrt_handler(GHookFunc fn, const void *data)
     g_hook_prepend(&abrt_hooks, hook);
 }
 
+void qtest_remove_abrt_handler(void *data)
+{
+    GHook *hook = g_hook_find_data(&abrt_hooks, TRUE, data);
+    g_hook_destroy_link(&abrt_hooks, hook);
+
+    /* Uninstall SIGABRT handler on last instance */
+    if (hook_list_is_empty(&abrt_hooks)) {
+        cleanup_sigabrt_handler();
+    }
+}
+
 static const char *qtest_qemu_binary(void)
 {
     const char *qemu_bin;
@@ -369,10 +395,7 @@ QTestState *qtest_init_with_serial(const char *extra_args, int *sock_fd)
 
 void qtest_quit(QTestState *s)
 {
-    g_hook_destroy_link(&abrt_hooks, g_hook_find_data(&abrt_hooks, TRUE, s));
-
-    /* Uninstall SIGABRT handler on last instance */
-    cleanup_sigabrt_handler();
+    qtest_remove_abrt_handler(s);
 
     qtest_kill_qemu(s);
     close(s->fd);
-- 
2.28.0

[PATCH v2 05/12] test: new qTest case to test the vhost-user-blk-server

[Stefan Hajnoczi]

From: Coiby Xu <coiby.xu@gmail.com>

This test case has the same tests as tests/virtio-blk-test.c except for
tests have block_resize. Since the vhost-user-blk export only serves one
client one time, two exports are started by qemu-storage-daemon for the
hotplug test.

Suggested-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Coiby Xu <coiby.xu@gmail.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 MAINTAINERS                         |   2 +
 tests/qtest/libqos/vhost-user-blk.h |  48 ++
 tests/qtest/libqos/vhost-user-blk.c | 130 +++++
 tests/qtest/vhost-user-blk-test.c   | 788 ++++++++++++++++++++++++++++
 tests/qtest/libqos/meson.build      |   1 +
 tests/qtest/meson.build             |   4 +
 6 files changed, 973 insertions(+)
 create mode 100644 tests/qtest/libqos/vhost-user-blk.h
 create mode 100644 tests/qtest/libqos/vhost-user-blk.c
 create mode 100644 tests/qtest/vhost-user-blk-test.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 68bc160f41..d351280d1f 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -3103,6 +3103,8 @@ F: block/export/vhost-user-blk-server.c
 F: block/export/vhost-user-blk-server.h
 F: include/qemu/vhost-user-server.h
 F: tests/qtest/libqos/vhost-user-blk.c
+F: tests/qtest/libqos/vhost-user-blk.h
+F: tests/qtest/vhost-user-blk-test.c
 F: util/vhost-user-server.c
 
 Replication
diff --git a/tests/qtest/libqos/vhost-user-blk.h b/tests/qtest/libqos/vhost-user-blk.h
new file mode 100644
index 0000000000..2a03456a45
--- /dev/null
+++ b/tests/qtest/libqos/vhost-user-blk.h
@@ -0,0 +1,48 @@
+/*
+ * libqos driver framework
+ *
+ * Based on tests/qtest/libqos/virtio-blk.c
+ *
+ * Copyright (c) 2020 Coiby Xu <coiby.xu@gmail.com>
+ *
+ * Copyright (c) 2018 Emanuele Giuseppe Esposito <e.emanuelegiuseppe@gmail.com>
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License version 2 as published by the Free Software Foundation.
+ *
+ * This library 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
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see <http://www.gnu.org/licenses/>
+ */
+
+#ifndef TESTS_LIBQOS_VHOST_USER_BLK_H
+#define TESTS_LIBQOS_VHOST_USER_BLK_H
+
+#include "qgraph.h"
+#include "virtio.h"
+#include "virtio-pci.h"
+
+typedef struct QVhostUserBlk QVhostUserBlk;
+typedef struct QVhostUserBlkPCI QVhostUserBlkPCI;
+typedef struct QVhostUserBlkDevice QVhostUserBlkDevice;
+
+struct QVhostUserBlk {
+    QVirtioDevice *vdev;
+};
+
+struct QVhostUserBlkPCI {
+    QVirtioPCIDevice pci_vdev;
+    QVhostUserBlk blk;
+};
+
+struct QVhostUserBlkDevice {
+    QOSGraphObject obj;
+    QVhostUserBlk blk;
+};
+
+#endif
diff --git a/tests/qtest/libqos/vhost-user-blk.c b/tests/qtest/libqos/vhost-user-blk.c
new file mode 100644
index 0000000000..568c3426ed
--- /dev/null
+++ b/tests/qtest/libqos/vhost-user-blk.c
@@ -0,0 +1,130 @@
+/*
+ * libqos driver framework
+ *
+ * Based on tests/qtest/libqos/virtio-blk.c
+ *
+ * Copyright (c) 2020 Coiby Xu <coiby.xu@gmail.com>
+ *
+ * Copyright (c) 2018 Emanuele Giuseppe Esposito <e.emanuelegiuseppe@gmail.com>
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License version 2.1 as published by the Free Software Foundation.
+ *
+ * This library 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
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see <http://www.gnu.org/licenses/>
+ */
+
+#include "qemu/osdep.h"
+#include "libqtest.h"
+#include "qemu/module.h"
+#include "standard-headers/linux/virtio_blk.h"
+#include "vhost-user-blk.h"
+
+#define PCI_SLOT                0x04
+#define PCI_FN                  0x00
+
+/* virtio-blk-device */
+static void *qvhost_user_blk_get_driver(QVhostUserBlk *v_blk,
+                                    const char *interface)
+{
+    if (!g_strcmp0(interface, "vhost-user-blk")) {
+        return v_blk;
+    }
+    if (!g_strcmp0(interface, "virtio")) {
+        return v_blk->vdev;
+    }
+
+    fprintf(stderr, "%s not present in vhost-user-blk-device\n", interface);
+    g_assert_not_reached();
+}
+
+static void *qvhost_user_blk_device_get_driver(void *object,
+                                           const char *interface)
+{
+    QVhostUserBlkDevice *v_blk = object;
+    return qvhost_user_blk_get_driver(&v_blk->blk, interface);
+}
+
+static void *vhost_user_blk_device_create(void *virtio_dev,
+                                      QGuestAllocator *t_alloc,
+                                      void *addr)
+{
+    QVhostUserBlkDevice *vhost_user_blk = g_new0(QVhostUserBlkDevice, 1);
+    QVhostUserBlk *interface = &vhost_user_blk->blk;
+
+    interface->vdev = virtio_dev;
+
+    vhost_user_blk->obj.get_driver = qvhost_user_blk_device_get_driver;
+
+    return &vhost_user_blk->obj;
+}
+
+/* virtio-blk-pci */
+static void *qvhost_user_blk_pci_get_driver(void *object, const char *interface)
+{
+    QVhostUserBlkPCI *v_blk = object;
+    if (!g_strcmp0(interface, "pci-device")) {
+        return v_blk->pci_vdev.pdev;
+    }
+    return qvhost_user_blk_get_driver(&v_blk->blk, interface);
+}
+
+static void *vhost_user_blk_pci_create(void *pci_bus, QGuestAllocator *t_alloc,
+                                      void *addr)
+{
+    QVhostUserBlkPCI *vhost_user_blk = g_new0(QVhostUserBlkPCI, 1);
+    QVhostUserBlk *interface = &vhost_user_blk->blk;
+    QOSGraphObject *obj = &vhost_user_blk->pci_vdev.obj;
+
+    virtio_pci_init(&vhost_user_blk->pci_vdev, pci_bus, addr);
+    interface->vdev = &vhost_user_blk->pci_vdev.vdev;
+
+    g_assert_cmphex(interface->vdev->device_type, ==, VIRTIO_ID_BLOCK);
+
+    obj->get_driver = qvhost_user_blk_pci_get_driver;
+
+    return obj;
+}
+
+static void vhost_user_blk_register_nodes(void)
+{
+    /*
+     * FIXME: every test using these two nodes needs to setup a
+     * -drive,id=drive0 otherwise QEMU is not going to start.
+     * Therefore, we do not include "produces" edge for virtio
+     * and pci-device yet.
+     */
+
+    char *arg = g_strdup_printf("id=drv0,chardev=char1,addr=%x.%x",
+                                PCI_SLOT, PCI_FN);
+
+    QPCIAddress addr = {
+        .devfn = QPCI_DEVFN(PCI_SLOT, PCI_FN),
+    };
+
+    QOSGraphEdgeOptions opts = { };
+
+    /* virtio-blk-device */
+    /** opts.extra_device_opts = "drive=drive0"; */
+    qos_node_create_driver("vhost-user-blk-device",
+                           vhost_user_blk_device_create);
+    qos_node_consumes("vhost-user-blk-device", "virtio-bus", &opts);
+    qos_node_produces("vhost-user-blk-device", "vhost-user-blk");
+
+    /* virtio-blk-pci */
+    opts.extra_device_opts = arg;
+    add_qpci_address(&opts, &addr);
+    qos_node_create_driver("vhost-user-blk-pci", vhost_user_blk_pci_create);
+    qos_node_consumes("vhost-user-blk-pci", "pci-bus", &opts);
+    qos_node_produces("vhost-user-blk-pci", "vhost-user-blk");
+
+    g_free(arg);
+}
+
+libqos_init(vhost_user_blk_register_nodes);
diff --git a/tests/qtest/vhost-user-blk-test.c b/tests/qtest/vhost-user-blk-test.c
new file mode 100644
index 0000000000..175dccab98
--- /dev/null
+++ b/tests/qtest/vhost-user-blk-test.c
@@ -0,0 +1,788 @@
+/*
+ * QTest testcase for Vhost-user Block Device
+ *
+ * Based on tests/qtest//virtio-blk-test.c
+
+ * Copyright (c) 2014 SUSE LINUX Products GmbH
+ * Copyright (c) 2014 Marc Marí
+ * Copyright (c) 2020 Coiby Xu
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#include "qemu/osdep.h"
+#include "libqtest-single.h"
+#include "qemu/bswap.h"
+#include "qemu/module.h"
+#include "standard-headers/linux/virtio_blk.h"
+#include "standard-headers/linux/virtio_pci.h"
+#include "libqos/qgraph.h"
+#include "libqos/vhost-user-blk.h"
+#include "libqos/libqos-pc.h"
+
+#define TEST_IMAGE_SIZE         (64 * 1024 * 1024)
+#define QVIRTIO_BLK_TIMEOUT_US  (30 * 1000 * 1000)
+#define PCI_SLOT_HP             0x06
+
+typedef struct {
+    pid_t pid;
+} QemuStorageDaemonState;
+
+typedef struct QVirtioBlkReq {
+    uint32_t type;
+    uint32_t ioprio;
+    uint64_t sector;
+    char *data;
+    uint8_t status;
+} QVirtioBlkReq;
+
+#ifdef HOST_WORDS_BIGENDIAN
+static const bool host_is_big_endian = true;
+#else
+static const bool host_is_big_endian; /* false */
+#endif
+
+static inline void virtio_blk_fix_request(QVirtioDevice *d, QVirtioBlkReq *req)
+{
+    if (qvirtio_is_big_endian(d) != host_is_big_endian) {
+        req->type = bswap32(req->type);
+        req->ioprio = bswap32(req->ioprio);
+        req->sector = bswap64(req->sector);
+    }
+}
+
+static inline void virtio_blk_fix_dwz_hdr(QVirtioDevice *d,
+    struct virtio_blk_discard_write_zeroes *dwz_hdr)
+{
+    if (qvirtio_is_big_endian(d) != host_is_big_endian) {
+        dwz_hdr->sector = bswap64(dwz_hdr->sector);
+        dwz_hdr->num_sectors = bswap32(dwz_hdr->num_sectors);
+        dwz_hdr->flags = bswap32(dwz_hdr->flags);
+    }
+}
+
+static uint64_t virtio_blk_request(QGuestAllocator *alloc, QVirtioDevice *d,
+                                   QVirtioBlkReq *req, uint64_t data_size)
+{
+    uint64_t addr;
+    uint8_t status = 0xFF;
+    QTestState *qts = global_qtest;
+
+    switch (req->type) {
+    case VIRTIO_BLK_T_IN:
+    case VIRTIO_BLK_T_OUT:
+        g_assert_cmpuint(data_size % 512, ==, 0);
+        break;
+    case VIRTIO_BLK_T_DISCARD:
+    case VIRTIO_BLK_T_WRITE_ZEROES:
+        g_assert_cmpuint(data_size %
+                         sizeof(struct virtio_blk_discard_write_zeroes), ==, 0);
+        break;
+    default:
+        g_assert_cmpuint(data_size, ==, 0);
+    }
+
+    addr = guest_alloc(alloc, sizeof(*req) + data_size);
+
+    virtio_blk_fix_request(d, req);
+
+    qtest_memwrite(qts, addr, req, 16);
+    qtest_memwrite(qts, addr + 16, req->data, data_size);
+    qtest_memwrite(qts, addr + 16 + data_size, &status, sizeof(status));
+
+    return addr;
+}
+
+/* Returns the request virtqueue so the caller can perform further tests */
+static QVirtQueue *test_basic(QVirtioDevice *dev, QGuestAllocator *alloc)
+{
+    QVirtioBlkReq req;
+    uint64_t req_addr;
+    uint64_t capacity;
+    uint64_t features;
+    uint32_t free_head;
+    uint8_t status;
+    char *data;
+    QTestState *qts = global_qtest;
+    QVirtQueue *vq;
+
+    features = qvirtio_get_features(dev);
+    features = features & ~(QVIRTIO_F_BAD_FEATURE |
+                    (1u << VIRTIO_RING_F_INDIRECT_DESC) |
+                    (1u << VIRTIO_RING_F_EVENT_IDX) |
+                    (1u << VIRTIO_BLK_F_SCSI));
+    qvirtio_set_features(dev, features);
+
+    capacity = qvirtio_config_readq(dev, 0);
+    g_assert_cmpint(capacity, ==, TEST_IMAGE_SIZE / 512);
+
+    vq = qvirtqueue_setup(dev, alloc, 0);
+
+    qvirtio_set_driver_ok(dev);
+
+    /* Write and read with 3 descriptor layout */
+    /* Write request */
+    req.type = VIRTIO_BLK_T_OUT;
+    req.ioprio = 1;
+    req.sector = 0;
+    req.data = g_malloc0(512);
+    strcpy(req.data, "TEST");
+
+    req_addr = virtio_blk_request(alloc, dev, &req, 512);
+
+    g_free(req.data);
+
+    free_head = qvirtqueue_add(qts, vq, req_addr, 16, false, true);
+    qvirtqueue_add(qts, vq, req_addr + 16, 512, false, true);
+    qvirtqueue_add(qts, vq, req_addr + 528, 1, true, false);
+
+    qvirtqueue_kick(qts, dev, vq, free_head);
+
+    qvirtio_wait_used_elem(qts, dev, vq, free_head, NULL,
+                           QVIRTIO_BLK_TIMEOUT_US);
+    status = readb(req_addr + 528);
+    g_assert_cmpint(status, ==, 0);
+
+    guest_free(alloc, req_addr);
+
+    /* Read request */
+    req.type = VIRTIO_BLK_T_IN;
+    req.ioprio = 1;
+    req.sector = 0;
+    req.data = g_malloc0(512);
+
+    req_addr = virtio_blk_request(alloc, dev, &req, 512);
+
+    g_free(req.data);
+
+    free_head = qvirtqueue_add(qts, vq, req_addr, 16, false, true);
+    qvirtqueue_add(qts, vq, req_addr + 16, 512, true, true);
+    qvirtqueue_add(qts, vq, req_addr + 528, 1, true, false);
+
+    qvirtqueue_kick(qts, dev, vq, free_head);
+
+    qvirtio_wait_used_elem(qts, dev, vq, free_head, NULL,
+                           QVIRTIO_BLK_TIMEOUT_US);
+    status = readb(req_addr + 528);
+    g_assert_cmpint(status, ==, 0);
+
+    data = g_malloc0(512);
+    qtest_memread(qts, req_addr + 16, data, 512);
+    g_assert_cmpstr(data, ==, "TEST");
+    g_free(data);
+
+    guest_free(alloc, req_addr);
+
+    if (features & (1u << VIRTIO_BLK_F_WRITE_ZEROES)) {
+        struct virtio_blk_discard_write_zeroes dwz_hdr;
+        void *expected;
+
+        /*
+         * WRITE_ZEROES request on the same sector of previous test where
+         * we wrote "TEST".
+         */
+        req.type = VIRTIO_BLK_T_WRITE_ZEROES;
+        req.data = (char *) &dwz_hdr;
+        dwz_hdr.sector = 0;
+        dwz_hdr.num_sectors = 1;
+        dwz_hdr.flags = 0;
+
+        virtio_blk_fix_dwz_hdr(dev, &dwz_hdr);
+
+        req_addr = virtio_blk_request(alloc, dev, &req, sizeof(dwz_hdr));
+
+        free_head = qvirtqueue_add(qts, vq, req_addr, 16, false, true);
+        qvirtqueue_add(qts, vq, req_addr + 16, sizeof(dwz_hdr), false, true);
+        qvirtqueue_add(qts, vq, req_addr + 16 + sizeof(dwz_hdr), 1, true,
+                       false);
+
+        qvirtqueue_kick(qts, dev, vq, free_head);
+
+        qvirtio_wait_used_elem(qts, dev, vq, free_head, NULL,
+                               QVIRTIO_BLK_TIMEOUT_US);
+        status = readb(req_addr + 16 + sizeof(dwz_hdr));
+        g_assert_cmpint(status, ==, 0);
+
+        guest_free(alloc, req_addr);
+
+        /* Read request to check if the sector contains all zeroes */
+        req.type = VIRTIO_BLK_T_IN;
+        req.ioprio = 1;
+        req.sector = 0;
+        req.data = g_malloc0(512);
+
+        req_addr = virtio_blk_request(alloc, dev, &req, 512);
+
+        g_free(req.data);
+
+        free_head = qvirtqueue_add(qts, vq, req_addr, 16, false, true);
+        qvirtqueue_add(qts, vq, req_addr + 16, 512, true, true);
+        qvirtqueue_add(qts, vq, req_addr + 528, 1, true, false);
+
+        qvirtqueue_kick(qts, dev, vq, free_head);
+
+        qvirtio_wait_used_elem(qts, dev, vq, free_head, NULL,
+                               QVIRTIO_BLK_TIMEOUT_US);
+        status = readb(req_addr + 528);
+        g_assert_cmpint(status, ==, 0);
+
+        data = g_malloc(512);
+        expected = g_malloc0(512);
+        qtest_memread(qts, req_addr + 16, data, 512);
+        g_assert_cmpmem(data, 512, expected, 512);
+        g_free(expected);
+        g_free(data);
+
+        guest_free(alloc, req_addr);
+    }
+
+    if (features & (1u << VIRTIO_BLK_F_DISCARD)) {
+        struct virtio_blk_discard_write_zeroes dwz_hdr;
+
+        req.type = VIRTIO_BLK_T_DISCARD;
+        req.data = (char *) &dwz_hdr;
+        dwz_hdr.sector = 0;
+        dwz_hdr.num_sectors = 1;
+        dwz_hdr.flags = 0;
+
+        virtio_blk_fix_dwz_hdr(dev, &dwz_hdr);
+
+        req_addr = virtio_blk_request(alloc, dev, &req, sizeof(dwz_hdr));
+
+        free_head = qvirtqueue_add(qts, vq, req_addr, 16, false, true);
+        qvirtqueue_add(qts, vq, req_addr + 16, sizeof(dwz_hdr), false, true);
+        qvirtqueue_add(qts, vq, req_addr + 16 + sizeof(dwz_hdr),
+                       1, true, false);
+
+        qvirtqueue_kick(qts, dev, vq, free_head);
+
+        qvirtio_wait_used_elem(qts, dev, vq, free_head, NULL,
+                               QVIRTIO_BLK_TIMEOUT_US);
+        status = readb(req_addr + 16 + sizeof(dwz_hdr));
+        g_assert_cmpint(status, ==, 0);
+
+        guest_free(alloc, req_addr);
+    }
+
+    if (features & (1u << VIRTIO_F_ANY_LAYOUT)) {
+        /* Write and read with 2 descriptor layout */
+        /* Write request */
+        req.type = VIRTIO_BLK_T_OUT;
+        req.ioprio = 1;
+        req.sector = 1;
+        req.data = g_malloc0(512);
+        strcpy(req.data, "TEST");
+
+        req_addr = virtio_blk_request(alloc, dev, &req, 512);
+
+        g_free(req.data);
+
+        free_head = qvirtqueue_add(qts, vq, req_addr, 528, false, true);
+        qvirtqueue_add(qts, vq, req_addr + 528, 1, true, false);
+        qvirtqueue_kick(qts, dev, vq, free_head);
+
+        qvirtio_wait_used_elem(qts, dev, vq, free_head, NULL,
+                               QVIRTIO_BLK_TIMEOUT_US);
+        status = readb(req_addr + 528);
+        g_assert_cmpint(status, ==, 0);
+
+        guest_free(alloc, req_addr);
+
+        /* Read request */
+        req.type = VIRTIO_BLK_T_IN;
+        req.ioprio = 1;
+        req.sector = 1;
+        req.data = g_malloc0(512);
+
+        req_addr = virtio_blk_request(alloc, dev, &req, 512);
+
+        g_free(req.data);
+
+        free_head = qvirtqueue_add(qts, vq, req_addr, 16, false, true);
+        qvirtqueue_add(qts, vq, req_addr + 16, 513, true, false);
+
+        qvirtqueue_kick(qts, dev, vq, free_head);
+
+        qvirtio_wait_used_elem(qts, dev, vq, free_head, NULL,
+                               QVIRTIO_BLK_TIMEOUT_US);
+        status = readb(req_addr + 528);
+        g_assert_cmpint(status, ==, 0);
+
+        data = g_malloc0(512);
+        qtest_memread(qts, req_addr + 16, data, 512);
+        g_assert_cmpstr(data, ==, "TEST");
+        g_free(data);
+
+        guest_free(alloc, req_addr);
+    }
+
+    return vq;
+}
+
+static void basic(void *obj, void *data, QGuestAllocator *t_alloc)
+{
+    QVhostUserBlk *blk_if = obj;
+    QVirtQueue *vq;
+
+    vq = test_basic(blk_if->vdev, t_alloc);
+    qvirtqueue_cleanup(blk_if->vdev->bus, vq, t_alloc);
+
+}
+
+static void indirect(void *obj, void *u_data, QGuestAllocator *t_alloc)
+{
+    QVirtQueue *vq;
+    QVhostUserBlk *blk_if = obj;
+    QVirtioDevice *dev = blk_if->vdev;
+    QVirtioBlkReq req;
+    QVRingIndirectDesc *indirect;
+    uint64_t req_addr;
+    uint64_t capacity;
+    uint64_t features;
+    uint32_t free_head;
+    uint8_t status;
+    char *data;
+    QTestState *qts = global_qtest;
+
+    features = qvirtio_get_features(dev);
+    g_assert_cmphex(features & (1u << VIRTIO_RING_F_INDIRECT_DESC), !=, 0);
+    features = features & ~(QVIRTIO_F_BAD_FEATURE |
+                            (1u << VIRTIO_RING_F_EVENT_IDX) |
+                            (1u << VIRTIO_BLK_F_SCSI));
+    qvirtio_set_features(dev, features);
+
+    capacity = qvirtio_config_readq(dev, 0);
+    g_assert_cmpint(capacity, ==, TEST_IMAGE_SIZE / 512);
+
+    vq = qvirtqueue_setup(dev, t_alloc, 0);
+    qvirtio_set_driver_ok(dev);
+
+    /* Write request */
+    req.type = VIRTIO_BLK_T_OUT;
+    req.ioprio = 1;
+    req.sector = 0;
+    req.data = g_malloc0(512);
+    strcpy(req.data, "TEST");
+
+    req_addr = virtio_blk_request(t_alloc, dev, &req, 512);
+
+    g_free(req.data);
+
+    indirect = qvring_indirect_desc_setup(qts, dev, t_alloc, 2);
+    qvring_indirect_desc_add(dev, qts, indirect, req_addr, 528, false);
+    qvring_indirect_desc_add(dev, qts, indirect, req_addr + 528, 1, true);
+    free_head = qvirtqueue_add_indirect(qts, vq, indirect);
+    qvirtqueue_kick(qts, dev, vq, free_head);
+
+    qvirtio_wait_used_elem(qts, dev, vq, free_head, NULL,
+                           QVIRTIO_BLK_TIMEOUT_US);
+    status = readb(req_addr + 528);
+    g_assert_cmpint(status, ==, 0);
+
+    g_free(indirect);
+    guest_free(t_alloc, req_addr);
+
+    /* Read request */
+    req.type = VIRTIO_BLK_T_IN;
+    req.ioprio = 1;
+    req.sector = 0;
+    req.data = g_malloc0(512);
+    strcpy(req.data, "TEST");
+
+    req_addr = virtio_blk_request(t_alloc, dev, &req, 512);
+
+    g_free(req.data);
+
+    indirect = qvring_indirect_desc_setup(qts, dev, t_alloc, 2);
+    qvring_indirect_desc_add(dev, qts, indirect, req_addr, 16, false);
+    qvring_indirect_desc_add(dev, qts, indirect, req_addr + 16, 513, true);
+    free_head = qvirtqueue_add_indirect(qts, vq, indirect);
+    qvirtqueue_kick(qts, dev, vq, free_head);
+
+    qvirtio_wait_used_elem(qts, dev, vq, free_head, NULL,
+                           QVIRTIO_BLK_TIMEOUT_US);
+    status = readb(req_addr + 528);
+    g_assert_cmpint(status, ==, 0);
+
+    data = g_malloc0(512);
+    qtest_memread(qts, req_addr + 16, data, 512);
+    g_assert_cmpstr(data, ==, "TEST");
+    g_free(data);
+
+    g_free(indirect);
+    guest_free(t_alloc, req_addr);
+    qvirtqueue_cleanup(dev->bus, vq, t_alloc);
+}
+
+static void idx(void *obj, void *u_data, QGuestAllocator *t_alloc)
+{
+    QVirtQueue *vq;
+    QVhostUserBlkPCI *blk = obj;
+    QVirtioPCIDevice *pdev = &blk->pci_vdev;
+    QVirtioDevice *dev = &pdev->vdev;
+    QVirtioBlkReq req;
+    uint64_t req_addr;
+    uint64_t capacity;
+    uint64_t features;
+    uint32_t free_head;
+    uint32_t write_head;
+    uint32_t desc_idx;
+    uint8_t status;
+    char *data;
+    QOSGraphObject *blk_object = obj;
+    QPCIDevice *pci_dev = blk_object->get_driver(blk_object, "pci-device");
+    QTestState *qts = global_qtest;
+
+    if (qpci_check_buggy_msi(pci_dev)) {
+        return;
+    }
+
+    qpci_msix_enable(pdev->pdev);
+    qvirtio_pci_set_msix_configuration_vector(pdev, t_alloc, 0);
+
+    features = qvirtio_get_features(dev);
+    features = features & ~(QVIRTIO_F_BAD_FEATURE |
+                            (1u << VIRTIO_RING_F_INDIRECT_DESC) |
+                            (1u << VIRTIO_F_NOTIFY_ON_EMPTY) |
+                            (1u << VIRTIO_BLK_F_SCSI));
+    qvirtio_set_features(dev, features);
+
+    capacity = qvirtio_config_readq(dev, 0);
+    g_assert_cmpint(capacity, ==, TEST_IMAGE_SIZE / 512);
+
+    vq = qvirtqueue_setup(dev, t_alloc, 0);
+    qvirtqueue_pci_msix_setup(pdev, (QVirtQueuePCI *)vq, t_alloc, 1);
+
+    qvirtio_set_driver_ok(dev);
+
+    /* Write request */
+    req.type = VIRTIO_BLK_T_OUT;
+    req.ioprio = 1;
+    req.sector = 0;
+    req.data = g_malloc0(512);
+    strcpy(req.data, "TEST");
+
+    req_addr = virtio_blk_request(t_alloc, dev, &req, 512);
+
+    g_free(req.data);
+
+    free_head = qvirtqueue_add(qts, vq, req_addr, 16, false, true);
+    qvirtqueue_add(qts, vq, req_addr + 16, 512, false, true);
+    qvirtqueue_add(qts, vq, req_addr + 528, 1, true, false);
+    qvirtqueue_kick(qts, dev, vq, free_head);
+
+    qvirtio_wait_used_elem(qts, dev, vq, free_head, NULL,
+                           QVIRTIO_BLK_TIMEOUT_US);
+
+    /* Write request */
+    req.type = VIRTIO_BLK_T_OUT;
+    req.ioprio = 1;
+    req.sector = 1;
+    req.data = g_malloc0(512);
+    strcpy(req.data, "TEST");
+
+    req_addr = virtio_blk_request(t_alloc, dev, &req, 512);
+
+    g_free(req.data);
+
+    /* Notify after processing the third request */
+    qvirtqueue_set_used_event(qts, vq, 2);
+    free_head = qvirtqueue_add(qts, vq, req_addr, 16, false, true);
+    qvirtqueue_add(qts, vq, req_addr + 16, 512, false, true);
+    qvirtqueue_add(qts, vq, req_addr + 528, 1, true, false);
+    qvirtqueue_kick(qts, dev, vq, free_head);
+    write_head = free_head;
+
+    /* No notification expected */
+    status = qvirtio_wait_status_byte_no_isr(qts, dev,
+                                             vq, req_addr + 528,
+                                             QVIRTIO_BLK_TIMEOUT_US);
+    g_assert_cmpint(status, ==, 0);
+
+    guest_free(t_alloc, req_addr);
+
+    /* Read request */
+    req.type = VIRTIO_BLK_T_IN;
+    req.ioprio = 1;
+    req.sector = 1;
+    req.data = g_malloc0(512);
+
+    req_addr = virtio_blk_request(t_alloc, dev, &req, 512);
+
+    g_free(req.data);
+
+    free_head = qvirtqueue_add(qts, vq, req_addr, 16, false, true);
+    qvirtqueue_add(qts, vq, req_addr + 16, 512, true, true);
+    qvirtqueue_add(qts, vq, req_addr + 528, 1, true, false);
+
+    qvirtqueue_kick(qts, dev, vq, free_head);
+
+    /* We get just one notification for both requests */
+    qvirtio_wait_used_elem(qts, dev, vq, write_head, NULL,
+                           QVIRTIO_BLK_TIMEOUT_US);
+    g_assert(qvirtqueue_get_buf(qts, vq, &desc_idx, NULL));
+    g_assert_cmpint(desc_idx, ==, free_head);
+
+    status = readb(req_addr + 528);
+    g_assert_cmpint(status, ==, 0);
+
+    data = g_malloc0(512);
+    qtest_memread(qts, req_addr + 16, data, 512);
+    g_assert_cmpstr(data, ==, "TEST");
+    g_free(data);
+
+    guest_free(t_alloc, req_addr);
+
+    /* End test */
+    qpci_msix_disable(pdev->pdev);
+
+    qvirtqueue_cleanup(dev->bus, vq, t_alloc);
+}
+
+static void pci_hotplug(void *obj, void *data, QGuestAllocator *t_alloc)
+{
+    QVirtioPCIDevice *dev1 = obj;
+    QVirtioPCIDevice *dev;
+    QTestState *qts = dev1->pdev->bus->qts;
+
+    /* plug secondary disk */
+    qtest_qmp_device_add(qts, "vhost-user-blk-pci", "drv1",
+                         "{'addr': %s, 'chardev': 'char2'}",
+                         stringify(PCI_SLOT_HP) ".0");
+
+    dev = virtio_pci_new(dev1->pdev->bus,
+                         &(QPCIAddress) { .devfn = QPCI_DEVFN(PCI_SLOT_HP, 0)
+                                        });
+    g_assert_nonnull(dev);
+    g_assert_cmpint(dev->vdev.device_type, ==, VIRTIO_ID_BLOCK);
+    qvirtio_pci_device_disable(dev);
+    qos_object_destroy((QOSGraphObject *)dev);
+
+    /* unplug secondary disk */
+    qpci_unplug_acpi_device_test(qts, "drv1", PCI_SLOT_HP);
+}
+
+/*
+ * Check that setting the vring addr on a non-existent virtqueue does
+ * not crash.
+ */
+static void test_nonexistent_virtqueue(void *obj, void *data,
+                                       QGuestAllocator *t_alloc)
+{
+    QVhostUserBlkPCI *blk = obj;
+    QVirtioPCIDevice *pdev = &blk->pci_vdev;
+    QPCIBar bar0;
+    QPCIDevice *dev;
+
+    dev = qpci_device_find(pdev->pdev->bus, QPCI_DEVFN(4, 0));
+    g_assert(dev != NULL);
+    qpci_device_enable(dev);
+
+    bar0 = qpci_iomap(dev, 0, NULL);
+
+    qpci_io_writeb(dev, bar0, VIRTIO_PCI_QUEUE_SEL, 2);
+    qpci_io_writel(dev, bar0, VIRTIO_PCI_QUEUE_PFN, 1);
+
+    g_free(dev);
+}
+
+static const char *qtest_qemu_storage_daemon_binary(void)
+{
+    const char *qemu_storage_daemon_bin;
+
+    qemu_storage_daemon_bin = getenv("QTEST_QEMU_STORAGE_DAEMON_BINARY");
+    if (!qemu_storage_daemon_bin) {
+        fprintf(stderr, "Environment variable "
+                        "QTEST_QEMU_STORAGE_DAEMON_BINARY required\n");
+        exit(0);
+    }
+
+    return qemu_storage_daemon_bin;
+}
+
+/* g_test_queue_destroy() cleanup function for files */
+static void destroy_file(void *path)
+{
+    unlink(path);
+    g_free(path);
+    qos_invalidate_command_line();
+}
+
+static char *drive_create(void)
+{
+    int fd, ret;
+    /** vhost-user-blk won't recognize drive located in /tmp */
+    char *t_path = g_strdup("qtest.XXXXXX");
+
+    /** Create a temporary raw image */
+    fd = mkstemp(t_path);
+    g_assert_cmpint(fd, >=, 0);
+    ret = ftruncate(fd, TEST_IMAGE_SIZE);
+    g_assert_cmpint(ret, ==, 0);
+    close(fd);
+
+    g_test_queue_destroy(destroy_file, t_path);
+    return t_path;
+}
+
+static char *create_listen_socket(int *fd)
+{
+    int tmp_fd;
+    char *path;
+
+    /* No race because our pid makes the path unique */
+    path = g_strdup_printf("/tmp/qtest-%d-sock.XXXXXX", getpid());
+    tmp_fd = mkstemp(path);
+    g_assert_cmpint(tmp_fd, >=, 0);
+    close(tmp_fd);
+    unlink(path);
+
+    *fd = qtest_socket_server(path);
+    g_test_queue_destroy(destroy_file, path);
+    return path;
+}
+
+/*
+ * g_test_queue_destroy() and qtest_add_abrt_handler() cleanup function for
+ * qemu-storage-daemon.
+ */
+static void quit_storage_daemon(void *data)
+{
+    QemuStorageDaemonState *qsd = data;
+    int wstatus;
+    pid_t pid;
+
+    /*
+     * If we were invoked as a g_test_queue_destroy() cleanup function we need
+     * to remove the abrt handler to avoid being called again if the code below
+     * aborts. Also, we must not leave the abrt handler installed after
+     * cleanup.
+     */
+    qtest_remove_abrt_handler(data);
+
+    /* Before quitting storage-daemon, quit qemu to avoid dubious messages */
+    qtest_kill_qemu(global_qtest);
+
+    kill(qsd->pid, SIGTERM);
+    pid = waitpid(qsd->pid, &wstatus, 0);
+    g_assert_cmpint(pid, ==, qsd->pid);
+    if (!WIFEXITED(wstatus)) {
+        fprintf(stderr, "%s: expected qemu-storage-daemon to exit\n",
+                __func__);
+        abort();
+    }
+    if (WEXITSTATUS(wstatus) != 0) {
+        fprintf(stderr, "%s: expected qemu-storage-daemon to exit "
+                "successfully, got %d\n",
+                __func__, WEXITSTATUS(wstatus));
+        abort();
+    }
+
+    g_free(data);
+}
+
+static void start_vhost_user_blk(GString *cmd_line, int vus_instances)
+{
+    const char *vhost_user_blk_bin = qtest_qemu_storage_daemon_binary();
+    int i;
+    gchar *img_path;
+    GString *storage_daemon_command = g_string_new(NULL);
+    QemuStorageDaemonState *qsd;
+
+    g_string_append_printf(storage_daemon_command,
+                           "exec %s ",
+                           vhost_user_blk_bin);
+
+    g_string_append_printf(cmd_line,
+            " -object memory-backend-memfd,id=mem,size=256M,share=on "
+            " -M memory-backend=mem ");
+
+    for (i = 0; i < vus_instances; i++) {
+        int fd;
+        char *sock_path = create_listen_socket(&fd);
+
+        /* create image file */
+        img_path = drive_create();
+        g_string_append_printf(storage_daemon_command,
+            "--blockdev driver=file,node-name=disk%d,filename=%s "
+            "--export type=vhost-user-blk,id=disk%d,addr.type=unix,addr.path=%s,"
+            "node-name=disk%i,writable=on ",
+            i, img_path, i, sock_path, i);
+
+        g_string_append_printf(cmd_line, "-chardev socket,id=char%d,path=%s ",
+                               i + 1, sock_path);
+    }
+
+    g_test_message("starting vhost-user backend: %s",
+                   storage_daemon_command->str);
+    pid_t pid = fork();
+    if (pid == 0) {
+        /*
+         * Close standard file descriptors so tap-driver.pl pipe detects when
+         * our parent terminates.
+         */
+        close(0);
+        close(1);
+        open("/dev/null", O_RDONLY);
+        open("/dev/null", O_WRONLY);
+
+        execlp("/bin/sh", "sh", "-c", storage_daemon_command->str, NULL);
+        exit(1);
+    }
+    g_string_free(storage_daemon_command, true);
+
+    qsd = g_new(QemuStorageDaemonState, 1);
+    qsd->pid = pid;
+
+    /* Make sure qemu-storage-daemon is stopped */
+    qtest_add_abrt_handler(quit_storage_daemon, qsd);
+    g_test_queue_destroy(quit_storage_daemon, qsd);
+}
+
+static void *vhost_user_blk_test_setup(GString *cmd_line, void *arg)
+{
+    start_vhost_user_blk(cmd_line, 1);
+    return arg;
+}
+
+/*
+ * Setup for hotplug.
+ *
+ * Since vhost-user server only serves one vhost-user client one time,
+ * another exprot
+ *
+ */
+static void *vhost_user_blk_hotplug_test_setup(GString *cmd_line, void *arg)
+{
+    /* "-chardev socket,id=char2" is used for pci_hotplug*/
+    start_vhost_user_blk(cmd_line, 2);
+    return arg;
+}
+
+static void register_vhost_user_blk_test(void)
+{
+    QOSGraphTestOptions opts = {
+        .before = vhost_user_blk_test_setup,
+    };
+
+    /*
+     * tests for vhost-user-blk and vhost-user-blk-pci
+     * The tests are borrowed from tests/virtio-blk-test.c. But some tests
+     * regarding block_resize don't work for vhost-user-blk.
+     * vhost-user-blk device doesn't have -drive, so tests containing
+     * block_resize are also abandoned,
+     *  - config
+     *  - resize
+     */
+    qos_add_test("basic", "vhost-user-blk", basic, &opts);
+    qos_add_test("indirect", "vhost-user-blk", indirect, &opts);
+    qos_add_test("idx", "vhost-user-blk-pci", idx, &opts);
+    qos_add_test("nxvirtq", "vhost-user-blk-pci",
+                 test_nonexistent_virtqueue, &opts);
+
+    opts.before = vhost_user_blk_hotplug_test_setup;
+    qos_add_test("hotplug", "vhost-user-blk-pci", pci_hotplug, &opts);
+}
+
+libqos_init(register_vhost_user_blk_test);
diff --git a/tests/qtest/libqos/meson.build b/tests/qtest/libqos/meson.build
index 1cddf5bdaa..1f5c8f1053 100644
--- a/tests/qtest/libqos/meson.build
+++ b/tests/qtest/libqos/meson.build
@@ -32,6 +32,7 @@ libqos_srcs = files('../libqtest.c',
         'virtio-9p.c',
         'virtio-balloon.c',
         'virtio-blk.c',
+        'vhost-user-blk.c',
         'virtio-mmio.c',
         'virtio-net.c',
         'virtio-pci.c',
diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build
index c19f1c8503..39af1cddc4 100644
--- a/tests/qtest/meson.build
+++ b/tests/qtest/meson.build
@@ -199,6 +199,9 @@ qos_test_ss.add(
 )
 qos_test_ss.add(when: 'CONFIG_VIRTFS', if_true: files('virtio-9p-test.c'))
 qos_test_ss.add(when: 'CONFIG_VHOST_USER', if_true: files('vhost-user-test.c'))
+if have_vhost_user_blk_server
+  qos_test_ss.add(files('vhost-user-blk-test.c'))
+endif
 
 tpmemu_files = ['tpm-emu.c', 'tpm-util.c', 'tpm-tests.c']
 
@@ -237,6 +240,7 @@ foreach dir : target_dirs
   endif
   qtest_env.set('G_TEST_DBUS_DAEMON', meson.source_root() / 'tests/dbus-vmstate-daemon.sh')
   qtest_env.set('QTEST_QEMU_BINARY', './qemu-system-' + target_base)
+  qtest_env.set('QTEST_QEMU_STORAGE_DAEMON_BINARY', './storage-daemon/qemu-storage-daemon')
   
   foreach test : target_qtests
     # Executables are shared across targets, declare them only the first time we
-- 
2.28.0

[PATCH v2 06/12] tests/qtest: add multi-queue test case to vhost-user-blk-test

[Stefan Hajnoczi]

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 tests/qtest/vhost-user-blk-test.c | 81 +++++++++++++++++++++++++++++--
 1 file changed, 76 insertions(+), 5 deletions(-)

diff --git a/tests/qtest/vhost-user-blk-test.c b/tests/qtest/vhost-user-blk-test.c
index 175dccab98..1f1e8a9b79 100644
--- a/tests/qtest/vhost-user-blk-test.c
+++ b/tests/qtest/vhost-user-blk-test.c
@@ -563,6 +563,67 @@ static void pci_hotplug(void *obj, void *data, QGuestAllocator *t_alloc)
     qpci_unplug_acpi_device_test(qts, "drv1", PCI_SLOT_HP);
 }
 
+static void multiqueue(void *obj, void *data, QGuestAllocator *t_alloc)
+{
+    QVirtioPCIDevice *pdev1 = obj;
+    QVirtioDevice *dev1 = &pdev1->vdev;
+    QVirtioPCIDevice *pdev8;
+    QVirtioDevice *dev8;
+    QTestState *qts = pdev1->pdev->bus->qts;
+    uint64_t features;
+    uint16_t num_queues;
+
+    /*
+     * The primary device has 1 queue and VIRTIO_BLK_F_MQ is not enabled. The
+     * VIRTIO specification allows VIRTIO_BLK_F_MQ to be enabled when there is
+     * only 1 virtqueue, but --device vhost-user-blk-pci doesn't do this (which
+     * is also spec-compliant).
+     */
+    features = qvirtio_get_features(dev1);
+    g_assert_cmpint(features & (1u << VIRTIO_BLK_F_MQ), ==, 0);
+    features = features & ~(QVIRTIO_F_BAD_FEATURE |
+                            (1u << VIRTIO_RING_F_INDIRECT_DESC) |
+                            (1u << VIRTIO_F_NOTIFY_ON_EMPTY) |
+                            (1u << VIRTIO_BLK_F_SCSI));
+    qvirtio_set_features(dev1, features);
+
+    /* Hotplug a secondary device with 8 queues */
+    qtest_qmp_device_add(qts, "vhost-user-blk-pci", "drv1",
+                         "{'addr': %s, 'chardev': 'char2', 'num-queues': 8}",
+                         stringify(PCI_SLOT_HP) ".0");
+
+    pdev8 = virtio_pci_new(pdev1->pdev->bus,
+                           &(QPCIAddress) {
+                               .devfn = QPCI_DEVFN(PCI_SLOT_HP, 0)
+                           });
+    g_assert_nonnull(pdev8);
+    g_assert_cmpint(pdev8->vdev.device_type, ==, VIRTIO_ID_BLOCK);
+
+    qos_object_start_hw(&pdev8->obj);
+
+    dev8 = &pdev8->vdev;
+    features = qvirtio_get_features(dev8);
+    g_assert_cmpint(features & (1u << VIRTIO_BLK_F_MQ),
+                    ==,
+                    (1u << VIRTIO_BLK_F_MQ));
+    features = features & ~(QVIRTIO_F_BAD_FEATURE |
+                            (1u << VIRTIO_RING_F_INDIRECT_DESC) |
+                            (1u << VIRTIO_F_NOTIFY_ON_EMPTY) |
+                            (1u << VIRTIO_BLK_F_SCSI) |
+                            (1u << VIRTIO_BLK_F_MQ));
+    qvirtio_set_features(dev8, features);
+
+    num_queues = qvirtio_config_readw(dev8,
+            offsetof(struct virtio_blk_config, num_queues));
+    g_assert_cmpint(num_queues, ==, 8);
+
+    qvirtio_pci_device_disable(pdev8);
+    qos_object_destroy(&pdev8->obj);
+
+    /* unplug secondary disk */
+    qpci_unplug_acpi_device_test(qts, "drv1", PCI_SLOT_HP);
+}
+
 /*
  * Check that setting the vring addr on a non-existent virtqueue does
  * not crash.
@@ -682,7 +743,8 @@ static void quit_storage_daemon(void *data)
     g_free(data);
 }
 
-static void start_vhost_user_blk(GString *cmd_line, int vus_instances)
+static void start_vhost_user_blk(GString *cmd_line, int vus_instances,
+                                 int num_queues)
 {
     const char *vhost_user_blk_bin = qtest_qemu_storage_daemon_binary();
     int i;
@@ -707,8 +769,8 @@ static void start_vhost_user_blk(GString *cmd_line, int vus_instances)
         g_string_append_printf(storage_daemon_command,
             "--blockdev driver=file,node-name=disk%d,filename=%s "
             "--export type=vhost-user-blk,id=disk%d,addr.type=unix,addr.path=%s,"
-            "node-name=disk%i,writable=on ",
-            i, img_path, i, sock_path, i);
+            "node-name=disk%i,writable=on,num-queues=%d ",
+            i, img_path, i, sock_path, i, num_queues);
 
         g_string_append_printf(cmd_line, "-chardev socket,id=char%d,path=%s ",
                                i + 1, sock_path);
@@ -742,7 +804,7 @@ static void start_vhost_user_blk(GString *cmd_line, int vus_instances)
 
 static void *vhost_user_blk_test_setup(GString *cmd_line, void *arg)
 {
-    start_vhost_user_blk(cmd_line, 1);
+    start_vhost_user_blk(cmd_line, 1, 1);
     return arg;
 }
 
@@ -756,7 +818,13 @@ static void *vhost_user_blk_test_setup(GString *cmd_line, void *arg)
 static void *vhost_user_blk_hotplug_test_setup(GString *cmd_line, void *arg)
 {
     /* "-chardev socket,id=char2" is used for pci_hotplug*/
-    start_vhost_user_blk(cmd_line, 2);
+    start_vhost_user_blk(cmd_line, 2, 1);
+    return arg;
+}
+
+static void *vhost_user_blk_multiqueue_test_setup(GString *cmd_line, void *arg)
+{
+    start_vhost_user_blk(cmd_line, 2, 8);
     return arg;
 }
 
@@ -783,6 +851,9 @@ static void register_vhost_user_blk_test(void)
 
     opts.before = vhost_user_blk_hotplug_test_setup;
     qos_add_test("hotplug", "vhost-user-blk-pci", pci_hotplug, &opts);
+
+    opts.before = vhost_user_blk_multiqueue_test_setup;
+    qos_add_test("multiqueue", "vhost-user-blk-pci", multiqueue, &opts);
 }
 
 libqos_init(register_vhost_user_blk_test);
-- 
2.28.0

[PATCH v2 07/12] block/export: fix blk_size double byteswap

[Stefan Hajnoczi]

The config->blk_size field is little-endian. Use the native-endian
blk_size variable to avoid double byteswapping.

Fixes: 11f60f7eaee2630dd6fa0c3a8c49f792e46c4cf1 ("block/export: make vhost-user-blk config space little-endian")
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 block/export/vhost-user-blk-server.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/block/export/vhost-user-blk-server.c b/block/export/vhost-user-blk-server.c
index 62672d1cb9..3003cff189 100644
--- a/block/export/vhost-user-blk-server.c
+++ b/block/export/vhost-user-blk-server.c
@@ -354,7 +354,7 @@ vu_blk_initialize_config(BlockDriverState *bs,
     config->num_queues = cpu_to_le16(num_queues);
     config->max_discard_sectors = cpu_to_le32(32768);
     config->max_discard_seg = cpu_to_le32(1);
-    config->discard_sector_alignment = cpu_to_le32(config->blk_size >> 9);
+    config->discard_sector_alignment = cpu_to_le32(blk_size >> 9);
     config->max_write_zeroes_sectors = cpu_to_le32(32768);
     config->max_write_zeroes_seg = cpu_to_le32(1);
 }
-- 
2.28.0

[PATCH v2 08/12] block/export: use VIRTIO_BLK_SECTOR_BITS

[Stefan Hajnoczi]

Use VIRTIO_BLK_SECTOR_BITS and VIRTIO_BLK_SECTOR_SIZE when dealing with
virtio-blk sector numbers. Although the values happen to be the same as
BDRV_SECTOR_BITS and BDRV_SECTOR_SIZE, they are conceptually different.
This makes it clearer when we are dealing with virtio-blk sector units.

Use VIRTIO_BLK_SECTOR_BITS in vu_blk_initialize_config(). Later patches
will use it the new constants the virtqueue request processing code
path.

Suggested-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 block/export/vhost-user-blk-server.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/block/export/vhost-user-blk-server.c b/block/export/vhost-user-blk-server.c
index 3003cff189..feb139e067 100644
--- a/block/export/vhost-user-blk-server.c
+++ b/block/export/vhost-user-blk-server.c
@@ -20,6 +20,13 @@
 #include "sysemu/block-backend.h"
 #include "util/block-helpers.h"
 
+/*
+ * Sector units are 512 bytes regardless of the
+ * virtio_blk_config->blk_size value.
+ */
+#define VIRTIO_BLK_SECTOR_BITS 9
+#define VIRTIO_BLK_SECTOR_SIZE (1ull << VIRTIO_BLK_SECTOR_BITS)
+
 enum {
     VHOST_USER_BLK_NUM_QUEUES_DEFAULT = 1,
 };
@@ -345,7 +352,8 @@ vu_blk_initialize_config(BlockDriverState *bs,
                          uint32_t blk_size,
                          uint16_t num_queues)
 {
-    config->capacity = cpu_to_le64(bdrv_getlength(bs) >> BDRV_SECTOR_BITS);
+    config->capacity =
+        cpu_to_le64(bdrv_getlength(bs) >> VIRTIO_BLK_SECTOR_BITS);
     config->blk_size = cpu_to_le32(blk_size);
     config->size_max = cpu_to_le32(0);
     config->seg_max = cpu_to_le32(128 - 2);
@@ -354,7 +362,8 @@ vu_blk_initialize_config(BlockDriverState *bs,
     config->num_queues = cpu_to_le16(num_queues);
     config->max_discard_sectors = cpu_to_le32(32768);
     config->max_discard_seg = cpu_to_le32(1);
-    config->discard_sector_alignment = cpu_to_le32(blk_size >> 9);
+    config->discard_sector_alignment =
+        cpu_to_le32(blk_size >> VIRTIO_BLK_SECTOR_BITS);
     config->max_write_zeroes_sectors = cpu_to_le32(32768);
     config->max_write_zeroes_seg = cpu_to_le32(1);
 }
@@ -381,7 +390,7 @@ static int vu_blk_exp_create(BlockExport *exp, BlockExportOptions *opts,
     if (vu_opts->has_logical_block_size) {
         logical_block_size = vu_opts->logical_block_size;
     } else {
-        logical_block_size = BDRV_SECTOR_SIZE;
+        logical_block_size = VIRTIO_BLK_SECTOR_SIZE;
     }
     check_block_size(exp->id, "logical-block-size", logical_block_size,
                      &local_err);
-- 
2.28.0

[PATCH v2 09/12] block/export: fix vhost-user-blk export sector number calculation

[Stefan Hajnoczi]

The driver is supposed to honor the blk_size field but the protocol
still uses 512-byte sector numbers. It is incorrect to multiply
req->sector_num by blk_size.

VIRTIO 1.1 5.2.5 Device Initialization says:

  blk_size can be read to determine the optimal sector size for the
  driver to use. This does not affect the units used in the protocol
  (always 512 bytes), but awareness of the correct value can affect
  performance.

Fixes: 3578389bcf76c824a5d82e6586a6f0c71e56f2aa ("block/export: vhost-user block device backend server")
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 block/export/vhost-user-blk-server.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/block/export/vhost-user-blk-server.c b/block/export/vhost-user-blk-server.c
index feb139e067..bb07f499c8 100644
--- a/block/export/vhost-user-blk-server.c
+++ b/block/export/vhost-user-blk-server.c
@@ -144,7 +144,7 @@ static void coroutine_fn vu_blk_virtio_process_req(void *opaque)
             break;
         }
 
-        int64_t offset = req->sector_num * vexp->blk_size;
+        int64_t offset = req->sector_num << VIRTIO_BLK_SECTOR_BITS;
         QEMUIOVector qiov;
         if (is_write) {
             qemu_iovec_init_external(&qiov, out_iov, out_num);
-- 
2.28.0

[PATCH v2 10/12] block/export: port virtio-blk discard/write zeroes input validation

[Stefan Hajnoczi]

Validate discard/write zeroes the same way we do for virtio-blk. Some of
these checks are mandated by the VIRTIO specification, others are
internal to QEMU.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 block/export/vhost-user-blk-server.c | 116 +++++++++++++++++++++------
 1 file changed, 93 insertions(+), 23 deletions(-)

diff --git a/block/export/vhost-user-blk-server.c b/block/export/vhost-user-blk-server.c
index bb07f499c8..937bb5e9b4 100644
--- a/block/export/vhost-user-blk-server.c
+++ b/block/export/vhost-user-blk-server.c
@@ -29,6 +29,8 @@
 
 enum {
     VHOST_USER_BLK_NUM_QUEUES_DEFAULT = 1,
+    VHOST_USER_BLK_MAX_DISCARD_SECTORS = 32768,
+    VHOST_USER_BLK_MAX_WRITE_ZEROES_SECTORS = 32768,
 };
 struct virtio_blk_inhdr {
     unsigned char status;
@@ -65,30 +67,102 @@ static void vu_blk_req_complete(VuBlkReq *req)
     free(req);
 }
 
+static bool vu_blk_sect_range_ok(VuBlkExport *vexp, uint64_t sector,
+                                 size_t size)
+{
+    uint64_t nb_sectors = size >> BDRV_SECTOR_BITS;
+    uint64_t total_sectors;
+
+    if (nb_sectors > BDRV_REQUEST_MAX_SECTORS) {
+        return false;
+    }
+    if ((sector << VIRTIO_BLK_SECTOR_BITS) % vexp->blk_size) {
+        return false;
+    }
+    blk_get_geometry(vexp->export.blk, &total_sectors);
+    if (sector > total_sectors || nb_sectors > total_sectors - sector) {
+        return false;
+    }
+    return true;
+}
+
 static int coroutine_fn
-vu_blk_discard_write_zeroes(BlockBackend *blk, struct iovec *iov,
+vu_blk_discard_write_zeroes(VuBlkExport *vexp, struct iovec *iov,
                             uint32_t iovcnt, uint32_t type)
 {
+    BlockBackend *blk = vexp->export.blk;
     struct virtio_blk_discard_write_zeroes desc;
-    ssize_t size = iov_to_buf(iov, iovcnt, 0, &desc, sizeof(desc));
+    ssize_t size;
+    uint64_t sector;
+    uint32_t num_sectors;
+    uint32_t max_sectors;
+    uint32_t flags;
+    int bytes;
+
+    /* Only one desc is currently supported */
+    if (unlikely(iov_size(iov, iovcnt) > sizeof(desc))) {
+        return VIRTIO_BLK_S_UNSUPP;
+    }
+
+    size = iov_to_buf(iov, iovcnt, 0, &desc, sizeof(desc));
     if (unlikely(size != sizeof(desc))) {
-        error_report("Invalid size %zd, expect %zu", size, sizeof(desc));
-        return -EINVAL;
+        error_report("Invalid size %zd, expected %zu", size, sizeof(desc));
+        return VIRTIO_BLK_S_IOERR;
     }
 
-    uint64_t range[2] = { le64_to_cpu(desc.sector) << 9,
-                          le32_to_cpu(desc.num_sectors) << 9 };
-    if (type == VIRTIO_BLK_T_DISCARD) {
-        if (blk_co_pdiscard(blk, range[0], range[1]) == 0) {
-            return 0;
+    sector = le64_to_cpu(desc.sector);
+    num_sectors = le32_to_cpu(desc.num_sectors);
+    flags = le32_to_cpu(desc.flags);
+    max_sectors = (type == VIRTIO_BLK_T_WRITE_ZEROES) ?
+                  VHOST_USER_BLK_MAX_WRITE_ZEROES_SECTORS :
+                  VHOST_USER_BLK_MAX_DISCARD_SECTORS;
+
+    /* This check ensures that 'bytes' fits in an int */
+    if (unlikely(num_sectors > max_sectors)) {
+        return VIRTIO_BLK_S_IOERR;
+    }
+
+    bytes = num_sectors << VIRTIO_BLK_SECTOR_BITS;
+
+    if (unlikely(!vu_blk_sect_range_ok(vexp, sector, bytes))) {
+        return VIRTIO_BLK_S_IOERR;
+    }
+
+    /*
+     * The device MUST set the status byte to VIRTIO_BLK_S_UNSUPP for discard
+     * and write zeroes commands if any unknown flag is set.
+     */
+    if (unlikely(flags & ~VIRTIO_BLK_WRITE_ZEROES_FLAG_UNMAP)) {
+        return VIRTIO_BLK_S_UNSUPP;
+    }
+
+    if (type == VIRTIO_BLK_T_WRITE_ZEROES) {
+        int blk_flags = 0;
+
+        if (flags & VIRTIO_BLK_WRITE_ZEROES_FLAG_UNMAP) {
+            blk_flags |= BDRV_REQ_MAY_UNMAP;
+        }
+
+        if (blk_co_pwrite_zeroes(blk, sector << VIRTIO_BLK_SECTOR_BITS,
+                                 bytes, blk_flags) == 0) {
+            return VIRTIO_BLK_S_OK;
+        }
+    } else if (type == VIRTIO_BLK_T_DISCARD) {
+        /*
+         * The device MUST set the status byte to VIRTIO_BLK_S_UNSUPP for
+         * discard commands if the unmap flag is set.
+         */
+        if (unlikely(flags & VIRTIO_BLK_WRITE_ZEROES_FLAG_UNMAP)) {
+            return VIRTIO_BLK_S_UNSUPP;
         }
-    } else if (type == VIRTIO_BLK_T_WRITE_ZEROES) {
-        if (blk_co_pwrite_zeroes(blk, range[0], range[1], 0) == 0) {
-            return 0;
+
+        if (blk_co_pdiscard(blk, sector << VIRTIO_BLK_SECTOR_BITS,
+                            bytes) == 0) {
+            return VIRTIO_BLK_S_OK;
         }
     }
 
-    return -EINVAL;
+    return VIRTIO_BLK_S_IOERR;
 }
 
 static void coroutine_fn vu_blk_virtio_process_req(void *opaque)
@@ -177,19 +251,13 @@ static void coroutine_fn vu_blk_virtio_process_req(void *opaque)
     }
     case VIRTIO_BLK_T_DISCARD:
     case VIRTIO_BLK_T_WRITE_ZEROES: {
-        int rc;
-
         if (!vexp->writable) {
             req->in->status = VIRTIO_BLK_S_IOERR;
             break;
         }
 
-        rc = vu_blk_discard_write_zeroes(blk, &elem->out_sg[1], out_num, type);
-        if (rc == 0) {
-            req->in->status = VIRTIO_BLK_S_OK;
-        } else {
-            req->in->status = VIRTIO_BLK_S_IOERR;
-        }
+        req->in->status = vu_blk_discard_write_zeroes(vexp, out_iov, out_num,
+                                                      type);
         break;
     }
     default:
@@ -360,11 +428,13 @@ vu_blk_initialize_config(BlockDriverState *bs,
     config->min_io_size = cpu_to_le16(1);
     config->opt_io_size = cpu_to_le32(1);
     config->num_queues = cpu_to_le16(num_queues);
-    config->max_discard_sectors = cpu_to_le32(32768);
+    config->max_discard_sectors =
+        cpu_to_le32(VHOST_USER_BLK_MAX_DISCARD_SECTORS);
     config->max_discard_seg = cpu_to_le32(1);
     config->discard_sector_alignment =
         cpu_to_le32(blk_size >> VIRTIO_BLK_SECTOR_BITS);
-    config->max_write_zeroes_sectors = cpu_to_le32(32768);
+    config->max_write_zeroes_sectors
+        = cpu_to_le32(VHOST_USER_BLK_MAX_WRITE_ZEROES_SECTORS);
     config->max_write_zeroes_seg = cpu_to_le32(1);
 }
 
-- 
2.28.0

[PATCH v2 11/12] vhost-user-blk-test: test discard/write zeroes invalid inputs

[Stefan Hajnoczi]

Exercise input validation code paths in
block/export/vhost-user-blk-server.c.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 tests/qtest/vhost-user-blk-test.c | 124 ++++++++++++++++++++++++++++++
 1 file changed, 124 insertions(+)

diff --git a/tests/qtest/vhost-user-blk-test.c b/tests/qtest/vhost-user-blk-test.c
index 1f1e8a9b79..78f8503dba 100644
--- a/tests/qtest/vhost-user-blk-test.c
+++ b/tests/qtest/vhost-user-blk-test.c
@@ -94,6 +94,124 @@ static uint64_t virtio_blk_request(QGuestAllocator *alloc, QVirtioDevice *d,
     return addr;
 }
 
+static void test_invalid_discard_write_zeroes(QVirtioDevice *dev,
+                                              QGuestAllocator *alloc,
+                                              QTestState *qts,
+                                              QVirtQueue *vq,
+                                              uint32_t type)
+{
+    QVirtioBlkReq req;
+    struct virtio_blk_discard_write_zeroes dwz_hdr;
+    struct virtio_blk_discard_write_zeroes dwz_hdr2[2];
+    uint64_t req_addr;
+    uint32_t free_head;
+    uint8_t status;
+
+    /* More than one dwz is not supported */
+    req.type = type;
+    req.data = (char *) dwz_hdr2;
+    dwz_hdr2[0].sector = 0;
+    dwz_hdr2[0].num_sectors = 1;
+    dwz_hdr2[0].flags = 0;
+    dwz_hdr2[1].sector = 1;
+    dwz_hdr2[1].num_sectors = 1;
+    dwz_hdr2[1].flags = 0;
+
+    virtio_blk_fix_dwz_hdr(dev, &dwz_hdr2[0]);
+    virtio_blk_fix_dwz_hdr(dev, &dwz_hdr2[1]);
+
+    req_addr = virtio_blk_request(alloc, dev, &req, sizeof(dwz_hdr2));
+
+    free_head = qvirtqueue_add(qts, vq, req_addr, 16, false, true);
+    qvirtqueue_add(qts, vq, req_addr + 16, sizeof(dwz_hdr2), false, true);
+    qvirtqueue_add(qts, vq, req_addr + 16 + sizeof(dwz_hdr2), 1, true,
+                   false);
+
+    qvirtqueue_kick(qts, dev, vq, free_head);
+
+    qvirtio_wait_used_elem(qts, dev, vq, free_head, NULL,
+                           QVIRTIO_BLK_TIMEOUT_US);
+    status = readb(req_addr + 16 + sizeof(dwz_hdr2));
+    g_assert_cmpint(status, ==, VIRTIO_BLK_S_UNSUPP);
+
+    guest_free(alloc, req_addr);
+
+    /* num_sectors must be less than config->max_write_zeroes_sectors */
+    req.type = type;
+    req.data = (char *) &dwz_hdr;
+    dwz_hdr.sector = 0;
+    dwz_hdr.num_sectors = 0xffffffff;
+    dwz_hdr.flags = 0;
+
+    virtio_blk_fix_dwz_hdr(dev, &dwz_hdr);
+
+    req_addr = virtio_blk_request(alloc, dev, &req, sizeof(dwz_hdr));
+
+    free_head = qvirtqueue_add(qts, vq, req_addr, 16, false, true);
+    qvirtqueue_add(qts, vq, req_addr + 16, sizeof(dwz_hdr), false, true);
+    qvirtqueue_add(qts, vq, req_addr + 16 + sizeof(dwz_hdr), 1, true,
+                   false);
+
+    qvirtqueue_kick(qts, dev, vq, free_head);
+
+    qvirtio_wait_used_elem(qts, dev, vq, free_head, NULL,
+                           QVIRTIO_BLK_TIMEOUT_US);
+    status = readb(req_addr + 16 + sizeof(dwz_hdr));
+    g_assert_cmpint(status, ==, VIRTIO_BLK_S_IOERR);
+
+    guest_free(alloc, req_addr);
+
+    /* sector must be less than the device capacity */
+    req.type = type;
+    req.data = (char *) &dwz_hdr;
+    dwz_hdr.sector = TEST_IMAGE_SIZE / 512 + 1;
+    dwz_hdr.num_sectors = 1;
+    dwz_hdr.flags = 0;
+
+    virtio_blk_fix_dwz_hdr(dev, &dwz_hdr);
+
+    req_addr = virtio_blk_request(alloc, dev, &req, sizeof(dwz_hdr));
+
+    free_head = qvirtqueue_add(qts, vq, req_addr, 16, false, true);
+    qvirtqueue_add(qts, vq, req_addr + 16, sizeof(dwz_hdr), false, true);
+    qvirtqueue_add(qts, vq, req_addr + 16 + sizeof(dwz_hdr), 1, true,
+                   false);
+
+    qvirtqueue_kick(qts, dev, vq, free_head);
+
+    qvirtio_wait_used_elem(qts, dev, vq, free_head, NULL,
+                           QVIRTIO_BLK_TIMEOUT_US);
+    status = readb(req_addr + 16 + sizeof(dwz_hdr));
+    g_assert_cmpint(status, ==, VIRTIO_BLK_S_IOERR);
+
+    guest_free(alloc, req_addr);
+
+    /* reserved flag bits must be zero */
+    req.type = type;
+    req.data = (char *) &dwz_hdr;
+    dwz_hdr.sector = 0;
+    dwz_hdr.num_sectors = 1;
+    dwz_hdr.flags = ~VIRTIO_BLK_WRITE_ZEROES_FLAG_UNMAP;
+
+    virtio_blk_fix_dwz_hdr(dev, &dwz_hdr);
+
+    req_addr = virtio_blk_request(alloc, dev, &req, sizeof(dwz_hdr));
+
+    free_head = qvirtqueue_add(qts, vq, req_addr, 16, false, true);
+    qvirtqueue_add(qts, vq, req_addr + 16, sizeof(dwz_hdr), false, true);
+    qvirtqueue_add(qts, vq, req_addr + 16 + sizeof(dwz_hdr), 1, true,
+                   false);
+
+    qvirtqueue_kick(qts, dev, vq, free_head);
+
+    qvirtio_wait_used_elem(qts, dev, vq, free_head, NULL,
+                           QVIRTIO_BLK_TIMEOUT_US);
+    status = readb(req_addr + 16 + sizeof(dwz_hdr));
+    g_assert_cmpint(status, ==, VIRTIO_BLK_S_UNSUPP);
+
+    guest_free(alloc, req_addr);
+}
+
 /* Returns the request virtqueue so the caller can perform further tests */
 static QVirtQueue *test_basic(QVirtioDevice *dev, QGuestAllocator *alloc)
 {
@@ -235,6 +353,9 @@ static QVirtQueue *test_basic(QVirtioDevice *dev, QGuestAllocator *alloc)
         g_free(data);
 
         guest_free(alloc, req_addr);
+
+        test_invalid_discard_write_zeroes(dev, alloc, qts, vq,
+                                          VIRTIO_BLK_T_WRITE_ZEROES);
     }
 
     if (features & (1u << VIRTIO_BLK_F_DISCARD)) {
@@ -263,6 +384,9 @@ static QVirtQueue *test_basic(QVirtioDevice *dev, QGuestAllocator *alloc)
         g_assert_cmpint(status, ==, 0);
 
         guest_free(alloc, req_addr);
+
+        test_invalid_discard_write_zeroes(dev, alloc, qts, vq,
+                                          VIRTIO_BLK_T_DISCARD);
     }
 
     if (features & (1u << VIRTIO_F_ANY_LAYOUT)) {
-- 
2.28.0

[PATCH v2 12/12] block/export: port virtio-blk read/write range check

[Stefan Hajnoczi]

Check that the sector number and byte count are valid.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 block/export/vhost-user-blk-server.c | 19 ++++++++++++++++---
 1 file changed, 16 insertions(+), 3 deletions(-)

diff --git a/block/export/vhost-user-blk-server.c b/block/export/vhost-user-blk-server.c
index 937bb5e9b4..dbe3cfb9e8 100644
--- a/block/export/vhost-user-blk-server.c
+++ b/block/export/vhost-user-blk-server.c
@@ -209,6 +209,8 @@ static void coroutine_fn vu_blk_virtio_process_req(void *opaque)
     switch (type & ~VIRTIO_BLK_T_BARRIER) {
     case VIRTIO_BLK_T_IN:
     case VIRTIO_BLK_T_OUT: {
+        QEMUIOVector qiov;
+        int64_t offset;
         ssize_t ret = 0;
         bool is_write = type & VIRTIO_BLK_T_OUT;
         req->sector_num = le64_to_cpu(req->out.sector);
@@ -218,13 +220,24 @@ static void coroutine_fn vu_blk_virtio_process_req(void *opaque)
             break;
         }
 
-        int64_t offset = req->sector_num << VIRTIO_BLK_SECTOR_BITS;
-        QEMUIOVector qiov;
         if (is_write) {
             qemu_iovec_init_external(&qiov, out_iov, out_num);
-            ret = blk_co_pwritev(blk, offset, qiov.size, &qiov, 0);
         } else {
             qemu_iovec_init_external(&qiov, in_iov, in_num);
+        }
+
+        if (unlikely(!vu_blk_sect_range_ok(vexp,
+                                           req->sector_num,
+                                           qiov.size))) {
+            req->in->status = VIRTIO_BLK_S_IOERR;
+            break;
+        }
+
+        offset = req->sector_num << VIRTIO_BLK_SECTOR_BITS;
+
+        if (is_write) {
+            ret = blk_co_pwritev(blk, offset, qiov.size, &qiov, 0);
+        } else {
             ret = blk_co_preadv(blk, offset, qiov.size, &qiov, 0);
         }
         if (ret >= 0) {
-- 
2.28.0

Re: [PATCH v2 02/12] libqtest: add qtest_socket_server()

[Thomas Huth]

On 07/12/2020 18.20, Stefan Hajnoczi wrote:
> Add an API that returns a new UNIX domain socket in the listen state.
> The code for this was already there but only used internally in
> init_socket().
> 
> This new API will be used by vhost-user-blk-test.
> 
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
>  tests/qtest/libqos/libqtest.h |  8 +++++++
>  tests/qtest/libqtest.c        | 40 ++++++++++++++++++++---------------
>  2 files changed, 31 insertions(+), 17 deletions(-)
> 
> diff --git a/tests/qtest/libqos/libqtest.h b/tests/qtest/libqos/libqtest.h
> index 724f65aa94..e5f1ec590c 100644
> --- a/tests/qtest/libqos/libqtest.h
> +++ b/tests/qtest/libqos/libqtest.h
> @@ -132,6 +132,14 @@ void qtest_qmp_send(QTestState *s, const char *fmt, ...)
>  void qtest_qmp_send_raw(QTestState *s, const char *fmt, ...)
>      GCC_FMT_ATTR(2, 3);

Reviewed-by: Thomas Huth <thuth@redhat.com>

Re: [PATCH v2 05/12] test: new qTest case to test the vhost-user-blk-server

[Coiby Xu]

On Mon, Dec 07, 2020 at 05:20:23PM +0000, Stefan Hajnoczi wrote:
>From: Coiby Xu <coiby.xu@gmail.com>
>
>This test case has the same tests as tests/virtio-blk-test.c except for
>tests have block_resize. Since the vhost-user-blk export only serves one
>client one time, two exports are started by qemu-storage-daemon for the
>hotplug test.
>
>Suggested-by: Thomas Huth <thuth@redhat.com>
>Signed-off-by: Coiby Xu <coiby.xu@gmail.com>
>Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
>---
> MAINTAINERS                         |   2 +
> tests/qtest/libqos/vhost-user-blk.h |  48 ++
> tests/qtest/libqos/vhost-user-blk.c | 130 +++++
> tests/qtest/vhost-user-blk-test.c   | 788 ++++++++++++++++++++++++++++
> tests/qtest/libqos/meson.build      |   1 +
> tests/qtest/meson.build             |   4 +
> 6 files changed, 973 insertions(+)
> create mode 100644 tests/qtest/libqos/vhost-user-blk.h
> create mode 100644 tests/qtest/libqos/vhost-user-blk.c
> create mode 100644 tests/qtest/vhost-user-blk-test.c
>
>diff --git a/MAINTAINERS b/MAINTAINERS
>index 68bc160f41..d351280d1f 100644
>--- a/MAINTAINERS
>+++ b/MAINTAINERS
>@@ -3103,6 +3103,8 @@ F: block/export/vhost-user-blk-server.c
> F: block/export/vhost-user-blk-server.h
> F: include/qemu/vhost-user-server.h
> F: tests/qtest/libqos/vhost-user-blk.c
>+F: tests/qtest/libqos/vhost-user-blk.h
>+F: tests/qtest/vhost-user-blk-test.c
> F: util/vhost-user-server.c
>
> Replication
>diff --git a/tests/qtest/libqos/vhost-user-blk.h b/tests/qtest/libqos/vhost-user-blk.h
>new file mode 100644
>index 0000000000..2a03456a45
>--- /dev/null
>+++ b/tests/qtest/libqos/vhost-user-blk.h
>@@ -0,0 +1,48 @@
>+/*
>+ * libqos driver framework
>+ *
>+ * Based on tests/qtest/libqos/virtio-blk.c
>+ *
>+ * Copyright (c) 2020 Coiby Xu <coiby.xu@gmail.com>
>+ *
>+ * Copyright (c) 2018 Emanuele Giuseppe Esposito <e.emanuelegiuseppe@gmail.com>
>+ *
>+ * This library is free software; you can redistribute it and/or
>+ * modify it under the terms of the GNU Lesser General Public
>+ * License version 2 as published by the Free Software Foundation.
>+ *
>+ * This library 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
>+ * Lesser General Public License for more details.
>+ *
>+ * You should have received a copy of the GNU Lesser General Public
>+ * License along with this library; if not, see <http://www.gnu.org/licenses/>
>+ */
>+
>+#ifndef TESTS_LIBQOS_VHOST_USER_BLK_H
>+#define TESTS_LIBQOS_VHOST_USER_BLK_H
>+
>+#include "qgraph.h"
>+#include "virtio.h"
>+#include "virtio-pci.h"
>+
>+typedef struct QVhostUserBlk QVhostUserBlk;
>+typedef struct QVhostUserBlkPCI QVhostUserBlkPCI;
>+typedef struct QVhostUserBlkDevice QVhostUserBlkDevice;
>+
>+struct QVhostUserBlk {
>+    QVirtioDevice *vdev;
>+};
>+
>+struct QVhostUserBlkPCI {
>+    QVirtioPCIDevice pci_vdev;
>+    QVhostUserBlk blk;
>+};
>+
>+struct QVhostUserBlkDevice {
>+    QOSGraphObject obj;
>+    QVhostUserBlk blk;
>+};
>+
>+#endif
>diff --git a/tests/qtest/libqos/vhost-user-blk.c b/tests/qtest/libqos/vhost-user-blk.c
>new file mode 100644
>index 0000000000..568c3426ed
>--- /dev/null
>+++ b/tests/qtest/libqos/vhost-user-blk.c
>@@ -0,0 +1,130 @@
>+/*
>+ * libqos driver framework
>+ *
>+ * Based on tests/qtest/libqos/virtio-blk.c
>+ *
>+ * Copyright (c) 2020 Coiby Xu <coiby.xu@gmail.com>
>+ *
>+ * Copyright (c) 2018 Emanuele Giuseppe Esposito <e.emanuelegiuseppe@gmail.com>
>+ *
>+ * This library is free software; you can redistribute it and/or
>+ * modify it under the terms of the GNU Lesser General Public
>+ * License version 2.1 as published by the Free Software Foundation.
>+ *
>+ * This library 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
>+ * Lesser General Public License for more details.
>+ *
>+ * You should have received a copy of the GNU Lesser General Public
>+ * License along with this library; if not, see <http://www.gnu.org/licenses/>
>+ */
>+
>+#include "qemu/osdep.h"
>+#include "libqtest.h"
>+#include "qemu/module.h"
>+#include "standard-headers/linux/virtio_blk.h"
>+#include "vhost-user-blk.h"
>+
>+#define PCI_SLOT                0x04
>+#define PCI_FN                  0x00
>+
>+/* virtio-blk-device */
>+static void *qvhost_user_blk_get_driver(QVhostUserBlk *v_blk,
>+                                    const char *interface)
>+{
>+    if (!g_strcmp0(interface, "vhost-user-blk")) {
>+        return v_blk;
>+    }
>+    if (!g_strcmp0(interface, "virtio")) {
>+        return v_blk->vdev;
>+    }
>+
>+    fprintf(stderr, "%s not present in vhost-user-blk-device\n", interface);
>+    g_assert_not_reached();
>+}
>+
>+static void *qvhost_user_blk_device_get_driver(void *object,
>+                                           const char *interface)
>+{
>+    QVhostUserBlkDevice *v_blk = object;
>+    return qvhost_user_blk_get_driver(&v_blk->blk, interface);
>+}
>+
>+static void *vhost_user_blk_device_create(void *virtio_dev,
>+                                      QGuestAllocator *t_alloc,
>+                                      void *addr)
>+{
>+    QVhostUserBlkDevice *vhost_user_blk = g_new0(QVhostUserBlkDevice, 1);
>+    QVhostUserBlk *interface = &vhost_user_blk->blk;
>+
>+    interface->vdev = virtio_dev;
>+
>+    vhost_user_blk->obj.get_driver = qvhost_user_blk_device_get_driver;
>+
>+    return &vhost_user_blk->obj;
>+}
>+
>+/* virtio-blk-pci */
>+static void *qvhost_user_blk_pci_get_driver(void *object, const char *interface)
>+{
>+    QVhostUserBlkPCI *v_blk = object;
>+    if (!g_strcmp0(interface, "pci-device")) {
>+        return v_blk->pci_vdev.pdev;
>+    }
>+    return qvhost_user_blk_get_driver(&v_blk->blk, interface);
>+}
>+
>+static void *vhost_user_blk_pci_create(void *pci_bus, QGuestAllocator *t_alloc,
>+                                      void *addr)
>+{
>+    QVhostUserBlkPCI *vhost_user_blk = g_new0(QVhostUserBlkPCI, 1);
>+    QVhostUserBlk *interface = &vhost_user_blk->blk;
>+    QOSGraphObject *obj = &vhost_user_blk->pci_vdev.obj;
>+
>+    virtio_pci_init(&vhost_user_blk->pci_vdev, pci_bus, addr);
>+    interface->vdev = &vhost_user_blk->pci_vdev.vdev;
>+
>+    g_assert_cmphex(interface->vdev->device_type, ==, VIRTIO_ID_BLOCK);
>+
>+    obj->get_driver = qvhost_user_blk_pci_get_driver;
>+
>+    return obj;
>+}
>+
>+static void vhost_user_blk_register_nodes(void)
>+{
>+    /*
>+     * FIXME: every test using these two nodes needs to setup a
>+     * -drive,id=drive0 otherwise QEMU is not going to start.
>+     * Therefore, we do not include "produces" edge for virtio
>+     * and pci-device yet.
>+     */
>+
>+    char *arg = g_strdup_printf("id=drv0,chardev=char1,addr=%x.%x",
>+                                PCI_SLOT, PCI_FN);
>+
>+    QPCIAddress addr = {
>+        .devfn = QPCI_DEVFN(PCI_SLOT, PCI_FN),
>+    };
>+
>+    QOSGraphEdgeOptions opts = { };
>+
>+    /* virtio-blk-device */
>+    /** opts.extra_device_opts = "drive=drive0"; */
>+    qos_node_create_driver("vhost-user-blk-device",
>+                           vhost_user_blk_device_create);
>+    qos_node_consumes("vhost-user-blk-device", "virtio-bus", &opts);
>+    qos_node_produces("vhost-user-blk-device", "vhost-user-blk");
>+
>+    /* virtio-blk-pci */
>+    opts.extra_device_opts = arg;
>+    add_qpci_address(&opts, &addr);
>+    qos_node_create_driver("vhost-user-blk-pci", vhost_user_blk_pci_create);
>+    qos_node_consumes("vhost-user-blk-pci", "pci-bus", &opts);
>+    qos_node_produces("vhost-user-blk-pci", "vhost-user-blk");
>+
>+    g_free(arg);
>+}
>+
>+libqos_init(vhost_user_blk_register_nodes);
>diff --git a/tests/qtest/vhost-user-blk-test.c b/tests/qtest/vhost-user-blk-test.c
>new file mode 100644
>index 0000000000..175dccab98
>--- /dev/null
>+++ b/tests/qtest/vhost-user-blk-test.c
>@@ -0,0 +1,788 @@
>+/*
>+ * QTest testcase for Vhost-user Block Device
>+ *
>+ * Based on tests/qtest//virtio-blk-test.c
>+
>+ * Copyright (c) 2014 SUSE LINUX Products GmbH
>+ * Copyright (c) 2014 Marc Marí
>+ * Copyright (c) 2020 Coiby Xu
>+ *
>+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
>+ * See the COPYING file in the top-level directory.
>+ */
>+
>+#include "qemu/osdep.h"
>+#include "libqtest-single.h"
>+#include "qemu/bswap.h"
>+#include "qemu/module.h"
>+#include "standard-headers/linux/virtio_blk.h"
>+#include "standard-headers/linux/virtio_pci.h"
>+#include "libqos/qgraph.h"
>+#include "libqos/vhost-user-blk.h"
>+#include "libqos/libqos-pc.h"
>+
>+#define TEST_IMAGE_SIZE         (64 * 1024 * 1024)
>+#define QVIRTIO_BLK_TIMEOUT_US  (30 * 1000 * 1000)
>+#define PCI_SLOT_HP             0x06
>+
>+typedef struct {
>+    pid_t pid;
>+} QemuStorageDaemonState;
>+
>+typedef struct QVirtioBlkReq {
>+    uint32_t type;
>+    uint32_t ioprio;
>+    uint64_t sector;
>+    char *data;
>+    uint8_t status;
>+} QVirtioBlkReq;
>+
>+#ifdef HOST_WORDS_BIGENDIAN
>+static const bool host_is_big_endian = true;
>+#else
>+static const bool host_is_big_endian; /* false */
>+#endif
>+
>+static inline void virtio_blk_fix_request(QVirtioDevice *d, QVirtioBlkReq *req)
>+{
>+    if (qvirtio_is_big_endian(d) != host_is_big_endian) {
>+        req->type = bswap32(req->type);
>+        req->ioprio = bswap32(req->ioprio);
>+        req->sector = bswap64(req->sector);
>+    }
>+}
>+
>+static inline void virtio_blk_fix_dwz_hdr(QVirtioDevice *d,
>+    struct virtio_blk_discard_write_zeroes *dwz_hdr)
>+{
>+    if (qvirtio_is_big_endian(d) != host_is_big_endian) {
>+        dwz_hdr->sector = bswap64(dwz_hdr->sector);
>+        dwz_hdr->num_sectors = bswap32(dwz_hdr->num_sectors);
>+        dwz_hdr->flags = bswap32(dwz_hdr->flags);
>+    }
>+}
>+
>+static uint64_t virtio_blk_request(QGuestAllocator *alloc, QVirtioDevice *d,
>+                                   QVirtioBlkReq *req, uint64_t data_size)
>+{
>+    uint64_t addr;
>+    uint8_t status = 0xFF;
>+    QTestState *qts = global_qtest;
>+
>+    switch (req->type) {
>+    case VIRTIO_BLK_T_IN:
>+    case VIRTIO_BLK_T_OUT:
>+        g_assert_cmpuint(data_size % 512, ==, 0);
>+        break;
>+    case VIRTIO_BLK_T_DISCARD:
>+    case VIRTIO_BLK_T_WRITE_ZEROES:
>+        g_assert_cmpuint(data_size %
>+                         sizeof(struct virtio_blk_discard_write_zeroes), ==, 0);
>+        break;
>+    default:
>+        g_assert_cmpuint(data_size, ==, 0);
>+    }
>+
>+    addr = guest_alloc(alloc, sizeof(*req) + data_size);
>+
>+    virtio_blk_fix_request(d, req);
>+
>+    qtest_memwrite(qts, addr, req, 16);
>+    qtest_memwrite(qts, addr + 16, req->data, data_size);
>+    qtest_memwrite(qts, addr + 16 + data_size, &status, sizeof(status));
>+
>+    return addr;
>+}
>+
>+/* Returns the request virtqueue so the caller can perform further tests */
>+static QVirtQueue *test_basic(QVirtioDevice *dev, QGuestAllocator *alloc)
>+{
>+    QVirtioBlkReq req;
>+    uint64_t req_addr;
>+    uint64_t capacity;
>+    uint64_t features;
>+    uint32_t free_head;
>+    uint8_t status;
>+    char *data;
>+    QTestState *qts = global_qtest;
>+    QVirtQueue *vq;
>+
>+    features = qvirtio_get_features(dev);
>+    features = features & ~(QVIRTIO_F_BAD_FEATURE |
>+                    (1u << VIRTIO_RING_F_INDIRECT_DESC) |
>+                    (1u << VIRTIO_RING_F_EVENT_IDX) |
>+                    (1u << VIRTIO_BLK_F_SCSI));
>+    qvirtio_set_features(dev, features);
>+
>+    capacity = qvirtio_config_readq(dev, 0);
>+    g_assert_cmpint(capacity, ==, TEST_IMAGE_SIZE / 512);
>+
>+    vq = qvirtqueue_setup(dev, alloc, 0);
>+
>+    qvirtio_set_driver_ok(dev);
>+
>+    /* Write and read with 3 descriptor layout */
>+    /* Write request */
>+    req.type = VIRTIO_BLK_T_OUT;
>+    req.ioprio = 1;
>+    req.sector = 0;
>+    req.data = g_malloc0(512);
>+    strcpy(req.data, "TEST");
>+
>+    req_addr = virtio_blk_request(alloc, dev, &req, 512);
>+
>+    g_free(req.data);
>+
>+    free_head = qvirtqueue_add(qts, vq, req_addr, 16, false, true);
>+    qvirtqueue_add(qts, vq, req_addr + 16, 512, false, true);
>+    qvirtqueue_add(qts, vq, req_addr + 528, 1, true, false);
>+
>+    qvirtqueue_kick(qts, dev, vq, free_head);
>+
>+    qvirtio_wait_used_elem(qts, dev, vq, free_head, NULL,
>+                           QVIRTIO_BLK_TIMEOUT_US);
>+    status = readb(req_addr + 528);
>+    g_assert_cmpint(status, ==, 0);
>+
>+    guest_free(alloc, req_addr);
>+
>+    /* Read request */
>+    req.type = VIRTIO_BLK_T_IN;
>+    req.ioprio = 1;
>+    req.sector = 0;
>+    req.data = g_malloc0(512);
>+
>+    req_addr = virtio_blk_request(alloc, dev, &req, 512);
>+
>+    g_free(req.data);
>+
>+    free_head = qvirtqueue_add(qts, vq, req_addr, 16, false, true);
>+    qvirtqueue_add(qts, vq, req_addr + 16, 512, true, true);
>+    qvirtqueue_add(qts, vq, req_addr + 528, 1, true, false);
>+
>+    qvirtqueue_kick(qts, dev, vq, free_head);
>+
>+    qvirtio_wait_used_elem(qts, dev, vq, free_head, NULL,
>+                           QVIRTIO_BLK_TIMEOUT_US);
>+    status = readb(req_addr + 528);
>+    g_assert_cmpint(status, ==, 0);
>+
>+    data = g_malloc0(512);
>+    qtest_memread(qts, req_addr + 16, data, 512);
>+    g_assert_cmpstr(data, ==, "TEST");
>+    g_free(data);
>+
>+    guest_free(alloc, req_addr);
>+
>+    if (features & (1u << VIRTIO_BLK_F_WRITE_ZEROES)) {
>+        struct virtio_blk_discard_write_zeroes dwz_hdr;
>+        void *expected;
>+
>+        /*
>+         * WRITE_ZEROES request on the same sector of previous test where
>+         * we wrote "TEST".
>+         */
>+        req.type = VIRTIO_BLK_T_WRITE_ZEROES;
>+        req.data = (char *) &dwz_hdr;
>+        dwz_hdr.sector = 0;
>+        dwz_hdr.num_sectors = 1;
>+        dwz_hdr.flags = 0;
>+
>+        virtio_blk_fix_dwz_hdr(dev, &dwz_hdr);
>+
>+        req_addr = virtio_blk_request(alloc, dev, &req, sizeof(dwz_hdr));
>+
>+        free_head = qvirtqueue_add(qts, vq, req_addr, 16, false, true);
>+        qvirtqueue_add(qts, vq, req_addr + 16, sizeof(dwz_hdr), false, true);
>+        qvirtqueue_add(qts, vq, req_addr + 16 + sizeof(dwz_hdr), 1, true,
>+                       false);
>+
>+        qvirtqueue_kick(qts, dev, vq, free_head);
>+
>+        qvirtio_wait_used_elem(qts, dev, vq, free_head, NULL,
>+                               QVIRTIO_BLK_TIMEOUT_US);
>+        status = readb(req_addr + 16 + sizeof(dwz_hdr));
>+        g_assert_cmpint(status, ==, 0);
>+
>+        guest_free(alloc, req_addr);
>+
>+        /* Read request to check if the sector contains all zeroes */
>+        req.type = VIRTIO_BLK_T_IN;
>+        req.ioprio = 1;
>+        req.sector = 0;
>+        req.data = g_malloc0(512);
>+
>+        req_addr = virtio_blk_request(alloc, dev, &req, 512);
>+
>+        g_free(req.data);
>+
>+        free_head = qvirtqueue_add(qts, vq, req_addr, 16, false, true);
>+        qvirtqueue_add(qts, vq, req_addr + 16, 512, true, true);
>+        qvirtqueue_add(qts, vq, req_addr + 528, 1, true, false);
>+
>+        qvirtqueue_kick(qts, dev, vq, free_head);
>+
>+        qvirtio_wait_used_elem(qts, dev, vq, free_head, NULL,
>+                               QVIRTIO_BLK_TIMEOUT_US);
>+        status = readb(req_addr + 528);
>+        g_assert_cmpint(status, ==, 0);
>+
>+        data = g_malloc(512);
>+        expected = g_malloc0(512);
>+        qtest_memread(qts, req_addr + 16, data, 512);
>+        g_assert_cmpmem(data, 512, expected, 512);
>+        g_free(expected);
>+        g_free(data);
>+
>+        guest_free(alloc, req_addr);
>+    }
>+
>+    if (features & (1u << VIRTIO_BLK_F_DISCARD)) {
>+        struct virtio_blk_discard_write_zeroes dwz_hdr;
>+
>+        req.type = VIRTIO_BLK_T_DISCARD;
>+        req.data = (char *) &dwz_hdr;
>+        dwz_hdr.sector = 0;
>+        dwz_hdr.num_sectors = 1;
>+        dwz_hdr.flags = 0;
>+
>+        virtio_blk_fix_dwz_hdr(dev, &dwz_hdr);
>+
>+        req_addr = virtio_blk_request(alloc, dev, &req, sizeof(dwz_hdr));
>+
>+        free_head = qvirtqueue_add(qts, vq, req_addr, 16, false, true);
>+        qvirtqueue_add(qts, vq, req_addr + 16, sizeof(dwz_hdr), false, true);
>+        qvirtqueue_add(qts, vq, req_addr + 16 + sizeof(dwz_hdr),
>+                       1, true, false);
>+
>+        qvirtqueue_kick(qts, dev, vq, free_head);
>+
>+        qvirtio_wait_used_elem(qts, dev, vq, free_head, NULL,
>+                               QVIRTIO_BLK_TIMEOUT_US);
>+        status = readb(req_addr + 16 + sizeof(dwz_hdr));
>+        g_assert_cmpint(status, ==, 0);
>+
>+        guest_free(alloc, req_addr);
>+    }
>+
>+    if (features & (1u << VIRTIO_F_ANY_LAYOUT)) {
>+        /* Write and read with 2 descriptor layout */
>+        /* Write request */
>+        req.type = VIRTIO_BLK_T_OUT;
>+        req.ioprio = 1;
>+        req.sector = 1;
>+        req.data = g_malloc0(512);
>+        strcpy(req.data, "TEST");
>+
>+        req_addr = virtio_blk_request(alloc, dev, &req, 512);
>+
>+        g_free(req.data);
>+
>+        free_head = qvirtqueue_add(qts, vq, req_addr, 528, false, true);
>+        qvirtqueue_add(qts, vq, req_addr + 528, 1, true, false);
>+        qvirtqueue_kick(qts, dev, vq, free_head);
>+
>+        qvirtio_wait_used_elem(qts, dev, vq, free_head, NULL,
>+                               QVIRTIO_BLK_TIMEOUT_US);
>+        status = readb(req_addr + 528);
>+        g_assert_cmpint(status, ==, 0);
>+
>+        guest_free(alloc, req_addr);
>+
>+        /* Read request */
>+        req.type = VIRTIO_BLK_T_IN;
>+        req.ioprio = 1;
>+        req.sector = 1;
>+        req.data = g_malloc0(512);
>+
>+        req_addr = virtio_blk_request(alloc, dev, &req, 512);
>+
>+        g_free(req.data);
>+
>+        free_head = qvirtqueue_add(qts, vq, req_addr, 16, false, true);
>+        qvirtqueue_add(qts, vq, req_addr + 16, 513, true, false);
>+
>+        qvirtqueue_kick(qts, dev, vq, free_head);
>+
>+        qvirtio_wait_used_elem(qts, dev, vq, free_head, NULL,
>+                               QVIRTIO_BLK_TIMEOUT_US);
>+        status = readb(req_addr + 528);
>+        g_assert_cmpint(status, ==, 0);
>+
>+        data = g_malloc0(512);
>+        qtest_memread(qts, req_addr + 16, data, 512);
>+        g_assert_cmpstr(data, ==, "TEST");
>+        g_free(data);
>+
>+        guest_free(alloc, req_addr);
>+    }
>+
>+    return vq;
>+}
>+
>+static void basic(void *obj, void *data, QGuestAllocator *t_alloc)
>+{
>+    QVhostUserBlk *blk_if = obj;
>+    QVirtQueue *vq;
>+
>+    vq = test_basic(blk_if->vdev, t_alloc);
>+    qvirtqueue_cleanup(blk_if->vdev->bus, vq, t_alloc);
>+
>+}
>+
>+static void indirect(void *obj, void *u_data, QGuestAllocator *t_alloc)
>+{
>+    QVirtQueue *vq;
>+    QVhostUserBlk *blk_if = obj;
>+    QVirtioDevice *dev = blk_if->vdev;
>+    QVirtioBlkReq req;
>+    QVRingIndirectDesc *indirect;
>+    uint64_t req_addr;
>+    uint64_t capacity;
>+    uint64_t features;
>+    uint32_t free_head;
>+    uint8_t status;
>+    char *data;
>+    QTestState *qts = global_qtest;
>+
>+    features = qvirtio_get_features(dev);
>+    g_assert_cmphex(features & (1u << VIRTIO_RING_F_INDIRECT_DESC), !=, 0);
>+    features = features & ~(QVIRTIO_F_BAD_FEATURE |
>+                            (1u << VIRTIO_RING_F_EVENT_IDX) |
>+                            (1u << VIRTIO_BLK_F_SCSI));
>+    qvirtio_set_features(dev, features);
>+
>+    capacity = qvirtio_config_readq(dev, 0);
>+    g_assert_cmpint(capacity, ==, TEST_IMAGE_SIZE / 512);
>+
>+    vq = qvirtqueue_setup(dev, t_alloc, 0);
>+    qvirtio_set_driver_ok(dev);
>+
>+    /* Write request */
>+    req.type = VIRTIO_BLK_T_OUT;
>+    req.ioprio = 1;
>+    req.sector = 0;
>+    req.data = g_malloc0(512);
>+    strcpy(req.data, "TEST");
>+
>+    req_addr = virtio_blk_request(t_alloc, dev, &req, 512);
>+
>+    g_free(req.data);
>+
>+    indirect = qvring_indirect_desc_setup(qts, dev, t_alloc, 2);
>+    qvring_indirect_desc_add(dev, qts, indirect, req_addr, 528, false);
>+    qvring_indirect_desc_add(dev, qts, indirect, req_addr + 528, 1, true);
>+    free_head = qvirtqueue_add_indirect(qts, vq, indirect);
>+    qvirtqueue_kick(qts, dev, vq, free_head);
>+
>+    qvirtio_wait_used_elem(qts, dev, vq, free_head, NULL,
>+                           QVIRTIO_BLK_TIMEOUT_US);
>+    status = readb(req_addr + 528);
>+    g_assert_cmpint(status, ==, 0);
>+
>+    g_free(indirect);
>+    guest_free(t_alloc, req_addr);
>+
>+    /* Read request */
>+    req.type = VIRTIO_BLK_T_IN;
>+    req.ioprio = 1;
>+    req.sector = 0;
>+    req.data = g_malloc0(512);
>+    strcpy(req.data, "TEST");
>+
>+    req_addr = virtio_blk_request(t_alloc, dev, &req, 512);
>+
>+    g_free(req.data);
>+
>+    indirect = qvring_indirect_desc_setup(qts, dev, t_alloc, 2);
>+    qvring_indirect_desc_add(dev, qts, indirect, req_addr, 16, false);
>+    qvring_indirect_desc_add(dev, qts, indirect, req_addr + 16, 513, true);
>+    free_head = qvirtqueue_add_indirect(qts, vq, indirect);
>+    qvirtqueue_kick(qts, dev, vq, free_head);
>+
>+    qvirtio_wait_used_elem(qts, dev, vq, free_head, NULL,
>+                           QVIRTIO_BLK_TIMEOUT_US);
>+    status = readb(req_addr + 528);
>+    g_assert_cmpint(status, ==, 0);
>+
>+    data = g_malloc0(512);
>+    qtest_memread(qts, req_addr + 16, data, 512);
>+    g_assert_cmpstr(data, ==, "TEST");
>+    g_free(data);
>+
>+    g_free(indirect);
>+    guest_free(t_alloc, req_addr);
>+    qvirtqueue_cleanup(dev->bus, vq, t_alloc);
>+}
>+
>+static void idx(void *obj, void *u_data, QGuestAllocator *t_alloc)
>+{
>+    QVirtQueue *vq;
>+    QVhostUserBlkPCI *blk = obj;
>+    QVirtioPCIDevice *pdev = &blk->pci_vdev;
>+    QVirtioDevice *dev = &pdev->vdev;
>+    QVirtioBlkReq req;
>+    uint64_t req_addr;
>+    uint64_t capacity;
>+    uint64_t features;
>+    uint32_t free_head;
>+    uint32_t write_head;
>+    uint32_t desc_idx;
>+    uint8_t status;
>+    char *data;
>+    QOSGraphObject *blk_object = obj;
>+    QPCIDevice *pci_dev = blk_object->get_driver(blk_object, "pci-device");
>+    QTestState *qts = global_qtest;
>+
>+    if (qpci_check_buggy_msi(pci_dev)) {
>+        return;
>+    }
>+
>+    qpci_msix_enable(pdev->pdev);
>+    qvirtio_pci_set_msix_configuration_vector(pdev, t_alloc, 0);
>+
>+    features = qvirtio_get_features(dev);
>+    features = features & ~(QVIRTIO_F_BAD_FEATURE |
>+                            (1u << VIRTIO_RING_F_INDIRECT_DESC) |
>+                            (1u << VIRTIO_F_NOTIFY_ON_EMPTY) |
>+                            (1u << VIRTIO_BLK_F_SCSI));
>+    qvirtio_set_features(dev, features);
>+
>+    capacity = qvirtio_config_readq(dev, 0);
>+    g_assert_cmpint(capacity, ==, TEST_IMAGE_SIZE / 512);
>+
>+    vq = qvirtqueue_setup(dev, t_alloc, 0);
>+    qvirtqueue_pci_msix_setup(pdev, (QVirtQueuePCI *)vq, t_alloc, 1);
>+
>+    qvirtio_set_driver_ok(dev);
>+
>+    /* Write request */
>+    req.type = VIRTIO_BLK_T_OUT;
>+    req.ioprio = 1;
>+    req.sector = 0;
>+    req.data = g_malloc0(512);
>+    strcpy(req.data, "TEST");
>+
>+    req_addr = virtio_blk_request(t_alloc, dev, &req, 512);
>+
>+    g_free(req.data);
>+
>+    free_head = qvirtqueue_add(qts, vq, req_addr, 16, false, true);
>+    qvirtqueue_add(qts, vq, req_addr + 16, 512, false, true);
>+    qvirtqueue_add(qts, vq, req_addr + 528, 1, true, false);
>+    qvirtqueue_kick(qts, dev, vq, free_head);
>+
>+    qvirtio_wait_used_elem(qts, dev, vq, free_head, NULL,
>+                           QVIRTIO_BLK_TIMEOUT_US);
>+
>+    /* Write request */
>+    req.type = VIRTIO_BLK_T_OUT;
>+    req.ioprio = 1;
>+    req.sector = 1;
>+    req.data = g_malloc0(512);
>+    strcpy(req.data, "TEST");
>+
>+    req_addr = virtio_blk_request(t_alloc, dev, &req, 512);
>+
>+    g_free(req.data);
>+
>+    /* Notify after processing the third request */
>+    qvirtqueue_set_used_event(qts, vq, 2);
>+    free_head = qvirtqueue_add(qts, vq, req_addr, 16, false, true);
>+    qvirtqueue_add(qts, vq, req_addr + 16, 512, false, true);
>+    qvirtqueue_add(qts, vq, req_addr + 528, 1, true, false);
>+    qvirtqueue_kick(qts, dev, vq, free_head);
>+    write_head = free_head;
>+
>+    /* No notification expected */
>+    status = qvirtio_wait_status_byte_no_isr(qts, dev,
>+                                             vq, req_addr + 528,
>+                                             QVIRTIO_BLK_TIMEOUT_US);
>+    g_assert_cmpint(status, ==, 0);
>+
>+    guest_free(t_alloc, req_addr);
>+
>+    /* Read request */
>+    req.type = VIRTIO_BLK_T_IN;
>+    req.ioprio = 1;
>+    req.sector = 1;
>+    req.data = g_malloc0(512);
>+
>+    req_addr = virtio_blk_request(t_alloc, dev, &req, 512);
>+
>+    g_free(req.data);
>+
>+    free_head = qvirtqueue_add(qts, vq, req_addr, 16, false, true);
>+    qvirtqueue_add(qts, vq, req_addr + 16, 512, true, true);
>+    qvirtqueue_add(qts, vq, req_addr + 528, 1, true, false);
>+
>+    qvirtqueue_kick(qts, dev, vq, free_head);
>+
>+    /* We get just one notification for both requests */
>+    qvirtio_wait_used_elem(qts, dev, vq, write_head, NULL,
>+                           QVIRTIO_BLK_TIMEOUT_US);
>+    g_assert(qvirtqueue_get_buf(qts, vq, &desc_idx, NULL));
>+    g_assert_cmpint(desc_idx, ==, free_head);
>+
>+    status = readb(req_addr + 528);
>+    g_assert_cmpint(status, ==, 0);
>+
>+    data = g_malloc0(512);
>+    qtest_memread(qts, req_addr + 16, data, 512);
>+    g_assert_cmpstr(data, ==, "TEST");
>+    g_free(data);
>+
>+    guest_free(t_alloc, req_addr);
>+
>+    /* End test */
>+    qpci_msix_disable(pdev->pdev);
>+
>+    qvirtqueue_cleanup(dev->bus, vq, t_alloc);
>+}
>+
>+static void pci_hotplug(void *obj, void *data, QGuestAllocator *t_alloc)
>+{
>+    QVirtioPCIDevice *dev1 = obj;
>+    QVirtioPCIDevice *dev;
>+    QTestState *qts = dev1->pdev->bus->qts;
>+
>+    /* plug secondary disk */
>+    qtest_qmp_device_add(qts, "vhost-user-blk-pci", "drv1",
>+                         "{'addr': %s, 'chardev': 'char2'}",
>+                         stringify(PCI_SLOT_HP) ".0");
>+
>+    dev = virtio_pci_new(dev1->pdev->bus,
>+                         &(QPCIAddress) { .devfn = QPCI_DEVFN(PCI_SLOT_HP, 0)
>+                                        });
>+    g_assert_nonnull(dev);
>+    g_assert_cmpint(dev->vdev.device_type, ==, VIRTIO_ID_BLOCK);
>+    qvirtio_pci_device_disable(dev);
>+    qos_object_destroy((QOSGraphObject *)dev);
>+
>+    /* unplug secondary disk */
>+    qpci_unplug_acpi_device_test(qts, "drv1", PCI_SLOT_HP);
>+}
>+
>+/*
>+ * Check that setting the vring addr on a non-existent virtqueue does
>+ * not crash.
>+ */
>+static void test_nonexistent_virtqueue(void *obj, void *data,
>+                                       QGuestAllocator *t_alloc)
>+{
>+    QVhostUserBlkPCI *blk = obj;
>+    QVirtioPCIDevice *pdev = &blk->pci_vdev;
>+    QPCIBar bar0;
>+    QPCIDevice *dev;
>+
>+    dev = qpci_device_find(pdev->pdev->bus, QPCI_DEVFN(4, 0));
>+    g_assert(dev != NULL);
>+    qpci_device_enable(dev);
>+
>+    bar0 = qpci_iomap(dev, 0, NULL);
>+
>+    qpci_io_writeb(dev, bar0, VIRTIO_PCI_QUEUE_SEL, 2);
>+    qpci_io_writel(dev, bar0, VIRTIO_PCI_QUEUE_PFN, 1);
>+
>+    g_free(dev);
>+}
>+
>+static const char *qtest_qemu_storage_daemon_binary(void)
>+{
>+    const char *qemu_storage_daemon_bin;
>+
>+    qemu_storage_daemon_bin = getenv("QTEST_QEMU_STORAGE_DAEMON_BINARY");
>+    if (!qemu_storage_daemon_bin) {
>+        fprintf(stderr, "Environment variable "
>+                        "QTEST_QEMU_STORAGE_DAEMON_BINARY required\n");
>+        exit(0);
>+    }
>+
>+    return qemu_storage_daemon_bin;
>+}
>+
>+/* g_test_queue_destroy() cleanup function for files */
>+static void destroy_file(void *path)
>+{
>+    unlink(path);
>+    g_free(path);
>+    qos_invalidate_command_line();
>+}
>+
>+static char *drive_create(void)
>+{
>+    int fd, ret;
>+    /** vhost-user-blk won't recognize drive located in /tmp */
>+    char *t_path = g_strdup("qtest.XXXXXX");
>+
>+    /** Create a temporary raw image */
>+    fd = mkstemp(t_path);
>+    g_assert_cmpint(fd, >=, 0);
>+    ret = ftruncate(fd, TEST_IMAGE_SIZE);
>+    g_assert_cmpint(ret, ==, 0);
>+    close(fd);
>+
>+    g_test_queue_destroy(destroy_file, t_path);
>+    return t_path;
>+}
>+
>+static char *create_listen_socket(int *fd)
>+{
>+    int tmp_fd;
>+    char *path;
>+
>+    /* No race because our pid makes the path unique */
>+    path = g_strdup_printf("/tmp/qtest-%d-sock.XXXXXX", getpid());
>+    tmp_fd = mkstemp(path);
>+    g_assert_cmpint(tmp_fd, >=, 0);
>+    close(tmp_fd);
>+    unlink(path);
>+
>+    *fd = qtest_socket_server(path);
>+    g_test_queue_destroy(destroy_file, path);
>+    return path;
>+}
>+
>+/*
>+ * g_test_queue_destroy() and qtest_add_abrt_handler() cleanup function for
>+ * qemu-storage-daemon.
>+ */
>+static void quit_storage_daemon(void *data)
>+{
>+    QemuStorageDaemonState *qsd = data;
>+    int wstatus;
>+    pid_t pid;
>+
>+    /*
>+     * If we were invoked as a g_test_queue_destroy() cleanup function we need
>+     * to remove the abrt handler to avoid being called again if the code below
>+     * aborts. Also, we must not leave the abrt handler installed after
>+     * cleanup.
>+     */
>+    qtest_remove_abrt_handler(data);
>+

There is an issue. If a test fails, quit_storage_daemon won't be
called. Since there no abrt_handler like kill_qemu_hook_func
installed to stop qemu-storage-daemon, qemu-storage-daemon would keep
running after QEMU is killed by kill_qemu_hook_func.

>+    /* Before quitting storage-daemon, quit qemu to avoid dubious messages */
>+    qtest_kill_qemu(global_qtest);
>+
>+    kill(qsd->pid, SIGTERM);
>+    pid = waitpid(qsd->pid, &wstatus, 0);
>+    g_assert_cmpint(pid, ==, qsd->pid);
>+    if (!WIFEXITED(wstatus)) {
>+        fprintf(stderr, "%s: expected qemu-storage-daemon to exit\n",
>+                __func__);
>+        abort();
>+    }
>+    if (WEXITSTATUS(wstatus) != 0) {
>+        fprintf(stderr, "%s: expected qemu-storage-daemon to exit "
>+                "successfully, got %d\n",
>+                __func__, WEXITSTATUS(wstatus));
>+        abort();
>+    }
>+
>+    g_free(data);
>+}
>+
>+static void start_vhost_user_blk(GString *cmd_line, int vus_instances)
>+{
>+    const char *vhost_user_blk_bin = qtest_qemu_storage_daemon_binary();
>+    int i;
>+    gchar *img_path;
>+    GString *storage_daemon_command = g_string_new(NULL);
>+    QemuStorageDaemonState *qsd;
>+
>+    g_string_append_printf(storage_daemon_command,
>+                           "exec %s ",
>+                           vhost_user_blk_bin);
>+
>+    g_string_append_printf(cmd_line,
>+            " -object memory-backend-memfd,id=mem,size=256M,share=on "
>+            " -M memory-backend=mem ");
>+
>+    for (i = 0; i < vus_instances; i++) {
>+        int fd;
>+        char *sock_path = create_listen_socket(&fd);
>+
>+        /* create image file */
>+        img_path = drive_create();
>+        g_string_append_printf(storage_daemon_command,
>+            "--blockdev driver=file,node-name=disk%d,filename=%s "
>+            "--export type=vhost-user-blk,id=disk%d,addr.type=unix,addr.path=%s,"
>+            "node-name=disk%i,writable=on ",
>+            i, img_path, i, sock_path, i);
>+
>+        g_string_append_printf(cmd_line, "-chardev socket,id=char%d,path=%s ",
>+                               i + 1, sock_path);
>+    }
>+
>+    g_test_message("starting vhost-user backend: %s",
>+                   storage_daemon_command->str);
>+    pid_t pid = fork();
>+    if (pid == 0) {
>+        /*
>+         * Close standard file descriptors so tap-driver.pl pipe detects when
>+         * our parent terminates.
>+         */
>+        close(0);
>+        close(1);
>+        open("/dev/null", O_RDONLY);
>+        open("/dev/null", O_WRONLY);
>+
>+        execlp("/bin/sh", "sh", "-c", storage_daemon_command->str, NULL);
>+        exit(1);
>+    }
>+    g_string_free(storage_daemon_command, true);
>+
>+    qsd = g_new(QemuStorageDaemonState, 1);
>+    qsd->pid = pid;
>+
>+    /* Make sure qemu-storage-daemon is stopped */
>+    qtest_add_abrt_handler(quit_storage_daemon, qsd);
>+    g_test_queue_destroy(quit_storage_daemon, qsd);
>+}
>+
>+static void *vhost_user_blk_test_setup(GString *cmd_line, void *arg)
>+{
>+    start_vhost_user_blk(cmd_line, 1);
>+    return arg;
>+}
>+
>+/*
>+ * Setup for hotplug.
>+ *
>+ * Since vhost-user server only serves one vhost-user client one time,
>+ * another exprot
>+ *
>+ */
>+static void *vhost_user_blk_hotplug_test_setup(GString *cmd_line, void *arg)
>+{
>+    /* "-chardev socket,id=char2" is used for pci_hotplug*/
>+    start_vhost_user_blk(cmd_line, 2);
>+    return arg;
>+}
>+
>+static void register_vhost_user_blk_test(void)
>+{
>+    QOSGraphTestOptions opts = {
>+        .before = vhost_user_blk_test_setup,
>+    };
>+
>+    /*
>+     * tests for vhost-user-blk and vhost-user-blk-pci
>+     * The tests are borrowed from tests/virtio-blk-test.c. But some tests
>+     * regarding block_resize don't work for vhost-user-blk.
>+     * vhost-user-blk device doesn't have -drive, so tests containing
>+     * block_resize are also abandoned,
>+     *  - config
>+     *  - resize
>+     */
>+    qos_add_test("basic", "vhost-user-blk", basic, &opts);
>+    qos_add_test("indirect", "vhost-user-blk", indirect, &opts);
>+    qos_add_test("idx", "vhost-user-blk-pci", idx, &opts);
>+    qos_add_test("nxvirtq", "vhost-user-blk-pci",
>+                 test_nonexistent_virtqueue, &opts);
>+
>+    opts.before = vhost_user_blk_hotplug_test_setup;
>+    qos_add_test("hotplug", "vhost-user-blk-pci", pci_hotplug, &opts);
>+}
>+
>+libqos_init(register_vhost_user_blk_test);
>diff --git a/tests/qtest/libqos/meson.build b/tests/qtest/libqos/meson.build
>index 1cddf5bdaa..1f5c8f1053 100644
>--- a/tests/qtest/libqos/meson.build
>+++ b/tests/qtest/libqos/meson.build
>@@ -32,6 +32,7 @@ libqos_srcs = files('../libqtest.c',
>         'virtio-9p.c',
>         'virtio-balloon.c',
>         'virtio-blk.c',
>+        'vhost-user-blk.c',
>         'virtio-mmio.c',
>         'virtio-net.c',
>         'virtio-pci.c',
>diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build
>index c19f1c8503..39af1cddc4 100644
>--- a/tests/qtest/meson.build
>+++ b/tests/qtest/meson.build
>@@ -199,6 +199,9 @@ qos_test_ss.add(
> )
> qos_test_ss.add(when: 'CONFIG_VIRTFS', if_true: files('virtio-9p-test.c'))
> qos_test_ss.add(when: 'CONFIG_VHOST_USER', if_true: files('vhost-user-test.c'))
>+if have_vhost_user_blk_server
>+  qos_test_ss.add(files('vhost-user-blk-test.c'))
>+endif
>
> tpmemu_files = ['tpm-emu.c', 'tpm-util.c', 'tpm-tests.c']
>
>@@ -237,6 +240,7 @@ foreach dir : target_dirs
>   endif
>   qtest_env.set('G_TEST_DBUS_DAEMON', meson.source_root() / 'tests/dbus-vmstate-daemon.sh')
>   qtest_env.set('QTEST_QEMU_BINARY', './qemu-system-' + target_base)
>+  qtest_env.set('QTEST_QEMU_STORAGE_DAEMON_BINARY', './storage-daemon/qemu-storage-daemon')
>
>   foreach test : target_qtests
>     # Executables are shared across targets, declare them only the first time we
>--
>2.28.0
>

--
Best regards,
Coiby

Re: [PATCH v2 01/12] vhost-user-blk: fix blkcfg->num_queues endianness

[Raphael Norwitz]

Apologies for the late review - will get to the rest of the series as
soon as I can.

On Mon, Dec 7, 2020 at 12:31 PM Stefan Hajnoczi <stefanha@redhat.com> wrote:
>
> Treat the num_queues field as virtio-endian. On big-endian hosts the
> vhost-user-blk num_queues field was in the wrong endianness.
>
> Move the blkcfg.num_queues store operation from realize to
> vhost_user_blk_update_config() so feature negotiation has finished and
> we know the endianness of the device. VIRTIO 1.0 devices are
> little-endian, but in case someone wants to use legacy VIRTIO we support
> all endianness cases.
>
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>

Reviewed-by: Raphael Norwitz <raphael.norwitz@nutanix.com>


> ---
>  hw/block/vhost-user-blk.c | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c
> index 2dd3d93ca0..d9d9dc8a89 100644
> --- a/hw/block/vhost-user-blk.c
> +++ b/hw/block/vhost-user-blk.c
> @@ -53,6 +53,9 @@ static void vhost_user_blk_update_config(VirtIODevice *vdev, uint8_t *config)
>  {
>      VHostUserBlk *s = VHOST_USER_BLK(vdev);
>
> +    /* Our num_queues overrides the device backend */
> +    virtio_stw_p(vdev, &s->blkcfg.num_queues, s->num_queues);
> +
>      memcpy(config, &s->blkcfg, sizeof(struct virtio_blk_config));
>  }
>
> @@ -490,10 +493,6 @@ reconnect:
>          goto reconnect;
>      }
>
> -    if (s->blkcfg.num_queues != s->num_queues) {
> -        s->blkcfg.num_queues = s->num_queues;
> -    }
> -
>      return;
>
>  virtio_err:
> --
> 2.28.0
>

Re: [PATCH v2 02/12] libqtest: add qtest_socket_server()

[Wainer dos Santos Moschetta]

On 12/7/20 2:20 PM, Stefan Hajnoczi wrote:
> Add an API that returns a new UNIX domain socket in the listen state.
> The code for this was already there but only used internally in
> init_socket().
>
> This new API will be used by vhost-user-blk-test.
>
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
>   tests/qtest/libqos/libqtest.h |  8 +++++++
>   tests/qtest/libqtest.c        | 40 ++++++++++++++++++++---------------
>   2 files changed, 31 insertions(+), 17 deletions(-)


Reviewed-by: Wainer dos Santos Moschetta <wainersm@redhat.com>


>
> diff --git a/tests/qtest/libqos/libqtest.h b/tests/qtest/libqos/libqtest.h
> index 724f65aa94..e5f1ec590c 100644
> --- a/tests/qtest/libqos/libqtest.h
> +++ b/tests/qtest/libqos/libqtest.h
> @@ -132,6 +132,14 @@ void qtest_qmp_send(QTestState *s, const char *fmt, ...)
>   void qtest_qmp_send_raw(QTestState *s, const char *fmt, ...)
>       GCC_FMT_ATTR(2, 3);
>   
> +/**
> + * qtest_socket_server:
> + * @socket_path: the UNIX domain socket path
> + *
> + * Create and return a listen socket file descriptor, or abort on failure.
> + */
> +int qtest_socket_server(const char *socket_path);
> +
>   /**
>    * qtest_vqmp_fds:
>    * @s: #QTestState instance to operate on.
> diff --git a/tests/qtest/libqtest.c b/tests/qtest/libqtest.c
> index e49f3a1e45..bc389d422b 100644
> --- a/tests/qtest/libqtest.c
> +++ b/tests/qtest/libqtest.c
> @@ -81,24 +81,8 @@ static void qtest_client_set_rx_handler(QTestState *s, QTestRecvFn recv);
>   
>   static int init_socket(const char *socket_path)
>   {
> -    struct sockaddr_un addr;
> -    int sock;
> -    int ret;
> -
> -    sock = socket(PF_UNIX, SOCK_STREAM, 0);
> -    g_assert_cmpint(sock, !=, -1);
> -
> -    addr.sun_family = AF_UNIX;
> -    snprintf(addr.sun_path, sizeof(addr.sun_path), "%s", socket_path);
> +    int sock = qtest_socket_server(socket_path);
>       qemu_set_cloexec(sock);
> -
> -    do {
> -        ret = bind(sock, (struct sockaddr *)&addr, sizeof(addr));
> -    } while (ret == -1 && errno == EINTR);
> -    g_assert_cmpint(ret, !=, -1);
> -    ret = listen(sock, 1);
> -    g_assert_cmpint(ret, !=, -1);
> -
>       return sock;
>   }
>   
> @@ -636,6 +620,28 @@ QDict *qtest_qmp_receive_dict(QTestState *s)
>       return qmp_fd_receive(s->qmp_fd);
>   }
>   
> +int qtest_socket_server(const char *socket_path)
> +{
> +    struct sockaddr_un addr;
> +    int sock;
> +    int ret;
> +
> +    sock = socket(PF_UNIX, SOCK_STREAM, 0);
> +    g_assert_cmpint(sock, !=, -1);
> +
> +    addr.sun_family = AF_UNIX;
> +    snprintf(addr.sun_path, sizeof(addr.sun_path), "%s", socket_path);
> +
> +    do {
> +        ret = bind(sock, (struct sockaddr *)&addr, sizeof(addr));
> +    } while (ret == -1 && errno == EINTR);
> +    g_assert_cmpint(ret, !=, -1);
> +    ret = listen(sock, 1);
> +    g_assert_cmpint(ret, !=, -1);
> +
> +    return sock;
> +}
> +
>   /**
>    * Allow users to send a message without waiting for the reply,
>    * in the case that they choose to discard all replies up until

Re: [PATCH v2 03/12] libqtest: add qtest_kill_qemu()

[Wainer dos Santos Moschetta]

On 12/7/20 2:20 PM, Stefan Hajnoczi wrote:
> Tests that manage multiple processes may wish to kill QEMU before
> destroying the QTestState. Expose a function to do that.
>
> The vhost-user-blk-test testcase will need this.
>
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
>   tests/qtest/libqos/libqtest.h | 11 +++++++++++
>   tests/qtest/libqtest.c        |  7 ++++---
>   2 files changed, 15 insertions(+), 3 deletions(-)


Reviewed-by: Wainer dos Santos Moschetta <wainersm@redhat.com>


>
> diff --git a/tests/qtest/libqos/libqtest.h b/tests/qtest/libqos/libqtest.h
> index e5f1ec590c..51287b9276 100644
> --- a/tests/qtest/libqos/libqtest.h
> +++ b/tests/qtest/libqos/libqtest.h
> @@ -74,6 +74,17 @@ QTestState *qtest_init_without_qmp_handshake(const char *extra_args);
>    */
>   QTestState *qtest_init_with_serial(const char *extra_args, int *sock_fd);
>   
> +/**
> + * qtest_kill_qemu:
> + * @s: #QTestState instance to operate on.
> + *
> + * Kill the QEMU process and wait for it to terminate. It is safe to call this
> + * function multiple times. Normally qtest_quit() is used instead because it
> + * also frees QTestState. Use qtest_kill_qemu() when you just want to kill QEMU
> + * and qtest_quit() will be called later.
> + */
> +void qtest_kill_qemu(QTestState *s);
> +
>   /**
>    * qtest_quit:
>    * @s: #QTestState instance to operate on.
> diff --git a/tests/qtest/libqtest.c b/tests/qtest/libqtest.c
> index bc389d422b..cc2cec4a35 100644
> --- a/tests/qtest/libqtest.c
> +++ b/tests/qtest/libqtest.c
> @@ -133,7 +133,7 @@ void qtest_set_expected_status(QTestState *s, int status)
>       s->expected_status = status;
>   }
>   
> -static void kill_qemu(QTestState *s)
> +void qtest_kill_qemu(QTestState *s)
>   {
>       pid_t pid = s->qemu_pid;
>       int wstatus;
> @@ -143,6 +143,7 @@ static void kill_qemu(QTestState *s)
>           kill(pid, SIGTERM);
>           TFR(pid = waitpid(s->qemu_pid, &s->wstatus, 0));
>           assert(pid == s->qemu_pid);
> +        s->qemu_pid = -1;
>       }
>   
>       /*
> @@ -169,7 +170,7 @@ static void kill_qemu(QTestState *s)
>   
>   static void kill_qemu_hook_func(void *s)
>   {
> -    kill_qemu(s);
> +    qtest_kill_qemu(s);
>   }
>   
>   static void sigabrt_handler(int signo)
> @@ -373,7 +374,7 @@ void qtest_quit(QTestState *s)
>       /* Uninstall SIGABRT handler on last instance */
>       cleanup_sigabrt_handler();
>   
> -    kill_qemu(s);
> +    qtest_kill_qemu(s);
>       close(s->fd);
>       close(s->qmp_fd);
>       g_string_free(s->rx, true);

Re: [PATCH v2 04/12] libqtest: add qtest_remove_abrt_handler()

[Wainer dos Santos Moschetta]

On 12/7/20 2:20 PM, Stefan Hajnoczi wrote:
> Add a function to remove previously-added abrt handler functions.
>
> Now that a symmetric pair of add/remove functions exists we can also
> balance the SIGABRT handler installation. The signal handler was
> installed each time qtest_add_abrt_handler() was called. Now it is
> installed when the abrt handler list becomes non-empty and removed again
> when the list becomes empty.
>
> The qtest_remove_abrt_handler() function will be used by
> vhost-user-blk-test.
>
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
>   tests/qtest/libqos/libqtest.h | 18 ++++++++++++++++++
>   tests/qtest/libqtest.c        | 35 +++++++++++++++++++++++++++++------
>   2 files changed, 47 insertions(+), 6 deletions(-)


Reviewed-by: Wainer dos Santos Moschetta <wainersm@redhat.com>


>
> diff --git a/tests/qtest/libqos/libqtest.h b/tests/qtest/libqos/libqtest.h
> index 51287b9276..a68dcd79d4 100644
> --- a/tests/qtest/libqos/libqtest.h
> +++ b/tests/qtest/libqos/libqtest.h
> @@ -649,8 +649,26 @@ void qtest_add_data_func_full(const char *str, void *data,
>           g_free(path); \
>       } while (0)
>   
> +/**
> + * qtest_add_abrt_handler:
> + * @fn: Handler function
> + * @data: Argument that is passed to the handler
> + *
> + * Add a handler function that is invoked on SIGABRT. This can be used to
> + * terminate processes and perform other cleanup. The handler can be removed
> + * with qtest_remove_abrt_handler().
> + */
>   void qtest_add_abrt_handler(GHookFunc fn, const void *data);
>   
> +/**
> + * qtest_remove_abrt_handler:
> + * @data: Argument previously passed to qtest_add_abrt_handler()
> + *
> + * Remove an abrt handler that was previously added with
> + * qtest_add_abrt_handler().
> + */
> +void qtest_remove_abrt_handler(void *data);
> +
>   /**
>    * qtest_qmp_assert_success:
>    * @qts: QTestState instance to operate on
> diff --git a/tests/qtest/libqtest.c b/tests/qtest/libqtest.c
> index cc2cec4a35..7cf247baf0 100644
> --- a/tests/qtest/libqtest.c
> +++ b/tests/qtest/libqtest.c
> @@ -196,15 +196,30 @@ static void cleanup_sigabrt_handler(void)
>       sigaction(SIGABRT, &sigact_old, NULL);
>   }
>   
> +static bool hook_list_is_empty(GHookList *hook_list)
> +{
> +    GHook *hook = g_hook_first_valid(hook_list, TRUE);
> +
> +    if (!hook) {
> +        return false;
> +    }
> +
> +    g_hook_unref(hook_list, hook);
> +    return true;
> +}
> +
>   void qtest_add_abrt_handler(GHookFunc fn, const void *data)
>   {
>       GHook *hook;
>   
> -    /* Only install SIGABRT handler once */
>       if (!abrt_hooks.is_setup) {
>           g_hook_list_init(&abrt_hooks, sizeof(GHook));
>       }
> -    setup_sigabrt_handler();
> +
> +    /* Only install SIGABRT handler once */
> +    if (hook_list_is_empty(&abrt_hooks)) {
> +        setup_sigabrt_handler();
> +    }
>   
>       hook = g_hook_alloc(&abrt_hooks);
>       hook->func = fn;
> @@ -213,6 +228,17 @@ void qtest_add_abrt_handler(GHookFunc fn, const void *data)
>       g_hook_prepend(&abrt_hooks, hook);
>   }
>   
> +void qtest_remove_abrt_handler(void *data)
> +{
> +    GHook *hook = g_hook_find_data(&abrt_hooks, TRUE, data);
> +    g_hook_destroy_link(&abrt_hooks, hook);
> +
> +    /* Uninstall SIGABRT handler on last instance */
> +    if (hook_list_is_empty(&abrt_hooks)) {
> +        cleanup_sigabrt_handler();
> +    }
> +}
> +
>   static const char *qtest_qemu_binary(void)
>   {
>       const char *qemu_bin;
> @@ -369,10 +395,7 @@ QTestState *qtest_init_with_serial(const char *extra_args, int *sock_fd)
>   
>   void qtest_quit(QTestState *s)
>   {
> -    g_hook_destroy_link(&abrt_hooks, g_hook_find_data(&abrt_hooks, TRUE, s));
> -
> -    /* Uninstall SIGABRT handler on last instance */
> -    cleanup_sigabrt_handler();
> +    qtest_remove_abrt_handler(s);
>   
>       qtest_kill_qemu(s);
>       close(s->fd);

Re: [PATCH v2 01/12] vhost-user-blk: fix blkcfg->num_queues endianness

[Michael S. Tsirkin]

On Mon, Dec 07, 2020 at 05:20:19PM +0000, Stefan Hajnoczi wrote:
> Treat the num_queues field as virtio-endian. On big-endian hosts the
> vhost-user-blk num_queues field was in the wrong endianness.
> 
> Move the blkcfg.num_queues store operation from realize to
> vhost_user_blk_update_config() so feature negotiation has finished and
> we know the endianness of the device. VIRTIO 1.0 devices are
> little-endian, but in case someone wants to use legacy VIRTIO we support
> all endianness cases.
> 
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>

Reviewed-by: Michael S. Tsirkin <mst@redhat.com>

Pls merge with rest of the series.
And maybe CC stable?



> ---
>  hw/block/vhost-user-blk.c | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c
> index 2dd3d93ca0..d9d9dc8a89 100644
> --- a/hw/block/vhost-user-blk.c
> +++ b/hw/block/vhost-user-blk.c
> @@ -53,6 +53,9 @@ static void vhost_user_blk_update_config(VirtIODevice *vdev, uint8_t *config)
>  {
>      VHostUserBlk *s = VHOST_USER_BLK(vdev);
>  
> +    /* Our num_queues overrides the device backend */
> +    virtio_stw_p(vdev, &s->blkcfg.num_queues, s->num_queues);
> +
>      memcpy(config, &s->blkcfg, sizeof(struct virtio_blk_config));
>  }
>  
> @@ -490,10 +493,6 @@ reconnect:
>          goto reconnect;
>      }
>  
> -    if (s->blkcfg.num_queues != s->num_queues) {
> -        s->blkcfg.num_queues = s->num_queues;
> -    }
> -
>      return;
>  
>  virtio_err:
> -- 
> 2.28.0
> 

Re: [PATCH v2 00/12] block/export: vhost-user-blk server tests and input validation

[Kevin Wolf]

Am 07.12.2020 um 18:20 hat Stefan Hajnoczi geschrieben:
> v2:
>  * Add abrt handler that terminates qemu-storage-daemon to
>    vhost-user-blk-test. No more orphaned processes on test failure. [Peter]
>  * Fix sector number calculation in vhost-user-blk-server.c
>  * Introduce VIRTIO_BLK_SECTOR_BITS/SIZE to make code clearer [Max]
>  * Fix vhost-user-blk-server.c blk_size double byteswap
>  * Fix vhost-user-blk blkcfg->num_queues endianness [Peter]
>  * Squashed cleanups into Coiby vhost-user-blk-test commit so the code is
>    easier to review
> 
> The vhost-user-blk server test was already in Michael Tsirkin's recent vhost
> pull request, but was dropped because it exposed vhost-user regressions
> (b7c1bd9d7848 and the Based-on tag below). Now that the vhost-user regressions
> are fixed we can re-introduce the test case.
> 
> This series adds missing input validation that led to a Coverity report. The
> virtio-blk read, write, discard, and write zeroes commands need to check
> sector/byte ranges and other inputs. This solves the issue Peter Maydell raised
> in "[PATCH for-5.2] block/export/vhost-user-blk-server.c: Avoid potential
> integer overflow".
> 
> Merging just the input validation patches would be possible too, but I prefer
> to merge the corresponding tests so the code is exercised by the CI.

Is this series still open? I don't see it in master.

Kevin

Re: [PATCH v2 00/12] block/export: vhost-user-blk server tests and input validation

[Peter Maydell]

On Mon, 15 Feb 2021 at 10:41, Kevin Wolf <kwolf@redhat.com> wrote:
>
> Am 07.12.2020 um 18:20 hat Stefan Hajnoczi geschrieben:
> > v2:
> >  * Add abrt handler that terminates qemu-storage-daemon to
> >    vhost-user-blk-test. No more orphaned processes on test failure. [Peter]
> >  * Fix sector number calculation in vhost-user-blk-server.c
> >  * Introduce VIRTIO_BLK_SECTOR_BITS/SIZE to make code clearer [Max]
> >  * Fix vhost-user-blk-server.c blk_size double byteswap
> >  * Fix vhost-user-blk blkcfg->num_queues endianness [Peter]
> >  * Squashed cleanups into Coiby vhost-user-blk-test commit so the code is
> >    easier to review
> >
> > The vhost-user-blk server test was already in Michael Tsirkin's recent vhost
> > pull request, but was dropped because it exposed vhost-user regressions
> > (b7c1bd9d7848 and the Based-on tag below). Now that the vhost-user regressions
> > are fixed we can re-introduce the test case.
> >
> > This series adds missing input validation that led to a Coverity report. The
> > virtio-blk read, write, discard, and write zeroes commands need to check
> > sector/byte ranges and other inputs. This solves the issue Peter Maydell raised
> > in "[PATCH for-5.2] block/export/vhost-user-blk-server.c: Avoid potential
> > integer overflow".
> >
> > Merging just the input validation patches would be possible too, but I prefer
> > to merge the corresponding tests so the code is exercised by the CI.
>
> Is this series still open? I don't see it in master.

The Coverity issue is still unfixed, at any rate...

-- PMM

Re: [PATCH v2 00/12] block/export: vhost-user-blk server tests and input validation

[Philippe Mathieu-Daudé]

On 2/19/21 11:38 PM, Peter Maydell wrote:
> On Mon, 15 Feb 2021 at 10:41, Kevin Wolf <kwolf@redhat.com> wrote:
>>
>> Am 07.12.2020 um 18:20 hat Stefan Hajnoczi geschrieben:
>>> v2:
>>>  * Add abrt handler that terminates qemu-storage-daemon to
>>>    vhost-user-blk-test. No more orphaned processes on test failure. [Peter]
>>>  * Fix sector number calculation in vhost-user-blk-server.c
>>>  * Introduce VIRTIO_BLK_SECTOR_BITS/SIZE to make code clearer [Max]
>>>  * Fix vhost-user-blk-server.c blk_size double byteswap
>>>  * Fix vhost-user-blk blkcfg->num_queues endianness [Peter]
>>>  * Squashed cleanups into Coiby vhost-user-blk-test commit so the code is
>>>    easier to review
>>>
>>> The vhost-user-blk server test was already in Michael Tsirkin's recent vhost
>>> pull request, but was dropped because it exposed vhost-user regressions
>>> (b7c1bd9d7848 and the Based-on tag below). Now that the vhost-user regressions
>>> are fixed we can re-introduce the test case.
>>>
>>> This series adds missing input validation that led to a Coverity report. The
>>> virtio-blk read, write, discard, and write zeroes commands need to check
>>> sector/byte ranges and other inputs. This solves the issue Peter Maydell raised
>>> in "[PATCH for-5.2] block/export/vhost-user-blk-server.c: Avoid potential
>>> integer overflow".
>>>
>>> Merging just the input validation patches would be possible too, but I prefer
>>> to merge the corresponding tests so the code is exercised by the CI.
>>
>> Is this series still open? I don't see it in master.
> 
> The Coverity issue is still unfixed, at any rate...

Copying Coverity report here:

CID 1435956 Unintentional integer overflow

In vu_blk_discard_write_zeroes: An integer overflow occurs, with the
result converted to a wider integer type (CWE-190)

 61 static int coroutine_fn
 62 vu_blk_discard_write_zeroes(BlockBackend *blk, struct iovec *iov,
 63                             uint32_t iovcnt, uint32_t type)
 64 {
 65     struct virtio_blk_discard_write_zeroes desc;
 66     ssize_t size = iov_to_buf(iov, iovcnt, 0, &desc, sizeof(desc));
 67     if (unlikely(size != sizeof(desc))) {
 68         error_report("Invalid size %zd, expect %zu", size,
sizeof(desc));
 69         return -EINVAL;
 70     }
 71
 72     uint64_t range[2] = { le64_to_cpu(desc.sector) << 9,

CID 1435956 (#1 of 1): Unintentional integer overflow
(OVERFLOW_BEFORE_WIDEN)
overflow_before_widen: Potentially overflowing expression
le32_to_cpu(desc.num_sectors) << 9 with type uint32_t (32 bits,
unsigned) is evaluated using 32-bit arithmetic, and then used in a
context that expects an expression of type uint64_t (64 bits, unsigned).

 73                           le32_to_cpu(desc.num_sectors) << 9 };

Re: [PATCH v2 05/12] test: new qTest case to test the vhost-user-blk-server

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 33550 bytes --]

On Fri, Dec 18, 2020 at 10:56:32PM +0800, Coiby Xu wrote:
> On Mon, Dec 07, 2020 at 05:20:23PM +0000, Stefan Hajnoczi wrote:
> > From: Coiby Xu <coiby.xu@gmail.com>
> > 
> > This test case has the same tests as tests/virtio-blk-test.c except for
> > tests have block_resize. Since the vhost-user-blk export only serves one
> > client one time, two exports are started by qemu-storage-daemon for the
> > hotplug test.
> > 
> > Suggested-by: Thomas Huth <thuth@redhat.com>
> > Signed-off-by: Coiby Xu <coiby.xu@gmail.com>
> > Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> > ---
> > MAINTAINERS                         |   2 +
> > tests/qtest/libqos/vhost-user-blk.h |  48 ++
> > tests/qtest/libqos/vhost-user-blk.c | 130 +++++
> > tests/qtest/vhost-user-blk-test.c   | 788 ++++++++++++++++++++++++++++
> > tests/qtest/libqos/meson.build      |   1 +
> > tests/qtest/meson.build             |   4 +
> > 6 files changed, 973 insertions(+)
> > create mode 100644 tests/qtest/libqos/vhost-user-blk.h
> > create mode 100644 tests/qtest/libqos/vhost-user-blk.c
> > create mode 100644 tests/qtest/vhost-user-blk-test.c
> > 
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index 68bc160f41..d351280d1f 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -3103,6 +3103,8 @@ F: block/export/vhost-user-blk-server.c
> > F: block/export/vhost-user-blk-server.h
> > F: include/qemu/vhost-user-server.h
> > F: tests/qtest/libqos/vhost-user-blk.c
> > +F: tests/qtest/libqos/vhost-user-blk.h
> > +F: tests/qtest/vhost-user-blk-test.c
> > F: util/vhost-user-server.c
> > 
> > Replication
> > diff --git a/tests/qtest/libqos/vhost-user-blk.h b/tests/qtest/libqos/vhost-user-blk.h
> > new file mode 100644
> > index 0000000000..2a03456a45
> > --- /dev/null
> > +++ b/tests/qtest/libqos/vhost-user-blk.h
> > @@ -0,0 +1,48 @@
> > +/*
> > + * libqos driver framework
> > + *
> > + * Based on tests/qtest/libqos/virtio-blk.c
> > + *
> > + * Copyright (c) 2020 Coiby Xu <coiby.xu@gmail.com>
> > + *
> > + * Copyright (c) 2018 Emanuele Giuseppe Esposito <e.emanuelegiuseppe@gmail.com>
> > + *
> > + * This library is free software; you can redistribute it and/or
> > + * modify it under the terms of the GNU Lesser General Public
> > + * License version 2 as published by the Free Software Foundation.
> > + *
> > + * This library 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
> > + * Lesser General Public License for more details.
> > + *
> > + * You should have received a copy of the GNU Lesser General Public
> > + * License along with this library; if not, see <http://www.gnu.org/licenses/>
> > + */
> > +
> > +#ifndef TESTS_LIBQOS_VHOST_USER_BLK_H
> > +#define TESTS_LIBQOS_VHOST_USER_BLK_H
> > +
> > +#include "qgraph.h"
> > +#include "virtio.h"
> > +#include "virtio-pci.h"
> > +
> > +typedef struct QVhostUserBlk QVhostUserBlk;
> > +typedef struct QVhostUserBlkPCI QVhostUserBlkPCI;
> > +typedef struct QVhostUserBlkDevice QVhostUserBlkDevice;
> > +
> > +struct QVhostUserBlk {
> > +    QVirtioDevice *vdev;
> > +};
> > +
> > +struct QVhostUserBlkPCI {
> > +    QVirtioPCIDevice pci_vdev;
> > +    QVhostUserBlk blk;
> > +};
> > +
> > +struct QVhostUserBlkDevice {
> > +    QOSGraphObject obj;
> > +    QVhostUserBlk blk;
> > +};
> > +
> > +#endif
> > diff --git a/tests/qtest/libqos/vhost-user-blk.c b/tests/qtest/libqos/vhost-user-blk.c
> > new file mode 100644
> > index 0000000000..568c3426ed
> > --- /dev/null
> > +++ b/tests/qtest/libqos/vhost-user-blk.c
> > @@ -0,0 +1,130 @@
> > +/*
> > + * libqos driver framework
> > + *
> > + * Based on tests/qtest/libqos/virtio-blk.c
> > + *
> > + * Copyright (c) 2020 Coiby Xu <coiby.xu@gmail.com>
> > + *
> > + * Copyright (c) 2018 Emanuele Giuseppe Esposito <e.emanuelegiuseppe@gmail.com>
> > + *
> > + * This library is free software; you can redistribute it and/or
> > + * modify it under the terms of the GNU Lesser General Public
> > + * License version 2.1 as published by the Free Software Foundation.
> > + *
> > + * This library 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
> > + * Lesser General Public License for more details.
> > + *
> > + * You should have received a copy of the GNU Lesser General Public
> > + * License along with this library; if not, see <http://www.gnu.org/licenses/>
> > + */
> > +
> > +#include "qemu/osdep.h"
> > +#include "libqtest.h"
> > +#include "qemu/module.h"
> > +#include "standard-headers/linux/virtio_blk.h"
> > +#include "vhost-user-blk.h"
> > +
> > +#define PCI_SLOT                0x04
> > +#define PCI_FN                  0x00
> > +
> > +/* virtio-blk-device */
> > +static void *qvhost_user_blk_get_driver(QVhostUserBlk *v_blk,
> > +                                    const char *interface)
> > +{
> > +    if (!g_strcmp0(interface, "vhost-user-blk")) {
> > +        return v_blk;
> > +    }
> > +    if (!g_strcmp0(interface, "virtio")) {
> > +        return v_blk->vdev;
> > +    }
> > +
> > +    fprintf(stderr, "%s not present in vhost-user-blk-device\n", interface);
> > +    g_assert_not_reached();
> > +}
> > +
> > +static void *qvhost_user_blk_device_get_driver(void *object,
> > +                                           const char *interface)
> > +{
> > +    QVhostUserBlkDevice *v_blk = object;
> > +    return qvhost_user_blk_get_driver(&v_blk->blk, interface);
> > +}
> > +
> > +static void *vhost_user_blk_device_create(void *virtio_dev,
> > +                                      QGuestAllocator *t_alloc,
> > +                                      void *addr)
> > +{
> > +    QVhostUserBlkDevice *vhost_user_blk = g_new0(QVhostUserBlkDevice, 1);
> > +    QVhostUserBlk *interface = &vhost_user_blk->blk;
> > +
> > +    interface->vdev = virtio_dev;
> > +
> > +    vhost_user_blk->obj.get_driver = qvhost_user_blk_device_get_driver;
> > +
> > +    return &vhost_user_blk->obj;
> > +}
> > +
> > +/* virtio-blk-pci */
> > +static void *qvhost_user_blk_pci_get_driver(void *object, const char *interface)
> > +{
> > +    QVhostUserBlkPCI *v_blk = object;
> > +    if (!g_strcmp0(interface, "pci-device")) {
> > +        return v_blk->pci_vdev.pdev;
> > +    }
> > +    return qvhost_user_blk_get_driver(&v_blk->blk, interface);
> > +}
> > +
> > +static void *vhost_user_blk_pci_create(void *pci_bus, QGuestAllocator *t_alloc,
> > +                                      void *addr)
> > +{
> > +    QVhostUserBlkPCI *vhost_user_blk = g_new0(QVhostUserBlkPCI, 1);
> > +    QVhostUserBlk *interface = &vhost_user_blk->blk;
> > +    QOSGraphObject *obj = &vhost_user_blk->pci_vdev.obj;
> > +
> > +    virtio_pci_init(&vhost_user_blk->pci_vdev, pci_bus, addr);
> > +    interface->vdev = &vhost_user_blk->pci_vdev.vdev;
> > +
> > +    g_assert_cmphex(interface->vdev->device_type, ==, VIRTIO_ID_BLOCK);
> > +
> > +    obj->get_driver = qvhost_user_blk_pci_get_driver;
> > +
> > +    return obj;
> > +}
> > +
> > +static void vhost_user_blk_register_nodes(void)
> > +{
> > +    /*
> > +     * FIXME: every test using these two nodes needs to setup a
> > +     * -drive,id=drive0 otherwise QEMU is not going to start.
> > +     * Therefore, we do not include "produces" edge for virtio
> > +     * and pci-device yet.
> > +     */
> > +
> > +    char *arg = g_strdup_printf("id=drv0,chardev=char1,addr=%x.%x",
> > +                                PCI_SLOT, PCI_FN);
> > +
> > +    QPCIAddress addr = {
> > +        .devfn = QPCI_DEVFN(PCI_SLOT, PCI_FN),
> > +    };
> > +
> > +    QOSGraphEdgeOptions opts = { };
> > +
> > +    /* virtio-blk-device */
> > +    /** opts.extra_device_opts = "drive=drive0"; */
> > +    qos_node_create_driver("vhost-user-blk-device",
> > +                           vhost_user_blk_device_create);
> > +    qos_node_consumes("vhost-user-blk-device", "virtio-bus", &opts);
> > +    qos_node_produces("vhost-user-blk-device", "vhost-user-blk");
> > +
> > +    /* virtio-blk-pci */
> > +    opts.extra_device_opts = arg;
> > +    add_qpci_address(&opts, &addr);
> > +    qos_node_create_driver("vhost-user-blk-pci", vhost_user_blk_pci_create);
> > +    qos_node_consumes("vhost-user-blk-pci", "pci-bus", &opts);
> > +    qos_node_produces("vhost-user-blk-pci", "vhost-user-blk");
> > +
> > +    g_free(arg);
> > +}
> > +
> > +libqos_init(vhost_user_blk_register_nodes);
> > diff --git a/tests/qtest/vhost-user-blk-test.c b/tests/qtest/vhost-user-blk-test.c
> > new file mode 100644
> > index 0000000000..175dccab98
> > --- /dev/null
> > +++ b/tests/qtest/vhost-user-blk-test.c
> > @@ -0,0 +1,788 @@
> > +/*
> > + * QTest testcase for Vhost-user Block Device
> > + *
> > + * Based on tests/qtest//virtio-blk-test.c
> > +
> > + * Copyright (c) 2014 SUSE LINUX Products GmbH
> > + * Copyright (c) 2014 Marc Marí
> > + * Copyright (c) 2020 Coiby Xu
> > + *
> > + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> > + * See the COPYING file in the top-level directory.
> > + */
> > +
> > +#include "qemu/osdep.h"
> > +#include "libqtest-single.h"
> > +#include "qemu/bswap.h"
> > +#include "qemu/module.h"
> > +#include "standard-headers/linux/virtio_blk.h"
> > +#include "standard-headers/linux/virtio_pci.h"
> > +#include "libqos/qgraph.h"
> > +#include "libqos/vhost-user-blk.h"
> > +#include "libqos/libqos-pc.h"
> > +
> > +#define TEST_IMAGE_SIZE         (64 * 1024 * 1024)
> > +#define QVIRTIO_BLK_TIMEOUT_US  (30 * 1000 * 1000)
> > +#define PCI_SLOT_HP             0x06
> > +
> > +typedef struct {
> > +    pid_t pid;
> > +} QemuStorageDaemonState;
> > +
> > +typedef struct QVirtioBlkReq {
> > +    uint32_t type;
> > +    uint32_t ioprio;
> > +    uint64_t sector;
> > +    char *data;
> > +    uint8_t status;
> > +} QVirtioBlkReq;
> > +
> > +#ifdef HOST_WORDS_BIGENDIAN
> > +static const bool host_is_big_endian = true;
> > +#else
> > +static const bool host_is_big_endian; /* false */
> > +#endif
> > +
> > +static inline void virtio_blk_fix_request(QVirtioDevice *d, QVirtioBlkReq *req)
> > +{
> > +    if (qvirtio_is_big_endian(d) != host_is_big_endian) {
> > +        req->type = bswap32(req->type);
> > +        req->ioprio = bswap32(req->ioprio);
> > +        req->sector = bswap64(req->sector);
> > +    }
> > +}
> > +
> > +static inline void virtio_blk_fix_dwz_hdr(QVirtioDevice *d,
> > +    struct virtio_blk_discard_write_zeroes *dwz_hdr)
> > +{
> > +    if (qvirtio_is_big_endian(d) != host_is_big_endian) {
> > +        dwz_hdr->sector = bswap64(dwz_hdr->sector);
> > +        dwz_hdr->num_sectors = bswap32(dwz_hdr->num_sectors);
> > +        dwz_hdr->flags = bswap32(dwz_hdr->flags);
> > +    }
> > +}
> > +
> > +static uint64_t virtio_blk_request(QGuestAllocator *alloc, QVirtioDevice *d,
> > +                                   QVirtioBlkReq *req, uint64_t data_size)
> > +{
> > +    uint64_t addr;
> > +    uint8_t status = 0xFF;
> > +    QTestState *qts = global_qtest;
> > +
> > +    switch (req->type) {
> > +    case VIRTIO_BLK_T_IN:
> > +    case VIRTIO_BLK_T_OUT:
> > +        g_assert_cmpuint(data_size % 512, ==, 0);
> > +        break;
> > +    case VIRTIO_BLK_T_DISCARD:
> > +    case VIRTIO_BLK_T_WRITE_ZEROES:
> > +        g_assert_cmpuint(data_size %
> > +                         sizeof(struct virtio_blk_discard_write_zeroes), ==, 0);
> > +        break;
> > +    default:
> > +        g_assert_cmpuint(data_size, ==, 0);
> > +    }
> > +
> > +    addr = guest_alloc(alloc, sizeof(*req) + data_size);
> > +
> > +    virtio_blk_fix_request(d, req);
> > +
> > +    qtest_memwrite(qts, addr, req, 16);
> > +    qtest_memwrite(qts, addr + 16, req->data, data_size);
> > +    qtest_memwrite(qts, addr + 16 + data_size, &status, sizeof(status));
> > +
> > +    return addr;
> > +}
> > +
> > +/* Returns the request virtqueue so the caller can perform further tests */
> > +static QVirtQueue *test_basic(QVirtioDevice *dev, QGuestAllocator *alloc)
> > +{
> > +    QVirtioBlkReq req;
> > +    uint64_t req_addr;
> > +    uint64_t capacity;
> > +    uint64_t features;
> > +    uint32_t free_head;
> > +    uint8_t status;
> > +    char *data;
> > +    QTestState *qts = global_qtest;
> > +    QVirtQueue *vq;
> > +
> > +    features = qvirtio_get_features(dev);
> > +    features = features & ~(QVIRTIO_F_BAD_FEATURE |
> > +                    (1u << VIRTIO_RING_F_INDIRECT_DESC) |
> > +                    (1u << VIRTIO_RING_F_EVENT_IDX) |
> > +                    (1u << VIRTIO_BLK_F_SCSI));
> > +    qvirtio_set_features(dev, features);
> > +
> > +    capacity = qvirtio_config_readq(dev, 0);
> > +    g_assert_cmpint(capacity, ==, TEST_IMAGE_SIZE / 512);
> > +
> > +    vq = qvirtqueue_setup(dev, alloc, 0);
> > +
> > +    qvirtio_set_driver_ok(dev);
> > +
> > +    /* Write and read with 3 descriptor layout */
> > +    /* Write request */
> > +    req.type = VIRTIO_BLK_T_OUT;
> > +    req.ioprio = 1;
> > +    req.sector = 0;
> > +    req.data = g_malloc0(512);
> > +    strcpy(req.data, "TEST");
> > +
> > +    req_addr = virtio_blk_request(alloc, dev, &req, 512);
> > +
> > +    g_free(req.data);
> > +
> > +    free_head = qvirtqueue_add(qts, vq, req_addr, 16, false, true);
> > +    qvirtqueue_add(qts, vq, req_addr + 16, 512, false, true);
> > +    qvirtqueue_add(qts, vq, req_addr + 528, 1, true, false);
> > +
> > +    qvirtqueue_kick(qts, dev, vq, free_head);
> > +
> > +    qvirtio_wait_used_elem(qts, dev, vq, free_head, NULL,
> > +                           QVIRTIO_BLK_TIMEOUT_US);
> > +    status = readb(req_addr + 528);
> > +    g_assert_cmpint(status, ==, 0);
> > +
> > +    guest_free(alloc, req_addr);
> > +
> > +    /* Read request */
> > +    req.type = VIRTIO_BLK_T_IN;
> > +    req.ioprio = 1;
> > +    req.sector = 0;
> > +    req.data = g_malloc0(512);
> > +
> > +    req_addr = virtio_blk_request(alloc, dev, &req, 512);
> > +
> > +    g_free(req.data);
> > +
> > +    free_head = qvirtqueue_add(qts, vq, req_addr, 16, false, true);
> > +    qvirtqueue_add(qts, vq, req_addr + 16, 512, true, true);
> > +    qvirtqueue_add(qts, vq, req_addr + 528, 1, true, false);
> > +
> > +    qvirtqueue_kick(qts, dev, vq, free_head);
> > +
> > +    qvirtio_wait_used_elem(qts, dev, vq, free_head, NULL,
> > +                           QVIRTIO_BLK_TIMEOUT_US);
> > +    status = readb(req_addr + 528);
> > +    g_assert_cmpint(status, ==, 0);
> > +
> > +    data = g_malloc0(512);
> > +    qtest_memread(qts, req_addr + 16, data, 512);
> > +    g_assert_cmpstr(data, ==, "TEST");
> > +    g_free(data);
> > +
> > +    guest_free(alloc, req_addr);
> > +
> > +    if (features & (1u << VIRTIO_BLK_F_WRITE_ZEROES)) {
> > +        struct virtio_blk_discard_write_zeroes dwz_hdr;
> > +        void *expected;
> > +
> > +        /*
> > +         * WRITE_ZEROES request on the same sector of previous test where
> > +         * we wrote "TEST".
> > +         */
> > +        req.type = VIRTIO_BLK_T_WRITE_ZEROES;
> > +        req.data = (char *) &dwz_hdr;
> > +        dwz_hdr.sector = 0;
> > +        dwz_hdr.num_sectors = 1;
> > +        dwz_hdr.flags = 0;
> > +
> > +        virtio_blk_fix_dwz_hdr(dev, &dwz_hdr);
> > +
> > +        req_addr = virtio_blk_request(alloc, dev, &req, sizeof(dwz_hdr));
> > +
> > +        free_head = qvirtqueue_add(qts, vq, req_addr, 16, false, true);
> > +        qvirtqueue_add(qts, vq, req_addr + 16, sizeof(dwz_hdr), false, true);
> > +        qvirtqueue_add(qts, vq, req_addr + 16 + sizeof(dwz_hdr), 1, true,
> > +                       false);
> > +
> > +        qvirtqueue_kick(qts, dev, vq, free_head);
> > +
> > +        qvirtio_wait_used_elem(qts, dev, vq, free_head, NULL,
> > +                               QVIRTIO_BLK_TIMEOUT_US);
> > +        status = readb(req_addr + 16 + sizeof(dwz_hdr));
> > +        g_assert_cmpint(status, ==, 0);
> > +
> > +        guest_free(alloc, req_addr);
> > +
> > +        /* Read request to check if the sector contains all zeroes */
> > +        req.type = VIRTIO_BLK_T_IN;
> > +        req.ioprio = 1;
> > +        req.sector = 0;
> > +        req.data = g_malloc0(512);
> > +
> > +        req_addr = virtio_blk_request(alloc, dev, &req, 512);
> > +
> > +        g_free(req.data);
> > +
> > +        free_head = qvirtqueue_add(qts, vq, req_addr, 16, false, true);
> > +        qvirtqueue_add(qts, vq, req_addr + 16, 512, true, true);
> > +        qvirtqueue_add(qts, vq, req_addr + 528, 1, true, false);
> > +
> > +        qvirtqueue_kick(qts, dev, vq, free_head);
> > +
> > +        qvirtio_wait_used_elem(qts, dev, vq, free_head, NULL,
> > +                               QVIRTIO_BLK_TIMEOUT_US);
> > +        status = readb(req_addr + 528);
> > +        g_assert_cmpint(status, ==, 0);
> > +
> > +        data = g_malloc(512);
> > +        expected = g_malloc0(512);
> > +        qtest_memread(qts, req_addr + 16, data, 512);
> > +        g_assert_cmpmem(data, 512, expected, 512);
> > +        g_free(expected);
> > +        g_free(data);
> > +
> > +        guest_free(alloc, req_addr);
> > +    }
> > +
> > +    if (features & (1u << VIRTIO_BLK_F_DISCARD)) {
> > +        struct virtio_blk_discard_write_zeroes dwz_hdr;
> > +
> > +        req.type = VIRTIO_BLK_T_DISCARD;
> > +        req.data = (char *) &dwz_hdr;
> > +        dwz_hdr.sector = 0;
> > +        dwz_hdr.num_sectors = 1;
> > +        dwz_hdr.flags = 0;
> > +
> > +        virtio_blk_fix_dwz_hdr(dev, &dwz_hdr);
> > +
> > +        req_addr = virtio_blk_request(alloc, dev, &req, sizeof(dwz_hdr));
> > +
> > +        free_head = qvirtqueue_add(qts, vq, req_addr, 16, false, true);
> > +        qvirtqueue_add(qts, vq, req_addr + 16, sizeof(dwz_hdr), false, true);
> > +        qvirtqueue_add(qts, vq, req_addr + 16 + sizeof(dwz_hdr),
> > +                       1, true, false);
> > +
> > +        qvirtqueue_kick(qts, dev, vq, free_head);
> > +
> > +        qvirtio_wait_used_elem(qts, dev, vq, free_head, NULL,
> > +                               QVIRTIO_BLK_TIMEOUT_US);
> > +        status = readb(req_addr + 16 + sizeof(dwz_hdr));
> > +        g_assert_cmpint(status, ==, 0);
> > +
> > +        guest_free(alloc, req_addr);
> > +    }
> > +
> > +    if (features & (1u << VIRTIO_F_ANY_LAYOUT)) {
> > +        /* Write and read with 2 descriptor layout */
> > +        /* Write request */
> > +        req.type = VIRTIO_BLK_T_OUT;
> > +        req.ioprio = 1;
> > +        req.sector = 1;
> > +        req.data = g_malloc0(512);
> > +        strcpy(req.data, "TEST");
> > +
> > +        req_addr = virtio_blk_request(alloc, dev, &req, 512);
> > +
> > +        g_free(req.data);
> > +
> > +        free_head = qvirtqueue_add(qts, vq, req_addr, 528, false, true);
> > +        qvirtqueue_add(qts, vq, req_addr + 528, 1, true, false);
> > +        qvirtqueue_kick(qts, dev, vq, free_head);
> > +
> > +        qvirtio_wait_used_elem(qts, dev, vq, free_head, NULL,
> > +                               QVIRTIO_BLK_TIMEOUT_US);
> > +        status = readb(req_addr + 528);
> > +        g_assert_cmpint(status, ==, 0);
> > +
> > +        guest_free(alloc, req_addr);
> > +
> > +        /* Read request */
> > +        req.type = VIRTIO_BLK_T_IN;
> > +        req.ioprio = 1;
> > +        req.sector = 1;
> > +        req.data = g_malloc0(512);
> > +
> > +        req_addr = virtio_blk_request(alloc, dev, &req, 512);
> > +
> > +        g_free(req.data);
> > +
> > +        free_head = qvirtqueue_add(qts, vq, req_addr, 16, false, true);
> > +        qvirtqueue_add(qts, vq, req_addr + 16, 513, true, false);
> > +
> > +        qvirtqueue_kick(qts, dev, vq, free_head);
> > +
> > +        qvirtio_wait_used_elem(qts, dev, vq, free_head, NULL,
> > +                               QVIRTIO_BLK_TIMEOUT_US);
> > +        status = readb(req_addr + 528);
> > +        g_assert_cmpint(status, ==, 0);
> > +
> > +        data = g_malloc0(512);
> > +        qtest_memread(qts, req_addr + 16, data, 512);
> > +        g_assert_cmpstr(data, ==, "TEST");
> > +        g_free(data);
> > +
> > +        guest_free(alloc, req_addr);
> > +    }
> > +
> > +    return vq;
> > +}
> > +
> > +static void basic(void *obj, void *data, QGuestAllocator *t_alloc)
> > +{
> > +    QVhostUserBlk *blk_if = obj;
> > +    QVirtQueue *vq;
> > +
> > +    vq = test_basic(blk_if->vdev, t_alloc);
> > +    qvirtqueue_cleanup(blk_if->vdev->bus, vq, t_alloc);
> > +
> > +}
> > +
> > +static void indirect(void *obj, void *u_data, QGuestAllocator *t_alloc)
> > +{
> > +    QVirtQueue *vq;
> > +    QVhostUserBlk *blk_if = obj;
> > +    QVirtioDevice *dev = blk_if->vdev;
> > +    QVirtioBlkReq req;
> > +    QVRingIndirectDesc *indirect;
> > +    uint64_t req_addr;
> > +    uint64_t capacity;
> > +    uint64_t features;
> > +    uint32_t free_head;
> > +    uint8_t status;
> > +    char *data;
> > +    QTestState *qts = global_qtest;
> > +
> > +    features = qvirtio_get_features(dev);
> > +    g_assert_cmphex(features & (1u << VIRTIO_RING_F_INDIRECT_DESC), !=, 0);
> > +    features = features & ~(QVIRTIO_F_BAD_FEATURE |
> > +                            (1u << VIRTIO_RING_F_EVENT_IDX) |
> > +                            (1u << VIRTIO_BLK_F_SCSI));
> > +    qvirtio_set_features(dev, features);
> > +
> > +    capacity = qvirtio_config_readq(dev, 0);
> > +    g_assert_cmpint(capacity, ==, TEST_IMAGE_SIZE / 512);
> > +
> > +    vq = qvirtqueue_setup(dev, t_alloc, 0);
> > +    qvirtio_set_driver_ok(dev);
> > +
> > +    /* Write request */
> > +    req.type = VIRTIO_BLK_T_OUT;
> > +    req.ioprio = 1;
> > +    req.sector = 0;
> > +    req.data = g_malloc0(512);
> > +    strcpy(req.data, "TEST");
> > +
> > +    req_addr = virtio_blk_request(t_alloc, dev, &req, 512);
> > +
> > +    g_free(req.data);
> > +
> > +    indirect = qvring_indirect_desc_setup(qts, dev, t_alloc, 2);
> > +    qvring_indirect_desc_add(dev, qts, indirect, req_addr, 528, false);
> > +    qvring_indirect_desc_add(dev, qts, indirect, req_addr + 528, 1, true);
> > +    free_head = qvirtqueue_add_indirect(qts, vq, indirect);
> > +    qvirtqueue_kick(qts, dev, vq, free_head);
> > +
> > +    qvirtio_wait_used_elem(qts, dev, vq, free_head, NULL,
> > +                           QVIRTIO_BLK_TIMEOUT_US);
> > +    status = readb(req_addr + 528);
> > +    g_assert_cmpint(status, ==, 0);
> > +
> > +    g_free(indirect);
> > +    guest_free(t_alloc, req_addr);
> > +
> > +    /* Read request */
> > +    req.type = VIRTIO_BLK_T_IN;
> > +    req.ioprio = 1;
> > +    req.sector = 0;
> > +    req.data = g_malloc0(512);
> > +    strcpy(req.data, "TEST");
> > +
> > +    req_addr = virtio_blk_request(t_alloc, dev, &req, 512);
> > +
> > +    g_free(req.data);
> > +
> > +    indirect = qvring_indirect_desc_setup(qts, dev, t_alloc, 2);
> > +    qvring_indirect_desc_add(dev, qts, indirect, req_addr, 16, false);
> > +    qvring_indirect_desc_add(dev, qts, indirect, req_addr + 16, 513, true);
> > +    free_head = qvirtqueue_add_indirect(qts, vq, indirect);
> > +    qvirtqueue_kick(qts, dev, vq, free_head);
> > +
> > +    qvirtio_wait_used_elem(qts, dev, vq, free_head, NULL,
> > +                           QVIRTIO_BLK_TIMEOUT_US);
> > +    status = readb(req_addr + 528);
> > +    g_assert_cmpint(status, ==, 0);
> > +
> > +    data = g_malloc0(512);
> > +    qtest_memread(qts, req_addr + 16, data, 512);
> > +    g_assert_cmpstr(data, ==, "TEST");
> > +    g_free(data);
> > +
> > +    g_free(indirect);
> > +    guest_free(t_alloc, req_addr);
> > +    qvirtqueue_cleanup(dev->bus, vq, t_alloc);
> > +}
> > +
> > +static void idx(void *obj, void *u_data, QGuestAllocator *t_alloc)
> > +{
> > +    QVirtQueue *vq;
> > +    QVhostUserBlkPCI *blk = obj;
> > +    QVirtioPCIDevice *pdev = &blk->pci_vdev;
> > +    QVirtioDevice *dev = &pdev->vdev;
> > +    QVirtioBlkReq req;
> > +    uint64_t req_addr;
> > +    uint64_t capacity;
> > +    uint64_t features;
> > +    uint32_t free_head;
> > +    uint32_t write_head;
> > +    uint32_t desc_idx;
> > +    uint8_t status;
> > +    char *data;
> > +    QOSGraphObject *blk_object = obj;
> > +    QPCIDevice *pci_dev = blk_object->get_driver(blk_object, "pci-device");
> > +    QTestState *qts = global_qtest;
> > +
> > +    if (qpci_check_buggy_msi(pci_dev)) {
> > +        return;
> > +    }
> > +
> > +    qpci_msix_enable(pdev->pdev);
> > +    qvirtio_pci_set_msix_configuration_vector(pdev, t_alloc, 0);
> > +
> > +    features = qvirtio_get_features(dev);
> > +    features = features & ~(QVIRTIO_F_BAD_FEATURE |
> > +                            (1u << VIRTIO_RING_F_INDIRECT_DESC) |
> > +                            (1u << VIRTIO_F_NOTIFY_ON_EMPTY) |
> > +                            (1u << VIRTIO_BLK_F_SCSI));
> > +    qvirtio_set_features(dev, features);
> > +
> > +    capacity = qvirtio_config_readq(dev, 0);
> > +    g_assert_cmpint(capacity, ==, TEST_IMAGE_SIZE / 512);
> > +
> > +    vq = qvirtqueue_setup(dev, t_alloc, 0);
> > +    qvirtqueue_pci_msix_setup(pdev, (QVirtQueuePCI *)vq, t_alloc, 1);
> > +
> > +    qvirtio_set_driver_ok(dev);
> > +
> > +    /* Write request */
> > +    req.type = VIRTIO_BLK_T_OUT;
> > +    req.ioprio = 1;
> > +    req.sector = 0;
> > +    req.data = g_malloc0(512);
> > +    strcpy(req.data, "TEST");
> > +
> > +    req_addr = virtio_blk_request(t_alloc, dev, &req, 512);
> > +
> > +    g_free(req.data);
> > +
> > +    free_head = qvirtqueue_add(qts, vq, req_addr, 16, false, true);
> > +    qvirtqueue_add(qts, vq, req_addr + 16, 512, false, true);
> > +    qvirtqueue_add(qts, vq, req_addr + 528, 1, true, false);
> > +    qvirtqueue_kick(qts, dev, vq, free_head);
> > +
> > +    qvirtio_wait_used_elem(qts, dev, vq, free_head, NULL,
> > +                           QVIRTIO_BLK_TIMEOUT_US);
> > +
> > +    /* Write request */
> > +    req.type = VIRTIO_BLK_T_OUT;
> > +    req.ioprio = 1;
> > +    req.sector = 1;
> > +    req.data = g_malloc0(512);
> > +    strcpy(req.data, "TEST");
> > +
> > +    req_addr = virtio_blk_request(t_alloc, dev, &req, 512);
> > +
> > +    g_free(req.data);
> > +
> > +    /* Notify after processing the third request */
> > +    qvirtqueue_set_used_event(qts, vq, 2);
> > +    free_head = qvirtqueue_add(qts, vq, req_addr, 16, false, true);
> > +    qvirtqueue_add(qts, vq, req_addr + 16, 512, false, true);
> > +    qvirtqueue_add(qts, vq, req_addr + 528, 1, true, false);
> > +    qvirtqueue_kick(qts, dev, vq, free_head);
> > +    write_head = free_head;
> > +
> > +    /* No notification expected */
> > +    status = qvirtio_wait_status_byte_no_isr(qts, dev,
> > +                                             vq, req_addr + 528,
> > +                                             QVIRTIO_BLK_TIMEOUT_US);
> > +    g_assert_cmpint(status, ==, 0);
> > +
> > +    guest_free(t_alloc, req_addr);
> > +
> > +    /* Read request */
> > +    req.type = VIRTIO_BLK_T_IN;
> > +    req.ioprio = 1;
> > +    req.sector = 1;
> > +    req.data = g_malloc0(512);
> > +
> > +    req_addr = virtio_blk_request(t_alloc, dev, &req, 512);
> > +
> > +    g_free(req.data);
> > +
> > +    free_head = qvirtqueue_add(qts, vq, req_addr, 16, false, true);
> > +    qvirtqueue_add(qts, vq, req_addr + 16, 512, true, true);
> > +    qvirtqueue_add(qts, vq, req_addr + 528, 1, true, false);
> > +
> > +    qvirtqueue_kick(qts, dev, vq, free_head);
> > +
> > +    /* We get just one notification for both requests */
> > +    qvirtio_wait_used_elem(qts, dev, vq, write_head, NULL,
> > +                           QVIRTIO_BLK_TIMEOUT_US);
> > +    g_assert(qvirtqueue_get_buf(qts, vq, &desc_idx, NULL));
> > +    g_assert_cmpint(desc_idx, ==, free_head);
> > +
> > +    status = readb(req_addr + 528);
> > +    g_assert_cmpint(status, ==, 0);
> > +
> > +    data = g_malloc0(512);
> > +    qtest_memread(qts, req_addr + 16, data, 512);
> > +    g_assert_cmpstr(data, ==, "TEST");
> > +    g_free(data);
> > +
> > +    guest_free(t_alloc, req_addr);
> > +
> > +    /* End test */
> > +    qpci_msix_disable(pdev->pdev);
> > +
> > +    qvirtqueue_cleanup(dev->bus, vq, t_alloc);
> > +}
> > +
> > +static void pci_hotplug(void *obj, void *data, QGuestAllocator *t_alloc)
> > +{
> > +    QVirtioPCIDevice *dev1 = obj;
> > +    QVirtioPCIDevice *dev;
> > +    QTestState *qts = dev1->pdev->bus->qts;
> > +
> > +    /* plug secondary disk */
> > +    qtest_qmp_device_add(qts, "vhost-user-blk-pci", "drv1",
> > +                         "{'addr': %s, 'chardev': 'char2'}",
> > +                         stringify(PCI_SLOT_HP) ".0");
> > +
> > +    dev = virtio_pci_new(dev1->pdev->bus,
> > +                         &(QPCIAddress) { .devfn = QPCI_DEVFN(PCI_SLOT_HP, 0)
> > +                                        });
> > +    g_assert_nonnull(dev);
> > +    g_assert_cmpint(dev->vdev.device_type, ==, VIRTIO_ID_BLOCK);
> > +    qvirtio_pci_device_disable(dev);
> > +    qos_object_destroy((QOSGraphObject *)dev);
> > +
> > +    /* unplug secondary disk */
> > +    qpci_unplug_acpi_device_test(qts, "drv1", PCI_SLOT_HP);
> > +}
> > +
> > +/*
> > + * Check that setting the vring addr on a non-existent virtqueue does
> > + * not crash.
> > + */
> > +static void test_nonexistent_virtqueue(void *obj, void *data,
> > +                                       QGuestAllocator *t_alloc)
> > +{
> > +    QVhostUserBlkPCI *blk = obj;
> > +    QVirtioPCIDevice *pdev = &blk->pci_vdev;
> > +    QPCIBar bar0;
> > +    QPCIDevice *dev;
> > +
> > +    dev = qpci_device_find(pdev->pdev->bus, QPCI_DEVFN(4, 0));
> > +    g_assert(dev != NULL);
> > +    qpci_device_enable(dev);
> > +
> > +    bar0 = qpci_iomap(dev, 0, NULL);
> > +
> > +    qpci_io_writeb(dev, bar0, VIRTIO_PCI_QUEUE_SEL, 2);
> > +    qpci_io_writel(dev, bar0, VIRTIO_PCI_QUEUE_PFN, 1);
> > +
> > +    g_free(dev);
> > +}
> > +
> > +static const char *qtest_qemu_storage_daemon_binary(void)
> > +{
> > +    const char *qemu_storage_daemon_bin;
> > +
> > +    qemu_storage_daemon_bin = getenv("QTEST_QEMU_STORAGE_DAEMON_BINARY");
> > +    if (!qemu_storage_daemon_bin) {
> > +        fprintf(stderr, "Environment variable "
> > +                        "QTEST_QEMU_STORAGE_DAEMON_BINARY required\n");
> > +        exit(0);
> > +    }
> > +
> > +    return qemu_storage_daemon_bin;
> > +}
> > +
> > +/* g_test_queue_destroy() cleanup function for files */
> > +static void destroy_file(void *path)
> > +{
> > +    unlink(path);
> > +    g_free(path);
> > +    qos_invalidate_command_line();
> > +}
> > +
> > +static char *drive_create(void)
> > +{
> > +    int fd, ret;
> > +    /** vhost-user-blk won't recognize drive located in /tmp */
> > +    char *t_path = g_strdup("qtest.XXXXXX");
> > +
> > +    /** Create a temporary raw image */
> > +    fd = mkstemp(t_path);
> > +    g_assert_cmpint(fd, >=, 0);
> > +    ret = ftruncate(fd, TEST_IMAGE_SIZE);
> > +    g_assert_cmpint(ret, ==, 0);
> > +    close(fd);
> > +
> > +    g_test_queue_destroy(destroy_file, t_path);
> > +    return t_path;
> > +}
> > +
> > +static char *create_listen_socket(int *fd)
> > +{
> > +    int tmp_fd;
> > +    char *path;
> > +
> > +    /* No race because our pid makes the path unique */
> > +    path = g_strdup_printf("/tmp/qtest-%d-sock.XXXXXX", getpid());
> > +    tmp_fd = mkstemp(path);
> > +    g_assert_cmpint(tmp_fd, >=, 0);
> > +    close(tmp_fd);
> > +    unlink(path);
> > +
> > +    *fd = qtest_socket_server(path);
> > +    g_test_queue_destroy(destroy_file, path);
> > +    return path;
> > +}
> > +
> > +/*
> > + * g_test_queue_destroy() and qtest_add_abrt_handler() cleanup function for
> > + * qemu-storage-daemon.
> > + */
> > +static void quit_storage_daemon(void *data)
> > +{
> > +    QemuStorageDaemonState *qsd = data;
> > +    int wstatus;
> > +    pid_t pid;
> > +
> > +    /*
> > +     * If we were invoked as a g_test_queue_destroy() cleanup function we need
> > +     * to remove the abrt handler to avoid being called again if the code below
> > +     * aborts. Also, we must not leave the abrt handler installed after
> > +     * cleanup.
> > +     */
> > +    qtest_remove_abrt_handler(data);
> > +
> 
> There is an issue. If a test fails, quit_storage_daemon won't be
> called. Since there no abrt_handler like kill_qemu_hook_func
> installed to stop qemu-storage-daemon, qemu-storage-daemon would keep
> running after QEMU is killed by kill_qemu_hook_func.

I'm not sure I understand. quit_storage_daemon() is installed as an abrt
handler and as a g_test_queue_destroy handler. The abrt handler is
executed when the test failed. The g_test_queue_destroy handler is
executed when the test completes successfully.

Can you describe the scenario where the test fails but
quit_storage_daemon() is not invoked by the abrt handler that this code
installs?

Thanks,
Stefan

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v2 00/12] block/export: vhost-user-blk server tests and input validation

[Peter Maydell]

On Fri, 19 Feb 2021 at 22:38, Peter Maydell <peter.maydell@linaro.org> wrote:
>
> On Mon, 15 Feb 2021 at 10:41, Kevin Wolf <kwolf@redhat.com> wrote:
> >
> > Am 07.12.2020 um 18:20 hat Stefan Hajnoczi geschrieben:
> > > v2:
> > >  * Add abrt handler that terminates qemu-storage-daemon to
> > >    vhost-user-blk-test. No more orphaned processes on test failure. [Peter]
> > >  * Fix sector number calculation in vhost-user-blk-server.c
> > >  * Introduce VIRTIO_BLK_SECTOR_BITS/SIZE to make code clearer [Max]
> > >  * Fix vhost-user-blk-server.c blk_size double byteswap
> > >  * Fix vhost-user-blk blkcfg->num_queues endianness [Peter]
> > >  * Squashed cleanups into Coiby vhost-user-blk-test commit so the code is
> > >    easier to review
> > >
> > > The vhost-user-blk server test was already in Michael Tsirkin's recent vhost
> > > pull request, but was dropped because it exposed vhost-user regressions
> > > (b7c1bd9d7848 and the Based-on tag below). Now that the vhost-user regressions
> > > are fixed we can re-introduce the test case.
> > >
> > > This series adds missing input validation that led to a Coverity report. The
> > > virtio-blk read, write, discard, and write zeroes commands need to check
> > > sector/byte ranges and other inputs. This solves the issue Peter Maydell raised
> > > in "[PATCH for-5.2] block/export/vhost-user-blk-server.c: Avoid potential
> > > integer overflow".
> > >
> > > Merging just the input validation patches would be possible too, but I prefer
> > > to merge the corresponding tests so the code is exercised by the CI.
> >
> > Is this series still open? I don't see it in master.
>
> The Coverity issue is still unfixed, at any rate...

Ping^2 ! I'd like to get us down to no outstanding Coverity issues for the
6.0 release, and this (CID 1435956) is one of the handful still unfixed.

thanks
-- PMM

Re: [PATCH v2 00/12] block/export: vhost-user-blk server tests and input validation

[Kevin Wolf]

Am 10.03.2021 um 16:51 hat Peter Maydell geschrieben:
> On Fri, 19 Feb 2021 at 22:38, Peter Maydell <peter.maydell@linaro.org> wrote:
> >
> > On Mon, 15 Feb 2021 at 10:41, Kevin Wolf <kwolf@redhat.com> wrote:
> > >
> > > Am 07.12.2020 um 18:20 hat Stefan Hajnoczi geschrieben:
> > > > v2:
> > > >  * Add abrt handler that terminates qemu-storage-daemon to
> > > >    vhost-user-blk-test. No more orphaned processes on test failure. [Peter]
> > > >  * Fix sector number calculation in vhost-user-blk-server.c
> > > >  * Introduce VIRTIO_BLK_SECTOR_BITS/SIZE to make code clearer [Max]
> > > >  * Fix vhost-user-blk-server.c blk_size double byteswap
> > > >  * Fix vhost-user-blk blkcfg->num_queues endianness [Peter]
> > > >  * Squashed cleanups into Coiby vhost-user-blk-test commit so the code is
> > > >    easier to review
> > > >
> > > > The vhost-user-blk server test was already in Michael Tsirkin's recent vhost
> > > > pull request, but was dropped because it exposed vhost-user regressions
> > > > (b7c1bd9d7848 and the Based-on tag below). Now that the vhost-user regressions
> > > > are fixed we can re-introduce the test case.
> > > >
> > > > This series adds missing input validation that led to a Coverity report. The
> > > > virtio-blk read, write, discard, and write zeroes commands need to check
> > > > sector/byte ranges and other inputs. This solves the issue Peter Maydell raised
> > > > in "[PATCH for-5.2] block/export/vhost-user-blk-server.c: Avoid potential
> > > > integer overflow".
> > > >
> > > > Merging just the input validation patches would be possible too, but I prefer
> > > > to merge the corresponding tests so the code is exercised by the CI.
> > >
> > > Is this series still open? I don't see it in master.
> >
> > The Coverity issue is still unfixed, at any rate...
> 
> Ping^2 ! I'd like to get us down to no outstanding Coverity issues for the
> 6.0 release, and this (CID 1435956) is one of the handful still unfixed.

You pulled a newer version of this series (minus the tests that caused CI
failures on some older OSes) earlier today, so I assume this is fixed
now.

Kevin