* [PULL 01/17] vhost-user: fix VHOST_USER_ADD/REM_MEM_REG truncation
2020-11-15 22:27 [PULL 00/17] pc,vhost: fixes, new test Michael S. Tsirkin
@ 2020-11-15 22:27 ` Michael S. Tsirkin
2020-11-15 22:27 ` [PULL 02/17] meson: move vhost_user_blk_server to meson.build Michael S. Tsirkin
` (16 subsequent siblings)
17 siblings, 0 replies; 23+ messages in thread
From: Michael S. Tsirkin @ 2020-11-15 22:27 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Maydell, Cornelia Huck, Christian Borntraeger,
Stefan Hajnoczi, Raphael Norwitz
From: Stefan Hajnoczi <stefanha@redhat.com>
QEMU currently truncates the mmap_offset field when sending
VHOST_USER_ADD_MEM_REG and VHOST_USER_REM_MEM_REG messages. The struct
layout looks like this:
typedef struct VhostUserMemoryRegion {
uint64_t guest_phys_addr;
uint64_t memory_size;
uint64_t userspace_addr;
uint64_t mmap_offset;
} VhostUserMemoryRegion;
typedef struct VhostUserMemRegMsg {
uint32_t padding;
/* WARNING: there is a 32-bit hole here! */
VhostUserMemoryRegion region;
} VhostUserMemRegMsg;
The payload size is calculated as follows when sending the message in
hw/virtio/vhost-user.c:
msg->hdr.size = sizeof(msg->payload.mem_reg.padding) +
sizeof(VhostUserMemoryRegion);
This calculation produces an incorrect result of only 36 bytes.
sizeof(VhostUserMemRegMsg) is actually 40 bytes.
The consequence of this is that the final field, mmap_offset, is
truncated. This breaks x86_64 TCG guests on s390 hosts. Other guest/host
combinations may get lucky if either of the following holds:
1. The guest memory layout does not need mmap_offset != 0.
2. The host is little-endian and mmap_offset <= 0xffffffff so the
truncation has no effect.
Fix this by extending the existing 32-bit padding field to 64-bit. Now
the padding reflects the actual compiler padding. This can be verified
using pahole(1).
Also document the layout properly in the vhost-user specification. The
vhost-user spec did not document the exact layout. It would be
impossible to implement the spec without looking at the QEMU source
code.
Existing vhost-user frontends and device backends continue to work after
this fix has been applied. The only change in the wire protocol is that
QEMU now sets hdr.size to 40 instead of 36. If a vhost-user
implementation has a hardcoded size check for 36 bytes, then it will
fail with new QEMUs. Both QEMU and DPDK/SPDK don't check the exact
payload size, so they continue to work.
Fixes: f1aeb14b0809e313c74244d838645ed25e85ea63 ("Transmit vhost-user memory regions individually")
Cc: Raphael Norwitz <raphael.norwitz@nutanix.com>
Cc: Cornelia Huck <cohuck@redhat.com>
Cc: Michael S. Tsirkin <mst@redhat.com>
Cc: Christian Borntraeger <borntraeger@de.ibm.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-Id: <20201109174355.1069147-1-stefanha@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Fixes: f1aeb14b0809 ("Transmit vhost-user memory regions individually")
Reviewed-by: Cornelia Huck <cohuck@redhat.com>
Reviewed-by: Raphael Norwitz <raphael.norwitz@nutanix.com>
---
contrib/libvhost-user/libvhost-user.h | 2 +-
hw/virtio/vhost-user.c | 5 ++---
docs/interop/vhost-user.rst | 21 +++++++++++++++++++--
3 files changed, 22 insertions(+), 6 deletions(-)
diff --git a/contrib/libvhost-user/libvhost-user.h b/contrib/libvhost-user/libvhost-user.h
index a1539dbb69..7d47f1364a 100644
--- a/contrib/libvhost-user/libvhost-user.h
+++ b/contrib/libvhost-user/libvhost-user.h
@@ -136,7 +136,7 @@ typedef struct VhostUserMemory {
} VhostUserMemory;
typedef struct VhostUserMemRegMsg {
- uint32_t padding;
+ uint64_t padding;
VhostUserMemoryRegion region;
} VhostUserMemRegMsg;
diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
index 9c5b4f7fbc..2fdd5daf74 100644
--- a/hw/virtio/vhost-user.c
+++ b/hw/virtio/vhost-user.c
@@ -149,7 +149,7 @@ typedef struct VhostUserMemory {
} VhostUserMemory;
typedef struct VhostUserMemRegMsg {
- uint32_t padding;
+ uint64_t padding;
VhostUserMemoryRegion region;
} VhostUserMemRegMsg;
@@ -800,8 +800,7 @@ static int vhost_user_add_remove_regions(struct vhost_dev *dev,
uint64_t shadow_pcb[VHOST_USER_MAX_RAM_SLOTS] = {};
int nr_add_reg, nr_rem_reg;
- msg->hdr.size = sizeof(msg->payload.mem_reg.padding) +
- sizeof(VhostUserMemoryRegion);
+ msg->hdr.size = sizeof(msg->payload.mem_reg);
/* Find the regions which need to be removed or added. */
scrub_shadow_regions(dev, add_reg, &nr_add_reg, rem_reg, &nr_rem_reg,
diff --git a/docs/interop/vhost-user.rst b/docs/interop/vhost-user.rst
index 988f154144..6d4025ba6a 100644
--- a/docs/interop/vhost-user.rst
+++ b/docs/interop/vhost-user.rst
@@ -131,6 +131,23 @@ A region is:
:mmap offset: 64-bit offset where region starts in the mapped memory
+Single memory region description
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
++---------+---------------+------+--------------+-------------+
+| padding | guest address | size | user address | mmap offset |
++---------+---------------+------+--------------+-------------+
+
+:padding: 64-bit
+
+:guest address: a 64-bit guest address of the region
+
+:size: a 64-bit size
+
+:user address: a 64-bit user address
+
+:mmap offset: 64-bit offset where region starts in the mapped memory
+
Log description
^^^^^^^^^^^^^^^
@@ -1281,7 +1298,7 @@ Master message types
``VHOST_USER_ADD_MEM_REG``
:id: 37
:equivalent ioctl: N/A
- :slave payload: memory region
+ :slave payload: single memory region description
When the ``VHOST_USER_PROTOCOL_F_CONFIGURE_MEM_SLOTS`` protocol
feature has been successfully negotiated, this message is submitted
@@ -1296,7 +1313,7 @@ Master message types
``VHOST_USER_REM_MEM_REG``
:id: 38
:equivalent ioctl: N/A
- :slave payload: memory region
+ :slave payload: single memory region description
When the ``VHOST_USER_PROTOCOL_F_CONFIGURE_MEM_SLOTS`` protocol
feature has been successfully negotiated, this message is submitted
--
MST
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PULL 02/17] meson: move vhost_user_blk_server to meson.build
2020-11-15 22:27 [PULL 00/17] pc,vhost: fixes, new test Michael S. Tsirkin
2020-11-15 22:27 ` [PULL 01/17] vhost-user: fix VHOST_USER_ADD/REM_MEM_REG truncation Michael S. Tsirkin
@ 2020-11-15 22:27 ` Michael S. Tsirkin
2020-11-15 22:27 ` [PULL 03/17] vhost-user-blk-server: depend on CONFIG_VHOST_USER Michael S. Tsirkin
` (15 subsequent siblings)
17 siblings, 0 replies; 23+ messages in thread
From: Michael S. Tsirkin @ 2020-11-15 22:27 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, Peter Maydell, qemu-block, Max Reitz,
Stefan Hajnoczi, Philippe Mathieu-Daudé
From: Stefan Hajnoczi <stefanha@redhat.com>
The --enable/disable-vhost-user-blk-server options were implemented in
./configure. There has been confusion about them and part of the problem
is that the shell syntax used for setting the default value is not easy
to read. Move the option over to meson where the conditions are easier
to understand:
have_vhost_user_blk_server = (targetos == 'linux')
if get_option('vhost_user_blk_server').enabled()
if targetos != 'linux'
error('vhost_user_blk_server requires linux')
endif
elif get_option('vhost_user_blk_server').disabled() or not have_system
have_vhost_user_blk_server = false
endif
This patch does not change behavior.
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-Id: <20201110171121.1265142-2-stefanha@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
meson_options.txt | 2 ++
configure | 16 ++++------------
block/export/meson.build | 5 ++++-
meson.build | 12 ++++++++++++
4 files changed, 22 insertions(+), 13 deletions(-)
diff --git a/meson_options.txt b/meson_options.txt
index b4f1801875..f6f64785fe 100644
--- a/meson_options.txt
+++ b/meson_options.txt
@@ -64,6 +64,8 @@ option('xkbcommon', type : 'feature', value : 'auto',
description: 'xkbcommon support')
option('virtiofsd', type: 'feature', value: 'auto',
description: 'build virtiofs daemon (virtiofsd)')
+option('vhost_user_blk_server', type: 'feature', value: 'auto',
+ description: 'build vhost-user-blk server')
option('capstone', type: 'combo', value: 'auto',
choices: ['disabled', 'enabled', 'auto', 'system', 'internal'],
diff --git a/configure b/configure
index 4cef321d9d..516f28a088 100755
--- a/configure
+++ b/configure
@@ -329,7 +329,7 @@ vhost_crypto=""
vhost_scsi=""
vhost_vsock=""
vhost_user=""
-vhost_user_blk_server=""
+vhost_user_blk_server="auto"
vhost_user_fs=""
kvm="auto"
hax="auto"
@@ -1247,9 +1247,9 @@ for opt do
;;
--enable-vhost-vsock) vhost_vsock="yes"
;;
- --disable-vhost-user-blk-server) vhost_user_blk_server="no"
+ --disable-vhost-user-blk-server) vhost_user_blk_server="disabled"
;;
- --enable-vhost-user-blk-server) vhost_user_blk_server="yes"
+ --enable-vhost-user-blk-server) vhost_user_blk_server="enabled"
;;
--disable-vhost-user-fs) vhost_user_fs="no"
;;
@@ -2390,12 +2390,6 @@ if test "$vhost_net" = ""; then
test "$vhost_kernel" = "yes" && vhost_net=yes
fi
-# libvhost-user is Linux-only
-test "$vhost_user_blk_server" = "" && vhost_user_blk_server=$linux
-if test "$vhost_user_blk_server" = "yes" && test "$linux" = "no"; then
- error_exit "--enable-vhost-user-blk-server is only available on Linux"
-fi
-
##########################################
# pkg-config probe
@@ -6289,9 +6283,6 @@ fi
if test "$vhost_vdpa" = "yes" ; then
echo "CONFIG_VHOST_VDPA=y" >> $config_host_mak
fi
-if test "$vhost_user_blk_server" = "yes" ; then
- echo "CONFIG_VHOST_USER_BLK_SERVER=y" >> $config_host_mak
-fi
if test "$vhost_user_fs" = "yes" ; then
echo "CONFIG_VHOST_USER_FS=y" >> $config_host_mak
fi
@@ -7012,6 +7003,7 @@ NINJA=$ninja $meson setup \
-Dcapstone=$capstone -Dslirp=$slirp -Dfdt=$fdt \
-Diconv=$iconv -Dcurses=$curses -Dlibudev=$libudev\
-Ddocs=$docs -Dsphinx_build=$sphinx_build -Dinstall_blobs=$blobs \
+ -Dvhost_user_blk_server=$vhost_user_blk_server \
$cross_arg \
"$PWD" "$source_path"
diff --git a/block/export/meson.build b/block/export/meson.build
index 19526435d8..135b356775 100644
--- a/block/export/meson.build
+++ b/block/export/meson.build
@@ -1,2 +1,5 @@
blockdev_ss.add(files('export.c'))
-blockdev_ss.add(when: 'CONFIG_VHOST_USER_BLK_SERVER', if_true: files('vhost-user-blk-server.c'))
+
+if have_vhost_user_blk_server
+ blockdev_ss.add(files('vhost-user-blk-server.c'))
+endif
diff --git a/meson.build b/meson.build
index b473620321..4b789f18c1 100644
--- a/meson.build
+++ b/meson.build
@@ -751,6 +751,16 @@ statx_test = '''
has_statx = cc.links(statx_test)
+have_vhost_user_blk_server = (targetos == 'linux')
+
+if get_option('vhost_user_blk_server').enabled()
+ if targetos != 'linux'
+ error('vhost_user_blk_server requires linux')
+ endif
+elif get_option('vhost_user_blk_server').disabled() or not have_system
+ have_vhost_user_blk_server = false
+endif
+
#################
# config-host.h #
#################
@@ -775,6 +785,7 @@ config_host_data.set('CONFIG_MPATH_NEW_API', mpathpersist_new_api)
config_host_data.set('CONFIG_CURSES', curses.found())
config_host_data.set('CONFIG_SDL', sdl.found())
config_host_data.set('CONFIG_SDL_IMAGE', sdl_image.found())
+config_host_data.set('CONFIG_VHOST_USER_BLK_SERVER', have_vhost_user_blk_server)
config_host_data.set('CONFIG_VNC', vnc.found())
config_host_data.set('CONFIG_VNC_JPEG', jpeg.found())
config_host_data.set('CONFIG_VNC_PNG', png.found())
@@ -2103,6 +2114,7 @@ summary_info += {'vhost-crypto support': config_host.has_key('CONFIG_VHOST_CRYPT
summary_info += {'vhost-scsi support': config_host.has_key('CONFIG_VHOST_SCSI')}
summary_info += {'vhost-vsock support': config_host.has_key('CONFIG_VHOST_VSOCK')}
summary_info += {'vhost-user support': config_host.has_key('CONFIG_VHOST_KERNEL')}
+summary_info += {'vhost-user-blk server support': have_vhost_user_blk_server}
summary_info += {'vhost-user-fs support': config_host.has_key('CONFIG_VHOST_USER_FS')}
summary_info += {'vhost-vdpa support': config_host.has_key('CONFIG_VHOST_VDPA')}
summary_info += {'Trace backends': config_host['TRACE_BACKENDS']}
--
MST
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PULL 03/17] vhost-user-blk-server: depend on CONFIG_VHOST_USER
2020-11-15 22:27 [PULL 00/17] pc,vhost: fixes, new test Michael S. Tsirkin
2020-11-15 22:27 ` [PULL 01/17] vhost-user: fix VHOST_USER_ADD/REM_MEM_REG truncation Michael S. Tsirkin
2020-11-15 22:27 ` [PULL 02/17] meson: move vhost_user_blk_server to meson.build Michael S. Tsirkin
@ 2020-11-15 22:27 ` Michael S. Tsirkin
2020-11-15 22:27 ` [PULL 04/17] configure: mark vhost-user Linux-only Michael S. Tsirkin
` (14 subsequent siblings)
17 siblings, 0 replies; 23+ messages in thread
From: Michael S. Tsirkin @ 2020-11-15 22:27 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Maydell, Philippe Mathieu-Daudé,
Richard Henderson, Stefan Hajnoczi, Paolo Bonzini,
Marc-André Lureau, Alex Bennée
From: Stefan Hajnoczi <stefanha@redhat.com>
I interpreted CONFIG_VHOST_USER as controlling only QEMU's vhost-user
device frontends. However, virtiofsd and contrib/ vhost-user device
backends are also controlled by CONFIG_VHOST_USER. Make the
vhost-user-blk server depend on CONFIG_VHOST_USER for consistency.
Now the following error is printed when the vhost-user-blk server is
enabled without CONFIG_VHOST_USER:
$ ./configure --disable-vhost-user --enable-vhost-user-blk ...
../meson.build:761:8: ERROR: Problem encountered: vhost_user_blk_server requires vhost-user support
Suggested-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Suggested-by: Alex Bennée <alex.bennee@linaro.org>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-Id: <20201110171121.1265142-3-stefanha@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
meson.build | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/meson.build b/meson.build
index 4b789f18c1..7fd874eec7 100644
--- a/meson.build
+++ b/meson.build
@@ -751,11 +751,14 @@ statx_test = '''
has_statx = cc.links(statx_test)
-have_vhost_user_blk_server = (targetos == 'linux')
+have_vhost_user_blk_server = (targetos == 'linux' and
+ 'CONFIG_VHOST_USER' in config_host)
if get_option('vhost_user_blk_server').enabled()
if targetos != 'linux'
error('vhost_user_blk_server requires linux')
+ elif 'CONFIG_VHOST_USER' not in config_host
+ error('vhost_user_blk_server requires vhost-user support')
endif
elif get_option('vhost_user_blk_server').disabled() or not have_system
have_vhost_user_blk_server = false
--
MST
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PULL 04/17] configure: mark vhost-user Linux-only
2020-11-15 22:27 [PULL 00/17] pc,vhost: fixes, new test Michael S. Tsirkin
` (2 preceding siblings ...)
2020-11-15 22:27 ` [PULL 03/17] vhost-user-blk-server: depend on CONFIG_VHOST_USER Michael S. Tsirkin
@ 2020-11-15 22:27 ` Michael S. Tsirkin
2020-11-15 22:27 ` [PULL 05/17] hw/i386/acpi-build: Fix maybe-uninitialized error when ACPI hotplug off Michael S. Tsirkin
` (13 subsequent siblings)
17 siblings, 0 replies; 23+ messages in thread
From: Michael S. Tsirkin @ 2020-11-15 22:27 UTC (permalink / raw)
To: qemu-devel
Cc: Laurent Vivier, Peter Maydell, Thomas Huth, Richard Henderson,
Stefan Hajnoczi, Marc-André Lureau, Paolo Bonzini,
Philippe Mathieu-Daudé
From: Stefan Hajnoczi <stefanha@redhat.com>
The vhost-user protocol uses the Linux eventfd feature and is typically
connected to Linux kvm.ko ioeventfd and irqfd file descriptors. The
protocol specification in docs/interop/vhost-user.rst does not describe
how platforms without eventfd support work.
The QEMU vhost-user devices compile on other POSIX host operating
systems because eventfd usage is abstracted in QEMU. The libvhost-user
programs in contrib/ do not compile but we failed to notice since they
are not built by default.
Make it clear that vhost-user is only supported on Linux for the time
being. If someone wishes to support it on other platforms then the
details can be added to vhost-user.rst and CI jobs can test the feature
to prevent bitrot.
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-Id: <20201110171121.1265142-4-stefanha@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
configure | 9 ++++-----
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/configure b/configure
index 516f28a088..3fbc2a0c68 100755
--- a/configure
+++ b/configure
@@ -328,7 +328,7 @@ vhost_net=""
vhost_crypto=""
vhost_scsi=""
vhost_vsock=""
-vhost_user=""
+vhost_user="no"
vhost_user_blk_server="auto"
vhost_user_fs=""
kvm="auto"
@@ -718,7 +718,6 @@ fi
case $targetos in
MINGW32*)
mingw32="yes"
- vhost_user="no"
audio_possible_drivers="dsound sdl"
if check_include dsound.h; then
audio_drv_list="dsound"
@@ -797,6 +796,7 @@ Linux)
audio_possible_drivers="oss alsa sdl pa"
linux="yes"
linux_user="yes"
+ vhost_user="yes"
;;
esac
@@ -2341,9 +2341,8 @@ fi
# vhost interdependencies and host support
# vhost backends
-test "$vhost_user" = "" && vhost_user=yes
-if test "$vhost_user" = "yes" && test "$mingw32" = "yes"; then
- error_exit "vhost-user isn't available on win32"
+if test "$vhost_user" = "yes" && test "$linux" != "yes"; then
+ error_exit "vhost-user is only available on Linux"
fi
test "$vhost_vdpa" = "" && vhost_vdpa=$linux
if test "$vhost_vdpa" = "yes" && test "$linux" != "yes"; then
--
MST
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PULL 05/17] hw/i386/acpi-build: Fix maybe-uninitialized error when ACPI hotplug off
2020-11-15 22:27 [PULL 00/17] pc,vhost: fixes, new test Michael S. Tsirkin
` (3 preceding siblings ...)
2020-11-15 22:27 ` [PULL 04/17] configure: mark vhost-user Linux-only Michael S. Tsirkin
@ 2020-11-15 22:27 ` Michael S. Tsirkin
2020-11-16 17:44 ` Ani Sinha
2020-11-15 22:27 ` [PULL 06/17] test: new qTest case to test the vhost-user-blk-server Michael S. Tsirkin
` (12 subsequent siblings)
17 siblings, 1 reply; 23+ messages in thread
From: Michael S. Tsirkin @ 2020-11-15 22:27 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Maydell, Eduardo Habkost, Igor Mammedov, Ani Sinha,
Paolo Bonzini, Philippe Mathieu-Daudé,
Richard Henderson
From: Philippe Mathieu-Daudé <philmd@redhat.com>
GCC 9.3.0 thinks that 'method' can be left uninitialized. This code
is already in the "if (bsel || pcihp_bridge_en)" block statement,
but it isn't smart enough to figure it out.
Restrict the code to be used only in the "if (bsel || pcihp_bridge_en)"
block statement to fix (on Ubuntu):
../hw/i386/acpi-build.c: In function 'build_append_pci_bus_devices':
../hw/i386/acpi-build.c:496:9: error: 'method' may be used uninitialized
in this function [-Werror=maybe-uninitialized]
496 | aml_append(parent_scope, method);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
cc1: all warnings being treated as errors
Fixes: df4008c9c59 ("piix4: don't reserve hw resources when hotplug is off globally")
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-Id: <20201107194045.438027-1-philmd@redhat.com>
Acked-by: Ani Sinha <ani@anisinha.ca>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
hw/i386/acpi-build.c | 45 +++++++++++++++++++++-----------------------
1 file changed, 21 insertions(+), 24 deletions(-)
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 4f66642d88..1f5c211245 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -465,34 +465,31 @@ static void build_append_pci_bus_devices(Aml *parent_scope, PCIBus *bus,
*/
if (bsel || pcihp_bridge_en) {
method = aml_method("PCNT", 0, AML_NOTSERIALIZED);
- }
- /* If bus supports hotplug select it and notify about local events */
- if (bsel) {
- uint64_t bsel_val = qnum_get_uint(qobject_to(QNum, bsel));
- aml_append(method, aml_store(aml_int(bsel_val), aml_name("BNUM")));
- aml_append(method,
- aml_call2("DVNT", aml_name("PCIU"), aml_int(1) /* Device Check */)
- );
- aml_append(method,
- aml_call2("DVNT", aml_name("PCID"), aml_int(3)/* Eject Request */)
- );
- }
+ /* If bus supports hotplug select it and notify about local events */
+ if (bsel) {
+ uint64_t bsel_val = qnum_get_uint(qobject_to(QNum, bsel));
- /* Notify about child bus events in any case */
- if (pcihp_bridge_en) {
- QLIST_FOREACH(sec, &bus->child, sibling) {
- int32_t devfn = sec->parent_dev->devfn;
-
- if (pci_bus_is_root(sec) || pci_bus_is_express(sec)) {
- continue;
- }
-
- aml_append(method, aml_name("^S%.02X.PCNT", devfn));
+ aml_append(method, aml_store(aml_int(bsel_val), aml_name("BNUM")));
+ aml_append(method, aml_call2("DVNT", aml_name("PCIU"),
+ aml_int(1))); /* Device Check */
+ aml_append(method, aml_call2("DVNT", aml_name("PCID"),
+ aml_int(3))); /* Eject Request */
+ }
+
+ /* Notify about child bus events in any case */
+ if (pcihp_bridge_en) {
+ QLIST_FOREACH(sec, &bus->child, sibling) {
+ int32_t devfn = sec->parent_dev->devfn;
+
+ if (pci_bus_is_root(sec) || pci_bus_is_express(sec)) {
+ continue;
+ }
+
+ aml_append(method, aml_name("^S%.02X.PCNT", devfn));
+ }
}
- }
- if (bsel || pcihp_bridge_en) {
aml_append(parent_scope, method);
}
qobject_unref(bsel);
--
MST
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [PULL 05/17] hw/i386/acpi-build: Fix maybe-uninitialized error when ACPI hotplug off
2020-11-15 22:27 ` [PULL 05/17] hw/i386/acpi-build: Fix maybe-uninitialized error when ACPI hotplug off Michael S. Tsirkin
@ 2020-11-16 17:44 ` Ani Sinha
2020-11-16 18:01 ` Philippe Mathieu-Daudé
0 siblings, 1 reply; 23+ messages in thread
From: Ani Sinha @ 2020-11-16 17:44 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Peter Maydell, Eduardo Habkost, qemu-devel, Paolo Bonzini,
Igor Mammedov, Philippe Mathieu-Daudé,
Richard Henderson
[-- Attachment #1: Type: text/plain, Size: 4006 bytes --]
On Mon, Nov 16, 2020 at 03:57 Michael S. Tsirkin <mst@redhat.com> wrote:
> From: Philippe Mathieu-Daudé <philmd@redhat.com>
>
> GCC 9.3.0 thinks that 'method' can be left uninitialized. This code
> is already in the "if (bsel || pcihp_bridge_en)" block statement,
> but it isn't smart enough to figure it out.
>
> Restrict the code to be used only in the "if (bsel || pcihp_bridge_en)"
> block statement to fix (on Ubuntu):
>
> ../hw/i386/acpi-build.c: In function 'build_append_pci_bus_devices':
> ../hw/i386/acpi-build.c:496:9: error: 'method' may be used uninitialized
> in this function [-Werror=maybe-uninitialized]
> 496 | aml_append(parent_scope, method);
> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> cc1: all warnings being treated as errors
>
> Fixes: df4008c9c59 ("piix4: don't reserve hw resources when hotplug is off
> globally")
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> Message-Id: <20201107194045.438027-1-philmd@redhat.com>
> Acked-by: Ani Sinha <ani@anisinha.ca>
> Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Is there any reason why my ack was removed from the patch that was
ultimately merged?
https://git.qemu.org/?p=qemu.git;a=commit;h=811c74fb657db0559274a710e50ef0096a1915a3
> ---
> hw/i386/acpi-build.c | 45 +++++++++++++++++++++-----------------------
> 1 file changed, 21 insertions(+), 24 deletions(-)
>
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index 4f66642d88..1f5c211245 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -465,34 +465,31 @@ static void build_append_pci_bus_devices(Aml
> *parent_scope, PCIBus *bus,
> */
> if (bsel || pcihp_bridge_en) {
> method = aml_method("PCNT", 0, AML_NOTSERIALIZED);
> - }
> - /* If bus supports hotplug select it and notify about local events */
> - if (bsel) {
> - uint64_t bsel_val = qnum_get_uint(qobject_to(QNum, bsel));
>
> - aml_append(method, aml_store(aml_int(bsel_val),
> aml_name("BNUM")));
> - aml_append(method,
> - aml_call2("DVNT", aml_name("PCIU"), aml_int(1) /* Device
> Check */)
> - );
> - aml_append(method,
> - aml_call2("DVNT", aml_name("PCID"), aml_int(3)/* Eject
> Request */)
> - );
> - }
> + /* If bus supports hotplug select it and notify about local
> events */
> + if (bsel) {
> + uint64_t bsel_val = qnum_get_uint(qobject_to(QNum, bsel));
>
> - /* Notify about child bus events in any case */
> - if (pcihp_bridge_en) {
> - QLIST_FOREACH(sec, &bus->child, sibling) {
> - int32_t devfn = sec->parent_dev->devfn;
> -
> - if (pci_bus_is_root(sec) || pci_bus_is_express(sec)) {
> - continue;
> - }
> -
> - aml_append(method, aml_name("^S%.02X.PCNT", devfn));
> + aml_append(method, aml_store(aml_int(bsel_val),
> aml_name("BNUM")));
> + aml_append(method, aml_call2("DVNT", aml_name("PCIU"),
> + aml_int(1))); /* Device Check */
> + aml_append(method, aml_call2("DVNT", aml_name("PCID"),
> + aml_int(3))); /* Eject Request */
> + }
> +
> + /* Notify about child bus events in any case */
> + if (pcihp_bridge_en) {
> + QLIST_FOREACH(sec, &bus->child, sibling) {
> + int32_t devfn = sec->parent_dev->devfn;
> +
> + if (pci_bus_is_root(sec) || pci_bus_is_express(sec)) {
> + continue;
> + }
> +
> + aml_append(method, aml_name("^S%.02X.PCNT", devfn));
> + }
> }
> - }
>
> - if (bsel || pcihp_bridge_en) {
> aml_append(parent_scope, method);
> }
> qobject_unref(bsel);
> --
> MST
>
>
[-- Attachment #2: Type: text/html, Size: 5676 bytes --]
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [PULL 05/17] hw/i386/acpi-build: Fix maybe-uninitialized error when ACPI hotplug off
2020-11-16 17:44 ` Ani Sinha
@ 2020-11-16 18:01 ` Philippe Mathieu-Daudé
2020-11-16 18:08 ` Ani Sinha
0 siblings, 1 reply; 23+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-11-16 18:01 UTC (permalink / raw)
To: Ani Sinha, Michael S. Tsirkin, Alex Bennée
Cc: Peter Maydell, Eduardo Habkost, qemu-devel, Paolo Bonzini,
Igor Mammedov, Richard Henderson
On 11/16/20 6:44 PM, Ani Sinha wrote:
>
>
> On Mon, Nov 16, 2020 at 03:57 Michael S. Tsirkin <mst@redhat.com
> <mailto:mst@redhat.com>> wrote:
>
> From: Philippe Mathieu-Daudé <philmd@redhat.com
> <mailto:philmd@redhat.com>>
>
> GCC 9.3.0 thinks that 'method' can be left uninitialized. This code
> is already in the "if (bsel || pcihp_bridge_en)" block statement,
> but it isn't smart enough to figure it out.
>
> Restrict the code to be used only in the "if (bsel || pcihp_bridge_en)"
> block statement to fix (on Ubuntu):
>
> ../hw/i386/acpi-build.c: In function 'build_append_pci_bus_devices':
> ../hw/i386/acpi-build.c:496:9: error: 'method' may be used
> uninitialized
> in this function [-Werror=maybe-uninitialized]
> 496 | aml_append(parent_scope, method);
> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> cc1: all warnings being treated as errors
>
> Fixes: df4008c9c59 ("piix4: don't reserve hw resources when hotplug
> is off globally")
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com
> <mailto:philmd@redhat.com>>
> Message-Id: <20201107194045.438027-1-philmd@redhat.com
> <mailto:20201107194045.438027-1-philmd@redhat.com>>
> Acked-by: Ani Sinha <ani@anisinha.ca <mailto:ani@anisinha.ca>>
> Reviewed-by: Michael S. Tsirkin <mst@redhat.com <mailto:mst@redhat.com>>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com
> <mailto:mst@redhat.com>>
>
>
> Is there any reason why my ack was removed from the patch that was
> ultimately merged?
The patch merged is not the patch Michael queued. So your Ack has not
been removed, simply Alex queued an older version previous to your Ack.
https://www.mail-archive.com/qemu-devel@nongnu.org/msg760119.html
>
> https://git.qemu.org/?p=qemu.git;a=commit;h=811c74fb657db0559274a710e50ef0096a1915a3
> <https://git.qemu.org/?p=qemu.git;a=commit;h=811c74fb657db0559274a710e50ef0096a1915a3>
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [PULL 05/17] hw/i386/acpi-build: Fix maybe-uninitialized error when ACPI hotplug off
2020-11-16 18:01 ` Philippe Mathieu-Daudé
@ 2020-11-16 18:08 ` Ani Sinha
0 siblings, 0 replies; 23+ messages in thread
From: Ani Sinha @ 2020-11-16 18:08 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: Peter Maydell, Eduardo Habkost, Michael S. Tsirkin, qemu-devel,
Paolo Bonzini, Igor Mammedov, Alex Bennée,
Richard Henderson
[-- Attachment #1: Type: text/plain, Size: 2558 bytes --]
On Mon, Nov 16, 2020 at 23:32 Philippe Mathieu-Daudé <philmd@redhat.com>
wrote:
> On 11/16/20 6:44 PM, Ani Sinha wrote:
> >
> >
> > On Mon, Nov 16, 2020 at 03:57 Michael S. Tsirkin <mst@redhat.com
> > <mailto:mst@redhat.com>> wrote:
> >
> > From: Philippe Mathieu-Daudé <philmd@redhat.com
> > <mailto:philmd@redhat.com>>
> >
> > GCC 9.3.0 thinks that 'method' can be left uninitialized. This code
> > is already in the "if (bsel || pcihp_bridge_en)" block statement,
> > but it isn't smart enough to figure it out.
> >
> > Restrict the code to be used only in the "if (bsel ||
> pcihp_bridge_en)"
> > block statement to fix (on Ubuntu):
> >
> > ../hw/i386/acpi-build.c: In function
> 'build_append_pci_bus_devices':
> > ../hw/i386/acpi-build.c:496:9: error: 'method' may be used
> > uninitialized
> > in this function [-Werror=maybe-uninitialized]
> > 496 | aml_append(parent_scope, method);
> > | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > cc1: all warnings being treated as errors
> >
> > Fixes: df4008c9c59 ("piix4: don't reserve hw resources when hotplug
> > is off globally")
> > Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com
> > <mailto:philmd@redhat.com>>
> > Message-Id: <20201107194045.438027-1-philmd@redhat.com
> > <mailto:20201107194045.438027-1-philmd@redhat.com>>
> > Acked-by: Ani Sinha <ani@anisinha.ca <mailto:ani@anisinha.ca>>
> > Reviewed-by: Michael S. Tsirkin <mst@redhat.com <mailto:
> mst@redhat.com>>
> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com
> > <mailto:mst@redhat.com>>
> >
> >
> > Is there any reason why my ack was removed from the patch that was
> > ultimately merged?
>
> The patch merged is not the patch Michael queued. So your Ack has not
> been removed, simply Alex queued an older version previous to your Ack.
> https://www.mail-archive.com/qemu-devel@nongnu.org/msg760119.htm
> <https://www.mail-archive.com/qemu-devel@nongnu.org/msg760119.html>
Ugh! So is there any material difference between those two patches? I took
a quick look and it seemed the same patch.
<https://www.mail-archive.com/qemu-devel@nongnu.org/msg760119.html>
<https://www.mail-archive.com/qemu-devel@nongnu.org/msg760119.html>
>
> >
> >
> https://git.qemu.org/?p=qemu.git;a=commit;h=811c74fb657db0559274a710e50ef0096a1915a3
> > <
> https://git.qemu.org/?p=qemu.git;a=commit;h=811c74fb657db0559274a710e50ef0096a1915a3
> >
>
>
[-- Attachment #2: Type: text/html, Size: 4783 bytes --]
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PULL 06/17] test: new qTest case to test the vhost-user-blk-server
2020-11-15 22:27 [PULL 00/17] pc,vhost: fixes, new test Michael S. Tsirkin
` (4 preceding siblings ...)
2020-11-15 22:27 ` [PULL 05/17] hw/i386/acpi-build: Fix maybe-uninitialized error when ACPI hotplug off Michael S. Tsirkin
@ 2020-11-15 22:27 ` Michael S. Tsirkin
2020-11-15 22:27 ` [PULL 07/17] tests/qtest: add multi-queue test case to vhost-user-blk-test Michael S. Tsirkin
` (11 subsequent siblings)
17 siblings, 0 replies; 23+ messages in thread
From: Michael S. Tsirkin @ 2020-11-15 22:27 UTC (permalink / raw)
To: qemu-devel
Cc: Laurent Vivier, Peter Maydell, Thomas Huth, Coiby Xu,
Stefan Hajnoczi, Paolo Bonzini, Marc-André Lureau
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>
Message-Id: <20201111124331.1393747-2-stefanha@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@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
--
MST
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PULL 07/17] tests/qtest: add multi-queue test case to vhost-user-blk-test
2020-11-15 22:27 [PULL 00/17] pc,vhost: fixes, new test Michael S. Tsirkin
` (5 preceding siblings ...)
2020-11-15 22:27 ` [PULL 06/17] test: new qTest case to test the vhost-user-blk-server Michael S. Tsirkin
@ 2020-11-15 22:27 ` Michael S. Tsirkin
2020-11-15 22:27 ` [PULL 08/17] libqtest: add qtest_socket_server() Michael S. Tsirkin
` (10 subsequent siblings)
17 siblings, 0 replies; 23+ messages in thread
From: Michael S. Tsirkin @ 2020-11-15 22:27 UTC (permalink / raw)
To: qemu-devel
Cc: Laurent Vivier, Peter Maydell, Thomas Huth, Stefan Hajnoczi,
Paolo Bonzini
From: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-id: 20201001144604.559733-3-stefanha@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-Id: <20201111124331.1393747-3-stefanha@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@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);
--
MST
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PULL 08/17] libqtest: add qtest_socket_server()
2020-11-15 22:27 [PULL 00/17] pc,vhost: fixes, new test Michael S. Tsirkin
` (6 preceding siblings ...)
2020-11-15 22:27 ` [PULL 07/17] tests/qtest: add multi-queue test case to vhost-user-blk-test Michael S. Tsirkin
@ 2020-11-15 22:27 ` Michael S. Tsirkin
2020-11-15 22:27 ` [PULL 09/17] vhost-user-blk-test: rename destroy_drive() to destroy_file() Michael S. Tsirkin
` (9 subsequent siblings)
17 siblings, 0 replies; 23+ messages in thread
From: Michael S. Tsirkin @ 2020-11-15 22:27 UTC (permalink / raw)
To: qemu-devel
Cc: Laurent Vivier, Peter Maydell, Thomas Huth, Stefan Hajnoczi,
Paolo Bonzini
From: Stefan Hajnoczi <stefanha@redhat.com>
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>
Message-Id: <20201111124331.1393747-4-stefanha@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@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;
--
MST
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PULL 09/17] vhost-user-blk-test: rename destroy_drive() to destroy_file()
2020-11-15 22:27 [PULL 00/17] pc,vhost: fixes, new test Michael S. Tsirkin
` (7 preceding siblings ...)
2020-11-15 22:27 ` [PULL 08/17] libqtest: add qtest_socket_server() Michael S. Tsirkin
@ 2020-11-15 22:27 ` Michael S. Tsirkin
2020-11-15 22:27 ` [PULL 10/17] vhost-user-blk-test: close fork child file descriptors Michael S. Tsirkin
` (8 subsequent siblings)
17 siblings, 0 replies; 23+ messages in thread
From: Michael S. Tsirkin @ 2020-11-15 22:27 UTC (permalink / raw)
To: qemu-devel
Cc: Laurent Vivier, Peter Maydell, Thomas Huth, Stefan Hajnoczi,
Paolo Bonzini
From: Stefan Hajnoczi <stefanha@redhat.com>
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>
Message-Id: <20201111124331.1393747-5-stefanha@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@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,
--
MST
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PULL 10/17] vhost-user-blk-test: close fork child file descriptors
2020-11-15 22:27 [PULL 00/17] pc,vhost: fixes, new test Michael S. Tsirkin
` (8 preceding siblings ...)
2020-11-15 22:27 ` [PULL 09/17] vhost-user-blk-test: rename destroy_drive() to destroy_file() Michael S. Tsirkin
@ 2020-11-15 22:27 ` Michael S. Tsirkin
2020-11-15 22:27 ` [PULL 11/17] vhost-user-blk-test: drop unused return value Michael S. Tsirkin
` (7 subsequent siblings)
17 siblings, 0 replies; 23+ messages in thread
From: Michael S. Tsirkin @ 2020-11-15 22:27 UTC (permalink / raw)
To: qemu-devel
Cc: Laurent Vivier, Peter Maydell, Thomas Huth, Stefan Hajnoczi,
Paolo Bonzini
From: Stefan Hajnoczi <stefanha@redhat.com>
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>
Message-Id: <20201111124331.1393747-6-stefanha@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@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);
}
--
MST
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PULL 11/17] vhost-user-blk-test: drop unused return value
2020-11-15 22:27 [PULL 00/17] pc,vhost: fixes, new test Michael S. Tsirkin
` (9 preceding siblings ...)
2020-11-15 22:27 ` [PULL 10/17] vhost-user-blk-test: close fork child file descriptors Michael S. Tsirkin
@ 2020-11-15 22:27 ` Michael S. Tsirkin
2020-11-15 22:27 ` [PULL 12/17] vhost-user-blk-test: fix races by using fd passing Michael S. Tsirkin
` (6 subsequent siblings)
17 siblings, 0 replies; 23+ messages in thread
From: Michael S. Tsirkin @ 2020-11-15 22:27 UTC (permalink / raw)
To: qemu-devel
Cc: Laurent Vivier, Peter Maydell, Thomas Huth, Stefan Hajnoczi,
Paolo Bonzini
From: Stefan Hajnoczi <stefanha@redhat.com>
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>
Message-Id: <20201111124331.1393747-7-stefanha@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@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)
--
MST
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PULL 12/17] vhost-user-blk-test: fix races by using fd passing
2020-11-15 22:27 [PULL 00/17] pc,vhost: fixes, new test Michael S. Tsirkin
` (10 preceding siblings ...)
2020-11-15 22:27 ` [PULL 11/17] vhost-user-blk-test: drop unused return value Michael S. Tsirkin
@ 2020-11-15 22:27 ` Michael S. Tsirkin
2020-11-15 22:27 ` [PULL 13/17] block/export: port virtio-blk discard/write zeroes input validation Michael S. Tsirkin
` (5 subsequent siblings)
17 siblings, 0 replies; 23+ messages in thread
From: Michael S. Tsirkin @ 2020-11-15 22:27 UTC (permalink / raw)
To: qemu-devel
Cc: Laurent Vivier, Peter Maydell, Thomas Huth, Stefan Hajnoczi,
Paolo Bonzini
From: Stefan Hajnoczi <stefanha@redhat.com>
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>
Message-Id: <20201111124331.1393747-8-stefanha@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@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);
--
MST
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PULL 13/17] block/export: port virtio-blk discard/write zeroes input validation
2020-11-15 22:27 [PULL 00/17] pc,vhost: fixes, new test Michael S. Tsirkin
` (11 preceding siblings ...)
2020-11-15 22:27 ` [PULL 12/17] vhost-user-blk-test: fix races by using fd passing Michael S. Tsirkin
@ 2020-11-15 22:27 ` Michael S. Tsirkin
2020-11-15 22:27 ` [PULL 14/17] vhost-user-blk-test: test discard/write zeroes invalid inputs Michael S. Tsirkin
` (4 subsequent siblings)
17 siblings, 0 replies; 23+ messages in thread
From: Michael S. Tsirkin @ 2020-11-15 22:27 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, Peter Maydell, qemu-block, Coiby Xu, Max Reitz,
Stefan Hajnoczi
From: Stefan Hajnoczi <stefanha@redhat.com>
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>
Message-Id: <20201111124331.1393747-9-stefanha@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@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;
}
- } 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_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;
+ }
+
+ 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);
}
--
MST
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PULL 14/17] vhost-user-blk-test: test discard/write zeroes invalid inputs
2020-11-15 22:27 [PULL 00/17] pc,vhost: fixes, new test Michael S. Tsirkin
` (12 preceding siblings ...)
2020-11-15 22:27 ` [PULL 13/17] block/export: port virtio-blk discard/write zeroes input validation Michael S. Tsirkin
@ 2020-11-15 22:27 ` Michael S. Tsirkin
2020-11-15 22:27 ` [PULL 15/17] block/export: port virtio-blk read/write range check Michael S. Tsirkin
` (3 subsequent siblings)
17 siblings, 0 replies; 23+ messages in thread
From: Michael S. Tsirkin @ 2020-11-15 22:27 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, Peter Maydell, Thomas Huth, qemu-block,
Laurent Vivier, Coiby Xu, Max Reitz, Stefan Hajnoczi,
Paolo Bonzini
From: Stefan Hajnoczi <stefanha@redhat.com>
Exercise input validation code paths in
block/export/vhost-user-blk-server.c.
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-Id: <20201111124331.1393747-10-stefanha@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@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)) {
--
MST
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PULL 15/17] block/export: port virtio-blk read/write range check
2020-11-15 22:27 [PULL 00/17] pc,vhost: fixes, new test Michael S. Tsirkin
` (13 preceding siblings ...)
2020-11-15 22:27 ` [PULL 14/17] vhost-user-blk-test: test discard/write zeroes invalid inputs Michael S. Tsirkin
@ 2020-11-15 22:27 ` Michael S. Tsirkin
2020-11-15 22:27 ` [PULL 16/17] contrib/libvhost-user: Fix bad printf format specifiers Michael S. Tsirkin
` (2 subsequent siblings)
17 siblings, 0 replies; 23+ messages in thread
From: Michael S. Tsirkin @ 2020-11-15 22:27 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, Peter Maydell, qemu-block, Coiby Xu, Max Reitz,
Stefan Hajnoczi
From: Stefan Hajnoczi <stefanha@redhat.com>
Check that the sector number and byte count are valid.
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-Id: <20201111124331.1393747-11-stefanha@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@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) {
--
MST
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PULL 16/17] contrib/libvhost-user: Fix bad printf format specifiers
2020-11-15 22:27 [PULL 00/17] pc,vhost: fixes, new test Michael S. Tsirkin
` (14 preceding siblings ...)
2020-11-15 22:27 ` [PULL 15/17] block/export: port virtio-blk read/write range check Michael S. Tsirkin
@ 2020-11-15 22:27 ` Michael S. Tsirkin
2020-11-15 22:27 ` [PULL 17/17] vhost-user-blk/scsi: Fix broken error handling for socket call Michael S. Tsirkin
2020-11-16 14:19 ` [PULL 00/17] pc,vhost: fixes, new test Peter Maydell
17 siblings, 0 replies; 23+ messages in thread
From: Michael S. Tsirkin @ 2020-11-15 22:27 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Maydell, Raphael Norwitz, AlexChen, Stefan Hajnoczi,
Euler Robot, Marc-André Lureau, Philippe Mathieu-Daudé
From: AlexChen <alex.chen@huawei.com>
We should use printf format specifier "%u" instead of "%d" for
argument of type "unsigned int".
Reported-by: Euler Robot <euler.robot@huawei.com>
Signed-off-by: Alex Chen <alex.chen@huawei.com>
Message-Id: <5FA28106.6000901@huawei.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
contrib/libvhost-user/libvhost-user.c | 24 ++++++++++++------------
1 file changed, 12 insertions(+), 12 deletions(-)
diff --git a/contrib/libvhost-user/libvhost-user.c b/contrib/libvhost-user/libvhost-user.c
index bfec8a881a..5c73ffdd6b 100644
--- a/contrib/libvhost-user/libvhost-user.c
+++ b/contrib/libvhost-user/libvhost-user.c
@@ -701,7 +701,7 @@ vu_add_mem_reg(VuDev *dev, VhostUserMsg *vmsg) {
return false;
}
- DPRINT("Adding region: %d\n", dev->nregions);
+ DPRINT("Adding region: %u\n", dev->nregions);
DPRINT(" guest_phys_addr: 0x%016"PRIx64"\n",
msg_region->guest_phys_addr);
DPRINT(" memory_size: 0x%016"PRIx64"\n",
@@ -848,7 +848,7 @@ vu_set_mem_table_exec_postcopy(VuDev *dev, VhostUserMsg *vmsg)
VhostUserMemory m = vmsg->payload.memory, *memory = &m;
dev->nregions = memory->nregions;
- DPRINT("Nregions: %d\n", memory->nregions);
+ DPRINT("Nregions: %u\n", memory->nregions);
for (i = 0; i < dev->nregions; i++) {
void *mmap_addr;
VhostUserMemoryRegion *msg_region = &memory->regions[i];
@@ -938,7 +938,7 @@ vu_set_mem_table_exec(VuDev *dev, VhostUserMsg *vmsg)
return vu_set_mem_table_exec_postcopy(dev, vmsg);
}
- DPRINT("Nregions: %d\n", memory->nregions);
+ DPRINT("Nregions: %u\n", memory->nregions);
for (i = 0; i < dev->nregions; i++) {
void *mmap_addr;
VhostUserMemoryRegion *msg_region = &memory->regions[i];
@@ -1049,8 +1049,8 @@ vu_set_vring_num_exec(VuDev *dev, VhostUserMsg *vmsg)
unsigned int index = vmsg->payload.state.index;
unsigned int num = vmsg->payload.state.num;
- DPRINT("State.index: %d\n", index);
- DPRINT("State.num: %d\n", num);
+ DPRINT("State.index: %u\n", index);
+ DPRINT("State.num: %u\n", num);
dev->vq[index].vring.num = num;
return false;
@@ -1105,8 +1105,8 @@ vu_set_vring_base_exec(VuDev *dev, VhostUserMsg *vmsg)
unsigned int index = vmsg->payload.state.index;
unsigned int num = vmsg->payload.state.num;
- DPRINT("State.index: %d\n", index);
- DPRINT("State.num: %d\n", num);
+ DPRINT("State.index: %u\n", index);
+ DPRINT("State.num: %u\n", num);
dev->vq[index].shadow_avail_idx = dev->vq[index].last_avail_idx = num;
return false;
@@ -1117,7 +1117,7 @@ vu_get_vring_base_exec(VuDev *dev, VhostUserMsg *vmsg)
{
unsigned int index = vmsg->payload.state.index;
- DPRINT("State.index: %d\n", index);
+ DPRINT("State.index: %u\n", index);
vmsg->payload.state.num = dev->vq[index].last_avail_idx;
vmsg->size = sizeof(vmsg->payload.state);
@@ -1478,8 +1478,8 @@ vu_set_vring_enable_exec(VuDev *dev, VhostUserMsg *vmsg)
unsigned int index = vmsg->payload.state.index;
unsigned int enable = vmsg->payload.state.num;
- DPRINT("State.index: %d\n", index);
- DPRINT("State.enable: %d\n", enable);
+ DPRINT("State.index: %u\n", index);
+ DPRINT("State.enable: %u\n", enable);
if (index >= dev->max_queues) {
vu_panic(dev, "Invalid vring_enable index: %u", index);
@@ -1728,7 +1728,7 @@ vu_handle_vring_kick(VuDev *dev, VhostUserMsg *vmsg)
return false;
}
- DPRINT("Got kick message: handler:%p idx:%d\n",
+ DPRINT("Got kick message: handler:%p idx:%u\n",
dev->vq[index].handler, index);
if (!dev->vq[index].started) {
@@ -1772,7 +1772,7 @@ vu_process_message(VuDev *dev, VhostUserMsg *vmsg)
DPRINT("Request: %s (%d)\n", vu_request_to_string(vmsg->request),
vmsg->request);
DPRINT("Flags: 0x%x\n", vmsg->flags);
- DPRINT("Size: %d\n", vmsg->size);
+ DPRINT("Size: %u\n", vmsg->size);
if (vmsg->fd_num) {
int i;
--
MST
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PULL 17/17] vhost-user-blk/scsi: Fix broken error handling for socket call
2020-11-15 22:27 [PULL 00/17] pc,vhost: fixes, new test Michael S. Tsirkin
` (15 preceding siblings ...)
2020-11-15 22:27 ` [PULL 16/17] contrib/libvhost-user: Fix bad printf format specifiers Michael S. Tsirkin
@ 2020-11-15 22:27 ` Michael S. Tsirkin
2020-11-16 14:19 ` [PULL 00/17] pc,vhost: fixes, new test Peter Maydell
17 siblings, 0 replies; 23+ messages in thread
From: Michael S. Tsirkin @ 2020-11-15 22:27 UTC (permalink / raw)
To: qemu-devel; +Cc: AlexChen, Peter Maydell, Raphael Norwitz, Euler Robot
From: AlexChen <alex.chen@huawei.com>
When socket() fails, it returns -1, 0 is the normal return value and should not return error.
Reported-by: Euler Robot <euler.robot@huawei.com>
Signed-off-by: AlexChen <alex.chen@huawei.com>
Message-Id: <5F9A5B48.9030509@huawei.com>
Reviewed-by: Raphael Norwitz <raphael.norwitz@nutanix.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
contrib/vhost-user-blk/vhost-user-blk.c | 2 +-
contrib/vhost-user-scsi/vhost-user-scsi.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/contrib/vhost-user-blk/vhost-user-blk.c b/contrib/vhost-user-blk/vhost-user-blk.c
index caad88637e..dc981bf945 100644
--- a/contrib/vhost-user-blk/vhost-user-blk.c
+++ b/contrib/vhost-user-blk/vhost-user-blk.c
@@ -476,7 +476,7 @@ static int unix_sock_new(char *unix_fn)
assert(unix_fn);
sock = socket(AF_UNIX, SOCK_STREAM, 0);
- if (sock <= 0) {
+ if (sock < 0) {
perror("socket");
return -1;
}
diff --git a/contrib/vhost-user-scsi/vhost-user-scsi.c b/contrib/vhost-user-scsi/vhost-user-scsi.c
index 3c912384e9..0f9ba4b2a2 100644
--- a/contrib/vhost-user-scsi/vhost-user-scsi.c
+++ b/contrib/vhost-user-scsi/vhost-user-scsi.c
@@ -320,7 +320,7 @@ static int unix_sock_new(char *unix_fn)
assert(unix_fn);
sock = socket(AF_UNIX, SOCK_STREAM, 0);
- if (sock <= 0) {
+ if (sock < 0) {
perror("socket");
return -1;
}
--
MST
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [PULL 00/17] pc,vhost: fixes, new test
2020-11-15 22:27 [PULL 00/17] pc,vhost: fixes, new test Michael S. Tsirkin
` (16 preceding siblings ...)
2020-11-15 22:27 ` [PULL 17/17] vhost-user-blk/scsi: Fix broken error handling for socket call Michael S. Tsirkin
@ 2020-11-16 14:19 ` Peter Maydell
2020-11-17 9:20 ` Michael S. Tsirkin
17 siblings, 1 reply; 23+ messages in thread
From: Peter Maydell @ 2020-11-16 14:19 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: QEMU Developers
On Sun, 15 Nov 2020 at 22:27, Michael S. Tsirkin <mst@redhat.com> wrote:
>
> The following changes since commit c6f28ed5075df79fef39c500362a3f4089256c9c:
>
> Update version for v5.2.0-rc1 release (2020-11-10 22:29:57 +0000)
>
> are available in the Git repository at:
>
> git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git tags/for_upstream
>
> for you to fetch changes up to fe8d9946228d4df6c020f2cb38b6ac08981727cf:
>
> vhost-user-blk/scsi: Fix broken error handling for socket call (2020-11-15 17:05:47 -0500)
>
> ----------------------------------------------------------------
> pc,vhost: fixes, new test
>
> Lots of fixes all over the place.
> A new test case which seems like a good idea even at
> this late stage: can't break things and will make
> sure we don't introduce regressions.
>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
>
> ----------------------------------------------------------------
Something in this seems to cause hangs in 'make check' on
my x86-64 Linux box: sample 'ps wafux' output:
petmay01 30354 0.0 0.0 17392 9348 ? S 13:40 0:00
\_ make --output-sync -C build/a
ll check V=1 -j8
petmay01 7093 0.0 0.0 13916 3608 ? S 13:41 0:00
\_ bash -o pipefail -c echo
'MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}
QTEST_QEMU_IMG=./qemu-img
G_TEST_DBUS_DAEMON=/home/petmay01/linaro/qemu-for-merges/tests/dbus-vmstate-daemon.sh
QTEST_QEMU_BINARY=./qemu-system-i386 tests/qtest/qos-test --tap -k' &&
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}
QTEST_QEMU_IMG=./qemu-img
G_TEST_DBUS_DAEMON=/home/petmay01/linaro/qemu-for-merges/tests/dbus-vmstate-daemon.sh
QTEST_QEMU_BINARY=./qemu-system-i386 tests/qtest/qos-test --tap -k <
/dev/null | ./scripts/tap-driver.pl --test-name="qtest-i386/qos-test"
petmay01 7095 0.0 0.0 37764 11744 ? S 13:41 0:00
| \_ perl ./scripts/tap-driver.pl
--test-name=qtest-i386/qos-test
petmay01 14023 0.0 0.0 13916 3568 ? S 13:41 0:00
\_ bash -o pipefail -c echo
'MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}
QTEST_QEMU_IMG=./qemu-img
G_TEST_DBUS_DAEMON=/home/petmay01/linaro/qemu-for-merges/tests/dbus-vmstate-daemon.sh
QTEST_QEMU_BINARY=./qemu-system-ppc64 tests/qtest/qos-test --tap -k'
&& MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}
QTEST_QEMU_IMG=./qemu-img
G_TEST_DBUS_DAEMON=/home/petmay01/linaro/qemu-for-merges/tests/dbus-vmstate-daemon.sh
QTEST_QEMU_BINARY=./qemu-system-ppc64 tests/qtest/qos-test --tap -k <
/dev/null | ./scripts/tap-driver.pl --test-name="qtest-ppc64/qos-test"
petmay01 14025 0.0 0.0 37828 11760 ? S 13:41 0:00
| \_ perl ./scripts/tap-driver.pl
--test-name=qtest-ppc64/qos-test
petmay01 22886 0.0 0.0 13916 3716 ? S 13:42 0:00
\_ bash -o pipefail -c echo
'MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}
QTEST_QEMU_IMG=./qemu-img
G_TEST_DBUS_DAEMON=/home/petmay01/linaro/qemu-for-merges/tests/dbus-vmstate-daemon.sh
QTEST_QEMU_BINARY=./qemu-system-x86_64 tests/qtest/qos-test --tap -k'
&& MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}
QTEST_QEMU_IMG=./qemu-img
G_TEST_DBUS_DAEMON=/home/petmay01/linaro/qemu-for-merges/tests/dbus-vmstate-daemon.sh
QTEST_QEMU_BINARY=./qemu-system-x86_64 tests/qtest/qos-test --tap -k <
/dev/null | ./scripts/tap-driver.pl
--test-name="qtest-x86_64/qos-test"
petmay01 22888 0.0 0.0 37764 11836 ? S 13:42 0:00
\_ perl ./scripts/tap-driver.pl
--test-name=qtest-x86_64/qos-test
Something somewhere is also apparently leaving a qemu-storage-daemon
process running on bigendian hosts only (?): I see this on my s390x
test box:
ubuntu 26700 0.0 0.5 330776 21552 ? Sl 08:40 0:00
./storage-daemon/qemu-storage-daemon --chardev
socket,id=qmp,fd=11,server,nowait --monitor chardev=qmp --blockdev
driver=file,node-name=disk0,filename=qtest.V5gfPm --export
type=vhost-user-blk,id=disk0,addr.type=fd,addr.str=16,node-name=disk0,writable=on,num-queues=8
--blockdev driver=file,node-name=disk1,filename=qtest.JM24xB --export
type=vhost-user-blk,id=disk1,addr.type=fd,addr.str=17,node-name=disk1,writable=on,num-queues=8
and similarly on the ppc64be box (but not on other machines).
This seems to be associated with this test failure:
ERROR:../../tests/qtest/vhost-user-blk-test.c:738:multiqueue:
assertion failed (num_queues == 8): (2048 == 8)
ERROR qtest-i386/qos-test - Bail out!
ERROR:../../tests/qtest/vhost-user-blk-test.c:738:multiqueue:
assertion failed (num_queues == 8): (2048 == 8)
Makefile.mtest:1857: recipe for target 'run-test-230' failed
which looks suspiciously like an endianness bug somewhere.
Ideally if the test case starts external processes it should
make sure they're cleaned up even if the test fails.
thanks
-- PMM
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [PULL 00/17] pc,vhost: fixes, new test
2020-11-16 14:19 ` [PULL 00/17] pc,vhost: fixes, new test Peter Maydell
@ 2020-11-17 9:20 ` Michael S. Tsirkin
0 siblings, 0 replies; 23+ messages in thread
From: Michael S. Tsirkin @ 2020-11-17 9:20 UTC (permalink / raw)
To: Peter Maydell; +Cc: QEMU Developers, stefanha
On Mon, Nov 16, 2020 at 02:19:11PM +0000, Peter Maydell wrote:
> On Sun, 15 Nov 2020 at 22:27, Michael S. Tsirkin <mst@redhat.com> wrote:
> >
> > The following changes since commit c6f28ed5075df79fef39c500362a3f4089256c9c:
> >
> > Update version for v5.2.0-rc1 release (2020-11-10 22:29:57 +0000)
> >
> > are available in the Git repository at:
> >
> > git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git tags/for_upstream
> >
> > for you to fetch changes up to fe8d9946228d4df6c020f2cb38b6ac08981727cf:
> >
> > vhost-user-blk/scsi: Fix broken error handling for socket call (2020-11-15 17:05:47 -0500)
> >
> > ----------------------------------------------------------------
> > pc,vhost: fixes, new test
> >
> > Lots of fixes all over the place.
> > A new test case which seems like a good idea even at
> > this late stage: can't break things and will make
> > sure we don't introduce regressions.
> >
> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> >
> > ----------------------------------------------------------------
>
> Something in this seems to cause hangs in 'make check' on
> my x86-64 Linux box: sample 'ps wafux' output:
>
> petmay01 30354 0.0 0.0 17392 9348 ? S 13:40 0:00
> \_ make --output-sync -C build/a
> ll check V=1 -j8
> petmay01 7093 0.0 0.0 13916 3608 ? S 13:41 0:00
> \_ bash -o pipefail -c echo
> 'MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}
> QTEST_QEMU_IMG=./qemu-img
> G_TEST_DBUS_DAEMON=/home/petmay01/linaro/qemu-for-merges/tests/dbus-vmstate-daemon.sh
> QTEST_QEMU_BINARY=./qemu-system-i386 tests/qtest/qos-test --tap -k' &&
> MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}
> QTEST_QEMU_IMG=./qemu-img
> G_TEST_DBUS_DAEMON=/home/petmay01/linaro/qemu-for-merges/tests/dbus-vmstate-daemon.sh
> QTEST_QEMU_BINARY=./qemu-system-i386 tests/qtest/qos-test --tap -k <
> /dev/null | ./scripts/tap-driver.pl --test-name="qtest-i386/qos-test"
> petmay01 7095 0.0 0.0 37764 11744 ? S 13:41 0:00
> | \_ perl ./scripts/tap-driver.pl
> --test-name=qtest-i386/qos-test
> petmay01 14023 0.0 0.0 13916 3568 ? S 13:41 0:00
> \_ bash -o pipefail -c echo
> 'MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}
> QTEST_QEMU_IMG=./qemu-img
> G_TEST_DBUS_DAEMON=/home/petmay01/linaro/qemu-for-merges/tests/dbus-vmstate-daemon.sh
> QTEST_QEMU_BINARY=./qemu-system-ppc64 tests/qtest/qos-test --tap -k'
> && MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}
> QTEST_QEMU_IMG=./qemu-img
> G_TEST_DBUS_DAEMON=/home/petmay01/linaro/qemu-for-merges/tests/dbus-vmstate-daemon.sh
> QTEST_QEMU_BINARY=./qemu-system-ppc64 tests/qtest/qos-test --tap -k <
> /dev/null | ./scripts/tap-driver.pl --test-name="qtest-ppc64/qos-test"
> petmay01 14025 0.0 0.0 37828 11760 ? S 13:41 0:00
> | \_ perl ./scripts/tap-driver.pl
> --test-name=qtest-ppc64/qos-test
> petmay01 22886 0.0 0.0 13916 3716 ? S 13:42 0:00
> \_ bash -o pipefail -c echo
> 'MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}
> QTEST_QEMU_IMG=./qemu-img
> G_TEST_DBUS_DAEMON=/home/petmay01/linaro/qemu-for-merges/tests/dbus-vmstate-daemon.sh
> QTEST_QEMU_BINARY=./qemu-system-x86_64 tests/qtest/qos-test --tap -k'
> && MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}
> QTEST_QEMU_IMG=./qemu-img
> G_TEST_DBUS_DAEMON=/home/petmay01/linaro/qemu-for-merges/tests/dbus-vmstate-daemon.sh
> QTEST_QEMU_BINARY=./qemu-system-x86_64 tests/qtest/qos-test --tap -k <
> /dev/null | ./scripts/tap-driver.pl
> --test-name="qtest-x86_64/qos-test"
> petmay01 22888 0.0 0.0 37764 11836 ? S 13:42 0:00
> \_ perl ./scripts/tap-driver.pl
> --test-name=qtest-x86_64/qos-test
>
>
> Something somewhere is also apparently leaving a qemu-storage-daemon
> process running on bigendian hosts only (?): I see this on my s390x
> test box:
>
> ubuntu 26700 0.0 0.5 330776 21552 ? Sl 08:40 0:00
> ./storage-daemon/qemu-storage-daemon --chardev
> socket,id=qmp,fd=11,server,nowait --monitor chardev=qmp --blockdev
> driver=file,node-name=disk0,filename=qtest.V5gfPm --export
> type=vhost-user-blk,id=disk0,addr.type=fd,addr.str=16,node-name=disk0,writable=on,num-queues=8
> --blockdev driver=file,node-name=disk1,filename=qtest.JM24xB --export
> type=vhost-user-blk,id=disk1,addr.type=fd,addr.str=17,node-name=disk1,writable=on,num-queues=8
>
> and similarly on the ppc64be box (but not on other machines).
> This seems to be associated with this test failure:
>
> ERROR:../../tests/qtest/vhost-user-blk-test.c:738:multiqueue:
> assertion failed (num_queues == 8): (2048 == 8)
> ERROR qtest-i386/qos-test - Bail out!
> ERROR:../../tests/qtest/vhost-user-blk-test.c:738:multiqueue:
> assertion failed (num_queues == 8): (2048 == 8)
> Makefile.mtest:1857: recipe for target 'run-test-230' failed
>
> which looks suspiciously like an endianness bug somewhere.
>
> Ideally if the test case starts external processes it should
> make sure they're cleaned up even if the test fails.
>
> thanks
> -- PMM
Must be vhost user blk test patch exposing some more latent races ...
I dropped that for now, and sent pull v2.
Same tag.
--
MST
^ permalink raw reply [flat|nested] 23+ messages in thread