[PATCH 0/4] block/export: add BlockExportOptions->iothread member

[PATCH 0/4] block/export: add BlockExportOptions->iothread member

[Stefan Hajnoczi]

This series adjusts the build system and then adds a
BlockExportOptions->iothread member so that it is possible to set the iothread
for an export.

Based-on: 20200924151549.913737-1-stefanha@redhat.com ("[PATCH v2 00/13] block/export: convert vhost-user-blk-server to block exports API")

Stefan Hajnoczi (4):
  util/vhost-user-server: use static library in meson.build
  qemu-storage-daemon: avoid compiling blockdev_ss twice
  block: move block exports to libblockdev
  block/export: add BlockExportOptions->iothread member

 qapi/block-export.json            |  4 ++++
 block/export/export.c             | 34 ++++++++++++++++++++++++++-----
 stubs/blk-exp-close-all.c         |  7 +++++++
 stubs/qemu-system-killed.c        | 10 +++++++++
 block/export/meson.build          |  4 ++--
 contrib/libvhost-user/meson.build |  1 +
 meson.build                       | 22 +++++++++++++++-----
 nbd/meson.build                   |  2 ++
 storage-daemon/meson.build        |  3 +--
 stubs/meson.build                 |  2 ++
 tests/qtest/meson.build           |  2 +-
 util/meson.build                  |  4 +++-
 12 files changed, 79 insertions(+), 16 deletions(-)
 create mode 100644 stubs/blk-exp-close-all.c
 create mode 100644 stubs/qemu-system-killed.c

-- 
2.26.2

[PATCH 1/4] util/vhost-user-server: use static library in meson.build

[Stefan Hajnoczi]

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

[PATCH 2/4] qemu-storage-daemon: avoid compiling blockdev_ss twice

[Stefan Hajnoczi]

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

[PATCH 3/4] block: move block exports to libblockdev

[Stefan Hajnoczi]

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

[PATCH 4/4] block/export: add BlockExportOptions->iothread member

[Stefan Hajnoczi]

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

Re: [PATCH 1/4] util/vhost-user-server: use static library in meson.build

[Paolo Bonzini]

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'))
> 

Re: [PATCH 2/4] qemu-storage-daemon: avoid compiling blockdev_ss twice

[Paolo Bonzini]

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>

Re: [PATCH 3/4] block: move block exports to libblockdev

[Paolo Bonzini]

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'))
> 

Re: [PATCH 4/4] block/export: add BlockExportOptions->iothread member

[Kevin Wolf]

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

Re: [PATCH 3/4] block: move block exports to libblockdev

[Stefan Hajnoczi]

[-- 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 --]

Re: [PATCH 4/4] block/export: add BlockExportOptions->iothread member

[Stefan Hajnoczi]

[-- 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 --]