[PATCH for-5.2 00/10] block/export: vhost-user-blk server tests and input validation

[PATCH for-5.2 00/10] block/export: vhost-user-blk server tests and input validation

[Stefan Hajnoczi]

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.

Based-on: 20201109174355.1069147-1-stefanha@redhat.com ("[PATCH for-5.2] vhost-user: fix VHOST_USER_ADD/REM_MEM_REG truncation")

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

Stefan Hajnoczi (9):
  tests/qtest: add multi-queue test case to vhost-user-blk-test
  libqtest: add qtest_socket_server()
  vhost-user-blk-test: rename destroy_drive() to destroy_file()
  vhost-user-blk-test: close fork child file descriptors
  vhost-user-blk-test: drop unused return value
  vhost-user-blk-test: fix races by using fd passing
  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

 tests/qtest/libqos/libqtest.h        |  25 +
 tests/qtest/libqos/vhost-user-blk.h  |  48 ++
 block/export/vhost-user-blk-server.c | 129 +++-
 tests/qtest/libqos/vhost-user-blk.c  | 129 ++++
 tests/qtest/libqtest.c               |  76 ++-
 tests/qtest/vhost-user-blk-test.c    | 965 +++++++++++++++++++++++++++
 tests/qtest/libqos/meson.build       |   1 +
 tests/qtest/meson.build              |   2 +
 8 files changed, 1333 insertions(+), 42 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 for-5.2 01/10] 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 vhost-user server can only server one
client one time, two instances of vhost-user-blk-server are started by
qemu-storage-daemon for the hotplug test.

In order to not block scripts/tap-driver.pl, vhost-user-blk-server will
send "quit" command to qemu-storage-daemon's QMP monitor. So a function
is added to libqtest.c to establish socket connection with socket
server.

Suggested-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Coiby Xu <coiby.xu@gmail.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Message-id: 20200918080912.321299-7-coiby.xu@gmail.com
[Update meson.build to only test when CONFIG_TOOLS has built
qemu-storage-daemon. This prevents CI failures with --disable-tools.
Also bump RAM to 256 MB because that is the minimum RAM granularity on
ppc64 spapr machines.
--Stefan]
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 tests/qtest/libqos/libqtest.h       |  17 +
 tests/qtest/libqos/vhost-user-blk.h |  48 ++
 tests/qtest/libqos/vhost-user-blk.c | 129 +++++
 tests/qtest/libqtest.c              |  36 +-
 tests/qtest/vhost-user-blk-test.c   | 751 ++++++++++++++++++++++++++++
 tests/qtest/libqos/meson.build      |   1 +
 tests/qtest/meson.build             |   2 +
 7 files changed, 982 insertions(+), 2 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

diff --git a/tests/qtest/libqos/libqtest.h b/tests/qtest/libqos/libqtest.h
index 724f65aa94..d6236ea7a0 100644
--- a/tests/qtest/libqos/libqtest.h
+++ b/tests/qtest/libqos/libqtest.h
@@ -132,6 +132,23 @@ 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_client:
+ * @server_socket_path: the socket server's path
+ *
+ * Connect to a socket server.
+ */
+int qtest_socket_client(char *server_socket_path);
+
+/**
+ * qtest_create_state_with_qmp_fd:
+ * @fd: socket fd
+ *
+ * Wrap socket fd in QTestState to make use of qtest_qmp*
+ * functions
+ */
+QTestState *qtest_create_state_with_qmp_fd(int fd);
+
 /**
  * qtest_vqmp_fds:
  * @s: #QTestState instance to operate on.
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..58c7e1eb69
--- /dev/null
+++ b/tests/qtest/libqos/vhost-user-blk.c
@@ -0,0 +1,129 @@
+/*
+ * 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/libqtest.c b/tests/qtest/libqtest.c
index be0fb430dd..ff563cbaba 100644
--- a/tests/qtest/libqtest.c
+++ b/tests/qtest/libqtest.c
@@ -4,11 +4,13 @@
  * Copyright IBM, Corp. 2012
  * Copyright Red Hat, Inc. 2012
  * Copyright SUSE LINUX Products GmbH 2013
+ * Copyright Copyright (c) Coiby Xu
  *
  * Authors:
  *  Anthony Liguori   <aliguori@us.ibm.com>
  *  Paolo Bonzini     <pbonzini@redhat.com>
  *  Andreas Färber    <afaerber@suse.de>
+ *  Coiby Xu          <coiby.xu@gmail.com>
  *
  * This work is licensed under the terms of the GNU GPL, version 2 or later.
  * See the COPYING file in the top-level directory.
@@ -52,8 +54,7 @@ typedef struct QTestClientTransportOps {
     QTestRecvFn     recv_line; /* for receiving qtest command responses */
 } QTestTransportOps;
 
-struct QTestState
-{
+struct QTestState {
     int fd;
     int qmp_fd;
     pid_t qemu_pid;  /* our child QEMU process */
@@ -635,6 +636,37 @@ QDict *qtest_qmp_receive_dict(QTestState *s)
     return qmp_fd_receive(s->qmp_fd);
 }
 
+QTestState *qtest_create_state_with_qmp_fd(int fd)
+{
+    QTestState *qmp_test_state = g_new0(QTestState, 1);
+    qmp_test_state->qmp_fd = fd;
+    return qmp_test_state;
+}
+
+int qtest_socket_client(char *server_socket_path)
+{
+    struct sockaddr_un serv_addr;
+    int sock;
+    int ret;
+    int retries = 0;
+    sock = socket(PF_UNIX, SOCK_STREAM, 0);
+    g_assert_cmpint(sock, !=, -1);
+    serv_addr.sun_family = AF_UNIX;
+    snprintf(serv_addr.sun_path, sizeof(serv_addr.sun_path), "%s",
+             server_socket_path);
+
+    for (retries = 0; retries < 3; retries++) {
+        ret = connect(sock, (struct sockaddr *)&serv_addr, sizeof(serv_addr));
+        if (ret == 0) {
+            break;
+        }
+        g_usleep(G_USEC_PER_SEC);
+    }
+
+    g_assert_cmpint(ret, ==, 0);
+    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
diff --git a/tests/qtest/vhost-user-blk-test.c b/tests/qtest/vhost-user-blk-test.c
new file mode 100644
index 0000000000..e7e44f9bf0
--- /dev/null
+++ b/tests/qtest/vhost-user-blk-test.c
@@ -0,0 +1,751 @@
+/*
+ * 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 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;
+}
+
+static void drive_destroy(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(drive_destroy, t_path);
+    return t_path;
+}
+
+static char sock_path_tempate[] = "/tmp/qtest.vhost_user_blk.XXXXXX";
+static char qmp_sock_path_tempate[] = "/tmp/qtest.vhost_user_blk.qmp.XXXXXX";
+
+static void quit_storage_daemon(void *qmp_test_state)
+{
+    const char quit_str[] = "{ 'execute': 'quit' }";
+
+    /* Before quiting storate-daemon, quit qemu to avoid dubious messages */
+    qobject_unref(qtest_qmp(global_qtest, quit_str));
+
+    /*
+     * Give storage-daemon enough time to wake up&terminate
+     * vu_client_trip coroutine so the Coroutine object could
+     * be cleaned up. Otherwise LeakSanitizer would complain
+     * about memory leaks.
+     */
+    g_usleep(1000);
+
+    qobject_unref(qtest_qmp((QTestState *)qmp_test_state, quit_str));
+    g_free(qmp_test_state);
+}
+
+static char *start_vhost_user_blk(GString *cmd_line, int vus_instances)
+{
+    const char *vhost_user_blk_bin = qtest_qemu_storage_daemon_binary();
+    int fd, qmp_fd, i;
+    QTestState *qmp_test_state;
+    gchar *img_path;
+    char *sock_path = NULL;
+    char *qmp_sock_path = g_strdup(qmp_sock_path_tempate);
+    GString *storage_daemon_command = g_string_new(NULL);
+
+    qmp_fd = mkstemp(qmp_sock_path);
+    g_assert_cmpint(qmp_fd, >=, 0);
+    g_test_queue_destroy(drive_destroy, qmp_sock_path);
+
+    g_string_append_printf(storage_daemon_command,
+            "exec %s "
+            "--chardev socket,id=qmp,path=%s,server,nowait --monitor chardev=qmp ",
+            vhost_user_blk_bin, qmp_sock_path);
+
+    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++) {
+        sock_path = g_strdup(sock_path_tempate);
+        fd = mkstemp(sock_path);
+        g_assert_cmpint(fd, >=, 0);
+        g_test_queue_destroy(drive_destroy, sock_path);
+        /* 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) {
+        execlp("/bin/sh", "sh", "-c", storage_daemon_command->str, NULL);
+        exit(1);
+    }
+    g_string_free(storage_daemon_command, true);
+
+    qmp_test_state = qtest_create_state_with_qmp_fd(
+                             qtest_socket_client(qmp_sock_path));
+    /*
+     * Ask qemu-storage-daemon to quit so it
+     * will not block scripts/tap-driver.pl.
+     */
+    g_test_queue_destroy(quit_storage_daemon, qmp_test_state);
+
+    qobject_unref(qtest_qmp(qmp_test_state, "{'execute': 'qmp_capabilities'}"));
+    return sock_path;
+}
+
+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..23a70b4613 100644
--- a/tests/qtest/meson.build
+++ b/tests/qtest/meson.build
@@ -199,6 +199,7 @@ 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'))
+qos_test_ss.add(when: ['CONFIG_LINUX', 'CONFIG_TOOLS'], if_true: files('vhost-user-blk-test.c'))
 
 tpmemu_files = ['tpm-emu.c', 'tpm-util.c', 'tpm-tests.c']
 
@@ -237,6 +238,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 for-5.2 02/10] tests/qtest: add multi-queue test case to vhost-user-blk-test

