* [PATCH 1/4] util/vhost-user-server: use static library in meson.build
2020-09-25 13:42 [PATCH 0/4] block/export: add BlockExportOptions->iothread member Stefan Hajnoczi
@ 2020-09-25 13:42 ` Stefan Hajnoczi
2020-09-25 14:07 ` Paolo Bonzini
2020-09-25 13:42 ` [PATCH 2/4] qemu-storage-daemon: avoid compiling blockdev_ss twice Stefan Hajnoczi
` (2 subsequent siblings)
3 siblings, 1 reply; 11+ messages in thread
From: Stefan Hajnoczi @ 2020-09-25 13:42 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, Laurent Vivier, Thomas Huth, qemu-block,
Markus Armbruster, Coiby Xu, Max Reitz, Stefan Hajnoczi,
Paolo Bonzini
Don't compile contrib/libvhost-user/libvhost-user.c again. Instead build
the static library once and then reuse it throughout QEMU.
Also switch from CONFIG_LINUX to CONFIG_VHOST_USER, which is what the
vhost-user tools (vhost-user-gpu, etc) do.
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
block/export/export.c | 8 ++++----
block/export/meson.build | 2 +-
contrib/libvhost-user/meson.build | 1 +
meson.build | 6 +++++-
tests/qtest/meson.build | 2 +-
util/meson.build | 4 +++-
6 files changed, 15 insertions(+), 8 deletions(-)
diff --git a/block/export/export.c b/block/export/export.c
index bd7cac241f..550897e236 100644
--- a/block/export/export.c
+++ b/block/export/export.c
@@ -17,17 +17,17 @@
#include "sysemu/block-backend.h"
#include "block/export.h"
#include "block/nbd.h"
-#if CONFIG_LINUX
-#include "block/export/vhost-user-blk-server.h"
-#endif
#include "qapi/error.h"
#include "qapi/qapi-commands-block-export.h"
#include "qapi/qapi-events-block-export.h"
#include "qemu/id.h"
+#ifdef CONFIG_VHOST_USER
+#include "vhost-user-blk-server.h"
+#endif
static const BlockExportDriver *blk_exp_drivers[] = {
&blk_exp_nbd,
-#if CONFIG_LINUX
+#ifdef CONFIG_VHOST_USER
&blk_exp_vhost_user_blk,
#endif
};
diff --git a/block/export/meson.build b/block/export/meson.build
index ef3a9576f7..469a7aa0f5 100644
--- a/block/export/meson.build
+++ b/block/export/meson.build
@@ -1,2 +1,2 @@
block_ss.add(files('export.c'))
-block_ss.add(when: 'CONFIG_LINUX', if_true: files('vhost-user-blk-server.c', '../../contrib/libvhost-user/libvhost-user.c'))
+block_ss.add(when: 'CONFIG_VHOST_USER', if_true: files('vhost-user-blk-server.c'))
diff --git a/contrib/libvhost-user/meson.build b/contrib/libvhost-user/meson.build
index e68dd1a581..a261e7665f 100644
--- a/contrib/libvhost-user/meson.build
+++ b/contrib/libvhost-user/meson.build
@@ -1,3 +1,4 @@
libvhost_user = static_library('vhost-user',
files('libvhost-user.c', 'libvhost-user-glib.c'),
build_by_default: false)
+vhost_user = declare_dependency(link_with: libvhost_user)
diff --git a/meson.build b/meson.build
index 4c6c7310fa..eb84b97ebb 100644
--- a/meson.build
+++ b/meson.build
@@ -788,6 +788,11 @@ trace_events_subdirs += [
'util',
]
+vhost_user = not_found
+if 'CONFIG_VHOST_USER' in config_host
+ subdir('contrib/libvhost-user')
+endif
+
subdir('qapi')
subdir('qobject')
subdir('stubs')
@@ -1169,7 +1174,6 @@ if have_tools
install: true)
if 'CONFIG_VHOST_USER' in config_host
- subdir('contrib/libvhost-user')
subdir('contrib/vhost-user-blk')
subdir('contrib/vhost-user-gpu')
subdir('contrib/vhost-user-input')
diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build
index c72821b09a..aa8d0985e1 100644
--- a/tests/qtest/meson.build
+++ b/tests/qtest/meson.build
@@ -191,7 +191,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', if_true: files('vhost-user-blk-test.c'))
+qos_test_ss.add(when: 'CONFIG_VHOST_USER', if_true: files('vhost-user-blk-test.c'))
extra_qtest_deps = {
'bios-tables-test': [io],
diff --git a/util/meson.build b/util/meson.build
index 2296e81b34..9b2a7a5de9 100644
--- a/util/meson.build
+++ b/util/meson.build
@@ -66,7 +66,9 @@ if have_block
util_ss.add(files('main-loop.c'))
util_ss.add(files('nvdimm-utils.c'))
util_ss.add(files('qemu-coroutine.c', 'qemu-coroutine-lock.c', 'qemu-coroutine-io.c'))
- util_ss.add(when: 'CONFIG_LINUX', if_true: files('vhost-user-server.c'))
+ util_ss.add(when: 'CONFIG_VHOST_USER', if_true: [
+ files('vhost-user-server.c'), vhost_user
+ ])
util_ss.add(files('block-helpers.c'))
util_ss.add(files('qemu-coroutine-sleep.c'))
util_ss.add(files('qemu-co-shared-resource.c'))
--
2.26.2
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH 1/4] util/vhost-user-server: use static library in meson.build
2020-09-25 13:42 ` [PATCH 1/4] util/vhost-user-server: use static library in meson.build Stefan Hajnoczi
@ 2020-09-25 14:07 ` Paolo Bonzini
0 siblings, 0 replies; 11+ messages in thread
From: Paolo Bonzini @ 2020-09-25 14:07 UTC (permalink / raw)
To: Stefan Hajnoczi, qemu-devel
Cc: Kevin Wolf, Laurent Vivier, Thomas Huth, qemu-block,
Markus Armbruster, Coiby Xu, Max Reitz
On 25/09/20 15:42, Stefan Hajnoczi wrote:
> Don't compile contrib/libvhost-user/libvhost-user.c again. Instead build
> the static library once and then reuse it throughout QEMU.
>
> Also switch from CONFIG_LINUX to CONFIG_VHOST_USER, which is what the
> vhost-user tools (vhost-user-gpu, etc) do.
>
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Looks good. Unrelated to this patch, probably the contrib/libvhost-user
directory should be moved under tools.
Paolo
> ---
> block/export/export.c | 8 ++++----
> block/export/meson.build | 2 +-
> contrib/libvhost-user/meson.build | 1 +
> meson.build | 6 +++++-
> tests/qtest/meson.build | 2 +-
> util/meson.build | 4 +++-
> 6 files changed, 15 insertions(+), 8 deletions(-)
>
> diff --git a/block/export/export.c b/block/export/export.c
> index bd7cac241f..550897e236 100644
> --- a/block/export/export.c
> +++ b/block/export/export.c
> @@ -17,17 +17,17 @@
> #include "sysemu/block-backend.h"
> #include "block/export.h"
> #include "block/nbd.h"
> -#if CONFIG_LINUX
> -#include "block/export/vhost-user-blk-server.h"
> -#endif
> #include "qapi/error.h"
> #include "qapi/qapi-commands-block-export.h"
> #include "qapi/qapi-events-block-export.h"
> #include "qemu/id.h"
> +#ifdef CONFIG_VHOST_USER
> +#include "vhost-user-blk-server.h"
> +#endif
>
> static const BlockExportDriver *blk_exp_drivers[] = {
> &blk_exp_nbd,
> -#if CONFIG_LINUX
> +#ifdef CONFIG_VHOST_USER
> &blk_exp_vhost_user_blk,
> #endif
> };
> diff --git a/block/export/meson.build b/block/export/meson.build
> index ef3a9576f7..469a7aa0f5 100644
> --- a/block/export/meson.build
> +++ b/block/export/meson.build
> @@ -1,2 +1,2 @@
> block_ss.add(files('export.c'))
> -block_ss.add(when: 'CONFIG_LINUX', if_true: files('vhost-user-blk-server.c', '../../contrib/libvhost-user/libvhost-user.c'))
> +block_ss.add(when: 'CONFIG_VHOST_USER', if_true: files('vhost-user-blk-server.c'))
> diff --git a/contrib/libvhost-user/meson.build b/contrib/libvhost-user/meson.build
> index e68dd1a581..a261e7665f 100644
> --- a/contrib/libvhost-user/meson.build
> +++ b/contrib/libvhost-user/meson.build
> @@ -1,3 +1,4 @@
> libvhost_user = static_library('vhost-user',
> files('libvhost-user.c', 'libvhost-user-glib.c'),
> build_by_default: false)
> +vhost_user = declare_dependency(link_with: libvhost_user)
> diff --git a/meson.build b/meson.build
> index 4c6c7310fa..eb84b97ebb 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -788,6 +788,11 @@ trace_events_subdirs += [
> 'util',
> ]
>
> +vhost_user = not_found
> +if 'CONFIG_VHOST_USER' in config_host
> + subdir('contrib/libvhost-user')
> +endif
> +
> subdir('qapi')
> subdir('qobject')
> subdir('stubs')
> @@ -1169,7 +1174,6 @@ if have_tools
> install: true)
>
> if 'CONFIG_VHOST_USER' in config_host
> - subdir('contrib/libvhost-user')
> subdir('contrib/vhost-user-blk')
> subdir('contrib/vhost-user-gpu')
> subdir('contrib/vhost-user-input')
> diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build
> index c72821b09a..aa8d0985e1 100644
> --- a/tests/qtest/meson.build
> +++ b/tests/qtest/meson.build
> @@ -191,7 +191,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', if_true: files('vhost-user-blk-test.c'))
> +qos_test_ss.add(when: 'CONFIG_VHOST_USER', if_true: files('vhost-user-blk-test.c'))
>
> extra_qtest_deps = {
> 'bios-tables-test': [io],
> diff --git a/util/meson.build b/util/meson.build
> index 2296e81b34..9b2a7a5de9 100644
> --- a/util/meson.build
> +++ b/util/meson.build
> @@ -66,7 +66,9 @@ if have_block
> util_ss.add(files('main-loop.c'))
> util_ss.add(files('nvdimm-utils.c'))
> util_ss.add(files('qemu-coroutine.c', 'qemu-coroutine-lock.c', 'qemu-coroutine-io.c'))
> - util_ss.add(when: 'CONFIG_LINUX', if_true: files('vhost-user-server.c'))
> + util_ss.add(when: 'CONFIG_VHOST_USER', if_true: [
> + files('vhost-user-server.c'), vhost_user
> + ])
> util_ss.add(files('block-helpers.c'))
> util_ss.add(files('qemu-coroutine-sleep.c'))
> util_ss.add(files('qemu-co-shared-resource.c'))
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 2/4] qemu-storage-daemon: avoid compiling blockdev_ss twice
2020-09-25 13:42 [PATCH 0/4] block/export: add BlockExportOptions->iothread member Stefan Hajnoczi
2020-09-25 13:42 ` [PATCH 1/4] util/vhost-user-server: use static library in meson.build Stefan Hajnoczi
@ 2020-09-25 13:42 ` Stefan Hajnoczi
2020-09-25 14:08 ` Paolo Bonzini
2020-09-25 13:42 ` [PATCH 3/4] block: move block exports to libblockdev Stefan Hajnoczi
2020-09-25 13:42 ` [PATCH 4/4] block/export: add BlockExportOptions->iothread member Stefan Hajnoczi
3 siblings, 1 reply; 11+ messages in thread
From: Stefan Hajnoczi @ 2020-09-25 13:42 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, Laurent Vivier, Thomas Huth, qemu-block,
Markus Armbruster, Coiby Xu, Max Reitz, Stefan Hajnoczi,
Paolo Bonzini
Introduce libblkdev.fa to avoid recompiling blockdev_ss twice.
Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
meson.build | 12 ++++++++++--
storage-daemon/meson.build | 3 +--
2 files changed, 11 insertions(+), 4 deletions(-)
diff --git a/meson.build b/meson.build
index eb84b97ebb..18d689b423 100644
--- a/meson.build
+++ b/meson.build
@@ -857,7 +857,6 @@ blockdev_ss.add(files(
blockdev_ss.add(when: 'CONFIG_POSIX', if_true: files('os-posix.c'))
softmmu_ss.add(when: 'CONFIG_WIN32', if_true: [files('os-win32.c')])
-softmmu_ss.add_all(blockdev_ss)
softmmu_ss.add(files(
'bootdevice.c',
'dma-helpers.c',
@@ -952,6 +951,15 @@ block = declare_dependency(link_whole: [libblock],
link_args: '@block.syms',
dependencies: [crypto, io])
+blockdev_ss = blockdev_ss.apply(config_host, strict: false)
+libblockdev = static_library('blockdev', blockdev_ss.sources() + genh,
+ dependencies: blockdev_ss.dependencies(),
+ name_suffix: 'fa',
+ build_by_default: false)
+
+blockdev = declare_dependency(link_whole: [libblockdev],
+ dependencies: [block])
+
qmp_ss = qmp_ss.apply(config_host, strict: false)
libqmp = static_library('qmp', qmp_ss.sources() + genh,
dependencies: qmp_ss.dependencies(),
@@ -968,7 +976,7 @@ foreach m : block_mods + softmmu_mods
install_dir: config_host['qemu_moddir'])
endforeach
-softmmu_ss.add(authz, block, chardev, crypto, io, qmp)
+softmmu_ss.add(authz, blockdev, chardev, crypto, io, qmp)
common_ss.add(qom, qemuutil)
common_ss.add_all(when: 'CONFIG_SOFTMMU', if_true: [softmmu_ss])
diff --git a/storage-daemon/meson.build b/storage-daemon/meson.build
index 0409acc3f5..c5adce81c3 100644
--- a/storage-daemon/meson.build
+++ b/storage-daemon/meson.build
@@ -1,7 +1,6 @@
qsd_ss = ss.source_set()
qsd_ss.add(files('qemu-storage-daemon.c'))
-qsd_ss.add(block, chardev, qmp, qom, qemuutil)
-qsd_ss.add_all(blockdev_ss)
+qsd_ss.add(blockdev, chardev, qmp, qom, qemuutil)
subdir('qapi')
--
2.26.2
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH 2/4] qemu-storage-daemon: avoid compiling blockdev_ss twice
2020-09-25 13:42 ` [PATCH 2/4] qemu-storage-daemon: avoid compiling blockdev_ss twice Stefan Hajnoczi
@ 2020-09-25 14:08 ` Paolo Bonzini
0 siblings, 0 replies; 11+ messages in thread
From: Paolo Bonzini @ 2020-09-25 14:08 UTC (permalink / raw)
To: Stefan Hajnoczi, qemu-devel
Cc: Kevin Wolf, Laurent Vivier, Thomas Huth, qemu-block,
Markus Armbruster, Coiby Xu, Max Reitz
On 25/09/20 15:42, Stefan Hajnoczi wrote:
> Introduce libblkdev.fa to avoid recompiling blockdev_ss twice.
>
> Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
> meson.build | 12 ++++++++++--
> storage-daemon/meson.build | 3 +--
> 2 files changed, 11 insertions(+), 4 deletions(-)
>
> diff --git a/meson.build b/meson.build
> index eb84b97ebb..18d689b423 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -857,7 +857,6 @@ blockdev_ss.add(files(
> blockdev_ss.add(when: 'CONFIG_POSIX', if_true: files('os-posix.c'))
> softmmu_ss.add(when: 'CONFIG_WIN32', if_true: [files('os-win32.c')])
>
> -softmmu_ss.add_all(blockdev_ss)
> softmmu_ss.add(files(
> 'bootdevice.c',
> 'dma-helpers.c',
> @@ -952,6 +951,15 @@ block = declare_dependency(link_whole: [libblock],
> link_args: '@block.syms',
> dependencies: [crypto, io])
>
> +blockdev_ss = blockdev_ss.apply(config_host, strict: false)
> +libblockdev = static_library('blockdev', blockdev_ss.sources() + genh,
> + dependencies: blockdev_ss.dependencies(),
> + name_suffix: 'fa',
> + build_by_default: false)
> +
> +blockdev = declare_dependency(link_whole: [libblockdev],
> + dependencies: [block])
> +
> qmp_ss = qmp_ss.apply(config_host, strict: false)
> libqmp = static_library('qmp', qmp_ss.sources() + genh,
> dependencies: qmp_ss.dependencies(),
> @@ -968,7 +976,7 @@ foreach m : block_mods + softmmu_mods
> install_dir: config_host['qemu_moddir'])
> endforeach
>
> -softmmu_ss.add(authz, block, chardev, crypto, io, qmp)
> +softmmu_ss.add(authz, blockdev, chardev, crypto, io, qmp)
> common_ss.add(qom, qemuutil)
>
> common_ss.add_all(when: 'CONFIG_SOFTMMU', if_true: [softmmu_ss])
> diff --git a/storage-daemon/meson.build b/storage-daemon/meson.build
> index 0409acc3f5..c5adce81c3 100644
> --- a/storage-daemon/meson.build
> +++ b/storage-daemon/meson.build
> @@ -1,7 +1,6 @@
> qsd_ss = ss.source_set()
> qsd_ss.add(files('qemu-storage-daemon.c'))
> -qsd_ss.add(block, chardev, qmp, qom, qemuutil)
> -qsd_ss.add_all(blockdev_ss)
> +qsd_ss.add(blockdev, chardev, qmp, qom, qemuutil)
>
> subdir('qapi')
>
>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 3/4] block: move block exports to libblockdev
2020-09-25 13:42 [PATCH 0/4] block/export: add BlockExportOptions->iothread member Stefan Hajnoczi
2020-09-25 13:42 ` [PATCH 1/4] util/vhost-user-server: use static library in meson.build Stefan Hajnoczi
2020-09-25 13:42 ` [PATCH 2/4] qemu-storage-daemon: avoid compiling blockdev_ss twice Stefan Hajnoczi
@ 2020-09-25 13:42 ` Stefan Hajnoczi
2020-09-25 14:11 ` Paolo Bonzini
2020-09-25 13:42 ` [PATCH 4/4] block/export: add BlockExportOptions->iothread member Stefan Hajnoczi
3 siblings, 1 reply; 11+ messages in thread
From: Stefan Hajnoczi @ 2020-09-25 13:42 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, Laurent Vivier, Thomas Huth, qemu-block,
Markus Armbruster, Coiby Xu, Max Reitz, Stefan Hajnoczi,
Paolo Bonzini
Block exports are used by softmmu, qemu-storage-daemon, and qemu-nbd.
They are not used by other programs and are not otherwise needed in
libblock.
Undo the recent move of blockdev-nbd.c from blockdev_ss into block_ss.
Two stubs are required to support this:
1. bdrv_close_all() (libblock) calls blk_exp_close_all() (libblockdev).
2. qemu_system_killed() is called by os-posix.c (libblockdev) and not
implemented in qemu-nbd.
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
stubs/blk-exp-close-all.c | 7 +++++++
stubs/qemu-system-killed.c | 10 ++++++++++
block/export/meson.build | 4 ++--
meson.build | 4 ++--
nbd/meson.build | 2 ++
stubs/meson.build | 2 ++
6 files changed, 25 insertions(+), 4 deletions(-)
create mode 100644 stubs/blk-exp-close-all.c
create mode 100644 stubs/qemu-system-killed.c
diff --git a/stubs/blk-exp-close-all.c b/stubs/blk-exp-close-all.c
new file mode 100644
index 0000000000..1c71316763
--- /dev/null
+++ b/stubs/blk-exp-close-all.c
@@ -0,0 +1,7 @@
+#include "qemu/osdep.h"
+#include "block/export.h"
+
+/* Only used in programs that support block exports (libblockdev.fa) */
+void blk_exp_close_all(void)
+{
+}
diff --git a/stubs/qemu-system-killed.c b/stubs/qemu-system-killed.c
new file mode 100644
index 0000000000..9af131917b
--- /dev/null
+++ b/stubs/qemu-system-killed.c
@@ -0,0 +1,10 @@
+#include "qemu/osdep.h"
+#include "sysemu/runstate.h"
+
+/*
+ * This function is needed by os-posix.c but only implemented by softmmu and
+ * qemu-storage-daemon. Other programs may have no need for it.
+ */
+void qemu_system_killed(int signal, pid_t pid)
+{
+}
diff --git a/block/export/meson.build b/block/export/meson.build
index 469a7aa0f5..a2772a0dce 100644
--- a/block/export/meson.build
+++ b/block/export/meson.build
@@ -1,2 +1,2 @@
-block_ss.add(files('export.c'))
-block_ss.add(when: 'CONFIG_VHOST_USER', if_true: files('vhost-user-blk-server.c'))
+blockdev_ss.add(files('export.c'))
+blockdev_ss.add(when: 'CONFIG_VHOST_USER', if_true: files('vhost-user-blk-server.c'))
diff --git a/meson.build b/meson.build
index 18d689b423..0e9528adab 100644
--- a/meson.build
+++ b/meson.build
@@ -835,7 +835,6 @@ subdir('dump')
block_ss.add(files(
'block.c',
- 'blockdev-nbd.c',
'blockjob.c',
'job.c',
'qemu-io-cmds.c',
@@ -848,6 +847,7 @@ subdir('block')
blockdev_ss.add(files(
'blockdev.c',
+ 'blockdev-nbd.c',
'iothread.c',
'job-qmp.c',
))
@@ -1171,7 +1171,7 @@ if have_tools
qemu_io = executable('qemu-io', files('qemu-io.c'),
dependencies: [block, qemuutil], install: true)
qemu_nbd = executable('qemu-nbd', files('qemu-nbd.c'),
- dependencies: [block, qemuutil], install: true)
+ dependencies: [blockdev, qemuutil], install: true)
subdir('storage-daemon')
subdir('contrib/rdmacm-mux')
diff --git a/nbd/meson.build b/nbd/meson.build
index 0c00a776d3..2baaa36948 100644
--- a/nbd/meson.build
+++ b/nbd/meson.build
@@ -1,5 +1,7 @@
block_ss.add(files(
'client.c',
'common.c',
+))
+blockdev_ss.add(files(
'server.c',
))
diff --git a/stubs/meson.build b/stubs/meson.build
index e0b322bc28..60234571b1 100644
--- a/stubs/meson.build
+++ b/stubs/meson.build
@@ -1,6 +1,7 @@
stub_ss.add(files('arch_type.c'))
stub_ss.add(files('bdrv-next-monitor-owned.c'))
stub_ss.add(files('blk-commit-all.c'))
+stub_ss.add(files('blk-exp-close-all.c'))
stub_ss.add(files('blockdev-close-all-bdrv-states.c'))
stub_ss.add(files('change-state-handler.c'))
stub_ss.add(files('clock-warp.c'))
@@ -25,6 +26,7 @@ stub_ss.add(files('monitor.c'))
stub_ss.add(files('monitor-core.c'))
stub_ss.add(files('pci-bus.c'))
stub_ss.add(files('pci-host-piix.c'))
+stub_ss.add(files('qemu-system-killed.c'))
stub_ss.add(files('qemu-timer-notify-cb.c'))
stub_ss.add(files('qmp_memory_device.c'))
stub_ss.add(files('qtest.c'))
--
2.26.2
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH 3/4] block: move block exports to libblockdev
2020-09-25 13:42 ` [PATCH 3/4] block: move block exports to libblockdev Stefan Hajnoczi
@ 2020-09-25 14:11 ` Paolo Bonzini
2020-09-25 15:06 ` Stefan Hajnoczi
0 siblings, 1 reply; 11+ messages in thread
From: Paolo Bonzini @ 2020-09-25 14:11 UTC (permalink / raw)
To: Stefan Hajnoczi, qemu-devel
Cc: Kevin Wolf, Laurent Vivier, Thomas Huth, qemu-block,
Markus Armbruster, Coiby Xu, Max Reitz
On 25/09/20 15:42, Stefan Hajnoczi wrote:
> Block exports are used by softmmu, qemu-storage-daemon, and qemu-nbd.
> They are not used by other programs and are not otherwise needed in
> libblock.
>
> Undo the recent move of blockdev-nbd.c from blockdev_ss into block_ss.
> Two stubs are required to support this:
> 1. bdrv_close_all() (libblock) calls blk_exp_close_all() (libblockdev).
> 2. qemu_system_killed() is called by os-posix.c (libblockdev) and not
> implemented in qemu-nbd.
>
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Another possibility is to call os_setup_signal_handling in qemu-nbd.c
where we currently set the SIGTERM handler. The existing
termsig_handler can be repurposed as qemu_system_killed.
Paolo
> ---
> stubs/blk-exp-close-all.c | 7 +++++++
> stubs/qemu-system-killed.c | 10 ++++++++++
> block/export/meson.build | 4 ++--
> meson.build | 4 ++--
> nbd/meson.build | 2 ++
> stubs/meson.build | 2 ++
> 6 files changed, 25 insertions(+), 4 deletions(-)
> create mode 100644 stubs/blk-exp-close-all.c
> create mode 100644 stubs/qemu-system-killed.c
>
> diff --git a/stubs/blk-exp-close-all.c b/stubs/blk-exp-close-all.c
> new file mode 100644
> index 0000000000..1c71316763
> --- /dev/null
> +++ b/stubs/blk-exp-close-all.c
> @@ -0,0 +1,7 @@
> +#include "qemu/osdep.h"
> +#include "block/export.h"
> +
> +/* Only used in programs that support block exports (libblockdev.fa) */
> +void blk_exp_close_all(void)
> +{
> +}
> diff --git a/stubs/qemu-system-killed.c b/stubs/qemu-system-killed.c
> new file mode 100644
> index 0000000000..9af131917b
> --- /dev/null
> +++ b/stubs/qemu-system-killed.c
> @@ -0,0 +1,10 @@
> +#include "qemu/osdep.h"
> +#include "sysemu/runstate.h"
> +
> +/*
> + * This function is needed by os-posix.c but only implemented by softmmu and
> + * qemu-storage-daemon. Other programs may have no need for it.
> + */
> +void qemu_system_killed(int signal, pid_t pid)
> +{
> +}
> diff --git a/block/export/meson.build b/block/export/meson.build
> index 469a7aa0f5..a2772a0dce 100644
> --- a/block/export/meson.build
> +++ b/block/export/meson.build
> @@ -1,2 +1,2 @@
> -block_ss.add(files('export.c'))
> -block_ss.add(when: 'CONFIG_VHOST_USER', if_true: files('vhost-user-blk-server.c'))
> +blockdev_ss.add(files('export.c'))
> +blockdev_ss.add(when: 'CONFIG_VHOST_USER', if_true: files('vhost-user-blk-server.c'))
> diff --git a/meson.build b/meson.build
> index 18d689b423..0e9528adab 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -835,7 +835,6 @@ subdir('dump')
>
> block_ss.add(files(
> 'block.c',
> - 'blockdev-nbd.c',
> 'blockjob.c',
> 'job.c',
> 'qemu-io-cmds.c',
> @@ -848,6 +847,7 @@ subdir('block')
>
> blockdev_ss.add(files(
> 'blockdev.c',
> + 'blockdev-nbd.c',
> 'iothread.c',
> 'job-qmp.c',
> ))
> @@ -1171,7 +1171,7 @@ if have_tools
> qemu_io = executable('qemu-io', files('qemu-io.c'),
> dependencies: [block, qemuutil], install: true)
> qemu_nbd = executable('qemu-nbd', files('qemu-nbd.c'),
> - dependencies: [block, qemuutil], install: true)
> + dependencies: [blockdev, qemuutil], install: true)
>
> subdir('storage-daemon')
> subdir('contrib/rdmacm-mux')
> diff --git a/nbd/meson.build b/nbd/meson.build
> index 0c00a776d3..2baaa36948 100644
> --- a/nbd/meson.build
> +++ b/nbd/meson.build
> @@ -1,5 +1,7 @@
> block_ss.add(files(
> 'client.c',
> 'common.c',
> +))
> +blockdev_ss.add(files(
> 'server.c',
> ))
> diff --git a/stubs/meson.build b/stubs/meson.build
> index e0b322bc28..60234571b1 100644
> --- a/stubs/meson.build
> +++ b/stubs/meson.build
> @@ -1,6 +1,7 @@
> stub_ss.add(files('arch_type.c'))
> stub_ss.add(files('bdrv-next-monitor-owned.c'))
> stub_ss.add(files('blk-commit-all.c'))
> +stub_ss.add(files('blk-exp-close-all.c'))
> stub_ss.add(files('blockdev-close-all-bdrv-states.c'))
> stub_ss.add(files('change-state-handler.c'))
> stub_ss.add(files('clock-warp.c'))
> @@ -25,6 +26,7 @@ stub_ss.add(files('monitor.c'))
> stub_ss.add(files('monitor-core.c'))
> stub_ss.add(files('pci-bus.c'))
> stub_ss.add(files('pci-host-piix.c'))
> +stub_ss.add(files('qemu-system-killed.c'))
> stub_ss.add(files('qemu-timer-notify-cb.c'))
> stub_ss.add(files('qmp_memory_device.c'))
> stub_ss.add(files('qtest.c'))
>
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH 3/4] block: move block exports to libblockdev
2020-09-25 14:11 ` Paolo Bonzini
@ 2020-09-25 15:06 ` Stefan Hajnoczi
0 siblings, 0 replies; 11+ messages in thread
From: Stefan Hajnoczi @ 2020-09-25 15:06 UTC (permalink / raw)
To: Paolo Bonzini
Cc: Kevin Wolf, Laurent Vivier, Thomas Huth, qemu-block,
Markus Armbruster, qemu-devel, Coiby Xu, Max Reitz
[-- Attachment #1: Type: text/plain, Size: 890 bytes --]
On Fri, Sep 25, 2020 at 04:11:54PM +0200, Paolo Bonzini wrote:
> On 25/09/20 15:42, Stefan Hajnoczi wrote:
> > Block exports are used by softmmu, qemu-storage-daemon, and qemu-nbd.
> > They are not used by other programs and are not otherwise needed in
> > libblock.
> >
> > Undo the recent move of blockdev-nbd.c from blockdev_ss into block_ss.
> > Two stubs are required to support this:
> > 1. bdrv_close_all() (libblock) calls blk_exp_close_all() (libblockdev).
> > 2. qemu_system_killed() is called by os-posix.c (libblockdev) and not
> > implemented in qemu-nbd.
> >
> > Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
>
> Another possibility is to call os_setup_signal_handling in qemu-nbd.c
> where we currently set the SIGTERM handler. The existing
> termsig_handler can be repurposed as qemu_system_killed.
That is nicer. Will fix in v2.
Stefan
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 4/4] block/export: add BlockExportOptions->iothread member
2020-09-25 13:42 [PATCH 0/4] block/export: add BlockExportOptions->iothread member Stefan Hajnoczi
` (2 preceding siblings ...)
2020-09-25 13:42 ` [PATCH 3/4] block: move block exports to libblockdev Stefan Hajnoczi
@ 2020-09-25 13:42 ` Stefan Hajnoczi
2020-09-25 15:01 ` Kevin Wolf
3 siblings, 1 reply; 11+ messages in thread
From: Stefan Hajnoczi @ 2020-09-25 13:42 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, Laurent Vivier, Thomas Huth, qemu-block,
Markus Armbruster, Coiby Xu, Max Reitz, Stefan Hajnoczi,
Paolo Bonzini
Make it possible to specify the iothread where the export will run.
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
Note the x-blockdev-set-iothread QMP command can be used to do the same,
but not from the command-line. And it requires sending an additional
command.
In the long run vhost-user-blk will support per-virtqueue iothread
mappings. But for now a single iothread makes sense and most other
transports will just use one iothread anyway.
---
qapi/block-export.json | 4 ++++
block/export/export.c | 26 +++++++++++++++++++++++++-
2 files changed, 29 insertions(+), 1 deletion(-)
diff --git a/qapi/block-export.json b/qapi/block-export.json
index 87ac5117cd..eba6f6eae9 100644
--- a/qapi/block-export.json
+++ b/qapi/block-export.json
@@ -219,11 +219,15 @@
# export before completion is signalled. (since: 5.2;
# default: false)
#
+# @iothread: The name of the iothread object where the export will run. The
+# default is the main loop thread. (since: 5.2)
+#
# Since: 4.2
##
{ 'union': 'BlockExportOptions',
'base': { 'type': 'BlockExportType',
'id': 'str',
+ '*iothread': 'str',
'node-name': 'str',
'*writable': 'bool',
'*writethrough': 'bool' },
diff --git a/block/export/export.c b/block/export/export.c
index 550897e236..0fb3d76ee3 100644
--- a/block/export/export.c
+++ b/block/export/export.c
@@ -15,6 +15,7 @@
#include "block/block.h"
#include "sysemu/block-backend.h"
+#include "sysemu/iothread.h"
#include "block/export.h"
#include "block/nbd.h"
#include "qapi/error.h"
@@ -66,7 +67,7 @@ BlockExport *blk_exp_add(BlockExportOptions *export, Error **errp)
const BlockExportDriver *drv;
BlockExport *exp = NULL;
BlockDriverState *bs;
- BlockBackend *blk;
+ BlockBackend *blk = NULL;
AioContext *ctx;
uint64_t perm;
int ret;
@@ -102,6 +103,29 @@ BlockExport *blk_exp_add(BlockExportOptions *export, Error **errp)
ctx = bdrv_get_aio_context(bs);
aio_context_acquire(ctx);
+ if (export->has_iothread) {
+ IOThread *iothread;
+ AioContext *new_ctx;
+
+ iothread = iothread_by_id(export->iothread);
+ if (!iothread) {
+ error_setg(errp, "iothread \"%s\" not found", export->iothread);
+ goto fail;
+ }
+
+ new_ctx = iothread_get_aio_context(iothread);
+
+ ret = bdrv_try_set_aio_context(bs, new_ctx, errp);
+ if (ret != 0) {
+ goto fail;
+ }
+
+ aio_context_release(ctx);
+ aio_context_acquire(new_ctx);
+
+ ctx = new_ctx;
+ }
+
/*
* Block exports are used for non-shared storage migration. Make sure
* that BDRV_O_INACTIVE is cleared and the image is ready for write
--
2.26.2
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH 4/4] block/export: add BlockExportOptions->iothread member
2020-09-25 13:42 ` [PATCH 4/4] block/export: add BlockExportOptions->iothread member Stefan Hajnoczi
@ 2020-09-25 15:01 ` Kevin Wolf
2020-09-28 8:37 ` Stefan Hajnoczi
0 siblings, 1 reply; 11+ messages in thread
From: Kevin Wolf @ 2020-09-25 15:01 UTC (permalink / raw)
To: Stefan Hajnoczi
Cc: Laurent Vivier, Thomas Huth, qemu-block, Markus Armbruster,
qemu-devel, Coiby Xu, Max Reitz, Paolo Bonzini
Am 25.09.2020 um 15:42 hat Stefan Hajnoczi geschrieben:
> Make it possible to specify the iothread where the export will run.
>
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
> Note the x-blockdev-set-iothread QMP command can be used to do the same,
> but not from the command-line. And it requires sending an additional
> command.
>
> In the long run vhost-user-blk will support per-virtqueue iothread
> mappings. But for now a single iothread makes sense and most other
> transports will just use one iothread anyway.
> ---
> qapi/block-export.json | 4 ++++
> block/export/export.c | 26 +++++++++++++++++++++++++-
> 2 files changed, 29 insertions(+), 1 deletion(-)
>
> diff --git a/qapi/block-export.json b/qapi/block-export.json
> index 87ac5117cd..eba6f6eae9 100644
> --- a/qapi/block-export.json
> +++ b/qapi/block-export.json
> @@ -219,11 +219,15 @@
> # export before completion is signalled. (since: 5.2;
> # default: false)
> #
> +# @iothread: The name of the iothread object where the export will run. The
> +# default is the main loop thread. (since: 5.2)
NBD exports currently switch automatically to a different AioContext if
another user (usually a guest device using the same node) tries to
change the AioContext. I believe this is also the most useful mode in
the context of the system emulator.
I can see the need for an iothread option in qemu-storage-daemon where
usually nobody else will move the node into a different AioContext.
But we need to define the semantics more precisely and specify what
happens if another user wants to change the AioContext later. Currently,
the NBD export will allow this and just switch the AioContext - after
this patch, ignoring what the user set explicitly with this new option.
I see two options to handle this more consistently:
1. If @iothread is set, move the block node into the requested
AioContext, and if that fails, block-export-add fails. Other users of
the node will be denied to change the AioContext while the export is
active.
If @iothread is not given, it behaves like today: Use whatever
AioContext the node is currently in and switch whenever another user
requests it.
2. Add a bool option @fixed-iothread that determines whether other users
can change the AioContext while the export is active.
Giving an @iothread and fixed-iothread == true means that we'll
enforce the given AioContext during the whole lifetime of the export.
With fixed-iothread == false it means that we try to move the block
node to the requested iothread if possible (but we won't fail if it
isn't possible) and will follow any other user switching the
AioContext of the node.
Not giving @iothread means that we start with the current AioContext
of the node, and @fixed-iothread then means the same as before.
Does this make sense to you?
Kevin
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 4/4] block/export: add BlockExportOptions->iothread member
2020-09-25 15:01 ` Kevin Wolf
@ 2020-09-28 8:37 ` Stefan Hajnoczi
0 siblings, 0 replies; 11+ messages in thread
From: Stefan Hajnoczi @ 2020-09-28 8:37 UTC (permalink / raw)
To: Kevin Wolf
Cc: Laurent Vivier, Thomas Huth, qemu-block, Markus Armbruster,
qemu-devel, Coiby Xu, Max Reitz, Paolo Bonzini
[-- Attachment #1: Type: text/plain, Size: 3413 bytes --]
On Fri, Sep 25, 2020 at 05:01:42PM +0200, Kevin Wolf wrote:
> Am 25.09.2020 um 15:42 hat Stefan Hajnoczi geschrieben:
> > Make it possible to specify the iothread where the export will run.
> >
> > Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> > ---
> > Note the x-blockdev-set-iothread QMP command can be used to do the same,
> > but not from the command-line. And it requires sending an additional
> > command.
> >
> > In the long run vhost-user-blk will support per-virtqueue iothread
> > mappings. But for now a single iothread makes sense and most other
> > transports will just use one iothread anyway.
> > ---
> > qapi/block-export.json | 4 ++++
> > block/export/export.c | 26 +++++++++++++++++++++++++-
> > 2 files changed, 29 insertions(+), 1 deletion(-)
> >
> > diff --git a/qapi/block-export.json b/qapi/block-export.json
> > index 87ac5117cd..eba6f6eae9 100644
> > --- a/qapi/block-export.json
> > +++ b/qapi/block-export.json
> > @@ -219,11 +219,15 @@
> > # export before completion is signalled. (since: 5.2;
> > # default: false)
> > #
> > +# @iothread: The name of the iothread object where the export will run. The
> > +# default is the main loop thread. (since: 5.2)
>
> NBD exports currently switch automatically to a different AioContext if
> another user (usually a guest device using the same node) tries to
> change the AioContext. I believe this is also the most useful mode in
> the context of the system emulator.
>
> I can see the need for an iothread option in qemu-storage-daemon where
> usually nobody else will move the node into a different AioContext.
>
> But we need to define the semantics more precisely and specify what
> happens if another user wants to change the AioContext later. Currently,
> the NBD export will allow this and just switch the AioContext - after
> this patch, ignoring what the user set explicitly with this new option.
>
> I see two options to handle this more consistently:
>
> 1. If @iothread is set, move the block node into the requested
> AioContext, and if that fails, block-export-add fails. Other users of
> the node will be denied to change the AioContext while the export is
> active.
>
> If @iothread is not given, it behaves like today: Use whatever
> AioContext the node is currently in and switch whenever another user
> requests it.
>
> 2. Add a bool option @fixed-iothread that determines whether other users
> can change the AioContext while the export is active.
>
> Giving an @iothread and fixed-iothread == true means that we'll
> enforce the given AioContext during the whole lifetime of the export.
> With fixed-iothread == false it means that we try to move the block
> node to the requested iothread if possible (but we won't fail if it
> isn't possible) and will follow any other user switching the
> AioContext of the node.
>
> Not giving @iothread means that we start with the current AioContext
> of the node, and @fixed-iothread then means the same as before.
>
> Does this make sense to you?
Thanks for raising this. #2 is more flexible. I suspect most users will
be happy with fixed-iothread = false by default (i.e. things keep
working even if the iothread is changed).
I can adjust this patch to follow those semantics.
Stefan
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 11+ messages in thread