[Stefan Hajnoczi]

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-id: 20201001144604.559733-3-stefanha@redhat.com
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 e7e44f9bf0..31f2335f97 100644
--- a/tests/qtest/vhost-user-blk-test.c
+++ b/tests/qtest/vhost-user-blk-test.c
@@ -559,6 +559,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.
@@ -643,7 +704,8 @@ static void quit_storage_daemon(void *qmp_test_state)
     g_free(qmp_test_state);
 }
 
-static char *start_vhost_user_blk(GString *cmd_line, int vus_instances)
+static char *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 fd, qmp_fd, i;
@@ -675,8 +737,8 @@ static char *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);
@@ -705,7 +767,7 @@ static char *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;
 }
 
@@ -719,7 +781,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;
 }
 
@@ -746,6 +814,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 for-5.2 03/10] libqtest: add qtest_socket_server()

[Stefan Hajnoczi]

There is a qtest_socket_client() API. Add an equivalent
qtest_socket_server() 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().

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 d6236ea7a0..8792c09398 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_socket_client:
  * @server_socket_path: the socket server's path
diff --git a/tests/qtest/libqtest.c b/tests/qtest/libqtest.c
index ff563cbaba..50a30f8e1f 100644
--- a/tests/qtest/libqtest.c
+++ b/tests/qtest/libqtest.c
@@ -82,24 +82,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;
 }
 
@@ -643,6 +627,28 @@ QTestState *qtest_create_state_with_qmp_fd(int fd)
     return qmp_test_state;
 }
 
+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;
+}
+
 int qtest_socket_client(char *server_socket_path)
 {
     struct sockaddr_un serv_addr;
-- 
2.28.0

[PATCH for-5.2 04/10] vhost-user-blk-test: rename destroy_drive() to destroy_file()

[Stefan Hajnoczi]

The function is used not just for image files but also for UNIX domain
sockets (QMP monitor and vhost-user-blk). Reflect that in the name.

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

diff --git a/tests/qtest/vhost-user-blk-test.c b/tests/qtest/vhost-user-blk-test.c
index 31f2335f97..f05f14c192 100644
--- a/tests/qtest/vhost-user-blk-test.c
+++ b/tests/qtest/vhost-user-blk-test.c
@@ -658,7 +658,8 @@ static const char *qtest_qemu_storage_daemon_binary(void)
     return qemu_storage_daemon_bin;
 }
 
-static void drive_destroy(void *path)
+/* g_test_queue_destroy() cleanup function for files */
+static void destroy_file(void *path)
 {
     unlink(path);
     g_free(path);
@@ -678,7 +679,7 @@ static char *drive_create(void)
     g_assert_cmpint(ret, ==, 0);
     close(fd);
 
-    g_test_queue_destroy(drive_destroy, t_path);
+    g_test_queue_destroy(destroy_file, t_path);
     return t_path;
 }
 
@@ -717,7 +718,7 @@ static char *start_vhost_user_blk(GString *cmd_line, int vus_instances,
 
     qmp_fd = mkstemp(qmp_sock_path);
     g_assert_cmpint(qmp_fd, >=, 0);
-    g_test_queue_destroy(drive_destroy, qmp_sock_path);
+    g_test_queue_destroy(destroy_file, qmp_sock_path);
 
     g_string_append_printf(storage_daemon_command,
             "exec %s "
@@ -731,7 +732,7 @@ static char *start_vhost_user_blk(GString *cmd_line, int vus_instances,
         sock_path = g_strdup(sock_path_tempate);
         fd = mkstemp(sock_path);
         g_assert_cmpint(fd, >=, 0);
-        g_test_queue_destroy(drive_destroy, sock_path);
+        g_test_queue_destroy(drive_file, sock_path);
         /* create image file */
         img_path = drive_create();
         g_string_append_printf(storage_daemon_command,
-- 
2.28.0

[PATCH for-5.2 05/10] vhost-user-blk-test: close fork child file descriptors

[Stefan Hajnoczi]

Do not leave stdin and stdout open after fork. stdout is the
tap-driver.pl pipe. If we keep the pipe open then tap-driver.pl will not
detect that qos-test has terminated and it will hang.

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

diff --git a/tests/qtest/vhost-user-blk-test.c b/tests/qtest/vhost-user-blk-test.c
index f05f14c192..4019a72ac0 100644
--- a/tests/qtest/vhost-user-blk-test.c
+++ b/tests/qtest/vhost-user-blk-test.c
@@ -749,6 +749,15 @@ static char *start_vhost_user_blk(GString *cmd_line, int vus_instances,
                    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);
     }
-- 
2.28.0

[PATCH for-5.2 06/10] vhost-user-blk-test: drop unused return value

[Stefan Hajnoczi]

The sock_path return value was unused and bogus (it doesn't make sense
when there are multiple drives because only the last path is arbitrarily
returned).

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

diff --git a/tests/qtest/vhost-user-blk-test.c b/tests/qtest/vhost-user-blk-test.c
index 4019a72ac0..c5ff610d7a 100644
--- a/tests/qtest/vhost-user-blk-test.c
+++ b/tests/qtest/vhost-user-blk-test.c
@@ -705,8 +705,8 @@ static void quit_storage_daemon(void *qmp_test_state)
     g_free(qmp_test_state);
 }
 
-static char *start_vhost_user_blk(GString *cmd_line, int vus_instances,
-                                  int num_queues)
+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 fd, qmp_fd, i;
@@ -772,7 +772,6 @@ static char *start_vhost_user_blk(GString *cmd_line, int vus_instances,
     g_test_queue_destroy(quit_storage_daemon, qmp_test_state);
 
     qobject_unref(qtest_qmp(qmp_test_state, "{'execute': 'qmp_capabilities'}"));
-    return sock_path;
 }
 
 static void *vhost_user_blk_test_setup(GString *cmd_line, void *arg)
-- 
2.28.0

[PATCH for-5.2 07/10] vhost-user-blk-test: fix races by using fd passing

[Stefan Hajnoczi]

Pass the QMP and vhost-user-blk server sockets as file descriptors. That
way the sockets are already open and in a listen state when the QEMU
process is launched.

This solves the race with qemu-storage-daemon startup where the UNIX
domain sockets may not be ready yet when QEMU attempts to connect. It
also saves us sleeping for 1 second if the qemu-storage-daemon QMP
socket is not ready yet.

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

diff --git a/tests/qtest/vhost-user-blk-test.c b/tests/qtest/vhost-user-blk-test.c
index c5ff610d7a..e52340cffb 100644
--- a/tests/qtest/vhost-user-blk-test.c
+++ b/tests/qtest/vhost-user-blk-test.c
@@ -683,8 +683,22 @@ static char *drive_create(void)
     return t_path;
 }
 
-static char sock_path_tempate[] = "/tmp/qtest.vhost_user_blk.XXXXXX";
-static char qmp_sock_path_tempate[] = "/tmp/qtest.vhost_user_blk.qmp.XXXXXX";
+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;
+}
 
 static void quit_storage_daemon(void *qmp_test_state)
 {
@@ -709,37 +723,33 @@ 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 fd, qmp_fd, i;
+    int qmp_fd, i;
     QTestState *qmp_test_state;
     gchar *img_path;
-    char *sock_path = NULL;
-    char *qmp_sock_path = g_strdup(qmp_sock_path_tempate);
+    char *qmp_sock_path;
     GString *storage_daemon_command = g_string_new(NULL);
 
-    qmp_fd = mkstemp(qmp_sock_path);
-    g_assert_cmpint(qmp_fd, >=, 0);
-    g_test_queue_destroy(destroy_file, qmp_sock_path);
+    qmp_sock_path = create_listen_socket(&qmp_fd);
 
     g_string_append_printf(storage_daemon_command,
             "exec %s "
-            "--chardev socket,id=qmp,path=%s,server,nowait --monitor chardev=qmp ",
-            vhost_user_blk_bin, qmp_sock_path);
+            "--chardev socket,id=qmp,fd=%d,server,nowait --monitor chardev=qmp ",
+            vhost_user_blk_bin, qmp_fd);
 
     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++) {
-        sock_path = g_strdup(sock_path_tempate);
-        fd = mkstemp(sock_path);
-        g_assert_cmpint(fd, >=, 0);
-        g_test_queue_destroy(drive_file, sock_path);
+        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,"
+            "--export type=vhost-user-blk,id=disk%d,addr.type=fd,addr.str=%d,"
             "node-name=disk%i,writable=on,num-queues=%d ",
-            i, img_path, i, sock_path, i, num_queues);
+            i, img_path, i, fd, i, num_queues);
 
         g_string_append_printf(cmd_line, "-chardev socket,id=char%d,path=%s ",
                                i + 1, sock_path);
-- 
2.28.0

[PATCH for-5.2 08/10] 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 | 115 +++++++++++++++++++++------
 1 file changed, 92 insertions(+), 23 deletions(-)

diff --git a/block/export/vhost-user-blk-server.c b/block/export/vhost-user-blk-server.c
index 62672d1cb9..3295d3c951 100644
--- a/block/export/vhost-user-blk-server.c
+++ b/block/export/vhost-user-blk-server.c
@@ -22,6 +22,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;
@@ -58,30 +60,101 @@ 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 << BDRV_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 "num_sectors << BDRV_SECTOR_BITS" fits in an int */
+    if (unlikely(num_sectors > max_sectors)) {
+        return VIRTIO_BLK_S_IOERR;
+    }
+
+    bytes = num_sectors << BDRV_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 << BDRV_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 << BDRV_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)
@@ -170,19 +243,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, elem->out_sg,
+                                                      out_num, type);
         break;
     }
     default:
@@ -352,10 +419,12 @@ 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(config->blk_size >> 9);
-    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 for-5.2 09/10] 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>
---
 block/export/vhost-user-blk-server.c |   4 +-
 tests/qtest/vhost-user-blk-test.c    | 124 +++++++++++++++++++++++++++
 2 files changed, 126 insertions(+), 2 deletions(-)

diff --git a/block/export/vhost-user-blk-server.c b/block/export/vhost-user-blk-server.c
index 3295d3c951..d88e41714d 100644
--- a/block/export/vhost-user-blk-server.c
+++ b/block/export/vhost-user-blk-server.c
@@ -248,8 +248,8 @@ static void coroutine_fn vu_blk_virtio_process_req(void *opaque)
             break;
         }
 
-        req->in->status = vu_blk_discard_write_zeroes(vexp, elem->out_sg,
-                                                      out_num, type);
+        req->in->status = vu_blk_discard_write_zeroes(vexp, out_iov, out_num,
+                                                      type);
         break;
     }
     default:
diff --git a/tests/qtest/vhost-user-blk-test.c b/tests/qtest/vhost-user-blk-test.c
index e52340cffb..991a018c55 100644
--- a/tests/qtest/vhost-user-blk-test.c
+++ b/tests/qtest/vhost-user-blk-test.c
@@ -90,6 +90,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)
 {
@@ -231,6 +349,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)) {
@@ -259,6 +380,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 for-5.2 10/10] 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 | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/block/export/vhost-user-blk-server.c b/block/export/vhost-user-blk-server.c
index d88e41714d..6d7fd0fec3 100644
--- a/block/export/vhost-user-blk-server.c
+++ b/block/export/vhost-user-blk-server.c
@@ -214,9 +214,23 @@ static void coroutine_fn vu_blk_virtio_process_req(void *opaque)
         QEMUIOVector qiov;
         if (is_write) {
             qemu_iovec_init_external(&qiov, out_iov, out_num);
+
+            if (unlikely(!vu_blk_sect_range_ok(vexp, req->sector_num,
+                                               qiov.size))) {
+                req->in->status = VIRTIO_BLK_S_IOERR;
+                break;
+            }
+
             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;
+            }
+
             ret = blk_co_preadv(blk, offset, qiov.size, &qiov, 0);
         }
         if (ret >= 0) {
-- 
2.28.0

Re: [PATCH for-5.2 04/10] vhost-user-blk-test: rename destroy_drive() to destroy_file()

[Max Reitz]

On 11.11.20 13:43, Stefan Hajnoczi wrote:
> The function is used not just for image files but also for UNIX domain
> sockets (QMP monitor and vhost-user-blk). Reflect that in the name.
> 
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
>   tests/qtest/vhost-user-blk-test.c | 9 +++++----
>   1 file changed, 5 insertions(+), 4 deletions(-)

[...]

> @@ -731,7 +732,7 @@ static char *start_vhost_user_blk(GString *cmd_line, int vus_instances,
>           sock_path = g_strdup(sock_path_tempate);
>           fd = mkstemp(sock_path);
>           g_assert_cmpint(fd, >=, 0);
> -        g_test_queue_destroy(drive_destroy, sock_path);
> +        g_test_queue_destroy(drive_file, sock_path);

s/drive_file/destroy_file/, I think :)

>           /* create image file */
>           img_path = drive_create();
>           g_string_append_printf(storage_daemon_command,
> 

Re: [PATCH for-5.2 08/10] block/export: port virtio-blk discard/write zeroes input validation

[Max Reitz]

On 11.11.20 13:43, Stefan Hajnoczi wrote:
> 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 | 115 +++++++++++++++++++++------
>   1 file changed, 92 insertions(+), 23 deletions(-)
> 
> diff --git a/block/export/vhost-user-blk-server.c b/block/export/vhost-user-blk-server.c
> index 62672d1cb9..3295d3c951 100644
> --- a/block/export/vhost-user-blk-server.c
> +++ b/block/export/vhost-user-blk-server.c

[...]

> @@ -58,30 +60,101 @@ 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) {

I wonder why you pass a byte length, then shift it down to sectors, when 
it’s kind of unsafe on the caller’s side to even calculate that byte 
length (because the caller has to verify that shifting the sector count 
up to be a byte length is safe).

Wouldn’t it be simpler to pass nb_sectors (perhaps even as uint64_t, 
because why not) and then compare that against BDRV_REQUEST_MAX_BYTES here?

(With how the code is written, it also has the problem of rounding down 
@size.  Which isn’t a problem in practice because the caller effectively 
guarantees that @size is aligned to sectors, but it still means that the 
code looks a bit strange.  As in, “Why is it safe to round down?  Ah, 
because the caller always produces an aligned value.  But why does the 
caller even pass a byte count, then?  Especially given that the offset 
is passed as a sector index, too.”)

> +        return false;
> +    }
> +    if ((sector << BDRV_SECTOR_BITS) % vexp->blk_size) {

This made me wonder why the discard/write-zeroes sector granularity 
would be BDRV_SECTOR_BITS and not blk_size, which is used for IN/OUT 
(read/write) requests.

I still don’t know, but judging from the only reference I could find 
quickly (contrib/vhost-user-blk/vhost-user-blk.c), it seems correct.

OTOH, I wonder whether BDRV_SECTOR_BITS/BDRV_SECTOR_SIZE are the correct 
constants.  Isn’t it just 9/512 as per some specification, i.e., 
shouldn’t it be independent of qemu’s block layer’s sector size?

> +        return false;
> +    }
> +    blk_get_geometry(vexp->export.blk, &total_sectors);
> +    if (sector > total_sectors || nb_sectors > total_sectors - sector) {
> +        return false;
> +    }
> +    return true;
> +}
> +

[...]

> @@ -170,19 +243,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, elem->out_sg,
> +                                                      out_num, type);

elem->out_sg is different from &elem->out_sg[1], but from what I can 
tell vu_blk_discard_write_zeroes() doesn’t really change in how it 
treats @iov.

I’m confused.  Is that a bug fix?  (If so, it isn’t mentioned in the 
commit message)

Apart from this, the patch looks good to me (although there are the two 
things mentioned above that I find a bit strange, but definitely not wrong).

Max

>           break;
>       }
>       default:

Re: [PATCH for-5.2 09/10] vhost-user-blk-test: test discard/write zeroes invalid inputs

[Max Reitz]

On 11.11.20 13:43, Stefan Hajnoczi wrote:
> Exercise input validation code paths in
> block/export/vhost-user-blk-server.c.
> 
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
>   block/export/vhost-user-blk-server.c |   4 +-
>   tests/qtest/vhost-user-blk-test.c    | 124 +++++++++++++++++++++++++++
>   2 files changed, 126 insertions(+), 2 deletions(-)
> 
> diff --git a/block/export/vhost-user-blk-server.c b/block/export/vhost-user-blk-server.c
> index 3295d3c951..d88e41714d 100644
> --- a/block/export/vhost-user-blk-server.c
> +++ b/block/export/vhost-user-blk-server.c
> @@ -248,8 +248,8 @@ static void coroutine_fn vu_blk_virtio_process_req(void *opaque)
>               break;
>           }
>   
> -        req->in->status = vu_blk_discard_write_zeroes(vexp, elem->out_sg,
> -                                                      out_num, type);
> +        req->in->status = vu_blk_discard_write_zeroes(vexp, out_iov, out_num,
> +                                                      type);
>           break;
>       }
>       default:

Looks like this hunk should be squashed into the previous patch.  I 
think that would lift my confusion.

Max

Re: [PATCH for-5.2 10/10] block/export: port virtio-blk read/write range check

[Max Reitz]

On 11.11.20 13:43, Stefan Hajnoczi wrote:
> 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 | 14 ++++++++++++++
>   1 file changed, 14 insertions(+)
> 
> diff --git a/block/export/vhost-user-blk-server.c b/block/export/vhost-user-blk-server.c
> index d88e41714d..6d7fd0fec3 100644
> --- a/block/export/vhost-user-blk-server.c
> +++ b/block/export/vhost-user-blk-server.c
> @@ -214,9 +214,23 @@ static void coroutine_fn vu_blk_virtio_process_req(void *opaque)
>           QEMUIOVector qiov;
>           if (is_write) {
>               qemu_iovec_init_external(&qiov, out_iov, out_num);
> +
> +            if (unlikely(!vu_blk_sect_range_ok(vexp, req->sector_num,
> +                                               qiov.size))) {
> +                req->in->status = VIRTIO_BLK_S_IOERR;
> +                break;
> +            }
> +
>               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;
> +            }
> +
>               ret = blk_co_preadv(blk, offset, qiov.size, &qiov, 0);
>           }
>           if (ret >= 0) {

req->sector_num is not a block layer sector, though (i.e. not a 512-byte 
sector); it references sectors of size vexp->blk_size (which I presume 
aren’t necessarily 512 bytes in length).

Second, I now understand why vu_blk_sect_range_ok() takes a byte length; 
but with an arbitrary length as given here, it must also round that down 
when converting that length to block layer sectors.  (Or just compare 
the byte length against the result of bdrv_getlength().)

Max

Re: [PATCH for-5.2 04/10] vhost-user-blk-test: rename destroy_drive() to destroy_file()

[Stefan Hajnoczi]

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

On Thu, Nov 12, 2020 at 03:32:58PM +0100, Max Reitz wrote:
> On 11.11.20 13:43, Stefan Hajnoczi wrote:
> > The function is used not just for image files but also for UNIX domain
> > sockets (QMP monitor and vhost-user-blk). Reflect that in the name.
> > 
> > Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> > ---
> >   tests/qtest/vhost-user-blk-test.c | 9 +++++----
> >   1 file changed, 5 insertions(+), 4 deletions(-)
> 
> [...]
> 
> > @@ -731,7 +732,7 @@ static char *start_vhost_user_blk(GString *cmd_line, int vus_instances,
> >           sock_path = g_strdup(sock_path_tempate);
> >           fd = mkstemp(sock_path);
> >           g_assert_cmpint(fd, >=, 0);
> > -        g_test_queue_destroy(drive_destroy, sock_path);
> > +        g_test_queue_destroy(drive_file, sock_path);
> 
> s/drive_file/destroy_file/, I think :)

Oops! The following commit removes this line so I didn't notice this
issue: "vhost-user-blk-test: fix races by using fd passing".

Michael: If you want this intermediate commit fixed for bisectability I
will respin. Otherwise the patches in your tree are fine.

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

Re: [PATCH for-5.2 00/10] block/export: vhost-user-blk server tests and input validation

[Michael S. Tsirkin]

On Wed, Nov 11, 2020 at 12:43:21PM +0000, Stefan Hajnoczi wrote:
> 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.


Peter reports this hanging for him so I dropped this for now.
Pls work with him to resolve, and resubmit.
If possible let's defer the addition of new tests and just fix existing
ones for 5.2.

Thanks!

> Based-on: 20201109174355.1069147-1-stefanha@redhat.com ("[PATCH for-5.2] vhost-user: fix VHOST_USER_ADD/REM_MEM_REG truncation")
> 
> Coiby Xu (1):
>   test: new qTest case to test the vhost-user-blk-server
> 
> Stefan Hajnoczi (9):
>   tests/qtest: add multi-queue test case to vhost-user-blk-test
>   libqtest: add qtest_socket_server()
>   vhost-user-blk-test: rename destroy_drive() to destroy_file()
>   vhost-user-blk-test: close fork child file descriptors
>   vhost-user-blk-test: drop unused return value
>   vhost-user-blk-test: fix races by using fd passing
>   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
> 
>  tests/qtest/libqos/libqtest.h        |  25 +
>  tests/qtest/libqos/vhost-user-blk.h  |  48 ++
>  block/export/vhost-user-blk-server.c | 129 +++-
>  tests/qtest/libqos/vhost-user-blk.c  | 129 ++++
>  tests/qtest/libqtest.c               |  76 ++-
>  tests/qtest/vhost-user-blk-test.c    | 965 +++++++++++++++++++++++++++
>  tests/qtest/libqos/meson.build       |   1 +
>  tests/qtest/meson.build              |   2 +
>  8 files changed, 1333 insertions(+), 42 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
> 

Re: [PATCH for-5.2 05/10] vhost-user-blk-test: close fork child file descriptors

[Coiby Xu]

Hi Stefan,

On Wed, Nov 11, 2020 at 12:43:26PM +0000, Stefan Hajnoczi wrote:
>Do not leave stdin and stdout open after fork. stdout is the
>tap-driver.pl pipe. If we keep the pipe open then tap-driver.pl will not
>detect that qos-test has terminated and it will hang.

I wonder under which situation this would happen. I couldn't re-produce
this issue locally.

>
>Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
>---
> tests/qtest/vhost-user-blk-test.c | 9 +++++++++
> 1 file changed, 9 insertions(+)
>
>diff --git a/tests/qtest/vhost-user-blk-test.c b/tests/qtest/vhost-user-blk-test.c
>index f05f14c192..4019a72ac0 100644
>--- a/tests/qtest/vhost-user-blk-test.c
>+++ b/tests/qtest/vhost-user-blk-test.c
>@@ -749,6 +749,15 @@ static char *start_vhost_user_blk(GString *cmd_line, int vus_instances,
>                    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);
>     }
>--
>2.28.0
>

--
Best regards,
Coiby

Re: [PATCH for-5.2 01/10] test: new qTest case to test the vhost-user-blk-server

[Coiby Xu]

On Wed, Nov 11, 2020 at 12:43:22PM +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 vhost-user server can only server one
>client one time, two instances of vhost-user-blk-server are started by
>qemu-storage-daemon for the hotplug test.
>
>In order to not block scripts/tap-driver.pl, vhost-user-blk-server will
>send "quit" command to qemu-storage-daemon's QMP monitor. So a function
>is added to libqtest.c to establish socket connection with socket
>server.
>
>Suggested-by: Thomas Huth <thuth@redhat.com>
>Signed-off-by: Coiby Xu <coiby.xu@gmail.com>
>Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
>Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>Message-id: 20200918080912.321299-7-coiby.xu@gmail.com
>[Update meson.build to only test when CONFIG_TOOLS has built
>qemu-storage-daemon. This prevents CI failures with --disable-tools.
>Also bump RAM to 256 MB because that is the minimum RAM granularity on
>ppc64 spapr machines.
>--Stefan]
>Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
>---
> tests/qtest/libqos/libqtest.h       |  17 +
> tests/qtest/libqos/vhost-user-blk.h |  48 ++
> tests/qtest/libqos/vhost-user-blk.c | 129 +++++
> tests/qtest/libqtest.c              |  36 +-
> tests/qtest/vhost-user-blk-test.c   | 751 ++++++++++++++++++++++++++++
> tests/qtest/libqos/meson.build      |   1 +
> tests/qtest/meson.build             |   2 +
> 7 files changed, 982 insertions(+), 2 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
>
>diff --git a/tests/qtest/libqos/libqtest.h b/tests/qtest/libqos/libqtest.h
>index 724f65aa94..d6236ea7a0 100644
>--- a/tests/qtest/libqos/libqtest.h
>+++ b/tests/qtest/libqos/libqtest.h
>@@ -132,6 +132,23 @@ 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_client:
>+ * @server_socket_path: the socket server's path
>+ *
>+ * Connect to a socket server.
>+ */
>+int qtest_socket_client(char *server_socket_path);
>+
>+/**
>+ * qtest_create_state_with_qmp_fd:
>+ * @fd: socket fd
>+ *
>+ * Wrap socket fd in QTestState to make use of qtest_qmp*
>+ * functions
>+ */
>+QTestState *qtest_create_state_with_qmp_fd(int fd);
>+
> /**
>  * qtest_vqmp_fds:
>  * @s: #QTestState instance to operate on.
>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..58c7e1eb69
>--- /dev/null
>+++ b/tests/qtest/libqos/vhost-user-blk.c
>@@ -0,0 +1,129 @@
>+/*
>+ * 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/libqtest.c b/tests/qtest/libqtest.c
>index be0fb430dd..ff563cbaba 100644
>--- a/tests/qtest/libqtest.c
>+++ b/tests/qtest/libqtest.c
>@@ -4,11 +4,13 @@
>  * Copyright IBM, Corp. 2012
>  * Copyright Red Hat, Inc. 2012
>  * Copyright SUSE LINUX Products GmbH 2013
>+ * Copyright Copyright (c) Coiby Xu
>  *
>  * Authors:
>  *  Anthony Liguori   <aliguori@us.ibm.com>
>  *  Paolo Bonzini     <pbonzini@redhat.com>
>  *  Andreas Färber    <afaerber@suse.de>
>+ *  Coiby Xu          <coiby.xu@gmail.com>
>  *
>  * This work is licensed under the terms of the GNU GPL, version 2 or later.
>  * See the COPYING file in the top-level directory.
>@@ -52,8 +54,7 @@ typedef struct QTestClientTransportOps {
>     QTestRecvFn     recv_line; /* for receiving qtest command responses */
> } QTestTransportOps;
>
>-struct QTestState
>-{
>+struct QTestState {
>     int fd;
>     int qmp_fd;
>     pid_t qemu_pid;  /* our child QEMU process */
>@@ -635,6 +636,37 @@ QDict *qtest_qmp_receive_dict(QTestState *s)
>     return qmp_fd_receive(s->qmp_fd);
> }
>
>+QTestState *qtest_create_state_with_qmp_fd(int fd)
>+{
>+    QTestState *qmp_test_state = g_new0(QTestState, 1);
>+    qmp_test_state->qmp_fd = fd;
>+    return qmp_test_state;
>+}
>+
>+int qtest_socket_client(char *server_socket_path)
>+{
>+    struct sockaddr_un serv_addr;
>+    int sock;
>+    int ret;
>+    int retries = 0;
>+    sock = socket(PF_UNIX, SOCK_STREAM, 0);
>+    g_assert_cmpint(sock, !=, -1);
>+    serv_addr.sun_family = AF_UNIX;
>+    snprintf(serv_addr.sun_path, sizeof(serv_addr.sun_path), "%s",
>+             server_socket_path);
>+
>+    for (retries = 0; retries < 3; retries++) {
>+        ret = connect(sock, (struct sockaddr *)&serv_addr, sizeof(serv_addr));
>+        if (ret == 0) {
>+            break;
>+        }
>+        g_usleep(G_USEC_PER_SEC);
>+    }
>+
>+    g_assert_cmpint(ret, ==, 0);
>+    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
>diff --git a/tests/qtest/vhost-user-blk-test.c b/tests/qtest/vhost-user-blk-test.c
>new file mode 100644
>index 0000000000..e7e44f9bf0
>--- /dev/null
>+++ b/tests/qtest/vhost-user-blk-test.c
>@@ -0,0 +1,751 @@
>+/*
>+ * 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 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;
>+}
>+
>+static void drive_destroy(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(drive_destroy, t_path);
>+    return t_path;
>+}
>+
>+static char sock_path_tempate[] = "/tmp/qtest.vhost_user_blk.XXXXXX";
>+static char qmp_sock_path_tempate[] = "/tmp/qtest.vhost_user_blk.qmp.XXXXXX";
>+
>+static void quit_storage_daemon(void *qmp_test_state)
>+{
>+    const char quit_str[] = "{ 'execute': 'quit' }";
>+
>+    /* Before quiting storate-daemon, quit qemu to avoid dubious messages */
>+    qobject_unref(qtest_qmp(global_qtest, quit_str));
>+
>+    /*
>+     * Give storage-daemon enough time to wake up&terminate
>+     * vu_client_trip coroutine so the Coroutine object could
>+     * be cleaned up. Otherwise LeakSanitizer would complain
>+     * about memory leaks.
>+     */
>+    g_usleep(1000);

Your "[PATCH for-5.2 07/10] vhost-user-blk-test: fix races by using fd passing"
prompts to me think if there is a race condition under which 1000 ms
is not enough for qemu-storage-daemon to do the cleanups. If this
situation could happen, is there a way to tell if Coroutine objects have
been freed by qemu-storage-daemon?
>+
>+    qobject_unref(qtest_qmp((QTestState *)qmp_test_state, quit_str));
>+    g_free(qmp_test_state);
>+}
>+

> [...]
>--
>2.28.0
>

--
Best regards,
Coiby

Re: [PATCH for-5.2 01/10] test: new qTest case to test the vhost-user-blk-server

[Coiby Xu]

On Wed, Nov 25, 2020 at 04:20:56PM +0800, Coiby Xu wrote:
>On Wed, Nov 11, 2020 at 12:43:22PM +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 vhost-user server can only server one
>>client one time, two instances of vhost-user-blk-server are started by
>>qemu-storage-daemon for the hotplug test.
>>
>>In order to not block scripts/tap-driver.pl, vhost-user-blk-server will
>>send "quit" command to qemu-storage-daemon's QMP monitor. So a function
>>is added to libqtest.c to establish socket connection with socket
>>server.
>>
>>Suggested-by: Thomas Huth <thuth@redhat.com>
>>Signed-off-by: Coiby Xu <coiby.xu@gmail.com>
>>Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
>>Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>>Message-id: 20200918080912.321299-7-coiby.xu@gmail.com
>>[Update meson.build to only test when CONFIG_TOOLS has built
>>qemu-storage-daemon. This prevents CI failures with --disable-tools.
>>Also bump RAM to 256 MB because that is the minimum RAM granularity on
>>ppc64 spapr machines.
>>--Stefan]
>>Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
>>---
>>tests/qtest/libqos/libqtest.h       |  17 +
>>tests/qtest/libqos/vhost-user-blk.h |  48 ++
>>tests/qtest/libqos/vhost-user-blk.c | 129 +++++
>>tests/qtest/libqtest.c              |  36 +-
>>tests/qtest/vhost-user-blk-test.c   | 751 ++++++++++++++++++++++++++++
>>tests/qtest/libqos/meson.build      |   1 +
>>tests/qtest/meson.build             |   2 +
>>7 files changed, 982 insertions(+), 2 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
>>
>>diff --git a/tests/qtest/libqos/libqtest.h b/tests/qtest/libqos/libqtest.h
>>index 724f65aa94..d6236ea7a0 100644
>>--- a/tests/qtest/libqos/libqtest.h
>>+++ b/tests/qtest/libqos/libqtest.h
>>@@ -132,6 +132,23 @@ 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_client:
>>+ * @server_socket_path: the socket server's path
>>+ *
>>+ * Connect to a socket server.
>>+ */
>>+int qtest_socket_client(char *server_socket_path);
>>+
>>+/**
>>+ * qtest_create_state_with_qmp_fd:
>>+ * @fd: socket fd
>>+ *
>>+ * Wrap socket fd in QTestState to make use of qtest_qmp*
>>+ * functions
>>+ */
>>+QTestState *qtest_create_state_with_qmp_fd(int fd);
>>+
>>/**
>> * qtest_vqmp_fds:
>> * @s: #QTestState instance to operate on.
>>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..58c7e1eb69
>>--- /dev/null
>>+++ b/tests/qtest/libqos/vhost-user-blk.c
>>@@ -0,0 +1,129 @@
>>+/*
>>+ * 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/libqtest.c b/tests/qtest/libqtest.c
>>index be0fb430dd..ff563cbaba 100644
>>--- a/tests/qtest/libqtest.c
>>+++ b/tests/qtest/libqtest.c
>>@@ -4,11 +4,13 @@
>> * Copyright IBM, Corp. 2012
>> * Copyright Red Hat, Inc. 2012
>> * Copyright SUSE LINUX Products GmbH 2013
>>+ * Copyright Copyright (c) Coiby Xu
>> *
>> * Authors:
>> *  Anthony Liguori   <aliguori@us.ibm.com>
>> *  Paolo Bonzini     <pbonzini@redhat.com>
>> *  Andreas Färber    <afaerber@suse.de>
>>+ *  Coiby Xu          <coiby.xu@gmail.com>
>> *
>> * This work is licensed under the terms of the GNU GPL, version 2 or later.
>> * See the COPYING file in the top-level directory.
>>@@ -52,8 +54,7 @@ typedef struct QTestClientTransportOps {
>>    QTestRecvFn     recv_line; /* for receiving qtest command responses */
>>} QTestTransportOps;
>>
>>-struct QTestState
>>-{
>>+struct QTestState {
>>    int fd;
>>    int qmp_fd;
>>    pid_t qemu_pid;  /* our child QEMU process */
>>@@ -635,6 +636,37 @@ QDict *qtest_qmp_receive_dict(QTestState *s)
>>    return qmp_fd_receive(s->qmp_fd);
>>}
>>
>>+QTestState *qtest_create_state_with_qmp_fd(int fd)
>>+{
>>+    QTestState *qmp_test_state = g_new0(QTestState, 1);
>>+    qmp_test_state->qmp_fd = fd;
>>+    return qmp_test_state;
>>+}
>>+
>>+int qtest_socket_client(char *server_socket_path)
>>+{
>>+    struct sockaddr_un serv_addr;
>>+    int sock;
>>+    int ret;
>>+    int retries = 0;
>>+    sock = socket(PF_UNIX, SOCK_STREAM, 0);
>>+    g_assert_cmpint(sock, !=, -1);
>>+    serv_addr.sun_family = AF_UNIX;
>>+    snprintf(serv_addr.sun_path, sizeof(serv_addr.sun_path), "%s",
>>+             server_socket_path);
>>+
>>+    for (retries = 0; retries < 3; retries++) {
>>+        ret = connect(sock, (struct sockaddr *)&serv_addr, sizeof(serv_addr));
>>+        if (ret == 0) {
>>+            break;
>>+        }
>>+        g_usleep(G_USEC_PER_SEC);
>>+    }
>>+
>>+    g_assert_cmpint(ret, ==, 0);
>>+    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
>>diff --git a/tests/qtest/vhost-user-blk-test.c b/tests/qtest/vhost-user-blk-test.c
>>new file mode 100644
>>index 0000000000..e7e44f9bf0
>>--- /dev/null
>>+++ b/tests/qtest/vhost-user-blk-test.c
>>@@ -0,0 +1,751 @@
>>+/*
>>+ * 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 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;
>>+}
>>+
>>+static void drive_destroy(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(drive_destroy, t_path);
>>+    return t_path;
>>+}
>>+
>>+static char sock_path_tempate[] = "/tmp/qtest.vhost_user_blk.XXXXXX";
>>+static char qmp_sock_path_tempate[] = "/tmp/qtest.vhost_user_blk.qmp.XXXXXX";
>>+
>>+static void quit_storage_daemon(void *qmp_test_state)
>>+{
>>+    const char quit_str[] = "{ 'execute': 'quit' }";
>>+
>>+    /* Before quiting storate-daemon, quit qemu to avoid dubious messages */
>>+    qobject_unref(qtest_qmp(global_qtest, quit_str));
>>+
>>+    /*
>>+     * Give storage-daemon enough time to wake up&terminate
>>+     * vu_client_trip coroutine so the Coroutine object could
>>+     * be cleaned up. Otherwise LeakSanitizer would complain
>>+     * about memory leaks.
>>+     */
>>+    g_usleep(1000);
>
>Your "[PATCH for-5.2 07/10] vhost-user-blk-test: fix races by using fd passing"
>prompts to me think if there is a race condition under which 1000 ms
                                                               ^^^^^^^
Sorry, I meant 1000 μs.

>is not enough for qemu-storage-daemon to do the cleanups. If this
>situation could happen, is there a way to tell if Coroutine objects have
>been freed by qemu-storage-daemon?
>>+
>>+    qobject_unref(qtest_qmp((QTestState *)qmp_test_state, quit_str));
>>+    g_free(qmp_test_state);
>>+}
>>+
>
>>[...]
>>--
>>2.28.0
>>
>
>--
>Best regards,
>Coiby

--
Best regards,
Coiby

Re: [PATCH for-5.2 05/10] vhost-user-blk-test: close fork child file descriptors

[Stefan Hajnoczi]

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

On Tue, Nov 24, 2020 at 08:08:26PM +0800, Coiby Xu wrote:
> Hi Stefan,
> 
> On Wed, Nov 11, 2020 at 12:43:26PM +0000, Stefan Hajnoczi wrote:
> > Do not leave stdin and stdout open after fork. stdout is the
> > tap-driver.pl pipe. If we keep the pipe open then tap-driver.pl will not
> > detect that qos-test has terminated and it will hang.
> 
> I wonder under which situation this would happen. I couldn't re-produce
> this issue locally.

Try adding g_assert_not_reached() to one of the tests so the qos-test
process aborts. Then tap-driver.pl will hang because the pipe hasn't
been closed and "make check" never completes.

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

Re: [PATCH for-5.2 01/10] test: new qTest case to test the vhost-user-blk-server

[Stefan Hajnoczi]

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

On Wed, Nov 25, 2020 at 04:28:20PM +0800, Coiby Xu wrote:
> On Wed, Nov 25, 2020 at 04:20:56PM +0800, Coiby Xu wrote:
> > On Wed, Nov 11, 2020 at 12:43:22PM +0000, Stefan Hajnoczi wrote:
> > > +static void quit_storage_daemon(void *qmp_test_state)
> > > +{
> > > +    const char quit_str[] = "{ 'execute': 'quit' }";
> > > +
> > > +    /* Before quiting storate-daemon, quit qemu to avoid dubious messages */
> > > +    qobject_unref(qtest_qmp(global_qtest, quit_str));
> > > +
> > > +    /*
> > > +     * Give storage-daemon enough time to wake up&terminate
> > > +     * vu_client_trip coroutine so the Coroutine object could
> > > +     * be cleaned up. Otherwise LeakSanitizer would complain
> > > +     * about memory leaks.
> > > +     */
> > > +    g_usleep(1000);
> > 
> > Your "[PATCH for-5.2 07/10] vhost-user-blk-test: fix races by using fd passing"
> > prompts to me think if there is a race condition under which 1000 ms
>                                                               ^^^^^^^
> Sorry, I meant 1000 μs.

In the next revision vhost-user-blk-test sends a SIGTERM signal to
qemu-storage-daemon and then calls waitpid(2). This way there is a clean
shutdown without a sleep.

Regarding the LeakSanitizer issue you saw, are you still able to
reproduce it with commit f10802d2c9fd8bfd92c70f465da1a5992445157f
("qemu-storage-daemon: add missing cleanup calls") applied? Maybe
qemu-storage-daemon is still missing some cleanup code (e.g. to stop
exports before terminating).

Stefan

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

Re: [PATCH for-5.2 09/10] vhost-user-blk-test: test discard/write zeroes invalid inputs

[Stefan Hajnoczi]

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

On Thu, Nov 12, 2020 at 04:40:07PM +0100, Max Reitz wrote:
> On 11.11.20 13:43, Stefan Hajnoczi wrote:
> > Exercise input validation code paths in
> > block/export/vhost-user-blk-server.c.
> > 
> > Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> > ---
> >   block/export/vhost-user-blk-server.c |   4 +-
> >   tests/qtest/vhost-user-blk-test.c    | 124 +++++++++++++++++++++++++++
> >   2 files changed, 126 insertions(+), 2 deletions(-)
> > 
> > diff --git a/block/export/vhost-user-blk-server.c b/block/export/vhost-user-blk-server.c
> > index 3295d3c951..d88e41714d 100644
> > --- a/block/export/vhost-user-blk-server.c
> > +++ b/block/export/vhost-user-blk-server.c
> > @@ -248,8 +248,8 @@ static void coroutine_fn vu_blk_virtio_process_req(void *opaque)
> >               break;
> >           }
> > -        req->in->status = vu_blk_discard_write_zeroes(vexp, elem->out_sg,
> > -                                                      out_num, type);
> > +        req->in->status = vu_blk_discard_write_zeroes(vexp, out_iov, out_num,
> > +                                                      type);
> >           break;
> >       }
> >       default:
> 
> Looks like this hunk should be squashed into the previous patch.  I think
> that would lift my confusion.

Thanks, will fix.

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

Re: [PATCH for-5.2 08/10] block/export: port virtio-blk discard/write zeroes input validation

[Stefan Hajnoczi]

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

On Thu, Nov 12, 2020 at 04:25:39PM +0100, Max Reitz wrote:
> On 11.11.20 13:43, Stefan Hajnoczi wrote:
> > @@ -58,30 +60,101 @@ 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) {
> 
> I wonder why you pass a byte length, then shift it down to sectors, when
> it’s kind of unsafe on the caller’s side to even calculate that byte length
> (because the caller has to verify that shifting the sector count up to be a
> byte length is safe).
> 
> Wouldn’t it be simpler to pass nb_sectors (perhaps even as uint64_t, because
> why not) and then compare that against BDRV_REQUEST_MAX_BYTES here?
> 
> (With how the code is written, it also has the problem of rounding down
> @size.  Which isn’t a problem in practice because the caller effectively
> guarantees that @size is aligned to sectors, but it still means that the
> code looks a bit strange.  As in, “Why is it safe to round down?  Ah,
> because the caller always produces an aligned value.  But why does the
> caller even pass a byte count, then?  Especially given that the offset is
> passed as a sector index, too.”)

Thanks for the suggestions, I'll take a look for the next revision.

> > +        return false;
> > +    }
> > +    if ((sector << BDRV_SECTOR_BITS) % vexp->blk_size) {
> 
> This made me wonder why the discard/write-zeroes sector granularity would be
> BDRV_SECTOR_BITS and not blk_size, which is used for IN/OUT (read/write)
> requests.
> 
> I still don’t know, but judging from the only reference I could find quickly
> (contrib/vhost-user-blk/vhost-user-blk.c), it seems correct.
> 
> OTOH, I wonder whether BDRV_SECTOR_BITS/BDRV_SECTOR_SIZE are the correct
> constants.  Isn’t it just 9/512 as per some specification, i.e., shouldn’t
> it be independent of qemu’s block layer’s sector size?

Yes, a new VIRTIO_BLK_SECTOR_SIZE constant would be clearer. According
to the VIRTIO 1.1 specification:

  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.

So all virtio-blk sector fields are in 512-byte units, regardless of the
blk_size config field.

Stefan

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

Re: [PATCH for-5.2 05/10] vhost-user-blk-test: close fork child file descriptors

[Coiby Xu]

On Thu, Dec 03, 2020 at 01:57:21PM +0000, Stefan Hajnoczi wrote:
>On Tue, Nov 24, 2020 at 08:08:26PM +0800, Coiby Xu wrote:
>> Hi Stefan,
>>
>> On Wed, Nov 11, 2020 at 12:43:26PM +0000, Stefan Hajnoczi wrote:
>> > Do not leave stdin and stdout open after fork. stdout is the
>> > tap-driver.pl pipe. If we keep the pipe open then tap-driver.pl will not
>> > detect that qos-test has terminated and it will hang.
>>
>> I wonder under which situation this would happen. I couldn't re-produce
>> this issue locally.
>
>Try adding g_assert_not_reached() to one of the tests so the qos-test
>process aborts. Then tap-driver.pl will hang because the pipe hasn't
>been closed and "make check" never completes.

Thank you for the explanation! I thought closing fork child file is
for dealing with a subtle race condition. So it's actually for dealing
with the situation when g_assert_* fails.

Do you think g_spawn_async_with_pipes is a better way than fork/exec
since g_spawn_* could help us take care of closing standard file
descriptors?

     g_test_message("starting vhost-user backend: %s",
                    storage_daemon_command->str);

     GPid child_pid;
     g_autoptr(GError) error = NULL;
     const gchar * const argv[] = { "/bin/sh", "-c", storage_daemon_command->str, NULL };
     g_spawn_async_with_pipes (NULL, argv, NULL, G_SPAWN_STDOUT_TO_DEV_NULL, NULL,
             NULL, &child_pid, NULL, NULL,
             NULL, &error);

     if (error != NULL)
     {
         g_error ("Starting qemu-storage-daemon failed: %s", error->message);
         abort();
     }
     g_string_free(storage_daemon_command, true);



--
Best regards,
Coiby

Re: [PATCH for-5.2 01/10] test: new qTest case to test the vhost-user-blk-server

[Coiby Xu]

On Mon, Dec 07, 2020 at 11:28:38AM +0000, Stefan Hajnoczi wrote:
>On Wed, Nov 25, 2020 at 04:28:20PM +0800, Coiby Xu wrote:
>> On Wed, Nov 25, 2020 at 04:20:56PM +0800, Coiby Xu wrote:
>> > On Wed, Nov 11, 2020 at 12:43:22PM +0000, Stefan Hajnoczi wrote:
>> > > +static void quit_storage_daemon(void *qmp_test_state)
>> > > +{
>> > > +    const char quit_str[] = "{ 'execute': 'quit' }";
>> > > +
>> > > +    /* Before quiting storate-daemon, quit qemu to avoid dubious messages */
>> > > +    qobject_unref(qtest_qmp(global_qtest, quit_str));
>> > > +
>> > > +    /*
>> > > +     * Give storage-daemon enough time to wake up&terminate
>> > > +     * vu_client_trip coroutine so the Coroutine object could
>> > > +     * be cleaned up. Otherwise LeakSanitizer would complain
>> > > +     * about memory leaks.
>> > > +     */
>> > > +    g_usleep(1000);
>> >
>> > Your "[PATCH for-5.2 07/10] vhost-user-blk-test: fix races by using fd passing"
>> > prompts to me think if there is a race condition under which 1000 ms
>>                                                               ^^^^^^^
>> Sorry, I meant 1000 μs.
>
>In the next revision vhost-user-blk-test sends a SIGTERM signal to
>qemu-storage-daemon and then calls waitpid(2). This way there is a clean
>shutdown without a sleep.
>

Thank you for the explaining! Do you think checking if
qemu-storage-daemon has exited by detecting if qemu-storage-daemon's
QMP monitor socket has been closed is simpler solution than
waitpid(2) in v2?

>Regarding the LeakSanitizer issue you saw, are you still able to
>reproduce it with commit f10802d2c9fd8bfd92c70f465da1a5992445157f
>("qemu-storage-daemon: add missing cleanup calls") applied? Maybe
>qemu-storage-daemon is still missing some cleanup code (e.g. to stop
>exports before terminating).

It seems commit f10802d2c9fd8bfd92c70f465da1a5992445157f
("qemu-storage-daemon: add missing cleanup calls") is not related to
the LeakSanitizer issue.

When applying v9 (no "g_usleep(1000)") on
commit eea8f5df4ecc607d64f091b8d916fcc11a697541
("Merge remote-tracking branch 'remotes/awilliam/tags/vfio-update-20200611.0' into staging")
which doesn't contain commit f10802d2c9fd8bfd92c70f465da1a5992445157f,
there is no LeakSanitizer issue.

However, if applying v9 on
commit d0ed6a69d399ae193959225cdeaa9382746c91cc ("Update version for v5.1.0 release")
which contains commit f10802d2c9fd8bfd92c70f465da1a5992445157f, there is
the LeakSanitizer issue.

And if applying this patch set on
commit 23895cbd82be95428e90168b12e925d0d3ca2f06
("Merge remote-tracking branch 'remotes/awilliam/tags/vfio-update-20201123.0' into staging"),
the LeakSanitizer issue couldn't be reproduced even without "g_usleep(1000)".

>
>Stefan



--
Best regards,
Coiby

Re: [PATCH for-5.2 00/10] block/export: vhost-user-blk server tests and input validation

[Peter Maydell]

On Tue, 17 Nov 2020 at 09:18, Michael S. Tsirkin <mst@redhat.com> wrote:
>
> On Wed, Nov 11, 2020 at 12:43:21PM +0000, Stefan Hajnoczi wrote:
> > 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.
>
>
> Peter reports this hanging for him so I dropped this for now.
> Pls work with him to resolve, and resubmit.
> If possible let's defer the addition of new tests and just fix existing
> ones for 5.2.

Ping! This series was trying to fix a Coverity error -- it got dropped
for 5.2 but now 6.0 is open what are the plans for it?

thanks
-- PMM