* [PATCH v2 00/31] block/export: Add infrastructure and QAPI for block exports
@ 2020-09-24 15:26 Kevin Wolf
2020-09-24 15:26 ` [PATCH v2 01/31] nbd: Remove unused nbd_export_get_blockdev() Kevin Wolf
` (33 more replies)
0 siblings, 34 replies; 44+ messages in thread
From: Kevin Wolf @ 2020-09-24 15:26 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, qemu-devel, stefanha, mreitz
We are planning to add more block export types than just NBD in the near
future (e.g. vhost-user-blk and FUSE). This series lays the ground for
this with some generic block export infrastructure and QAPI interfaces
that will allow managing all of them (for now add/remove/query).
As a side effect, qemu-storage-daemon can now map --export directly to
the block-export-add QMP command, similar to other command line options.
The built-in NBD servers also gains new options that bring it at least a
little closer to feature parity with qemu-nbd.
v2:
- Rebased on current master
- Fixed assumption of &blk_exp_nbd instead of drv in generic code [Max]
- Documented blk_exp_request_shutdown() better [Max]
- iotests 140: Fixed race between QMP return value and event [Max]
- Improved the commit message for patch 26
- Removed copy&paste error in deprecated.rst [Max]
- iotests: Take Sequence instead of positional arguments in
qemu_tool_pipe_and_status() [Max]
- iotests: Separate patch for qemu_nbd_list_log [Max]
- iotests 307: [Max]
* Allow more image formats
* Use sock_dir for the socket
* Use f-strings instead of % operator
* Log events after deleting an export
* Test force removing an export
Kevin Wolf (31):
nbd: Remove unused nbd_export_get_blockdev()
qapi: Create block-export module
qapi: Rename BlockExport to BlockExportOptions
block/export: Add BlockExport infrastructure and block-export-add
qemu-storage-daemon: Use qmp_block_export_add()
qemu-nbd: Use raw block driver for --offset
block/export: Remove magic from block-export-add
nbd: Add max-connections to nbd-server-start
nbd: Add writethrough to block-export-add
nbd: Remove NBDExport.close callback
qemu-nbd: Use blk_exp_add() to create the export
nbd/server: Simplify export shutdown
block/export: Move refcount from NBDExport to BlockExport
block/export: Move AioContext from NBDExport to BlockExport
block/export: Add node-name to BlockExportOptions
block/export: Allocate BlockExport in blk_exp_add()
block/export: Add blk_exp_close_all(_type)
block/export: Add 'id' option to block-export-add
block/export: Move strong user reference to block_exports
block/export: Add block-export-del
block/export: Add BLOCK_EXPORT_DELETED event
block/export: Move blk to BlockExport
block/export: Create BlockBackend in blk_exp_add()
block/export: Add query-block-exports
block/export: Move writable to BlockExportOptions
nbd: Merge nbd_export_new() and nbd_export_create()
nbd: Deprecate nbd-server-add/remove
iotests: Factor out qemu_tool_pipe_and_status()
iotests: Introduce qemu_nbd_list_log()
iotests: Allow supported and unsupported formats at the same time
iotests: Test block-export-* QMP interface
qapi/block-core.json | 166 --------------
qapi/block-export.json | 291 ++++++++++++++++++++++++
qapi/qapi-schema.json | 1 +
docs/system/deprecated.rst | 6 +
include/block/export.h | 89 ++++++++
include/block/nbd.h | 22 +-
block.c | 2 +-
block/export/export.c | 325 +++++++++++++++++++++++++++
block/monitor/block-hmp-cmds.c | 13 +-
blockdev-nbd.c | 171 +++++++-------
nbd/server.c | 309 +++++++++++--------------
qemu-nbd.c | 67 +++---
storage-daemon/qemu-storage-daemon.c | 27 +--
tests/qemu-iotests/iotests.py | 68 +++---
block/export/meson.build | 1 +
block/meson.build | 2 +
meson.build | 2 +-
qapi/meson.build | 4 +-
storage-daemon/qapi/qapi-schema.json | 1 +
tests/qemu-iotests/140 | 9 +-
tests/qemu-iotests/140.out | 2 +-
tests/qemu-iotests/223.out | 8 +-
tests/qemu-iotests/307 | 132 +++++++++++
tests/qemu-iotests/307.out | 124 ++++++++++
tests/qemu-iotests/group | 1 +
25 files changed, 1311 insertions(+), 532 deletions(-)
create mode 100644 qapi/block-export.json
create mode 100644 include/block/export.h
create mode 100644 block/export/export.c
create mode 100644 block/export/meson.build
create mode 100755 tests/qemu-iotests/307
create mode 100644 tests/qemu-iotests/307.out
--
2.25.4
^ permalink raw reply [flat|nested] 44+ messages in thread
* [PATCH v2 01/31] nbd: Remove unused nbd_export_get_blockdev()
2020-09-24 15:26 [PATCH v2 00/31] block/export: Add infrastructure and QAPI for block exports Kevin Wolf
@ 2020-09-24 15:26 ` Kevin Wolf
2020-09-24 15:26 ` [PATCH v2 02/31] qapi: Create block-export module Kevin Wolf
` (32 subsequent siblings)
33 siblings, 0 replies; 44+ messages in thread
From: Kevin Wolf @ 2020-09-24 15:26 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, qemu-devel, stefanha, mreitz
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
include/block/nbd.h | 2 --
nbd/server.c | 5 -----
2 files changed, 7 deletions(-)
diff --git a/include/block/nbd.h b/include/block/nbd.h
index 9bc3bfaeec..0451683d03 100644
--- a/include/block/nbd.h
+++ b/include/block/nbd.h
@@ -338,8 +338,6 @@ void nbd_export_remove(NBDExport *exp, NbdServerRemoveMode mode, Error **errp);
void nbd_export_get(NBDExport *exp);
void nbd_export_put(NBDExport *exp);
-BlockBackend *nbd_export_get_blockdev(NBDExport *exp);
-
AioContext *nbd_export_aio_context(NBDExport *exp);
NBDExport *nbd_export_find(const char *name);
void nbd_export_close_all(void);
diff --git a/nbd/server.c b/nbd/server.c
index 982de67816..bd53f7baea 100644
--- a/nbd/server.c
+++ b/nbd/server.c
@@ -1731,11 +1731,6 @@ void nbd_export_put(NBDExport *exp)
}
}
-BlockBackend *nbd_export_get_blockdev(NBDExport *exp)
-{
- return exp->blk;
-}
-
void nbd_export_close_all(void)
{
NBDExport *exp, *next;
--
2.25.4
^ permalink raw reply related [flat|nested] 44+ messages in thread
* [PATCH v2 02/31] qapi: Create block-export module
2020-09-24 15:26 [PATCH v2 00/31] block/export: Add infrastructure and QAPI for block exports Kevin Wolf
2020-09-24 15:26 ` [PATCH v2 01/31] nbd: Remove unused nbd_export_get_blockdev() Kevin Wolf
@ 2020-09-24 15:26 ` Kevin Wolf
2020-09-24 15:26 ` [PATCH v2 03/31] qapi: Rename BlockExport to BlockExportOptions Kevin Wolf
` (31 subsequent siblings)
33 siblings, 0 replies; 44+ messages in thread
From: Kevin Wolf @ 2020-09-24 15:26 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, qemu-devel, stefanha, mreitz
Move all block export related types and commands from block-core to the
new QAPI module block-export.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
qapi/block-core.json | 166 -------------------------
qapi/block-export.json | 175 +++++++++++++++++++++++++++
qapi/qapi-schema.json | 1 +
include/block/nbd.h | 2 +-
block/monitor/block-hmp-cmds.c | 1 +
blockdev-nbd.c | 2 +-
storage-daemon/qemu-storage-daemon.c | 2 +-
qapi/meson.build | 4 +-
storage-daemon/qapi/qapi-schema.json | 1 +
9 files changed, 184 insertions(+), 170 deletions(-)
create mode 100644 qapi/block-export.json
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 3c16f1e11d..d620bd1302 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -5192,172 +5192,6 @@
'iothread': 'StrOrNull',
'*force': 'bool' } }
-##
-# @NbdServerOptions:
-#
-# @addr: Address on which to listen.
-# @tls-creds: ID of the TLS credentials object (since 2.6).
-# @tls-authz: ID of the QAuthZ authorization object used to validate
-# the client's x509 distinguished name. This object is
-# is only resolved at time of use, so can be deleted and
-# recreated on the fly while the NBD server is active.
-# If missing, it will default to denying access (since 4.0).
-#
-# Keep this type consistent with the nbd-server-start arguments. The only
-# intended difference is using SocketAddress instead of SocketAddressLegacy.
-#
-# Since: 4.2
-##
-{ 'struct': 'NbdServerOptions',
- 'data': { 'addr': 'SocketAddress',
- '*tls-creds': 'str',
- '*tls-authz': 'str'} }
-
-##
-# @nbd-server-start:
-#
-# Start an NBD server listening on the given host and port. Block
-# devices can then be exported using @nbd-server-add. The NBD
-# server will present them as named exports; for example, another
-# QEMU instance could refer to them as "nbd:HOST:PORT:exportname=NAME".
-#
-# Keep this type consistent with the NbdServerOptions type. The only intended
-# difference is using SocketAddressLegacy instead of SocketAddress.
-#
-# @addr: Address on which to listen.
-# @tls-creds: ID of the TLS credentials object (since 2.6).
-# @tls-authz: ID of the QAuthZ authorization object used to validate
-# the client's x509 distinguished name. This object is
-# is only resolved at time of use, so can be deleted and
-# recreated on the fly while the NBD server is active.
-# If missing, it will default to denying access (since 4.0).
-#
-# Returns: error if the server is already running.
-#
-# Since: 1.3.0
-##
-{ 'command': 'nbd-server-start',
- 'data': { 'addr': 'SocketAddressLegacy',
- '*tls-creds': 'str',
- '*tls-authz': 'str'} }
-
-##
-# @BlockExportNbd:
-#
-# An NBD block export.
-#
-# @device: The device name or node name of the node to be exported
-#
-# @name: Export name. If unspecified, the @device parameter is used as the
-# export name. (Since 2.12)
-#
-# @description: Free-form description of the export, up to 4096 bytes.
-# (Since 5.0)
-#
-# @writable: Whether clients should be able to write to the device via the
-# NBD connection (default false).
-#
-# @bitmap: Also export the dirty bitmap reachable from @device, so the
-# NBD client can use NBD_OPT_SET_META_CONTEXT with
-# "qemu:dirty-bitmap:NAME" to inspect the bitmap. (since 4.0)
-#
-# Since: 5.0
-##
-{ 'struct': 'BlockExportNbd',
- 'data': {'device': 'str', '*name': 'str', '*description': 'str',
- '*writable': 'bool', '*bitmap': 'str' } }
-
-##
-# @nbd-server-add:
-#
-# Export a block node to QEMU's embedded NBD server.
-#
-# Returns: error if the server is not running, or export with the same name
-# already exists.
-#
-# Since: 1.3.0
-##
-{ 'command': 'nbd-server-add',
- 'data': 'BlockExportNbd', 'boxed': true }
-
-##
-# @NbdServerRemoveMode:
-#
-# Mode for removing an NBD export.
-#
-# @safe: Remove export if there are no existing connections, fail otherwise.
-#
-# @hard: Drop all connections immediately and remove export.
-#
-# Potential additional modes to be added in the future:
-#
-# hide: Just hide export from new clients, leave existing connections as is.
-# Remove export after all clients are disconnected.
-#
-# soft: Hide export from new clients, answer with ESHUTDOWN for all further
-# requests from existing clients.
-#
-# Since: 2.12
-##
-{'enum': 'NbdServerRemoveMode', 'data': ['safe', 'hard']}
-
-##
-# @nbd-server-remove:
-#
-# Remove NBD export by name.
-#
-# @name: Export name.
-#
-# @mode: Mode of command operation. See @NbdServerRemoveMode description.
-# Default is 'safe'.
-#
-# Returns: error if
-# - the server is not running
-# - export is not found
-# - mode is 'safe' and there are existing connections
-#
-# Since: 2.12
-##
-{ 'command': 'nbd-server-remove',
- 'data': {'name': 'str', '*mode': 'NbdServerRemoveMode'} }
-
-##
-# @nbd-server-stop:
-#
-# Stop QEMU's embedded NBD server, and unregister all devices previously
-# added via @nbd-server-add.
-#
-# Since: 1.3.0
-##
-{ 'command': 'nbd-server-stop' }
-
-##
-# @BlockExportType:
-#
-# An enumeration of block export types
-#
-# @nbd: NBD export
-#
-# Since: 4.2
-##
-{ 'enum': 'BlockExportType',
- 'data': [ 'nbd' ] }
-
-##
-# @BlockExport:
-#
-# Describes a block export, i.e. how single node should be exported on an
-# external interface.
-#
-# Since: 4.2
-##
-{ 'union': 'BlockExport',
- 'base': { 'type': 'BlockExportType' },
- 'discriminator': 'type',
- 'data': {
- 'nbd': 'BlockExportNbd'
- } }
-
##
# @QuorumOpType:
#
diff --git a/qapi/block-export.json b/qapi/block-export.json
new file mode 100644
index 0000000000..f799fef36d
--- /dev/null
+++ b/qapi/block-export.json
@@ -0,0 +1,175 @@
+# -*- Mode: Python -*-
+# vim: filetype=python
+
+##
+# == Block device exports
+##
+
+{ 'include': 'sockets.json' }
+
+##
+# @NbdServerOptions:
+#
+# Keep this type consistent with the nbd-server-start arguments. The only
+# intended difference is using SocketAddress instead of SocketAddressLegacy.
+#
+# @addr: Address on which to listen.
+# @tls-creds: ID of the TLS credentials object (since 2.6).
+# @tls-authz: ID of the QAuthZ authorization object used to validate
+# the client's x509 distinguished name. This object is
+# is only resolved at time of use, so can be deleted and
+# recreated on the fly while the NBD server is active.
+# If missing, it will default to denying access (since 4.0).
+#
+# Since: 4.2
+##
+{ 'struct': 'NbdServerOptions',
+ 'data': { 'addr': 'SocketAddress',
+ '*tls-creds': 'str',
+ '*tls-authz': 'str'} }
+
+##
+# @nbd-server-start:
+#
+# Start an NBD server listening on the given host and port. Block
+# devices can then be exported using @nbd-server-add. The NBD
+# server will present them as named exports; for example, another
+# QEMU instance could refer to them as "nbd:HOST:PORT:exportname=NAME".
+#
+# @addr: Address on which to listen.
+# @tls-creds: ID of the TLS credentials object (since 2.6).
+# @tls-authz: ID of the QAuthZ authorization object used to validate
+# the client's x509 distinguished name. This object is
+# is only resolved at time of use, so can be deleted and
+# recreated on the fly while the NBD server is active.
+# If missing, it will default to denying access (since 4.0).
+#
+# Returns: error if the server is already running.
+#
+# Keep this type consistent with the NbdServerOptions type. The only intended
+# difference is using SocketAddressLegacy instead of SocketAddress.
+#
+# Since: 1.3.0
+##
+{ 'command': 'nbd-server-start',
+ 'data': { 'addr': 'SocketAddressLegacy',
+ '*tls-creds': 'str',
+ '*tls-authz': 'str'} }
+
+##
+# @BlockExportNbd:
+#
+# An NBD block export.
+#
+# @device: The device name or node name of the node to be exported
+#
+# @name: Export name. If unspecified, the @device parameter is used as the
+# export name. (Since 2.12)
+#
+# @description: Free-form description of the export, up to 4096 bytes.
+# (Since 5.0)
+#
+# @writable: Whether clients should be able to write to the device via the
+# NBD connection (default false).
+#
+# @bitmap: Also export the dirty bitmap reachable from @device, so the
+# NBD client can use NBD_OPT_SET_META_CONTEXT with
+# "qemu:dirty-bitmap:NAME" to inspect the bitmap. (since 4.0)
+#
+# Since: 5.0
+##
+{ 'struct': 'BlockExportNbd',
+ 'data': {'device': 'str', '*name': 'str', '*description': 'str',
+ '*writable': 'bool', '*bitmap': 'str' } }
+
+##
+# @nbd-server-add:
+#
+# Export a block node to QEMU's embedded NBD server.
+#
+# Returns: error if the server is not running, or export with the same name
+# already exists.
+#
+# Since: 1.3.0
+##
+{ 'command': 'nbd-server-add',
+ 'data': 'BlockExportNbd', 'boxed': true }
+
+##
+# @NbdServerRemoveMode:
+#
+# Mode for removing an NBD export.
+#
+# @safe: Remove export if there are no existing connections, fail otherwise.
+#
+# @hard: Drop all connections immediately and remove export.
+#
+# Potential additional modes to be added in the future:
+#
+# hide: Just hide export from new clients, leave existing connections as is.
+# Remove export after all clients are disconnected.
+#
+# soft: Hide export from new clients, answer with ESHUTDOWN for all further
+# requests from existing clients.
+#
+# Since: 2.12
+##
+{'enum': 'NbdServerRemoveMode', 'data': ['safe', 'hard']}
+
+##
+# @nbd-server-remove:
+#
+# Remove NBD export by name.
+#
+# @name: Export name.
+#
+# @mode: Mode of command operation. See @NbdServerRemoveMode description.
+# Default is 'safe'.
+#
+# Returns: error if
+# - the server is not running
+# - export is not found
+# - mode is 'safe' and there are existing connections
+#
+# Since: 2.12
+##
+{ 'command': 'nbd-server-remove',
+ 'data': {'name': 'str', '*mode': 'NbdServerRemoveMode'} }
+
+##
+# @nbd-server-stop:
+#
+# Stop QEMU's embedded NBD server, and unregister all devices previously
+# added via @nbd-server-add.
+#
+# Since: 1.3.0
+##
+{ 'command': 'nbd-server-stop' }
+
+##
+# @BlockExportType:
+#
+# An enumeration of block export types
+#
+# @nbd: NBD export
+#
+# Since: 4.2
+##
+{ 'enum': 'BlockExportType',
+ 'data': [ 'nbd' ] }
+
+##
+# @BlockExport:
+#
+# Describes a block export, i.e. how single node should be exported on an
+# external interface.
+#
+# Since: 4.2
+##
+{ 'union': 'BlockExport',
+ 'base': { 'type': 'BlockExportType' },
+ 'discriminator': 'type',
+ 'data': {
+ 'nbd': 'BlockExportNbd'
+ } }
+
diff --git a/qapi/qapi-schema.json b/qapi/qapi-schema.json
index f03ff91ceb..b78fc399a4 100644
--- a/qapi/qapi-schema.json
+++ b/qapi/qapi-schema.json
@@ -64,6 +64,7 @@
{ 'include': 'run-state.json' }
{ 'include': 'crypto.json' }
{ 'include': 'block.json' }
+{ 'include': 'block-export.json' }
{ 'include': 'char.json' }
{ 'include': 'dump.json' }
{ 'include': 'job.json' }
diff --git a/include/block/nbd.h b/include/block/nbd.h
index 0451683d03..262f6da2ce 100644
--- a/include/block/nbd.h
+++ b/include/block/nbd.h
@@ -20,7 +20,7 @@
#ifndef NBD_H
#define NBD_H
-#include "qapi/qapi-types-block.h"
+#include "qapi/qapi-types-block-export.h"
#include "io/channel-socket.h"
#include "crypto/tlscreds.h"
#include "qapi/error.h"
diff --git a/block/monitor/block-hmp-cmds.c b/block/monitor/block-hmp-cmds.c
index 4d3db5ed3c..6ce0f8f87c 100644
--- a/block/monitor/block-hmp-cmds.c
+++ b/block/monitor/block-hmp-cmds.c
@@ -40,6 +40,7 @@
#include "sysemu/block-backend.h"
#include "sysemu/blockdev.h"
#include "qapi/qapi-commands-block.h"
+#include "qapi/qapi-commands-block-export.h"
#include "qapi/qmp/qdict.h"
#include "qapi/error.h"
#include "qapi/qmp/qerror.h"
diff --git a/blockdev-nbd.c b/blockdev-nbd.c
index 1a95d89f00..0f6b80c58f 100644
--- a/blockdev-nbd.c
+++ b/blockdev-nbd.c
@@ -14,7 +14,7 @@
#include "sysemu/block-backend.h"
#include "hw/block/block.h"
#include "qapi/error.h"
-#include "qapi/qapi-commands-block.h"
+#include "qapi/qapi-commands-block-export.h"
#include "block/nbd.h"
#include "io/channel-socket.h"
#include "io/net-listener.h"
diff --git a/storage-daemon/qemu-storage-daemon.c b/storage-daemon/qemu-storage-daemon.c
index 7e9b0e0d3f..ed9d2afcf3 100644
--- a/storage-daemon/qemu-storage-daemon.c
+++ b/storage-daemon/qemu-storage-daemon.c
@@ -35,8 +35,8 @@
#include "monitor/monitor-internal.h"
#include "qapi/error.h"
-#include "qapi/qapi-visit-block.h"
#include "qapi/qapi-visit-block-core.h"
+#include "qapi/qapi-visit-block-export.h"
#include "qapi/qapi-visit-control.h"
#include "qapi/qmp/qdict.h"
#include "qapi/qmp/qstring.h"
diff --git a/qapi/meson.build b/qapi/meson.build
index 2b2872a41d..1ee19a07de 100644
--- a/qapi/meson.build
+++ b/qapi/meson.build
@@ -16,8 +16,9 @@ util_ss.add(files(
qapi_all_modules = [
'audio',
'authz',
- 'block-core',
'block',
+ 'block-core',
+ 'block-export',
'char',
'common',
'control',
@@ -47,6 +48,7 @@ qapi_all_modules = [
qapi_storage_daemon_modules = [
'block-core',
+ 'block-export',
'char',
'common',
'control',
diff --git a/storage-daemon/qapi/qapi-schema.json b/storage-daemon/qapi/qapi-schema.json
index 6100d1f0c9..c6ad5ae1e3 100644
--- a/storage-daemon/qapi/qapi-schema.json
+++ b/storage-daemon/qapi/qapi-schema.json
@@ -16,6 +16,7 @@
{ 'include': '../../qapi/pragma.json' }
{ 'include': '../../qapi/block-core.json' }
+{ 'include': '../../qapi/block-export.json' }
{ 'include': '../../qapi/char.json' }
{ 'include': '../../qapi/common.json' }
{ 'include': '../../qapi/control.json' }
--
2.25.4
^ permalink raw reply related [flat|nested] 44+ messages in thread
* [PATCH v2 03/31] qapi: Rename BlockExport to BlockExportOptions
2020-09-24 15:26 [PATCH v2 00/31] block/export: Add infrastructure and QAPI for block exports Kevin Wolf
2020-09-24 15:26 ` [PATCH v2 01/31] nbd: Remove unused nbd_export_get_blockdev() Kevin Wolf
2020-09-24 15:26 ` [PATCH v2 02/31] qapi: Create block-export module Kevin Wolf
@ 2020-09-24 15:26 ` Kevin Wolf
2020-09-24 15:26 ` [PATCH v2 04/31] block/export: Add BlockExport infrastructure and block-export-add Kevin Wolf
` (30 subsequent siblings)
33 siblings, 0 replies; 44+ messages in thread
From: Kevin Wolf @ 2020-09-24 15:26 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, qemu-devel, stefanha, mreitz
The name BlockExport will be used for the struct containing the runtime
state of block exports, so change the name of export creation options.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
qapi/block-export.json | 12 ++++++------
block/monitor/block-hmp-cmds.c | 6 +++---
blockdev-nbd.c | 2 +-
storage-daemon/qemu-storage-daemon.c | 8 ++++----
4 files changed, 14 insertions(+), 14 deletions(-)
diff --git a/qapi/block-export.json b/qapi/block-export.json
index f799fef36d..d540bcb25a 100644
--- a/qapi/block-export.json
+++ b/qapi/block-export.json
@@ -57,7 +57,7 @@
'*tls-authz': 'str'} }
##
-# @BlockExportNbd:
+# @BlockExportOptionsNbd:
#
# An NBD block export.
#
@@ -78,7 +78,7 @@
#
# Since: 5.0
##
-{ 'struct': 'BlockExportNbd',
+{ 'struct': 'BlockExportOptionsNbd',
'data': {'device': 'str', '*name': 'str', '*description': 'str',
'*writable': 'bool', '*bitmap': 'str' } }
@@ -93,7 +93,7 @@
# Since: 1.3.0
##
{ 'command': 'nbd-server-add',
- 'data': 'BlockExportNbd', 'boxed': true }
+ 'data': 'BlockExportOptionsNbd', 'boxed': true }
##
# @NbdServerRemoveMode:
@@ -159,17 +159,17 @@
'data': [ 'nbd' ] }
##
-# @BlockExport:
+# @BlockExportOptions:
#
# Describes a block export, i.e. how single node should be exported on an
# external interface.
#
# Since: 4.2
##
-{ 'union': 'BlockExport',
+{ 'union': 'BlockExportOptions',
'base': { 'type': 'BlockExportType' },
'discriminator': 'type',
'data': {
- 'nbd': 'BlockExportNbd'
+ 'nbd': 'BlockExportOptionsNbd'
} }
diff --git a/block/monitor/block-hmp-cmds.c b/block/monitor/block-hmp-cmds.c
index 6ce0f8f87c..fb632b1189 100644
--- a/block/monitor/block-hmp-cmds.c
+++ b/block/monitor/block-hmp-cmds.c
@@ -398,7 +398,7 @@ void hmp_nbd_server_start(Monitor *mon, const QDict *qdict)
Error *local_err = NULL;
BlockInfoList *block_list, *info;
SocketAddress *addr;
- BlockExportNbd export;
+ BlockExportOptionsNbd export;
if (writable && !all) {
error_setg(&local_err, "-w only valid together with -a");
@@ -431,7 +431,7 @@ void hmp_nbd_server_start(Monitor *mon, const QDict *qdict)
continue;
}
- export = (BlockExportNbd) {
+ export = (BlockExportOptionsNbd) {
.device = info->value->device,
.has_writable = true,
.writable = writable,
@@ -458,7 +458,7 @@ void hmp_nbd_server_add(Monitor *mon, const QDict *qdict)
bool writable = qdict_get_try_bool(qdict, "writable", false);
Error *local_err = NULL;
- BlockExportNbd export = {
+ BlockExportOptionsNbd export = {
.device = (char *) device,
.has_name = !!name,
.name = (char *) name,
diff --git a/blockdev-nbd.c b/blockdev-nbd.c
index 0f6b80c58f..98ee1b6170 100644
--- a/blockdev-nbd.c
+++ b/blockdev-nbd.c
@@ -148,7 +148,7 @@ void qmp_nbd_server_start(SocketAddressLegacy *addr,
qapi_free_SocketAddress(addr_flat);
}
-void qmp_nbd_server_add(BlockExportNbd *arg, Error **errp)
+void qmp_nbd_server_add(BlockExportOptionsNbd *arg, Error **errp)
{
BlockDriverState *bs = NULL;
BlockBackend *on_eject_blk;
diff --git a/storage-daemon/qemu-storage-daemon.c b/storage-daemon/qemu-storage-daemon.c
index ed9d2afcf3..ed26097254 100644
--- a/storage-daemon/qemu-storage-daemon.c
+++ b/storage-daemon/qemu-storage-daemon.c
@@ -150,7 +150,7 @@ static void init_qmp_commands(void)
qmp_marshal_qmp_capabilities, QCO_ALLOW_PRECONFIG);
}
-static void init_export(BlockExport *export, Error **errp)
+static void init_export(BlockExportOptions *export, Error **errp)
{
switch (export->type) {
case BLOCK_EXPORT_TYPE_NBD:
@@ -235,14 +235,14 @@ static void process_options(int argc, char *argv[])
case OPTION_EXPORT:
{
Visitor *v;
- BlockExport *export;
+ BlockExportOptions *export;
v = qobject_input_visitor_new_str(optarg, "type", &error_fatal);
- visit_type_BlockExport(v, NULL, &export, &error_fatal);
+ visit_type_BlockExportOptions(v, NULL, &export, &error_fatal);
visit_free(v);
init_export(export, &error_fatal);
- qapi_free_BlockExport(export);
+ qapi_free_BlockExportOptions(export);
break;
}
case OPTION_MONITOR:
--
2.25.4
^ permalink raw reply related [flat|nested] 44+ messages in thread
* [PATCH v2 04/31] block/export: Add BlockExport infrastructure and block-export-add
2020-09-24 15:26 [PATCH v2 00/31] block/export: Add infrastructure and QAPI for block exports Kevin Wolf
` (2 preceding siblings ...)
2020-09-24 15:26 ` [PATCH v2 03/31] qapi: Rename BlockExport to BlockExportOptions Kevin Wolf
@ 2020-09-24 15:26 ` Kevin Wolf
2020-09-25 13:18 ` Eric Blake
2020-09-24 15:26 ` [PATCH v2 05/31] qemu-storage-daemon: Use qmp_block_export_add() Kevin Wolf
` (29 subsequent siblings)
33 siblings, 1 reply; 44+ messages in thread
From: Kevin Wolf @ 2020-09-24 15:26 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, qemu-devel, stefanha, mreitz
We want to have a common set of commands for all types of block exports.
Currently, this is only NBD, but we're going to add more types.
This patch adds the basic BlockExport and BlockExportDriver structs and
a QMP command block-export-add that creates a new export based on the
given BlockExportOptions.
qmp_nbd_server_add() becomes a wrapper around qmp_block_export_add().
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
---
qapi/block-export.json | 9 ++++++++
include/block/export.h | 33 +++++++++++++++++++++++++++
include/block/nbd.h | 5 ++++-
block/export/export.c | 48 ++++++++++++++++++++++++++++++++++++++++
blockdev-nbd.c | 28 +++++++++++++++++------
nbd/server.c | 15 ++++++++++++-
block/export/meson.build | 1 +
block/meson.build | 2 ++
meson.build | 2 +-
9 files changed, 133 insertions(+), 10 deletions(-)
create mode 100644 include/block/export.h
create mode 100644 block/export/export.c
create mode 100644 block/export/meson.build
diff --git a/qapi/block-export.json b/qapi/block-export.json
index d540bcb25a..71c5730bc0 100644
--- a/qapi/block-export.json
+++ b/qapi/block-export.json
@@ -173,3 +173,12 @@
'nbd': 'BlockExportOptionsNbd'
} }
+##
+# @block-export-add:
+#
+# Creates a new block export.
+#
+# Since: 5.2
+##
+{ 'command': 'block-export-add',
+ 'data': 'BlockExportOptions', 'boxed': true }
diff --git a/include/block/export.h b/include/block/export.h
new file mode 100644
index 0000000000..42e3c055fc
--- /dev/null
+++ b/include/block/export.h
@@ -0,0 +1,33 @@
+/*
+ * Declarations for block exports
+ *
+ * Copyright (c) 2012, 2020 Red Hat, Inc.
+ *
+ * Authors:
+ * Paolo Bonzini <pbonzini@redhat.com>
+ * Kevin Wolf <kwolf@redhat.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.
+ */
+
+#ifndef BLOCK_EXPORT_H
+#define BLOCK_EXPORT_H
+
+#include "qapi/qapi-types-block-export.h"
+
+typedef struct BlockExport BlockExport;
+
+typedef struct BlockExportDriver {
+ /* The export type that this driver services */
+ BlockExportType type;
+
+ /* Creates and starts a new block export */
+ BlockExport *(*create)(BlockExportOptions *, Error **);
+} BlockExportDriver;
+
+struct BlockExport {
+ const BlockExportDriver *drv;
+};
+
+#endif
diff --git a/include/block/nbd.h b/include/block/nbd.h
index 262f6da2ce..7698453fb2 100644
--- a/include/block/nbd.h
+++ b/include/block/nbd.h
@@ -20,11 +20,13 @@
#ifndef NBD_H
#define NBD_H
-#include "qapi/qapi-types-block-export.h"
+#include "block/export.h"
#include "io/channel-socket.h"
#include "crypto/tlscreds.h"
#include "qapi/error.h"
+extern const BlockExportDriver blk_exp_nbd;
+
/* Handshake phase structs - this struct is passed on the wire */
struct NBDOption {
@@ -328,6 +330,7 @@ int nbd_errno_to_system_errno(int err);
typedef struct NBDExport NBDExport;
typedef struct NBDClient NBDClient;
+BlockExport *nbd_export_create(BlockExportOptions *exp_args, Error **errp);
NBDExport *nbd_export_new(BlockDriverState *bs, uint64_t dev_offset,
uint64_t size, const char *name, const char *desc,
const char *bitmap, bool readonly, bool shared,
diff --git a/block/export/export.c b/block/export/export.c
new file mode 100644
index 0000000000..fd65541963
--- /dev/null
+++ b/block/export/export.c
@@ -0,0 +1,48 @@
+/*
+ * Common block export infrastructure
+ *
+ * Copyright (c) 2012, 2020 Red Hat, Inc.
+ *
+ * Authors:
+ * Paolo Bonzini <pbonzini@redhat.com>
+ * Kevin Wolf <kwolf@redhat.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.
+ */
+
+#include "qemu/osdep.h"
+
+#include "block/export.h"
+#include "block/nbd.h"
+#include "qapi/error.h"
+#include "qapi/qapi-commands-block-export.h"
+
+static const BlockExportDriver *blk_exp_drivers[] = {
+ &blk_exp_nbd,
+};
+
+static const BlockExportDriver *blk_exp_find_driver(BlockExportType type)
+{
+ int i;
+
+ for (i = 0; i < ARRAY_SIZE(blk_exp_drivers); i++) {
+ if (blk_exp_drivers[i]->type == type) {
+ return blk_exp_drivers[i];
+ }
+ }
+ return NULL;
+}
+
+void qmp_block_export_add(BlockExportOptions *export, Error **errp)
+{
+ const BlockExportDriver *drv;
+
+ drv = blk_exp_find_driver(export->type);
+ if (!drv) {
+ error_setg(errp, "No driver found for the requested export type");
+ return;
+ }
+
+ drv->create(export, errp);
+}
diff --git a/blockdev-nbd.c b/blockdev-nbd.c
index 98ee1b6170..47b04f166a 100644
--- a/blockdev-nbd.c
+++ b/blockdev-nbd.c
@@ -148,17 +148,20 @@ void qmp_nbd_server_start(SocketAddressLegacy *addr,
qapi_free_SocketAddress(addr_flat);
}
-void qmp_nbd_server_add(BlockExportOptionsNbd *arg, Error **errp)
+BlockExport *nbd_export_create(BlockExportOptions *exp_args, Error **errp)
{
+ BlockExportOptionsNbd *arg = &exp_args->u.nbd;
BlockDriverState *bs = NULL;
BlockBackend *on_eject_blk;
- NBDExport *exp;
+ NBDExport *exp = NULL;
int64_t len;
AioContext *aio_context;
+ assert(exp_args->type == BLOCK_EXPORT_TYPE_NBD);
+
if (!nbd_server) {
error_setg(errp, "NBD server not running");
- return;
+ return NULL;
}
if (!arg->has_name) {
@@ -167,24 +170,24 @@ void qmp_nbd_server_add(BlockExportOptionsNbd *arg, Error **errp)
if (strlen(arg->name) > NBD_MAX_STRING_SIZE) {
error_setg(errp, "export name '%s' too long", arg->name);
- return;
+ return NULL;
}
if (arg->description && strlen(arg->description) > NBD_MAX_STRING_SIZE) {
error_setg(errp, "description '%s' too long", arg->description);
- return;
+ return NULL;
}
if (nbd_export_find(arg->name)) {
error_setg(errp, "NBD server already has export named '%s'", arg->name);
- return;
+ return NULL;
}
on_eject_blk = blk_by_name(arg->device);
bs = bdrv_lookup_bs(arg->device, arg->device, errp);
if (!bs) {
- return;
+ return NULL;
}
aio_context = bdrv_get_aio_context(bs);
@@ -217,6 +220,17 @@ void qmp_nbd_server_add(BlockExportOptionsNbd *arg, Error **errp)
out:
aio_context_release(aio_context);
+ /* TODO Remove the cast: nbd_export_new() will return a BlockExport. */
+ return (BlockExport*) exp;
+}
+
+void qmp_nbd_server_add(BlockExportOptionsNbd *arg, Error **errp)
+{
+ BlockExportOptions export = {
+ .type = BLOCK_EXPORT_TYPE_NBD,
+ .u.nbd = *arg,
+ };
+ qmp_block_export_add(&export, errp);
}
void qmp_nbd_server_remove(const char *name,
diff --git a/nbd/server.c b/nbd/server.c
index bd53f7baea..f5af93c253 100644
--- a/nbd/server.c
+++ b/nbd/server.c
@@ -18,6 +18,8 @@
*/
#include "qemu/osdep.h"
+
+#include "block/export.h"
#include "qapi/error.h"
#include "qemu/queue.h"
#include "trace.h"
@@ -80,6 +82,7 @@ struct NBDRequestData {
};
struct NBDExport {
+ BlockExport common;
int refcount;
void (*close)(NBDExport *exp);
@@ -1512,10 +1515,15 @@ NBDExport *nbd_export_new(BlockDriverState *bs, uint64_t dev_offset,
{
AioContext *ctx;
BlockBackend *blk;
- NBDExport *exp = g_new0(NBDExport, 1);
+ NBDExport *exp;
uint64_t perm;
int ret;
+ exp = g_new0(NBDExport, 1);
+ exp->common = (BlockExport) {
+ .drv = &blk_exp_nbd,
+ };
+
/*
* NBD exports are used for non-shared storage migration. Make sure
* that BDRV_O_INACTIVE is cleared and the image is ready for write
@@ -1731,6 +1739,11 @@ void nbd_export_put(NBDExport *exp)
}
}
+const BlockExportDriver blk_exp_nbd = {
+ .type = BLOCK_EXPORT_TYPE_NBD,
+ .create = nbd_export_create,
+};
+
void nbd_export_close_all(void)
{
NBDExport *exp, *next;
diff --git a/block/export/meson.build b/block/export/meson.build
new file mode 100644
index 0000000000..558ef35d38
--- /dev/null
+++ b/block/export/meson.build
@@ -0,0 +1 @@
+block_ss.add(files('export.c'))
diff --git a/block/meson.build b/block/meson.build
index a3e56b7cd1..0b38dc36f7 100644
--- a/block/meson.build
+++ b/block/meson.build
@@ -110,6 +110,8 @@ block_ss.add(module_block_h)
block_ss.add(files('stream.c'))
softmmu_ss.add(files('qapi-sysemu.c'))
+
+subdir('export')
subdir('monitor')
modules += {'block': block_modules}
diff --git a/meson.build b/meson.build
index f4d1ab1096..4c6c7310fa 100644
--- a/meson.build
+++ b/meson.build
@@ -830,6 +830,7 @@ subdir('dump')
block_ss.add(files(
'block.c',
+ 'blockdev-nbd.c',
'blockjob.c',
'job.c',
'qemu-io-cmds.c',
@@ -842,7 +843,6 @@ subdir('block')
blockdev_ss.add(files(
'blockdev.c',
- 'blockdev-nbd.c',
'iothread.c',
'job-qmp.c',
))
--
2.25.4
^ permalink raw reply related [flat|nested] 44+ messages in thread
* [PATCH v2 05/31] qemu-storage-daemon: Use qmp_block_export_add()
2020-09-24 15:26 [PATCH v2 00/31] block/export: Add infrastructure and QAPI for block exports Kevin Wolf
` (3 preceding siblings ...)
2020-09-24 15:26 ` [PATCH v2 04/31] block/export: Add BlockExport infrastructure and block-export-add Kevin Wolf
@ 2020-09-24 15:26 ` Kevin Wolf
2020-09-24 15:26 ` [PATCH v2 06/31] qemu-nbd: Use raw block driver for --offset Kevin Wolf
` (28 subsequent siblings)
33 siblings, 0 replies; 44+ messages in thread
From: Kevin Wolf @ 2020-09-24 15:26 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, qemu-devel, stefanha, mreitz
No reason to duplicate the functionality locally, we can now just reuse
the QMP command block-export-add for --export.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
storage-daemon/qemu-storage-daemon.c | 13 +------------
1 file changed, 1 insertion(+), 12 deletions(-)
diff --git a/storage-daemon/qemu-storage-daemon.c b/storage-daemon/qemu-storage-daemon.c
index ed26097254..b6f678d3ab 100644
--- a/storage-daemon/qemu-storage-daemon.c
+++ b/storage-daemon/qemu-storage-daemon.c
@@ -150,17 +150,6 @@ static void init_qmp_commands(void)
qmp_marshal_qmp_capabilities, QCO_ALLOW_PRECONFIG);
}
-static void init_export(BlockExportOptions *export, Error **errp)
-{
- switch (export->type) {
- case BLOCK_EXPORT_TYPE_NBD:
- qmp_nbd_server_add(&export->u.nbd, errp);
- break;
- default:
- g_assert_not_reached();
- }
-}
-
static void process_options(int argc, char *argv[])
{
int c;
@@ -241,7 +230,7 @@ static void process_options(int argc, char *argv[])
visit_type_BlockExportOptions(v, NULL, &export, &error_fatal);
visit_free(v);
- init_export(export, &error_fatal);
+ qmp_block_export_add(export, &error_fatal);
qapi_free_BlockExportOptions(export);
break;
}
--
2.25.4
^ permalink raw reply related [flat|nested] 44+ messages in thread
* [PATCH v2 06/31] qemu-nbd: Use raw block driver for --offset
2020-09-24 15:26 [PATCH v2 00/31] block/export: Add infrastructure and QAPI for block exports Kevin Wolf
` (4 preceding siblings ...)
2020-09-24 15:26 ` [PATCH v2 05/31] qemu-storage-daemon: Use qmp_block_export_add() Kevin Wolf
@ 2020-09-24 15:26 ` Kevin Wolf
2020-09-24 15:26 ` [PATCH v2 07/31] block/export: Remove magic from block-export-add Kevin Wolf
` (27 subsequent siblings)
33 siblings, 0 replies; 44+ messages in thread
From: Kevin Wolf @ 2020-09-24 15:26 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, qemu-devel, stefanha, mreitz
Instead of implementing qemu-nbd --offset in the NBD code, just put a
raw block node with the requested offset on top of the user image and
rely on that doing the job.
This does not only simplify the nbd_export_new() interface and bring it
closer to the set of options that the nbd-server-add QMP command offers,
but in fact it also eliminates a potential source for bugs in the NBD
code which previously had to add the offset manually in all relevant
places.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
include/block/nbd.h | 4 ++--
blockdev-nbd.c | 9 +--------
nbd/server.c | 33 ++++++++++++++++-----------------
qemu-nbd.c | 27 ++++++++++++---------------
4 files changed, 31 insertions(+), 42 deletions(-)
diff --git a/include/block/nbd.h b/include/block/nbd.h
index 7698453fb2..451f399b0a 100644
--- a/include/block/nbd.h
+++ b/include/block/nbd.h
@@ -331,8 +331,8 @@ typedef struct NBDExport NBDExport;
typedef struct NBDClient NBDClient;
BlockExport *nbd_export_create(BlockExportOptions *exp_args, Error **errp);
-NBDExport *nbd_export_new(BlockDriverState *bs, uint64_t dev_offset,
- uint64_t size, const char *name, const char *desc,
+NBDExport *nbd_export_new(BlockDriverState *bs,
+ const char *name, const char *desc,
const char *bitmap, bool readonly, bool shared,
void (*close)(NBDExport *), bool writethrough,
BlockBackend *on_eject_blk, Error **errp);
diff --git a/blockdev-nbd.c b/blockdev-nbd.c
index 47b04f166a..96cb0100e9 100644
--- a/blockdev-nbd.c
+++ b/blockdev-nbd.c
@@ -154,7 +154,6 @@ BlockExport *nbd_export_create(BlockExportOptions *exp_args, Error **errp)
BlockDriverState *bs = NULL;
BlockBackend *on_eject_blk;
NBDExport *exp = NULL;
- int64_t len;
AioContext *aio_context;
assert(exp_args->type == BLOCK_EXPORT_TYPE_NBD);
@@ -192,12 +191,6 @@ BlockExport *nbd_export_create(BlockExportOptions *exp_args, Error **errp)
aio_context = bdrv_get_aio_context(bs);
aio_context_acquire(aio_context);
- len = bdrv_getlength(bs);
- if (len < 0) {
- error_setg_errno(errp, -len,
- "Failed to determine the NBD export's length");
- goto out;
- }
if (!arg->has_writable) {
arg->writable = false;
@@ -206,7 +199,7 @@ BlockExport *nbd_export_create(BlockExportOptions *exp_args, Error **errp)
arg->writable = false;
}
- exp = nbd_export_new(bs, 0, len, arg->name, arg->description, arg->bitmap,
+ exp = nbd_export_new(bs, arg->name, arg->description, arg->bitmap,
!arg->writable, !arg->writable,
NULL, false, on_eject_blk, errp);
if (!exp) {
diff --git a/nbd/server.c b/nbd/server.c
index f5af93c253..33aaca918c 100644
--- a/nbd/server.c
+++ b/nbd/server.c
@@ -89,7 +89,6 @@ struct NBDExport {
BlockBackend *blk;
char *name;
char *description;
- uint64_t dev_offset;
uint64_t size;
uint16_t nbdflags;
QTAILQ_HEAD(, NBDClient) clients;
@@ -1507,8 +1506,8 @@ static void nbd_eject_notifier(Notifier *n, void *data)
aio_context_release(aio_context);
}
-NBDExport *nbd_export_new(BlockDriverState *bs, uint64_t dev_offset,
- uint64_t size, const char *name, const char *desc,
+NBDExport *nbd_export_new(BlockDriverState *bs,
+ const char *name, const char *desc,
const char *bitmap, bool readonly, bool shared,
void (*close)(NBDExport *), bool writethrough,
BlockBackend *on_eject_blk, Error **errp)
@@ -1516,9 +1515,17 @@ NBDExport *nbd_export_new(BlockDriverState *bs, uint64_t dev_offset,
AioContext *ctx;
BlockBackend *blk;
NBDExport *exp;
+ int64_t size;
uint64_t perm;
int ret;
+ size = bdrv_getlength(bs);
+ if (size < 0) {
+ error_setg_errno(errp, -size,
+ "Failed to determine the NBD export's length");
+ return NULL;
+ }
+
exp = g_new0(NBDExport, 1);
exp->common = (BlockExport) {
.drv = &blk_exp_nbd,
@@ -1553,8 +1560,6 @@ NBDExport *nbd_export_new(BlockDriverState *bs, uint64_t dev_offset,
exp->refcount = 1;
QTAILQ_INIT(&exp->clients);
exp->blk = blk;
- assert(dev_offset <= INT64_MAX);
- exp->dev_offset = dev_offset;
exp->name = g_strdup(name);
assert(!desc || strlen(desc) <= NBD_MAX_STRING_SIZE);
exp->description = g_strdup(desc);
@@ -1569,7 +1574,6 @@ NBDExport *nbd_export_new(BlockDriverState *bs, uint64_t dev_offset,
exp->nbdflags |= (NBD_FLAG_SEND_TRIM | NBD_FLAG_SEND_WRITE_ZEROES |
NBD_FLAG_SEND_FAST_ZERO);
}
- assert(size <= INT64_MAX - dev_offset);
exp->size = QEMU_ALIGN_DOWN(size, BDRV_SECTOR_SIZE);
if (bitmap) {
@@ -1928,8 +1932,7 @@ static int coroutine_fn nbd_co_send_sparse_read(NBDClient *client,
stl_be_p(&chunk.length, pnum);
ret = nbd_co_send_iov(client, iov, 1, errp);
} else {
- ret = blk_pread(exp->blk, offset + progress + exp->dev_offset,
- data + progress, pnum);
+ ret = blk_pread(exp->blk, offset + progress, data + progress, pnum);
if (ret < 0) {
error_setg_errno(errp, -ret, "reading from file failed");
break;
@@ -2303,8 +2306,7 @@ static coroutine_fn int nbd_do_cmd_read(NBDClient *client, NBDRequest *request,
data, request->len, errp);
}
- ret = blk_pread(exp->blk, request->from + exp->dev_offset, data,
- request->len);
+ ret = blk_pread(exp->blk, request->from, data, request->len);
if (ret < 0) {
return nbd_send_generic_reply(client, request->handle, ret,
"reading from file failed", errp);
@@ -2339,7 +2341,7 @@ static coroutine_fn int nbd_do_cmd_cache(NBDClient *client, NBDRequest *request,
assert(request->type == NBD_CMD_CACHE);
- ret = blk_co_preadv(exp->blk, request->from + exp->dev_offset, request->len,
+ ret = blk_co_preadv(exp->blk, request->from, request->len,
NULL, BDRV_REQ_COPY_ON_READ | BDRV_REQ_PREFETCH);
return nbd_send_generic_reply(client, request->handle, ret,
@@ -2370,8 +2372,7 @@ static coroutine_fn int nbd_handle_request(NBDClient *client,
if (request->flags & NBD_CMD_FLAG_FUA) {
flags |= BDRV_REQ_FUA;
}
- ret = blk_pwrite(exp->blk, request->from + exp->dev_offset,
- data, request->len, flags);
+ ret = blk_pwrite(exp->blk, request->from, data, request->len, flags);
return nbd_send_generic_reply(client, request->handle, ret,
"writing to file failed", errp);
@@ -2392,8 +2393,7 @@ static coroutine_fn int nbd_handle_request(NBDClient *client,
int align = client->check_align ?: 1;
int len = MIN(request->len, QEMU_ALIGN_DOWN(BDRV_REQUEST_MAX_BYTES,
align));
- ret = blk_pwrite_zeroes(exp->blk, request->from + exp->dev_offset,
- len, flags);
+ ret = blk_pwrite_zeroes(exp->blk, request->from, len, flags);
request->len -= len;
request->from += len;
}
@@ -2416,8 +2416,7 @@ static coroutine_fn int nbd_handle_request(NBDClient *client,
int align = client->check_align ?: 1;
int len = MIN(request->len, QEMU_ALIGN_DOWN(BDRV_REQUEST_MAX_BYTES,
align));
- ret = blk_co_pdiscard(exp->blk, request->from + exp->dev_offset,
- len);
+ ret = blk_co_pdiscard(exp->blk, request->from, len);
request->len -= len;
request->from += len;
}
diff --git a/qemu-nbd.c b/qemu-nbd.c
index 33476a1000..1752daa4c4 100644
--- a/qemu-nbd.c
+++ b/qemu-nbd.c
@@ -524,7 +524,6 @@ int main(int argc, char **argv)
const char *port = NULL;
char *sockpath = NULL;
char *device = NULL;
- int64_t fd_size;
QemuOpts *sn_opts = NULL;
const char *sn_id_or_name = NULL;
const char *sopt = "hVb:o:p:rsnc:dvk:e:f:tl:x:T:D:B:L";
@@ -1037,6 +1036,17 @@ int main(int argc, char **argv)
}
bs = blk_bs(blk);
+ if (dev_offset) {
+ QDict *raw_opts = qdict_new();
+ qdict_put_str(raw_opts, "driver", "raw");
+ qdict_put_str(raw_opts, "file", bs->node_name);
+ qdict_put_int(raw_opts, "offset", dev_offset);
+ bs = bdrv_open(NULL, NULL, raw_opts, flags, &error_fatal);
+ blk_remove_bs(blk);
+ blk_insert_bs(blk, bs, &error_fatal);
+ bdrv_unref(bs);
+ }
+
blk_set_enable_write_cache(blk, !writethrough);
if (sn_opts) {
@@ -1054,21 +1064,8 @@ int main(int argc, char **argv)
}
bs->detect_zeroes = detect_zeroes;
- fd_size = blk_getlength(blk);
- if (fd_size < 0) {
- error_report("Failed to determine the image length: %s",
- strerror(-fd_size));
- exit(EXIT_FAILURE);
- }
-
- if (dev_offset >= fd_size) {
- error_report("Offset (%" PRIu64 ") has to be smaller than the image "
- "size (%" PRId64 ")", dev_offset, fd_size);
- exit(EXIT_FAILURE);
- }
- fd_size -= dev_offset;
- export = nbd_export_new(bs, dev_offset, fd_size, export_name,
+ export = nbd_export_new(bs, export_name,
export_description, bitmap, readonly, shared > 1,
nbd_export_closed, writethrough, NULL,
&error_fatal);
--
2.25.4
^ permalink raw reply related [flat|nested] 44+ messages in thread
* [PATCH v2 07/31] block/export: Remove magic from block-export-add
2020-09-24 15:26 [PATCH v2 00/31] block/export: Add infrastructure and QAPI for block exports Kevin Wolf
` (5 preceding siblings ...)
2020-09-24 15:26 ` [PATCH v2 06/31] qemu-nbd: Use raw block driver for --offset Kevin Wolf
@ 2020-09-24 15:26 ` Kevin Wolf
2020-09-24 15:26 ` [PATCH v2 08/31] nbd: Add max-connections to nbd-server-start Kevin Wolf
` (26 subsequent siblings)
33 siblings, 0 replies; 44+ messages in thread
From: Kevin Wolf @ 2020-09-24 15:26 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, qemu-devel, stefanha, mreitz
nbd-server-add tries to be convenient and adds two questionable
features that we don't want to share in block-export-add, even for NBD
exports:
1. When requesting a writable export of a read-only device, the export
is silently downgraded to read-only. This should be an error in the
context of block-export-add.
2. When using a BlockBackend name, unplugging the device from the guest
will automatically stop the NBD server, too. This may sometimes be
what you want, but it could also be very surprising. Let's keep
things explicit with block-export-add. If the user wants to stop the
export, they should tell us so.
Move these things into the nbd-server-add QMP command handler so that
they apply only there.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
---
include/block/export.h | 2 ++
include/block/nbd.h | 3 ++-
block/export/export.c | 13 +++++++++---
blockdev-nbd.c | 47 +++++++++++++++++++++++++++++++++++-------
nbd/server.c | 20 +++++++++++-------
qemu-nbd.c | 3 +--
6 files changed, 67 insertions(+), 21 deletions(-)
diff --git a/include/block/export.h b/include/block/export.h
index 42e3c055fc..e7af2c7687 100644
--- a/include/block/export.h
+++ b/include/block/export.h
@@ -30,4 +30,6 @@ struct BlockExport {
const BlockExportDriver *drv;
};
+BlockExport *blk_exp_add(BlockExportOptions *export, Error **errp);
+
#endif
diff --git a/include/block/nbd.h b/include/block/nbd.h
index 451f399b0a..f55f5b710b 100644
--- a/include/block/nbd.h
+++ b/include/block/nbd.h
@@ -335,7 +335,8 @@ NBDExport *nbd_export_new(BlockDriverState *bs,
const char *name, const char *desc,
const char *bitmap, bool readonly, bool shared,
void (*close)(NBDExport *), bool writethrough,
- BlockBackend *on_eject_blk, Error **errp);
+ Error **errp);
+void nbd_export_set_on_eject_blk(BlockExport *exp, BlockBackend *blk);
void nbd_export_close(NBDExport *exp);
void nbd_export_remove(NBDExport *exp, NbdServerRemoveMode mode, Error **errp);
void nbd_export_get(NBDExport *exp);
diff --git a/block/export/export.c b/block/export/export.c
index fd65541963..05bc5e3744 100644
--- a/block/export/export.c
+++ b/block/export/export.c
@@ -13,6 +13,8 @@
#include "qemu/osdep.h"
+#include "block/block.h"
+#include "sysemu/block-backend.h"
#include "block/export.h"
#include "block/nbd.h"
#include "qapi/error.h"
@@ -34,15 +36,20 @@ static const BlockExportDriver *blk_exp_find_driver(BlockExportType type)
return NULL;
}
-void qmp_block_export_add(BlockExportOptions *export, Error **errp)
+BlockExport *blk_exp_add(BlockExportOptions *export, Error **errp)
{
const BlockExportDriver *drv;
drv = blk_exp_find_driver(export->type);
if (!drv) {
error_setg(errp, "No driver found for the requested export type");
- return;
+ return NULL;
}
- drv->create(export, errp);
+ return drv->create(export, errp);
+}
+
+void qmp_block_export_add(BlockExportOptions *export, Error **errp)
+{
+ blk_exp_add(export, errp);
}
diff --git a/blockdev-nbd.c b/blockdev-nbd.c
index 96cb0100e9..7bcca105f9 100644
--- a/blockdev-nbd.c
+++ b/blockdev-nbd.c
@@ -152,7 +152,6 @@ BlockExport *nbd_export_create(BlockExportOptions *exp_args, Error **errp)
{
BlockExportOptionsNbd *arg = &exp_args->u.nbd;
BlockDriverState *bs = NULL;
- BlockBackend *on_eject_blk;
NBDExport *exp = NULL;
AioContext *aio_context;
@@ -182,8 +181,6 @@ BlockExport *nbd_export_create(BlockExportOptions *exp_args, Error **errp)
return NULL;
}
- on_eject_blk = blk_by_name(arg->device);
-
bs = bdrv_lookup_bs(arg->device, arg->device, errp);
if (!bs) {
return NULL;
@@ -195,13 +192,14 @@ BlockExport *nbd_export_create(BlockExportOptions *exp_args, Error **errp)
if (!arg->has_writable) {
arg->writable = false;
}
- if (bdrv_is_read_only(bs)) {
- arg->writable = false;
+ if (bdrv_is_read_only(bs) && arg->writable) {
+ error_setg(errp, "Cannot export read-only node as writable");
+ goto out;
}
exp = nbd_export_new(bs, arg->name, arg->description, arg->bitmap,
!arg->writable, !arg->writable,
- NULL, false, on_eject_blk, errp);
+ NULL, false, errp);
if (!exp) {
goto out;
}
@@ -219,11 +217,44 @@ BlockExport *nbd_export_create(BlockExportOptions *exp_args, Error **errp)
void qmp_nbd_server_add(BlockExportOptionsNbd *arg, Error **errp)
{
- BlockExportOptions export = {
+ BlockExport *export;
+ BlockDriverState *bs;
+ BlockBackend *on_eject_blk;
+ BlockExportOptions export_opts;
+
+ bs = bdrv_lookup_bs(arg->device, arg->device, errp);
+ if (!bs) {
+ return;
+ }
+
+ export_opts = (BlockExportOptions) {
.type = BLOCK_EXPORT_TYPE_NBD,
.u.nbd = *arg,
};
- qmp_block_export_add(&export, errp);
+
+ /*
+ * nbd-server-add doesn't complain when a read-only device should be
+ * exported as writable, but simply downgrades it. This is an error with
+ * block-export-add.
+ */
+ if (bdrv_is_read_only(bs)) {
+ export_opts.u.nbd.has_writable = true;
+ export_opts.u.nbd.writable = false;
+ }
+
+ export = blk_exp_add(&export_opts, errp);
+ if (!export) {
+ return;
+ }
+
+ /*
+ * nbd-server-add removes the export when the named BlockBackend used for
+ * @device goes away.
+ */
+ on_eject_blk = blk_by_name(arg->device);
+ if (on_eject_blk) {
+ nbd_export_set_on_eject_blk(export, on_eject_blk);
+ }
}
void qmp_nbd_server_remove(const char *name,
diff --git a/nbd/server.c b/nbd/server.c
index 33aaca918c..23d9a53094 100644
--- a/nbd/server.c
+++ b/nbd/server.c
@@ -1506,11 +1506,23 @@ static void nbd_eject_notifier(Notifier *n, void *data)
aio_context_release(aio_context);
}
+void nbd_export_set_on_eject_blk(BlockExport *exp, BlockBackend *blk)
+{
+ NBDExport *nbd_exp = container_of(exp, NBDExport, common);
+ assert(exp->drv == &blk_exp_nbd);
+ assert(nbd_exp->eject_notifier_blk == NULL);
+
+ blk_ref(blk);
+ nbd_exp->eject_notifier_blk = blk;
+ nbd_exp->eject_notifier.notify = nbd_eject_notifier;
+ blk_add_remove_bs_notifier(blk, &nbd_exp->eject_notifier);
+}
+
NBDExport *nbd_export_new(BlockDriverState *bs,
const char *name, const char *desc,
const char *bitmap, bool readonly, bool shared,
void (*close)(NBDExport *), bool writethrough,
- BlockBackend *on_eject_blk, Error **errp)
+ Error **errp)
{
AioContext *ctx;
BlockBackend *blk;
@@ -1617,12 +1629,6 @@ NBDExport *nbd_export_new(BlockDriverState *bs,
exp->ctx = ctx;
blk_add_aio_context_notifier(blk, blk_aio_attached, blk_aio_detach, exp);
- if (on_eject_blk) {
- blk_ref(on_eject_blk);
- exp->eject_notifier_blk = on_eject_blk;
- exp->eject_notifier.notify = nbd_eject_notifier;
- blk_add_remove_bs_notifier(on_eject_blk, &exp->eject_notifier);
- }
QTAILQ_INSERT_TAIL(&exports, exp, next);
nbd_export_get(exp);
return exp;
diff --git a/qemu-nbd.c b/qemu-nbd.c
index 1752daa4c4..2bba6c0db3 100644
--- a/qemu-nbd.c
+++ b/qemu-nbd.c
@@ -1067,8 +1067,7 @@ int main(int argc, char **argv)
export = nbd_export_new(bs, export_name,
export_description, bitmap, readonly, shared > 1,
- nbd_export_closed, writethrough, NULL,
- &error_fatal);
+ nbd_export_closed, writethrough, &error_fatal);
if (device) {
#if HAVE_NBD_DEVICE
--
2.25.4
^ permalink raw reply related [flat|nested] 44+ messages in thread
* [PATCH v2 08/31] nbd: Add max-connections to nbd-server-start
2020-09-24 15:26 [PATCH v2 00/31] block/export: Add infrastructure and QAPI for block exports Kevin Wolf
` (6 preceding siblings ...)
2020-09-24 15:26 ` [PATCH v2 07/31] block/export: Remove magic from block-export-add Kevin Wolf
@ 2020-09-24 15:26 ` Kevin Wolf
2020-09-24 15:26 ` [PATCH v2 09/31] nbd: Add writethrough to block-export-add Kevin Wolf
` (25 subsequent siblings)
33 siblings, 0 replies; 44+ messages in thread
From: Kevin Wolf @ 2020-09-24 15:26 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, qemu-devel, stefanha, mreitz
This is a QMP equivalent of qemu-nbd's --shared option, limiting the
maximum number of clients that can attach at the same time.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
qapi/block-export.json | 10 +++++++--
include/block/nbd.h | 3 ++-
block/monitor/block-hmp-cmds.c | 2 +-
blockdev-nbd.c | 33 ++++++++++++++++++++++------
storage-daemon/qemu-storage-daemon.c | 4 ++--
5 files changed, 39 insertions(+), 13 deletions(-)
diff --git a/qapi/block-export.json b/qapi/block-export.json
index 71c5730bc0..60474bfacd 100644
--- a/qapi/block-export.json
+++ b/qapi/block-export.json
@@ -20,13 +20,16 @@
# is only resolved at time of use, so can be deleted and
# recreated on the fly while the NBD server is active.
# If missing, it will default to denying access (since 4.0).
+# @max-connections: The maximum number of connections to allow at the same
+# time, 0 for unlimited. (since 5.2; default: 0)
#
# Since: 4.2
##
{ 'struct': 'NbdServerOptions',
'data': { 'addr': 'SocketAddress',
'*tls-creds': 'str',
- '*tls-authz': 'str'} }
+ '*tls-authz': 'str',
+ '*max-connections': 'uint32' } }
##
# @nbd-server-start:
@@ -43,6 +46,8 @@
# is only resolved at time of use, so can be deleted and
# recreated on the fly while the NBD server is active.
# If missing, it will default to denying access (since 4.0).
+# @max-connections: The maximum number of connections to allow at the same
+# time, 0 for unlimited. (since 5.2; default: 0)
#
# Returns: error if the server is already running.
#
@@ -54,7 +59,8 @@
{ 'command': 'nbd-server-start',
'data': { 'addr': 'SocketAddressLegacy',
'*tls-creds': 'str',
- '*tls-authz': 'str'} }
+ '*tls-authz': 'str',
+ '*max-connections': 'uint32' } }
##
# @BlockExportOptionsNbd:
diff --git a/include/block/nbd.h b/include/block/nbd.h
index f55f5b710b..acccdb3180 100644
--- a/include/block/nbd.h
+++ b/include/block/nbd.h
@@ -354,7 +354,8 @@ void nbd_client_get(NBDClient *client);
void nbd_client_put(NBDClient *client);
void nbd_server_start(SocketAddress *addr, const char *tls_creds,
- const char *tls_authz, Error **errp);
+ const char *tls_authz, uint32_t max_connections,
+ Error **errp);
void nbd_server_start_options(NbdServerOptions *arg, Error **errp);
/* nbd_read
diff --git a/block/monitor/block-hmp-cmds.c b/block/monitor/block-hmp-cmds.c
index fb632b1189..662b7f7d00 100644
--- a/block/monitor/block-hmp-cmds.c
+++ b/block/monitor/block-hmp-cmds.c
@@ -411,7 +411,7 @@ void hmp_nbd_server_start(Monitor *mon, const QDict *qdict)
goto exit;
}
- nbd_server_start(addr, NULL, NULL, &local_err);
+ nbd_server_start(addr, NULL, NULL, 0, &local_err);
qapi_free_SocketAddress(addr);
if (local_err != NULL) {
goto exit;
diff --git a/blockdev-nbd.c b/blockdev-nbd.c
index 7bcca105f9..41d5542987 100644
--- a/blockdev-nbd.c
+++ b/blockdev-nbd.c
@@ -23,23 +23,41 @@ typedef struct NBDServerData {
QIONetListener *listener;
QCryptoTLSCreds *tlscreds;
char *tlsauthz;
+ uint32_t max_connections;
+ uint32_t connections;
} NBDServerData;
static NBDServerData *nbd_server;
+static void nbd_update_server_watch(NBDServerData *s);
+
static void nbd_blockdev_client_closed(NBDClient *client, bool ignored)
{
nbd_client_put(client);
+ assert(nbd_server->connections > 0);
+ nbd_server->connections--;
+ nbd_update_server_watch(nbd_server);
}
static void nbd_accept(QIONetListener *listener, QIOChannelSocket *cioc,
gpointer opaque)
{
+ nbd_server->connections++;
+ nbd_update_server_watch(nbd_server);
+
qio_channel_set_name(QIO_CHANNEL(cioc), "nbd-server");
nbd_client_new(cioc, nbd_server->tlscreds, nbd_server->tlsauthz,
nbd_blockdev_client_closed);
}
+static void nbd_update_server_watch(NBDServerData *s)
+{
+ if (!s->max_connections || s->connections < s->max_connections) {
+ qio_net_listener_set_client_func(s->listener, nbd_accept, NULL, NULL);
+ } else {
+ qio_net_listener_set_client_func(s->listener, NULL, NULL, NULL);
+ }
+}
static void nbd_server_free(NBDServerData *server)
{
@@ -88,7 +106,8 @@ static QCryptoTLSCreds *nbd_get_tls_creds(const char *id, Error **errp)
void nbd_server_start(SocketAddress *addr, const char *tls_creds,
- const char *tls_authz, Error **errp)
+ const char *tls_authz, uint32_t max_connections,
+ Error **errp)
{
if (nbd_server) {
error_setg(errp, "NBD server already running");
@@ -96,6 +115,7 @@ void nbd_server_start(SocketAddress *addr, const char *tls_creds,
}
nbd_server = g_new0(NBDServerData, 1);
+ nbd_server->max_connections = max_connections;
nbd_server->listener = qio_net_listener_new();
qio_net_listener_set_name(nbd_server->listener,
@@ -120,10 +140,7 @@ void nbd_server_start(SocketAddress *addr, const char *tls_creds,
nbd_server->tlsauthz = g_strdup(tls_authz);
- qio_net_listener_set_client_func(nbd_server->listener,
- nbd_accept,
- NULL,
- NULL);
+ nbd_update_server_watch(nbd_server);
return;
@@ -134,17 +151,19 @@ void nbd_server_start(SocketAddress *addr, const char *tls_creds,
void nbd_server_start_options(NbdServerOptions *arg, Error **errp)
{
- nbd_server_start(arg->addr, arg->tls_creds, arg->tls_authz, errp);
+ nbd_server_start(arg->addr, arg->tls_creds, arg->tls_authz,
+ arg->max_connections, errp);
}
void qmp_nbd_server_start(SocketAddressLegacy *addr,
bool has_tls_creds, const char *tls_creds,
bool has_tls_authz, const char *tls_authz,
+ bool has_max_connections, uint32_t max_connections,
Error **errp)
{
SocketAddress *addr_flat = socket_address_flatten(addr);
- nbd_server_start(addr_flat, tls_creds, tls_authz, errp);
+ nbd_server_start(addr_flat, tls_creds, tls_authz, max_connections, errp);
qapi_free_SocketAddress(addr_flat);
}
diff --git a/storage-daemon/qemu-storage-daemon.c b/storage-daemon/qemu-storage-daemon.c
index b6f678d3ab..0fcab6ed2d 100644
--- a/storage-daemon/qemu-storage-daemon.c
+++ b/storage-daemon/qemu-storage-daemon.c
@@ -101,9 +101,9 @@ static void help(void)
" configure a QMP monitor\n"
"\n"
" --nbd-server addr.type=inet,addr.host=<host>,addr.port=<port>\n"
-" [,tls-creds=<id>][,tls-authz=<id>]\n"
+" [,tls-creds=<id>][,tls-authz=<id>][,max-connections=<n>]\n"
" --nbd-server addr.type=unix,addr.path=<path>\n"
-" [,tls-creds=<id>][,tls-authz=<id>]\n"
+" [,tls-creds=<id>][,tls-authz=<id>][,max-connections=<n>]\n"
" start an NBD server for exporting block nodes\n"
"\n"
" --object help list object types that can be added\n"
--
2.25.4
^ permalink raw reply related [flat|nested] 44+ messages in thread
* [PATCH v2 09/31] nbd: Add writethrough to block-export-add
2020-09-24 15:26 [PATCH v2 00/31] block/export: Add infrastructure and QAPI for block exports Kevin Wolf
` (7 preceding siblings ...)
2020-09-24 15:26 ` [PATCH v2 08/31] nbd: Add max-connections to nbd-server-start Kevin Wolf
@ 2020-09-24 15:26 ` Kevin Wolf
2020-09-24 15:26 ` [PATCH v2 10/31] nbd: Remove NBDExport.close callback Kevin Wolf
` (24 subsequent siblings)
33 siblings, 0 replies; 44+ messages in thread
From: Kevin Wolf @ 2020-09-24 15:26 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, qemu-devel, stefanha, mreitz
qemu-nbd allows use of writethrough cache modes, which mean that write
requests made through NBD will cause a flush before they complete.
Expose the same functionality in block-export-add.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
---
qapi/block-export.json | 7 ++++++-
blockdev-nbd.c | 6 +++++-
2 files changed, 11 insertions(+), 2 deletions(-)
diff --git a/qapi/block-export.json b/qapi/block-export.json
index 60474bfacd..319a38be88 100644
--- a/qapi/block-export.json
+++ b/qapi/block-export.json
@@ -170,10 +170,15 @@
# Describes a block export, i.e. how single node should be exported on an
# external interface.
#
+# @writethrough: If true, caches are flushed after every write request to the
+# export before completion is signalled. (since: 5.2;
+# default: false)
+#
# Since: 4.2
##
{ 'union': 'BlockExportOptions',
- 'base': { 'type': 'BlockExportType' },
+ 'base': { 'type': 'BlockExportType',
+ '*writethrough': 'bool' },
'discriminator': 'type',
'data': {
'nbd': 'BlockExportOptionsNbd'
diff --git a/blockdev-nbd.c b/blockdev-nbd.c
index 41d5542987..09247a8ded 100644
--- a/blockdev-nbd.c
+++ b/blockdev-nbd.c
@@ -216,9 +216,13 @@ BlockExport *nbd_export_create(BlockExportOptions *exp_args, Error **errp)
goto out;
}
+ if (!exp_args->has_writethrough) {
+ exp_args->writethrough = false;
+ }
+
exp = nbd_export_new(bs, arg->name, arg->description, arg->bitmap,
!arg->writable, !arg->writable,
- NULL, false, errp);
+ NULL, exp_args->writethrough, errp);
if (!exp) {
goto out;
}
--
2.25.4
^ permalink raw reply related [flat|nested] 44+ messages in thread
* [PATCH v2 10/31] nbd: Remove NBDExport.close callback
2020-09-24 15:26 [PATCH v2 00/31] block/export: Add infrastructure and QAPI for block exports Kevin Wolf
` (8 preceding siblings ...)
2020-09-24 15:26 ` [PATCH v2 09/31] nbd: Add writethrough to block-export-add Kevin Wolf
@ 2020-09-24 15:26 ` Kevin Wolf
2020-09-24 15:26 ` [PATCH v2 11/31] qemu-nbd: Use blk_exp_add() to create the export Kevin Wolf
` (23 subsequent siblings)
33 siblings, 0 replies; 44+ messages in thread
From: Kevin Wolf @ 2020-09-24 15:26 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, qemu-devel, stefanha, mreitz
The export close callback is unused by the built-in NBD server. qemu-nbd
uses it only during shutdown to wait for the unrefed export to actually
go away. It can just use nbd_export_close_all() instead and do without
the callback.
This removes the close callback from nbd_export_new() and makes both
callers of it more similar.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
include/block/nbd.h | 3 +--
blockdev-nbd.c | 2 +-
nbd/server.c | 9 +--------
qemu-nbd.c | 14 ++++----------
4 files changed, 7 insertions(+), 21 deletions(-)
diff --git a/include/block/nbd.h b/include/block/nbd.h
index acccdb3180..9449b3dac4 100644
--- a/include/block/nbd.h
+++ b/include/block/nbd.h
@@ -334,8 +334,7 @@ BlockExport *nbd_export_create(BlockExportOptions *exp_args, Error **errp);
NBDExport *nbd_export_new(BlockDriverState *bs,
const char *name, const char *desc,
const char *bitmap, bool readonly, bool shared,
- void (*close)(NBDExport *), bool writethrough,
- Error **errp);
+ bool writethrough, Error **errp);
void nbd_export_set_on_eject_blk(BlockExport *exp, BlockBackend *blk);
void nbd_export_close(NBDExport *exp);
void nbd_export_remove(NBDExport *exp, NbdServerRemoveMode mode, Error **errp);
diff --git a/blockdev-nbd.c b/blockdev-nbd.c
index 09247a8ded..3a51b3e792 100644
--- a/blockdev-nbd.c
+++ b/blockdev-nbd.c
@@ -222,7 +222,7 @@ BlockExport *nbd_export_create(BlockExportOptions *exp_args, Error **errp)
exp = nbd_export_new(bs, arg->name, arg->description, arg->bitmap,
!arg->writable, !arg->writable,
- NULL, exp_args->writethrough, errp);
+ exp_args->writethrough, errp);
if (!exp) {
goto out;
}
diff --git a/nbd/server.c b/nbd/server.c
index 23d9a53094..1cc915f01d 100644
--- a/nbd/server.c
+++ b/nbd/server.c
@@ -84,7 +84,6 @@ struct NBDRequestData {
struct NBDExport {
BlockExport common;
int refcount;
- void (*close)(NBDExport *exp);
BlockBackend *blk;
char *name;
@@ -1521,8 +1520,7 @@ void nbd_export_set_on_eject_blk(BlockExport *exp, BlockBackend *blk)
NBDExport *nbd_export_new(BlockDriverState *bs,
const char *name, const char *desc,
const char *bitmap, bool readonly, bool shared,
- void (*close)(NBDExport *), bool writethrough,
- Error **errp)
+ bool writethrough, Error **errp)
{
AioContext *ctx;
BlockBackend *blk;
@@ -1625,7 +1623,6 @@ NBDExport *nbd_export_new(BlockDriverState *bs,
assert(strlen(exp->export_bitmap_context) < NBD_MAX_STRING_SIZE);
}
- exp->close = close;
exp->ctx = ctx;
blk_add_aio_context_notifier(blk, blk_aio_attached, blk_aio_detach, exp);
@@ -1723,10 +1720,6 @@ void nbd_export_put(NBDExport *exp)
assert(exp->name == NULL);
assert(exp->description == NULL);
- if (exp->close) {
- exp->close(exp);
- }
-
if (exp->blk) {
if (exp->eject_notifier_blk) {
notifier_remove(&exp->eject_notifier);
diff --git a/qemu-nbd.c b/qemu-nbd.c
index 2bba6c0db3..cab65529a5 100644
--- a/qemu-nbd.c
+++ b/qemu-nbd.c
@@ -70,7 +70,7 @@ static int verbose;
static char *srcpath;
static SocketAddress *saddr;
static int persistent = 0;
-static enum { RUNNING, TERMINATE, TERMINATING, TERMINATED } state;
+static enum { RUNNING, TERMINATE, TERMINATED } state;
static int shared = 1;
static int nb_fds;
static QIONetListener *server;
@@ -332,12 +332,6 @@ static int nbd_can_accept(void)
return state == RUNNING && nb_fds < shared;
}
-static void nbd_export_closed(NBDExport *export)
-{
- assert(state == TERMINATING);
- state = TERMINATED;
-}
-
static void nbd_update_server_watch(void);
static void nbd_client_closed(NBDClient *client, bool negotiated)
@@ -1067,7 +1061,7 @@ int main(int argc, char **argv)
export = nbd_export_new(bs, export_name,
export_description, bitmap, readonly, shared > 1,
- nbd_export_closed, writethrough, &error_fatal);
+ writethrough, &error_fatal);
if (device) {
#if HAVE_NBD_DEVICE
@@ -1107,10 +1101,10 @@ int main(int argc, char **argv)
do {
main_loop_wait(false);
if (state == TERMINATE) {
- state = TERMINATING;
- nbd_export_close(export);
nbd_export_put(export);
+ nbd_export_close_all();
export = NULL;
+ state = TERMINATED;
}
} while (state != TERMINATED);
--
2.25.4
^ permalink raw reply related [flat|nested] 44+ messages in thread
* [PATCH v2 11/31] qemu-nbd: Use blk_exp_add() to create the export
2020-09-24 15:26 [PATCH v2 00/31] block/export: Add infrastructure and QAPI for block exports Kevin Wolf
` (9 preceding siblings ...)
2020-09-24 15:26 ` [PATCH v2 10/31] nbd: Remove NBDExport.close callback Kevin Wolf
@ 2020-09-24 15:26 ` Kevin Wolf
2020-09-24 15:26 ` [PATCH v2 12/31] nbd/server: Simplify export shutdown Kevin Wolf
` (22 subsequent siblings)
33 siblings, 0 replies; 44+ messages in thread
From: Kevin Wolf @ 2020-09-24 15:26 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, qemu-devel, stefanha, mreitz
With this change, NBD exports are now only created through the
BlockExport interface. This allows us finally to move things from the
NBD layer to the BlockExport layer if they make sense for other export
types, too.
blk_exp_add() returns only a weak reference, so the explicit
nbd_export_put() goes away.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
---
include/block/nbd.h | 1 +
blockdev-nbd.c | 8 +++++++-
qemu-nbd.c | 28 ++++++++++++++++++++++------
3 files changed, 30 insertions(+), 7 deletions(-)
diff --git a/include/block/nbd.h b/include/block/nbd.h
index 9449b3dac4..1dafe70615 100644
--- a/include/block/nbd.h
+++ b/include/block/nbd.h
@@ -352,6 +352,7 @@ void nbd_client_new(QIOChannelSocket *sioc,
void nbd_client_get(NBDClient *client);
void nbd_client_put(NBDClient *client);
+void nbd_server_is_qemu_nbd(bool value);
void nbd_server_start(SocketAddress *addr, const char *tls_creds,
const char *tls_authz, uint32_t max_connections,
Error **errp);
diff --git a/blockdev-nbd.c b/blockdev-nbd.c
index 3a51b3e792..7bb0b09697 100644
--- a/blockdev-nbd.c
+++ b/blockdev-nbd.c
@@ -28,9 +28,15 @@ typedef struct NBDServerData {
} NBDServerData;
static NBDServerData *nbd_server;
+static bool is_qemu_nbd;
static void nbd_update_server_watch(NBDServerData *s);
+void nbd_server_is_qemu_nbd(bool value)
+{
+ is_qemu_nbd = value;
+}
+
static void nbd_blockdev_client_closed(NBDClient *client, bool ignored)
{
nbd_client_put(client);
@@ -176,7 +182,7 @@ BlockExport *nbd_export_create(BlockExportOptions *exp_args, Error **errp)
assert(exp_args->type == BLOCK_EXPORT_TYPE_NBD);
- if (!nbd_server) {
+ if (!nbd_server && !is_qemu_nbd) {
error_setg(errp, "NBD server not running");
return NULL;
}
diff --git a/qemu-nbd.c b/qemu-nbd.c
index cab65529a5..a863741e8f 100644
--- a/qemu-nbd.c
+++ b/qemu-nbd.c
@@ -65,7 +65,6 @@
#define MBR_SIZE 512
-static NBDExport *export;
static int verbose;
static char *srcpath;
static SocketAddress *saddr;
@@ -580,6 +579,7 @@ int main(int argc, char **argv)
int old_stderr = -1;
unsigned socket_activation;
const char *pid_file_name = NULL;
+ BlockExportOptions *export_opts;
#if HAVE_NBD_DEVICE
/* The client thread uses SIGTERM to interrupt the server. A signal
@@ -1059,9 +1059,27 @@ int main(int argc, char **argv)
bs->detect_zeroes = detect_zeroes;
- export = nbd_export_new(bs, export_name,
- export_description, bitmap, readonly, shared > 1,
- writethrough, &error_fatal);
+ nbd_server_is_qemu_nbd(true);
+
+ export_opts = g_new(BlockExportOptions, 1);
+ *export_opts = (BlockExportOptions) {
+ .type = BLOCK_EXPORT_TYPE_NBD,
+ .has_writethrough = true,
+ .writethrough = writethrough,
+ .u.nbd = {
+ .device = g_strdup(bdrv_get_node_name(bs)),
+ .has_name = true,
+ .name = g_strdup(export_name),
+ .has_description = !!export_description,
+ .description = g_strdup(export_description),
+ .has_writable = true,
+ .writable = !readonly,
+ .has_bitmap = !!bitmap,
+ .bitmap = g_strdup(bitmap),
+ },
+ };
+ blk_exp_add(export_opts, &error_fatal);
+ qapi_free_BlockExportOptions(export_opts);
if (device) {
#if HAVE_NBD_DEVICE
@@ -1101,9 +1119,7 @@ int main(int argc, char **argv)
do {
main_loop_wait(false);
if (state == TERMINATE) {
- nbd_export_put(export);
nbd_export_close_all();
- export = NULL;
state = TERMINATED;
}
} while (state != TERMINATED);
--
2.25.4
^ permalink raw reply related [flat|nested] 44+ messages in thread
* [PATCH v2 12/31] nbd/server: Simplify export shutdown
2020-09-24 15:26 [PATCH v2 00/31] block/export: Add infrastructure and QAPI for block exports Kevin Wolf
` (10 preceding siblings ...)
2020-09-24 15:26 ` [PATCH v2 11/31] qemu-nbd: Use blk_exp_add() to create the export Kevin Wolf
@ 2020-09-24 15:26 ` Kevin Wolf
2020-09-24 15:26 ` [PATCH v2 13/31] block/export: Move refcount from NBDExport to BlockExport Kevin Wolf
` (21 subsequent siblings)
33 siblings, 0 replies; 44+ messages in thread
From: Kevin Wolf @ 2020-09-24 15:26 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, qemu-devel, stefanha, mreitz
Closing export is somewhat convoluted because nbd_export_close() and
nbd_export_put() call each other and the ways they actually end up being
nested is not necessarily obvious.
However, it is not really necessary to call nbd_export_close() from
nbd_export_put() when putting the last reference because it only does
three things:
1. Close all clients. We're going to refcount 0 and all clients hold a
reference, so we know there is no active client any more.
2. Close the user reference (represented by exp->name being non-NULL).
The same argument applies: If the export were still named, we would
still have a reference.
3. Freeing exp->description. This is really cleanup work to be done when
the export is finally freed. There is no reason to already clear it
while clients are still in the process of shutting down.
So after moving the cleanup of exp->description, the code can be
simplified so that only nbd_export_close() calls nbd_export_put(), but
never the other way around.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
nbd/server.c | 17 ++++-------------
1 file changed, 4 insertions(+), 13 deletions(-)
diff --git a/nbd/server.c b/nbd/server.c
index 1cc915f01d..fb70374df5 100644
--- a/nbd/server.c
+++ b/nbd/server.c
@@ -1678,8 +1678,6 @@ void nbd_export_close(NBDExport *exp)
QTAILQ_REMOVE(&exports, exp, next);
QTAILQ_INSERT_TAIL(&closed_exports, exp, next);
}
- g_free(exp->description);
- exp->description = NULL;
nbd_export_put(exp);
}
@@ -1706,19 +1704,12 @@ void nbd_export_get(NBDExport *exp)
void nbd_export_put(NBDExport *exp)
{
assert(exp->refcount > 0);
- if (exp->refcount == 1) {
- nbd_export_close(exp);
- }
-
- /* nbd_export_close() may theoretically reduce refcount to 0. It may happen
- * if someone calls nbd_export_put() on named export not through
- * nbd_export_set_name() when refcount is 1. So, let's assert that
- * it is > 0.
- */
- assert(exp->refcount > 0);
if (--exp->refcount == 0) {
assert(exp->name == NULL);
- assert(exp->description == NULL);
+ assert(QTAILQ_EMPTY(&exp->clients));
+
+ g_free(exp->description);
+ exp->description = NULL;
if (exp->blk) {
if (exp->eject_notifier_blk) {
--
2.25.4
^ permalink raw reply related [flat|nested] 44+ messages in thread
* [PATCH v2 13/31] block/export: Move refcount from NBDExport to BlockExport
2020-09-24 15:26 [PATCH v2 00/31] block/export: Add infrastructure and QAPI for block exports Kevin Wolf
` (11 preceding siblings ...)
2020-09-24 15:26 ` [PATCH v2 12/31] nbd/server: Simplify export shutdown Kevin Wolf
@ 2020-09-24 15:26 ` Kevin Wolf
2020-09-24 15:27 ` [PATCH v2 14/31] block/export: Move AioContext " Kevin Wolf
` (20 subsequent siblings)
33 siblings, 0 replies; 44+ messages in thread
From: Kevin Wolf @ 2020-09-24 15:26 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, qemu-devel, stefanha, mreitz
Having a refcount makes sense for all types of block exports. It is also
a prerequisite for keeping a list of all exports at the BlockExport
level.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
---
include/block/export.h | 15 +++++++++
include/block/nbd.h | 2 --
block/export/export.c | 14 ++++++++
blockdev-nbd.c | 2 +-
nbd/server.c | 72 +++++++++++++++++++-----------------------
5 files changed, 63 insertions(+), 42 deletions(-)
diff --git a/include/block/export.h b/include/block/export.h
index e7af2c7687..5236a35e12 100644
--- a/include/block/export.h
+++ b/include/block/export.h
@@ -24,12 +24,27 @@ typedef struct BlockExportDriver {
/* Creates and starts a new block export */
BlockExport *(*create)(BlockExportOptions *, Error **);
+
+ /*
+ * Frees a removed block export. This function is only called after all
+ * references have been dropped.
+ */
+ void (*delete)(BlockExport *);
} BlockExportDriver;
struct BlockExport {
const BlockExportDriver *drv;
+
+ /*
+ * Reference count for this block export. This includes strong references
+ * both from the owner (qemu-nbd or the monitor) and clients connected to
+ * the export.
+ */
+ int refcount;
};
BlockExport *blk_exp_add(BlockExportOptions *export, Error **errp);
+void blk_exp_ref(BlockExport *exp);
+void blk_exp_unref(BlockExport *exp);
#endif
diff --git a/include/block/nbd.h b/include/block/nbd.h
index 1dafe70615..e3bd112227 100644
--- a/include/block/nbd.h
+++ b/include/block/nbd.h
@@ -338,8 +338,6 @@ NBDExport *nbd_export_new(BlockDriverState *bs,
void nbd_export_set_on_eject_blk(BlockExport *exp, BlockBackend *blk);
void nbd_export_close(NBDExport *exp);
void nbd_export_remove(NBDExport *exp, NbdServerRemoveMode mode, Error **errp);
-void nbd_export_get(NBDExport *exp);
-void nbd_export_put(NBDExport *exp);
AioContext *nbd_export_aio_context(NBDExport *exp);
NBDExport *nbd_export_find(const char *name);
diff --git a/block/export/export.c b/block/export/export.c
index 05bc5e3744..baba4e94ff 100644
--- a/block/export/export.c
+++ b/block/export/export.c
@@ -49,6 +49,20 @@ BlockExport *blk_exp_add(BlockExportOptions *export, Error **errp)
return drv->create(export, errp);
}
+void blk_exp_ref(BlockExport *exp)
+{
+ assert(exp->refcount > 0);
+ exp->refcount++;
+}
+
+void blk_exp_unref(BlockExport *exp)
+{
+ assert(exp->refcount > 0);
+ if (--exp->refcount == 0) {
+ exp->drv->delete(exp);
+ }
+}
+
void qmp_block_export_add(BlockExportOptions *export, Error **errp)
{
blk_exp_add(export, errp);
diff --git a/blockdev-nbd.c b/blockdev-nbd.c
index 7bb0b09697..e208324d42 100644
--- a/blockdev-nbd.c
+++ b/blockdev-nbd.c
@@ -236,7 +236,7 @@ BlockExport *nbd_export_create(BlockExportOptions *exp_args, Error **errp)
/* The list of named exports has a strong reference to this export now and
* our only way of accessing it is through nbd_export_find(), so we can drop
* the strong reference that is @exp. */
- nbd_export_put(exp);
+ blk_exp_unref((BlockExport*) exp);
out:
aio_context_release(aio_context);
diff --git a/nbd/server.c b/nbd/server.c
index fb70374df5..7be81c7bda 100644
--- a/nbd/server.c
+++ b/nbd/server.c
@@ -83,7 +83,6 @@ struct NBDRequestData {
struct NBDExport {
BlockExport common;
- int refcount;
BlockBackend *blk;
char *name;
@@ -499,7 +498,7 @@ static int nbd_negotiate_handle_export_name(NBDClient *client, bool no_zeroes,
}
QTAILQ_INSERT_TAIL(&client->exp->clients, client, next);
- nbd_export_get(client->exp);
+ blk_exp_ref(&client->exp->common);
nbd_check_meta_export(client);
return 0;
@@ -707,7 +706,7 @@ static int nbd_negotiate_handle_info(NBDClient *client, Error **errp)
client->exp = exp;
client->check_align = check_align;
QTAILQ_INSERT_TAIL(&client->exp->clients, client, next);
- nbd_export_get(client->exp);
+ blk_exp_ref(&client->exp->common);
nbd_check_meta_export(client);
rc = 1;
}
@@ -1406,7 +1405,7 @@ void nbd_client_put(NBDClient *client)
g_free(client->tlsauthz);
if (client->exp) {
QTAILQ_REMOVE(&client->exp->clients, client, next);
- nbd_export_put(client->exp);
+ blk_exp_unref(&client->exp->common);
}
g_free(client);
}
@@ -1538,7 +1537,8 @@ NBDExport *nbd_export_new(BlockDriverState *bs,
exp = g_new0(NBDExport, 1);
exp->common = (BlockExport) {
- .drv = &blk_exp_nbd,
+ .drv = &blk_exp_nbd,
+ .refcount = 1,
};
/*
@@ -1567,7 +1567,6 @@ NBDExport *nbd_export_new(BlockDriverState *bs,
blk_set_enable_write_cache(blk, !writethrough);
blk_set_allow_aio_context_change(blk, true);
- exp->refcount = 1;
QTAILQ_INIT(&exp->clients);
exp->blk = blk;
exp->name = g_strdup(name);
@@ -1626,8 +1625,9 @@ NBDExport *nbd_export_new(BlockDriverState *bs,
exp->ctx = ctx;
blk_add_aio_context_notifier(blk, blk_aio_attached, blk_aio_detach, exp);
+ blk_exp_ref(&exp->common);
QTAILQ_INSERT_TAIL(&exports, exp, next);
- nbd_export_get(exp);
+
return exp;
fail:
@@ -1660,7 +1660,7 @@ void nbd_export_close(NBDExport *exp)
{
NBDClient *client, *next;
- nbd_export_get(exp);
+ blk_exp_ref(&exp->common);
/*
* TODO: Should we expand QMP NbdServerRemoveNode enum to allow a
* close mode that stops advertising the export to new clients but
@@ -1672,13 +1672,13 @@ void nbd_export_close(NBDExport *exp)
client_close(client, true);
}
if (exp->name) {
- nbd_export_put(exp);
+ blk_exp_unref(&exp->common);
g_free(exp->name);
exp->name = NULL;
QTAILQ_REMOVE(&exports, exp, next);
QTAILQ_INSERT_TAIL(&closed_exports, exp, next);
}
- nbd_export_put(exp);
+ blk_exp_unref(&exp->common);
}
void nbd_export_remove(NBDExport *exp, NbdServerRemoveMode mode, Error **errp)
@@ -1695,47 +1695,41 @@ void nbd_export_remove(NBDExport *exp, NbdServerRemoveMode mode, Error **errp)
error_append_hint(errp, "Use mode='hard' to force client disconnect\n");
}
-void nbd_export_get(NBDExport *exp)
-{
- assert(exp->refcount > 0);
- exp->refcount++;
-}
-
-void nbd_export_put(NBDExport *exp)
+static void nbd_export_delete(BlockExport *blk_exp)
{
- assert(exp->refcount > 0);
- if (--exp->refcount == 0) {
- assert(exp->name == NULL);
- assert(QTAILQ_EMPTY(&exp->clients));
+ NBDExport *exp = container_of(blk_exp, NBDExport, common);
- g_free(exp->description);
- exp->description = NULL;
+ assert(exp->name == NULL);
+ assert(QTAILQ_EMPTY(&exp->clients));
- if (exp->blk) {
- if (exp->eject_notifier_blk) {
- notifier_remove(&exp->eject_notifier);
- blk_unref(exp->eject_notifier_blk);
- }
- blk_remove_aio_context_notifier(exp->blk, blk_aio_attached,
- blk_aio_detach, exp);
- blk_unref(exp->blk);
- exp->blk = NULL;
- }
+ g_free(exp->description);
+ exp->description = NULL;
- if (exp->export_bitmap) {
- bdrv_dirty_bitmap_set_busy(exp->export_bitmap, false);
- g_free(exp->export_bitmap_context);
+ if (exp->blk) {
+ if (exp->eject_notifier_blk) {
+ notifier_remove(&exp->eject_notifier);
+ blk_unref(exp->eject_notifier_blk);
}
+ blk_remove_aio_context_notifier(exp->blk, blk_aio_attached,
+ blk_aio_detach, exp);
+ blk_unref(exp->blk);
+ exp->blk = NULL;
+ }
- QTAILQ_REMOVE(&closed_exports, exp, next);
- g_free(exp);
- aio_wait_kick();
+ if (exp->export_bitmap) {
+ bdrv_dirty_bitmap_set_busy(exp->export_bitmap, false);
+ g_free(exp->export_bitmap_context);
}
+
+ QTAILQ_REMOVE(&closed_exports, exp, next);
+ g_free(exp);
+ aio_wait_kick();
}
const BlockExportDriver blk_exp_nbd = {
.type = BLOCK_EXPORT_TYPE_NBD,
.create = nbd_export_create,
+ .delete = nbd_export_delete,
};
void nbd_export_close_all(void)
--
2.25.4
^ permalink raw reply related [flat|nested] 44+ messages in thread
* [PATCH v2 14/31] block/export: Move AioContext from NBDExport to BlockExport
2020-09-24 15:26 [PATCH v2 00/31] block/export: Add infrastructure and QAPI for block exports Kevin Wolf
` (12 preceding siblings ...)
2020-09-24 15:26 ` [PATCH v2 13/31] block/export: Move refcount from NBDExport to BlockExport Kevin Wolf
@ 2020-09-24 15:27 ` Kevin Wolf
2020-09-24 15:27 ` [PATCH v2 15/31] block/export: Add node-name to BlockExportOptions Kevin Wolf
` (19 subsequent siblings)
33 siblings, 0 replies; 44+ messages in thread
From: Kevin Wolf @ 2020-09-24 15:27 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, qemu-devel, stefanha, mreitz
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
---
include/block/export.h | 3 +++
block/export/export.c | 2 ++
nbd/server.c | 26 +++++++++++++-------------
3 files changed, 18 insertions(+), 13 deletions(-)
diff --git a/include/block/export.h b/include/block/export.h
index 5236a35e12..e6f96f4e1e 100644
--- a/include/block/export.h
+++ b/include/block/export.h
@@ -41,6 +41,9 @@ struct BlockExport {
* the export.
*/
int refcount;
+
+ /* The AioContext whose lock protects this BlockExport object. */
+ AioContext *ctx;
};
BlockExport *blk_exp_add(BlockExportOptions *export, Error **errp);
diff --git a/block/export/export.c b/block/export/export.c
index baba4e94ff..8635318ef1 100644
--- a/block/export/export.c
+++ b/block/export/export.c
@@ -49,12 +49,14 @@ BlockExport *blk_exp_add(BlockExportOptions *export, Error **errp)
return drv->create(export, errp);
}
+/* Callers must hold exp->ctx lock */
void blk_exp_ref(BlockExport *exp)
{
assert(exp->refcount > 0);
exp->refcount++;
}
+/* Callers must hold exp->ctx lock */
void blk_exp_unref(BlockExport *exp)
{
assert(exp->refcount > 0);
diff --git a/nbd/server.c b/nbd/server.c
index 7be81c7bda..7cf81646fc 100644
--- a/nbd/server.c
+++ b/nbd/server.c
@@ -92,8 +92,6 @@ struct NBDExport {
QTAILQ_HEAD(, NBDClient) clients;
QTAILQ_ENTRY(NBDExport) next;
- AioContext *ctx;
-
BlockBackend *eject_notifier_blk;
Notifier eject_notifier;
@@ -1333,8 +1331,8 @@ static coroutine_fn int nbd_negotiate(NBDClient *client, Error **errp)
}
/* Attach the channel to the same AioContext as the export */
- if (client->exp && client->exp->ctx) {
- qio_channel_attach_aio_context(client->ioc, client->exp->ctx);
+ if (client->exp && client->exp->common.ctx) {
+ qio_channel_attach_aio_context(client->ioc, client->exp->common.ctx);
}
assert(!client->optlen);
@@ -1466,7 +1464,7 @@ static void blk_aio_attached(AioContext *ctx, void *opaque)
trace_nbd_blk_aio_attached(exp->name, ctx);
- exp->ctx = ctx;
+ exp->common.ctx = ctx;
QTAILQ_FOREACH(client, &exp->clients, next) {
qio_channel_attach_aio_context(client->ioc, ctx);
@@ -1484,13 +1482,13 @@ static void blk_aio_detach(void *opaque)
NBDExport *exp = opaque;
NBDClient *client;
- trace_nbd_blk_aio_detach(exp->name, exp->ctx);
+ trace_nbd_blk_aio_detach(exp->name, exp->common.ctx);
QTAILQ_FOREACH(client, &exp->clients, next) {
qio_channel_detach_aio_context(client->ioc);
}
- exp->ctx = NULL;
+ exp->common.ctx = NULL;
}
static void nbd_eject_notifier(Notifier *n, void *data)
@@ -1498,7 +1496,7 @@ static void nbd_eject_notifier(Notifier *n, void *data)
NBDExport *exp = container_of(n, NBDExport, eject_notifier);
AioContext *aio_context;
- aio_context = exp->ctx;
+ aio_context = exp->common.ctx;
aio_context_acquire(aio_context);
nbd_export_close(exp);
aio_context_release(aio_context);
@@ -1535,10 +1533,13 @@ NBDExport *nbd_export_new(BlockDriverState *bs,
return NULL;
}
+ ctx = bdrv_get_aio_context(bs);
+
exp = g_new0(NBDExport, 1);
exp->common = (BlockExport) {
.drv = &blk_exp_nbd,
.refcount = 1,
+ .ctx = ctx,
};
/*
@@ -1548,7 +1549,7 @@ NBDExport *nbd_export_new(BlockDriverState *bs,
* ctx was acquired in the caller.
*/
assert(name && strlen(name) <= NBD_MAX_STRING_SIZE);
- ctx = bdrv_get_aio_context(bs);
+
bdrv_invalidate_cache(bs, NULL);
/* Don't allow resize while the NBD server is running, otherwise we don't
@@ -1622,7 +1623,6 @@ NBDExport *nbd_export_new(BlockDriverState *bs,
assert(strlen(exp->export_bitmap_context) < NBD_MAX_STRING_SIZE);
}
- exp->ctx = ctx;
blk_add_aio_context_notifier(blk, blk_aio_attached, blk_aio_detach, exp);
blk_exp_ref(&exp->common);
@@ -1653,7 +1653,7 @@ NBDExport *nbd_export_find(const char *name)
AioContext *
nbd_export_aio_context(NBDExport *exp)
{
- return exp->ctx;
+ return exp->common.ctx;
}
void nbd_export_close(NBDExport *exp)
@@ -1738,7 +1738,7 @@ void nbd_export_close_all(void)
AioContext *aio_context;
QTAILQ_FOREACH_SAFE(exp, &exports, next, next) {
- aio_context = exp->ctx;
+ aio_context = exp->common.ctx;
aio_context_acquire(aio_context);
nbd_export_close(exp);
aio_context_release(aio_context);
@@ -2537,7 +2537,7 @@ static void nbd_client_receive_next_request(NBDClient *client)
if (!client->recv_coroutine && client->nb_requests < MAX_NBD_REQUESTS) {
nbd_client_get(client);
client->recv_coroutine = qemu_coroutine_create(nbd_trip, client);
- aio_co_schedule(client->exp->ctx, client->recv_coroutine);
+ aio_co_schedule(client->exp->common.ctx, client->recv_coroutine);
}
}
--
2.25.4
^ permalink raw reply related [flat|nested] 44+ messages in thread
* [PATCH v2 15/31] block/export: Add node-name to BlockExportOptions
2020-09-24 15:26 [PATCH v2 00/31] block/export: Add infrastructure and QAPI for block exports Kevin Wolf
` (13 preceding siblings ...)
2020-09-24 15:27 ` [PATCH v2 14/31] block/export: Move AioContext " Kevin Wolf
@ 2020-09-24 15:27 ` Kevin Wolf
2020-09-24 15:27 ` [PATCH v2 16/31] block/export: Allocate BlockExport in blk_exp_add() Kevin Wolf
` (18 subsequent siblings)
33 siblings, 0 replies; 44+ messages in thread
From: Kevin Wolf @ 2020-09-24 15:27 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, qemu-devel, stefanha, mreitz
Every block export needs a block node to export, so add a 'node-name'
option to BlockExportOptions and remove the replaced option 'device'
from BlockExportOptionsNbd.
To maintain compatibility in nbd-server-add, BlockExportOptionsNbd needs
to be wrapped by a new type NbdServerAddOptions that adds 'device' back
because nbd-server-add doesn't use the BlockExportOptions base type at
all (so even without changing it to a 'node-name' option in
block-export-add, this compatibility code would be necessary).
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
---
qapi/block-export.json | 27 ++++++++++++++++-----
block/monitor/block-hmp-cmds.c | 6 ++---
blockdev-nbd.c | 44 +++++++++++++++++++++++++---------
qemu-nbd.c | 2 +-
4 files changed, 58 insertions(+), 21 deletions(-)
diff --git a/qapi/block-export.json b/qapi/block-export.json
index 319a38be88..d20b3a085d 100644
--- a/qapi/block-export.json
+++ b/qapi/block-export.json
@@ -65,9 +65,8 @@
##
# @BlockExportOptionsNbd:
#
-# An NBD block export.
-#
-# @device: The device name or node name of the node to be exported
+# An NBD block export (options shared between nbd-server-add and the NBD branch
+# of block-export-add).
#
# @name: Export name. If unspecified, the @device parameter is used as the
# export name. (Since 2.12)
@@ -85,8 +84,21 @@
# Since: 5.0
##
{ 'struct': 'BlockExportOptionsNbd',
- 'data': {'device': 'str', '*name': 'str', '*description': 'str',
- '*writable': 'bool', '*bitmap': 'str' } }
+ 'data': { '*name': 'str', '*description': 'str',
+ '*writable': 'bool', '*bitmap': 'str' } }
+
+##
+# @NbdServerAddOptions:
+#
+# An NBD block export.
+#
+# @device: The device name or node name of the node to be exported
+#
+# Since: 5.0
+##
+{ 'struct': 'NbdServerAddOptions',
+ 'base': 'BlockExportOptionsNbd',
+ 'data': { 'device': 'str' } }
##
# @nbd-server-add:
@@ -99,7 +111,7 @@
# Since: 1.3.0
##
{ 'command': 'nbd-server-add',
- 'data': 'BlockExportOptionsNbd', 'boxed': true }
+ 'data': 'NbdServerAddOptions', 'boxed': true }
##
# @NbdServerRemoveMode:
@@ -170,6 +182,8 @@
# Describes a block export, i.e. how single node should be exported on an
# external interface.
#
+# @node-name: The node name of the block node to be exported (since: 5.2)
+#
# @writethrough: If true, caches are flushed after every write request to the
# export before completion is signalled. (since: 5.2;
# default: false)
@@ -178,6 +192,7 @@
##
{ 'union': 'BlockExportOptions',
'base': { 'type': 'BlockExportType',
+ 'node-name': 'str',
'*writethrough': 'bool' },
'discriminator': 'type',
'data': {
diff --git a/block/monitor/block-hmp-cmds.c b/block/monitor/block-hmp-cmds.c
index 662b7f7d00..db357cafcb 100644
--- a/block/monitor/block-hmp-cmds.c
+++ b/block/monitor/block-hmp-cmds.c
@@ -398,7 +398,7 @@ void hmp_nbd_server_start(Monitor *mon, const QDict *qdict)
Error *local_err = NULL;
BlockInfoList *block_list, *info;
SocketAddress *addr;
- BlockExportOptionsNbd export;
+ NbdServerAddOptions export;
if (writable && !all) {
error_setg(&local_err, "-w only valid together with -a");
@@ -431,7 +431,7 @@ void hmp_nbd_server_start(Monitor *mon, const QDict *qdict)
continue;
}
- export = (BlockExportOptionsNbd) {
+ export = (NbdServerAddOptions) {
.device = info->value->device,
.has_writable = true,
.writable = writable,
@@ -458,7 +458,7 @@ void hmp_nbd_server_add(Monitor *mon, const QDict *qdict)
bool writable = qdict_get_try_bool(qdict, "writable", false);
Error *local_err = NULL;
- BlockExportOptionsNbd export = {
+ NbdServerAddOptions export = {
.device = (char *) device,
.has_name = !!name,
.name = (char *) name,
diff --git a/blockdev-nbd.c b/blockdev-nbd.c
index e208324d42..ef14303b25 100644
--- a/blockdev-nbd.c
+++ b/blockdev-nbd.c
@@ -188,7 +188,7 @@ BlockExport *nbd_export_create(BlockExportOptions *exp_args, Error **errp)
}
if (!arg->has_name) {
- arg->name = arg->device;
+ arg->name = exp_args->node_name;
}
if (strlen(arg->name) > NBD_MAX_STRING_SIZE) {
@@ -206,7 +206,7 @@ BlockExport *nbd_export_create(BlockExportOptions *exp_args, Error **errp)
return NULL;
}
- bs = bdrv_lookup_bs(arg->device, arg->device, errp);
+ bs = bdrv_lookup_bs(NULL, exp_args->node_name, errp);
if (!bs) {
return NULL;
}
@@ -244,21 +244,40 @@ BlockExport *nbd_export_create(BlockExportOptions *exp_args, Error **errp)
return (BlockExport*) exp;
}
-void qmp_nbd_server_add(BlockExportOptionsNbd *arg, Error **errp)
+void qmp_nbd_server_add(NbdServerAddOptions *arg, Error **errp)
{
BlockExport *export;
BlockDriverState *bs;
BlockBackend *on_eject_blk;
- BlockExportOptions export_opts;
+ BlockExportOptions *export_opts;
bs = bdrv_lookup_bs(arg->device, arg->device, errp);
if (!bs) {
return;
}
- export_opts = (BlockExportOptions) {
- .type = BLOCK_EXPORT_TYPE_NBD,
- .u.nbd = *arg,
+ /*
+ * block-export-add would default to the node-name, but we may have to use
+ * the device name as a default here for compatibility.
+ */
+ if (!arg->has_name) {
+ arg->name = arg->device;
+ }
+
+ export_opts = g_new(BlockExportOptions, 1);
+ *export_opts = (BlockExportOptions) {
+ .type = BLOCK_EXPORT_TYPE_NBD,
+ .node_name = g_strdup(bdrv_get_node_name(bs)),
+ .u.nbd = {
+ .has_name = true,
+ .name = g_strdup(arg->name),
+ .has_description = arg->has_description,
+ .description = g_strdup(arg->description),
+ .has_writable = arg->has_writable,
+ .writable = arg->writable,
+ .has_bitmap = arg->has_bitmap,
+ .bitmap = g_strdup(arg->bitmap),
+ },
};
/*
@@ -267,13 +286,13 @@ void qmp_nbd_server_add(BlockExportOptionsNbd *arg, Error **errp)
* block-export-add.
*/
if (bdrv_is_read_only(bs)) {
- export_opts.u.nbd.has_writable = true;
- export_opts.u.nbd.writable = false;
+ export_opts->u.nbd.has_writable = true;
+ export_opts->u.nbd.writable = false;
}
- export = blk_exp_add(&export_opts, errp);
+ export = blk_exp_add(export_opts, errp);
if (!export) {
- return;
+ goto fail;
}
/*
@@ -284,6 +303,9 @@ void qmp_nbd_server_add(BlockExportOptionsNbd *arg, Error **errp)
if (on_eject_blk) {
nbd_export_set_on_eject_blk(export, on_eject_blk);
}
+
+fail:
+ qapi_free_BlockExportOptions(export_opts);
}
void qmp_nbd_server_remove(const char *name,
diff --git a/qemu-nbd.c b/qemu-nbd.c
index a863741e8f..23c2b7da8e 100644
--- a/qemu-nbd.c
+++ b/qemu-nbd.c
@@ -1064,10 +1064,10 @@ int main(int argc, char **argv)
export_opts = g_new(BlockExportOptions, 1);
*export_opts = (BlockExportOptions) {
.type = BLOCK_EXPORT_TYPE_NBD,
+ .node_name = g_strdup(bdrv_get_node_name(bs)),
.has_writethrough = true,
.writethrough = writethrough,
.u.nbd = {
- .device = g_strdup(bdrv_get_node_name(bs)),
.has_name = true,
.name = g_strdup(export_name),
.has_description = !!export_description,
--
2.25.4
^ permalink raw reply related [flat|nested] 44+ messages in thread
* [PATCH v2 16/31] block/export: Allocate BlockExport in blk_exp_add()
2020-09-24 15:26 [PATCH v2 00/31] block/export: Add infrastructure and QAPI for block exports Kevin Wolf
` (14 preceding siblings ...)
2020-09-24 15:27 ` [PATCH v2 15/31] block/export: Add node-name to BlockExportOptions Kevin Wolf
@ 2020-09-24 15:27 ` Kevin Wolf
2020-09-24 15:27 ` [PATCH v2 17/31] block/export: Add blk_exp_close_all(_type) Kevin Wolf
` (17 subsequent siblings)
33 siblings, 0 replies; 44+ messages in thread
From: Kevin Wolf @ 2020-09-24 15:27 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, qemu-devel, stefanha, mreitz
Instead of letting the driver allocate and return the BlockExport
object, allocate it already in blk_exp_add() and pass it. This allows us
to initialise the generic part before calling into the driver so that
the driver can just use these values instead of having to parse the
options a second time.
For symmetry, move freeing the BlockExport to blk_exp_unref().
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
---
include/block/export.h | 8 +++++++-
include/block/nbd.h | 11 ++++++-----
block/export/export.c | 18 +++++++++++++++++-
blockdev-nbd.c | 26 ++++++++++++++------------
nbd/server.c | 30 +++++++++++++-----------------
5 files changed, 57 insertions(+), 36 deletions(-)
diff --git a/include/block/export.h b/include/block/export.h
index e6f96f4e1e..cf9b1c9dad 100644
--- a/include/block/export.h
+++ b/include/block/export.h
@@ -22,8 +22,14 @@ typedef struct BlockExportDriver {
/* The export type that this driver services */
BlockExportType type;
+ /*
+ * The size of the driver-specific state that contains BlockExport as its
+ * first field.
+ */
+ size_t instance_size;
+
/* Creates and starts a new block export */
- BlockExport *(*create)(BlockExportOptions *, Error **);
+ int (*create)(BlockExport *, BlockExportOptions *, Error **);
/*
* Frees a removed block export. This function is only called after all
diff --git a/include/block/nbd.h b/include/block/nbd.h
index e3bd112227..fc2c153d5b 100644
--- a/include/block/nbd.h
+++ b/include/block/nbd.h
@@ -330,11 +330,12 @@ int nbd_errno_to_system_errno(int err);
typedef struct NBDExport NBDExport;
typedef struct NBDClient NBDClient;
-BlockExport *nbd_export_create(BlockExportOptions *exp_args, Error **errp);
-NBDExport *nbd_export_new(BlockDriverState *bs,
- const char *name, const char *desc,
- const char *bitmap, bool readonly, bool shared,
- bool writethrough, Error **errp);
+int nbd_export_create(BlockExport *exp, BlockExportOptions *exp_args,
+ Error **errp);
+int nbd_export_new(BlockExport *blk_exp, BlockDriverState *bs,
+ const char *name, const char *desc,
+ const char *bitmap, bool readonly, bool shared,
+ bool writethrough, Error **errp);
void nbd_export_set_on_eject_blk(BlockExport *exp, BlockBackend *blk);
void nbd_export_close(NBDExport *exp);
void nbd_export_remove(NBDExport *exp, NbdServerRemoveMode mode, Error **errp);
diff --git a/block/export/export.c b/block/export/export.c
index 8635318ef1..6b2b29078b 100644
--- a/block/export/export.c
+++ b/block/export/export.c
@@ -39,6 +39,8 @@ static const BlockExportDriver *blk_exp_find_driver(BlockExportType type)
BlockExport *blk_exp_add(BlockExportOptions *export, Error **errp)
{
const BlockExportDriver *drv;
+ BlockExport *exp;
+ int ret;
drv = blk_exp_find_driver(export->type);
if (!drv) {
@@ -46,7 +48,20 @@ BlockExport *blk_exp_add(BlockExportOptions *export, Error **errp)
return NULL;
}
- return drv->create(export, errp);
+ assert(drv->instance_size >= sizeof(BlockExport));
+ exp = g_malloc0(drv->instance_size);
+ *exp = (BlockExport) {
+ .drv = drv,
+ .refcount = 1,
+ };
+
+ ret = drv->create(exp, export, errp);
+ if (ret < 0) {
+ g_free(exp);
+ return NULL;
+ }
+
+ return exp;
}
/* Callers must hold exp->ctx lock */
@@ -62,6 +77,7 @@ void blk_exp_unref(BlockExport *exp)
assert(exp->refcount > 0);
if (--exp->refcount == 0) {
exp->drv->delete(exp);
+ g_free(exp);
}
}
diff --git a/blockdev-nbd.c b/blockdev-nbd.c
index ef14303b25..b34f159888 100644
--- a/blockdev-nbd.c
+++ b/blockdev-nbd.c
@@ -173,18 +173,19 @@ void qmp_nbd_server_start(SocketAddressLegacy *addr,
qapi_free_SocketAddress(addr_flat);
}
-BlockExport *nbd_export_create(BlockExportOptions *exp_args, Error **errp)
+int nbd_export_create(BlockExport *exp, BlockExportOptions *exp_args,
+ Error **errp)
{
BlockExportOptionsNbd *arg = &exp_args->u.nbd;
BlockDriverState *bs = NULL;
- NBDExport *exp = NULL;
AioContext *aio_context;
+ int ret;
assert(exp_args->type == BLOCK_EXPORT_TYPE_NBD);
if (!nbd_server && !is_qemu_nbd) {
error_setg(errp, "NBD server not running");
- return NULL;
+ return -EINVAL;
}
if (!arg->has_name) {
@@ -193,22 +194,22 @@ BlockExport *nbd_export_create(BlockExportOptions *exp_args, Error **errp)
if (strlen(arg->name) > NBD_MAX_STRING_SIZE) {
error_setg(errp, "export name '%s' too long", arg->name);
- return NULL;
+ return -EINVAL;
}
if (arg->description && strlen(arg->description) > NBD_MAX_STRING_SIZE) {
error_setg(errp, "description '%s' too long", arg->description);
- return NULL;
+ return -EINVAL;
}
if (nbd_export_find(arg->name)) {
error_setg(errp, "NBD server already has export named '%s'", arg->name);
- return NULL;
+ return -EEXIST;
}
bs = bdrv_lookup_bs(NULL, exp_args->node_name, errp);
if (!bs) {
- return NULL;
+ return -ENOENT;
}
aio_context = bdrv_get_aio_context(bs);
@@ -218,6 +219,7 @@ BlockExport *nbd_export_create(BlockExportOptions *exp_args, Error **errp)
arg->writable = false;
}
if (bdrv_is_read_only(bs) && arg->writable) {
+ ret = -EINVAL;
error_setg(errp, "Cannot export read-only node as writable");
goto out;
}
@@ -226,22 +228,22 @@ BlockExport *nbd_export_create(BlockExportOptions *exp_args, Error **errp)
exp_args->writethrough = false;
}
- exp = nbd_export_new(bs, arg->name, arg->description, arg->bitmap,
+ ret = nbd_export_new(exp, bs, arg->name, arg->description, arg->bitmap,
!arg->writable, !arg->writable,
exp_args->writethrough, errp);
- if (!exp) {
+ if (ret < 0) {
goto out;
}
/* The list of named exports has a strong reference to this export now and
* our only way of accessing it is through nbd_export_find(), so we can drop
* the strong reference that is @exp. */
- blk_exp_unref((BlockExport*) exp);
+ blk_exp_unref(exp);
+ ret = 0;
out:
aio_context_release(aio_context);
- /* TODO Remove the cast: nbd_export_new() will return a BlockExport. */
- return (BlockExport*) exp;
+ return ret;
}
void qmp_nbd_server_add(NbdServerAddOptions *arg, Error **errp)
diff --git a/nbd/server.c b/nbd/server.c
index 7cf81646fc..f31d8bbb60 100644
--- a/nbd/server.c
+++ b/nbd/server.c
@@ -1514,14 +1514,14 @@ void nbd_export_set_on_eject_blk(BlockExport *exp, BlockBackend *blk)
blk_add_remove_bs_notifier(blk, &nbd_exp->eject_notifier);
}
-NBDExport *nbd_export_new(BlockDriverState *bs,
- const char *name, const char *desc,
- const char *bitmap, bool readonly, bool shared,
- bool writethrough, Error **errp)
+int nbd_export_new(BlockExport *blk_exp, BlockDriverState *bs,
+ const char *name, const char *desc,
+ const char *bitmap, bool readonly, bool shared,
+ bool writethrough, Error **errp)
{
+ NBDExport *exp = container_of(blk_exp, NBDExport, common);
AioContext *ctx;
BlockBackend *blk;
- NBDExport *exp;
int64_t size;
uint64_t perm;
int ret;
@@ -1530,17 +1530,11 @@ NBDExport *nbd_export_new(BlockDriverState *bs,
if (size < 0) {
error_setg_errno(errp, -size,
"Failed to determine the NBD export's length");
- return NULL;
+ return size;
}
ctx = bdrv_get_aio_context(bs);
-
- exp = g_new0(NBDExport, 1);
- exp->common = (BlockExport) {
- .drv = &blk_exp_nbd,
- .refcount = 1,
- .ctx = ctx,
- };
+ blk_exp->ctx = ctx;
/*
* NBD exports are used for non-shared storage migration. Make sure
@@ -1599,16 +1593,19 @@ NBDExport *nbd_export_new(BlockDriverState *bs,
}
if (bm == NULL) {
+ ret = -ENOENT;
error_setg(errp, "Bitmap '%s' is not found", bitmap);
goto fail;
}
if (bdrv_dirty_bitmap_check(bm, BDRV_BITMAP_ALLOW_RO, errp)) {
+ ret = -EINVAL;
goto fail;
}
if (readonly && bdrv_is_writable(bs) &&
bdrv_dirty_bitmap_enabled(bm)) {
+ ret = -EINVAL;
error_setg(errp,
"Enabled bitmap '%s' incompatible with readonly export",
bitmap);
@@ -1628,14 +1625,13 @@ NBDExport *nbd_export_new(BlockDriverState *bs,
blk_exp_ref(&exp->common);
QTAILQ_INSERT_TAIL(&exports, exp, next);
- return exp;
+ return 0;
fail:
blk_unref(blk);
g_free(exp->name);
g_free(exp->description);
- g_free(exp);
- return NULL;
+ return ret;
}
NBDExport *nbd_export_find(const char *name)
@@ -1722,12 +1718,12 @@ static void nbd_export_delete(BlockExport *blk_exp)
}
QTAILQ_REMOVE(&closed_exports, exp, next);
- g_free(exp);
aio_wait_kick();
}
const BlockExportDriver blk_exp_nbd = {
.type = BLOCK_EXPORT_TYPE_NBD,
+ .instance_size = sizeof(NBDExport),
.create = nbd_export_create,
.delete = nbd_export_delete,
};
--
2.25.4
^ permalink raw reply related [flat|nested] 44+ messages in thread
* [PATCH v2 17/31] block/export: Add blk_exp_close_all(_type)
2020-09-24 15:26 [PATCH v2 00/31] block/export: Add infrastructure and QAPI for block exports Kevin Wolf
` (15 preceding siblings ...)
2020-09-24 15:27 ` [PATCH v2 16/31] block/export: Allocate BlockExport in blk_exp_add() Kevin Wolf
@ 2020-09-24 15:27 ` Kevin Wolf
2020-09-24 15:27 ` [PATCH v2 18/31] block/export: Add 'id' option to block-export-add Kevin Wolf
` (16 subsequent siblings)
33 siblings, 0 replies; 44+ messages in thread
From: Kevin Wolf @ 2020-09-24 15:27 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, qemu-devel, stefanha, mreitz
This adds a function to shut down all block exports, and another one to
shut down the block exports of a single type. The latter is used for now
when stopping the NBD server. As soon as we implement support for
multiple NBD servers, we'll need a per-server list of exports and it
will be replaced by a function using that.
As a side effect, the BlockExport layer has a list tracking all existing
exports now. closed_exports loses its only user and can go away.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
---
include/block/export.h | 15 ++++++++
include/block/nbd.h | 2 -
block.c | 2 +-
block/export/export.c | 86 +++++++++++++++++++++++++++++++++++++++++-
blockdev-nbd.c | 2 +-
nbd/server.c | 34 +++--------------
qemu-nbd.c | 2 +-
7 files changed, 107 insertions(+), 36 deletions(-)
diff --git a/include/block/export.h b/include/block/export.h
index cf9b1c9dad..6fffcb5651 100644
--- a/include/block/export.h
+++ b/include/block/export.h
@@ -15,6 +15,7 @@
#define BLOCK_EXPORT_H
#include "qapi/qapi-types-block-export.h"
+#include "qemu/queue.h"
typedef struct BlockExport BlockExport;
@@ -36,6 +37,14 @@ typedef struct BlockExportDriver {
* references have been dropped.
*/
void (*delete)(BlockExport *);
+
+ /*
+ * Start to disconnect all clients and drop other references held
+ * internally by the export driver. When the function returns, there may
+ * still be active references while the export is in the process of
+ * shutting down.
+ */
+ void (*request_shutdown)(BlockExport *);
} BlockExportDriver;
struct BlockExport {
@@ -50,10 +59,16 @@ struct BlockExport {
/* The AioContext whose lock protects this BlockExport object. */
AioContext *ctx;
+
+ /* List entry for block_exports */
+ QLIST_ENTRY(BlockExport) next;
};
BlockExport *blk_exp_add(BlockExportOptions *export, Error **errp);
void blk_exp_ref(BlockExport *exp);
void blk_exp_unref(BlockExport *exp);
+void blk_exp_request_shutdown(BlockExport *exp);
+void blk_exp_close_all(void);
+void blk_exp_close_all_type(BlockExportType type);
#endif
diff --git a/include/block/nbd.h b/include/block/nbd.h
index fc2c153d5b..0b9f3e5d4e 100644
--- a/include/block/nbd.h
+++ b/include/block/nbd.h
@@ -337,12 +337,10 @@ int nbd_export_new(BlockExport *blk_exp, BlockDriverState *bs,
const char *bitmap, bool readonly, bool shared,
bool writethrough, Error **errp);
void nbd_export_set_on_eject_blk(BlockExport *exp, BlockBackend *blk);
-void nbd_export_close(NBDExport *exp);
void nbd_export_remove(NBDExport *exp, NbdServerRemoveMode mode, Error **errp);
AioContext *nbd_export_aio_context(NBDExport *exp);
NBDExport *nbd_export_find(const char *name);
-void nbd_export_close_all(void);
void nbd_client_new(QIOChannelSocket *sioc,
QCryptoTLSCreds *tlscreds,
diff --git a/block.c b/block.c
index 11ab55f80b..47a9d91fb4 100644
--- a/block.c
+++ b/block.c
@@ -4462,7 +4462,7 @@ static void bdrv_close(BlockDriverState *bs)
void bdrv_close_all(void)
{
assert(job_next(NULL) == NULL);
- nbd_export_close_all();
+ blk_exp_close_all();
/* Drop references from requests still in flight, such as canceled block
* jobs whose AIO context has not been polled yet */
diff --git a/block/export/export.c b/block/export/export.c
index 6b2b29078b..e94a68c183 100644
--- a/block/export/export.c
+++ b/block/export/export.c
@@ -24,6 +24,10 @@ static const BlockExportDriver *blk_exp_drivers[] = {
&blk_exp_nbd,
};
+/* Only accessed from the main thread */
+static QLIST_HEAD(, BlockExport) block_exports =
+ QLIST_HEAD_INITIALIZER(block_exports);
+
static const BlockExportDriver *blk_exp_find_driver(BlockExportType type)
{
int i;
@@ -61,6 +65,7 @@ BlockExport *blk_exp_add(BlockExportOptions *export, Error **errp)
return NULL;
}
+ QLIST_INSERT_HEAD(&block_exports, exp, next);
return exp;
}
@@ -71,14 +76,91 @@ void blk_exp_ref(BlockExport *exp)
exp->refcount++;
}
+/* Runs in the main thread */
+static void blk_exp_delete_bh(void *opaque)
+{
+ BlockExport *exp = opaque;
+ AioContext *aio_context = exp->ctx;
+
+ aio_context_acquire(aio_context);
+
+ assert(exp->refcount == 0);
+ QLIST_REMOVE(exp, next);
+ exp->drv->delete(exp);
+ g_free(exp);
+
+ aio_context_release(aio_context);
+}
+
/* Callers must hold exp->ctx lock */
void blk_exp_unref(BlockExport *exp)
{
assert(exp->refcount > 0);
if (--exp->refcount == 0) {
- exp->drv->delete(exp);
- g_free(exp);
+ /* Touch the block_exports list only in the main thread */
+ aio_bh_schedule_oneshot(qemu_get_aio_context(), blk_exp_delete_bh,
+ exp);
+ }
+}
+
+/*
+ * Drops the user reference to the export and requests that all client
+ * connections and other internally held references start to shut down. When
+ * the function returns, there may still be active references while the export
+ * is in the process of shutting down.
+ *
+ * Acquires exp->ctx internally. Callers must *not* hold the lock.
+ */
+void blk_exp_request_shutdown(BlockExport *exp)
+{
+ AioContext *aio_context = exp->ctx;
+
+ aio_context_acquire(aio_context);
+ exp->drv->request_shutdown(exp);
+ aio_context_release(aio_context);
+}
+
+/*
+ * Returns whether a block export of the given type exists.
+ * type == BLOCK_EXPORT_TYPE__MAX checks for an export of any type.
+ */
+static bool blk_exp_has_type(BlockExportType type)
+{
+ BlockExport *exp;
+
+ if (type == BLOCK_EXPORT_TYPE__MAX) {
+ return !QLIST_EMPTY(&block_exports);
+ }
+
+ QLIST_FOREACH(exp, &block_exports, next) {
+ if (exp->drv->type == type) {
+ return true;
+ }
}
+
+ return false;
+}
+
+/* type == BLOCK_EXPORT_TYPE__MAX for all types */
+void blk_exp_close_all_type(BlockExportType type)
+{
+ BlockExport *exp, *next;
+
+ assert(in_aio_context_home_thread(qemu_get_aio_context()));
+
+ QLIST_FOREACH_SAFE(exp, &block_exports, next, next) {
+ if (type != BLOCK_EXPORT_TYPE__MAX && exp->drv->type != type) {
+ continue;
+ }
+ blk_exp_request_shutdown(exp);
+ }
+
+ AIO_WAIT_WHILE(NULL, blk_exp_has_type(type));
+}
+
+void blk_exp_close_all(void)
+{
+ blk_exp_close_all_type(BLOCK_EXPORT_TYPE__MAX);
}
void qmp_block_export_add(BlockExportOptions *export, Error **errp)
diff --git a/blockdev-nbd.c b/blockdev-nbd.c
index b34f159888..f927264777 100644
--- a/blockdev-nbd.c
+++ b/blockdev-nbd.c
@@ -345,7 +345,7 @@ void qmp_nbd_server_stop(Error **errp)
return;
}
- nbd_export_close_all();
+ blk_exp_close_all_type(BLOCK_EXPORT_TYPE_NBD);
nbd_server_free(nbd_server);
nbd_server = NULL;
diff --git a/nbd/server.c b/nbd/server.c
index f31d8bbb60..32147e4871 100644
--- a/nbd/server.c
+++ b/nbd/server.c
@@ -100,8 +100,6 @@ struct NBDExport {
};
static QTAILQ_HEAD(, NBDExport) exports = QTAILQ_HEAD_INITIALIZER(exports);
-static QTAILQ_HEAD(, NBDExport) closed_exports =
- QTAILQ_HEAD_INITIALIZER(closed_exports);
/* NBDExportMetaContexts represents a list of contexts to be exported,
* as selected by NBD_OPT_SET_META_CONTEXT. Also used for
@@ -1494,12 +1492,8 @@ static void blk_aio_detach(void *opaque)
static void nbd_eject_notifier(Notifier *n, void *data)
{
NBDExport *exp = container_of(n, NBDExport, eject_notifier);
- AioContext *aio_context;
- aio_context = exp->common.ctx;
- aio_context_acquire(aio_context);
- nbd_export_close(exp);
- aio_context_release(aio_context);
+ blk_exp_request_shutdown(&exp->common);
}
void nbd_export_set_on_eject_blk(BlockExport *exp, BlockBackend *blk)
@@ -1652,8 +1646,9 @@ nbd_export_aio_context(NBDExport *exp)
return exp->common.ctx;
}
-void nbd_export_close(NBDExport *exp)
+static void nbd_export_request_shutdown(BlockExport *blk_exp)
{
+ NBDExport *exp = container_of(blk_exp, NBDExport, common);
NBDClient *client, *next;
blk_exp_ref(&exp->common);
@@ -1672,7 +1667,6 @@ void nbd_export_close(NBDExport *exp)
g_free(exp->name);
exp->name = NULL;
QTAILQ_REMOVE(&exports, exp, next);
- QTAILQ_INSERT_TAIL(&closed_exports, exp, next);
}
blk_exp_unref(&exp->common);
}
@@ -1681,7 +1675,7 @@ void nbd_export_remove(NBDExport *exp, NbdServerRemoveMode mode, Error **errp)
{
ERRP_GUARD();
if (mode == NBD_SERVER_REMOVE_MODE_HARD || QTAILQ_EMPTY(&exp->clients)) {
- nbd_export_close(exp);
+ nbd_export_request_shutdown(&exp->common);
return;
}
@@ -1716,9 +1710,6 @@ static void nbd_export_delete(BlockExport *blk_exp)
bdrv_dirty_bitmap_set_busy(exp->export_bitmap, false);
g_free(exp->export_bitmap_context);
}
-
- QTAILQ_REMOVE(&closed_exports, exp, next);
- aio_wait_kick();
}
const BlockExportDriver blk_exp_nbd = {
@@ -1726,24 +1717,9 @@ const BlockExportDriver blk_exp_nbd = {
.instance_size = sizeof(NBDExport),
.create = nbd_export_create,
.delete = nbd_export_delete,
+ .request_shutdown = nbd_export_request_shutdown,
};
-void nbd_export_close_all(void)
-{
- NBDExport *exp, *next;
- AioContext *aio_context;
-
- QTAILQ_FOREACH_SAFE(exp, &exports, next, next) {
- aio_context = exp->common.ctx;
- aio_context_acquire(aio_context);
- nbd_export_close(exp);
- aio_context_release(aio_context);
- }
-
- AIO_WAIT_WHILE(NULL, !(QTAILQ_EMPTY(&exports) &&
- QTAILQ_EMPTY(&closed_exports)));
-}
-
static int coroutine_fn nbd_co_send_iov(NBDClient *client, struct iovec *iov,
unsigned niov, Error **errp)
{
diff --git a/qemu-nbd.c b/qemu-nbd.c
index 23c2b7da8e..c64f83618b 100644
--- a/qemu-nbd.c
+++ b/qemu-nbd.c
@@ -1119,7 +1119,7 @@ int main(int argc, char **argv)
do {
main_loop_wait(false);
if (state == TERMINATE) {
- nbd_export_close_all();
+ blk_exp_close_all();
state = TERMINATED;
}
} while (state != TERMINATED);
--
2.25.4
^ permalink raw reply related [flat|nested] 44+ messages in thread
* [PATCH v2 18/31] block/export: Add 'id' option to block-export-add
2020-09-24 15:26 [PATCH v2 00/31] block/export: Add infrastructure and QAPI for block exports Kevin Wolf
` (16 preceding siblings ...)
2020-09-24 15:27 ` [PATCH v2 17/31] block/export: Add blk_exp_close_all(_type) Kevin Wolf
@ 2020-09-24 15:27 ` Kevin Wolf
2020-09-24 15:27 ` [PATCH v2 19/31] block/export: Move strong user reference to block_exports Kevin Wolf
` (15 subsequent siblings)
33 siblings, 0 replies; 44+ messages in thread
From: Kevin Wolf @ 2020-09-24 15:27 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, qemu-devel, stefanha, mreitz
We'll need an id to identify block exports in monitor commands. This
adds one.
Note that this is different from the 'name' option in the NBD server,
which is the externally visible export name. While block export ids need
to be unique in the whole process, export names must be unique only for
the same server. Different export types or (potentially in the future)
multiple NBD servers can have the same export name externally, but still
need different block export ids internally.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
---
qapi/block-export.json | 5 +++++
include/block/export.h | 3 +++
block/export/export.c | 26 ++++++++++++++++++++++++++
blockdev-nbd.c | 1 +
qemu-nbd.c | 1 +
storage-daemon/qemu-storage-daemon.c | 2 +-
tests/qemu-iotests/223.out | 4 ++--
7 files changed, 39 insertions(+), 3 deletions(-)
diff --git a/qapi/block-export.json b/qapi/block-export.json
index d20b3a085d..47c75a1794 100644
--- a/qapi/block-export.json
+++ b/qapi/block-export.json
@@ -105,6 +105,8 @@
#
# Export a block node to QEMU's embedded NBD server.
#
+# The export name will be used as the id for the resulting block export.
+#
# Returns: error if the server is not running, or export with the same name
# already exists.
#
@@ -182,6 +184,8 @@
# Describes a block export, i.e. how single node should be exported on an
# external interface.
#
+# @id: A unique identifier for the block export (across all export types)
+#
# @node-name: The node name of the block node to be exported (since: 5.2)
#
# @writethrough: If true, caches are flushed after every write request to the
@@ -192,6 +196,7 @@
##
{ 'union': 'BlockExportOptions',
'base': { 'type': 'BlockExportType',
+ 'id': 'str',
'node-name': 'str',
'*writethrough': 'bool' },
'discriminator': 'type',
diff --git a/include/block/export.h b/include/block/export.h
index 6fffcb5651..cdc6e161ea 100644
--- a/include/block/export.h
+++ b/include/block/export.h
@@ -50,6 +50,9 @@ typedef struct BlockExportDriver {
struct BlockExport {
const BlockExportDriver *drv;
+ /* Unique identifier for the export */
+ char *id;
+
/*
* Reference count for this block export. This includes strong references
* both from the owner (qemu-nbd or the monitor) and clients connected to
diff --git a/block/export/export.c b/block/export/export.c
index e94a68c183..7a4a78449a 100644
--- a/block/export/export.c
+++ b/block/export/export.c
@@ -19,6 +19,7 @@
#include "block/nbd.h"
#include "qapi/error.h"
#include "qapi/qapi-commands-block-export.h"
+#include "qemu/id.h"
static const BlockExportDriver *blk_exp_drivers[] = {
&blk_exp_nbd,
@@ -28,6 +29,19 @@ static const BlockExportDriver *blk_exp_drivers[] = {
static QLIST_HEAD(, BlockExport) block_exports =
QLIST_HEAD_INITIALIZER(block_exports);
+static BlockExport *blk_exp_find(const char *id)
+{
+ BlockExport *exp;
+
+ QLIST_FOREACH(exp, &block_exports, next) {
+ if (strcmp(id, exp->id) == 0) {
+ return exp;
+ }
+ }
+
+ return NULL;
+}
+
static const BlockExportDriver *blk_exp_find_driver(BlockExportType type)
{
int i;
@@ -46,6 +60,15 @@ BlockExport *blk_exp_add(BlockExportOptions *export, Error **errp)
BlockExport *exp;
int ret;
+ if (!id_wellformed(export->id)) {
+ error_setg(errp, "Invalid block export id");
+ return NULL;
+ }
+ if (blk_exp_find(export->id)) {
+ error_setg(errp, "Block export id '%s' is already in use", export->id);
+ return NULL;
+ }
+
drv = blk_exp_find_driver(export->type);
if (!drv) {
error_setg(errp, "No driver found for the requested export type");
@@ -57,10 +80,12 @@ BlockExport *blk_exp_add(BlockExportOptions *export, Error **errp)
*exp = (BlockExport) {
.drv = drv,
.refcount = 1,
+ .id = g_strdup(export->id),
};
ret = drv->create(exp, export, errp);
if (ret < 0) {
+ g_free(exp->id);
g_free(exp);
return NULL;
}
@@ -87,6 +112,7 @@ static void blk_exp_delete_bh(void *opaque)
assert(exp->refcount == 0);
QLIST_REMOVE(exp, next);
exp->drv->delete(exp);
+ g_free(exp->id);
g_free(exp);
aio_context_release(aio_context);
diff --git a/blockdev-nbd.c b/blockdev-nbd.c
index f927264777..814554dd90 100644
--- a/blockdev-nbd.c
+++ b/blockdev-nbd.c
@@ -269,6 +269,7 @@ void qmp_nbd_server_add(NbdServerAddOptions *arg, Error **errp)
export_opts = g_new(BlockExportOptions, 1);
*export_opts = (BlockExportOptions) {
.type = BLOCK_EXPORT_TYPE_NBD,
+ .id = g_strdup(arg->name),
.node_name = g_strdup(bdrv_get_node_name(bs)),
.u.nbd = {
.has_name = true,
diff --git a/qemu-nbd.c b/qemu-nbd.c
index c64f83618b..ac82acb4ac 100644
--- a/qemu-nbd.c
+++ b/qemu-nbd.c
@@ -1064,6 +1064,7 @@ int main(int argc, char **argv)
export_opts = g_new(BlockExportOptions, 1);
*export_opts = (BlockExportOptions) {
.type = BLOCK_EXPORT_TYPE_NBD,
+ .id = g_strdup("qemu-nbd-export"),
.node_name = g_strdup(bdrv_get_node_name(bs)),
.has_writethrough = true,
.writethrough = writethrough,
diff --git a/storage-daemon/qemu-storage-daemon.c b/storage-daemon/qemu-storage-daemon.c
index 0fcab6ed2d..e6157ff518 100644
--- a/storage-daemon/qemu-storage-daemon.c
+++ b/storage-daemon/qemu-storage-daemon.c
@@ -92,7 +92,7 @@ static void help(void)
" --chardev <options> configure a character device backend\n"
" (see the qemu(1) man page for possible options)\n"
"\n"
-" --export [type=]nbd,device=<node-name>[,name=<export-name>]\n"
+" --export [type=]nbd,device=<node-name>,id=<id>,[,name=<export-name>]\n"
" [,writable=on|off][,bitmap=<name>]\n"
" export the specified block node over NBD\n"
" (requires --nbd-server)\n"
diff --git a/tests/qemu-iotests/223.out b/tests/qemu-iotests/223.out
index e1eaaedb55..31ce9e6fe0 100644
--- a/tests/qemu-iotests/223.out
+++ b/tests/qemu-iotests/223.out
@@ -45,7 +45,7 @@ exports available: 0
{"execute":"nbd-server-add", "arguments":{"device":"nosuch"}}
{"error": {"class": "GenericError", "desc": "Cannot find device=nosuch nor node_name=nosuch"}}
{"execute":"nbd-server-add", "arguments":{"device":"n"}}
-{"error": {"class": "GenericError", "desc": "NBD server already has export named 'n'"}}
+{"error": {"class": "GenericError", "desc": "Block export id 'n' is already in use"}}
{"execute":"nbd-server-add", "arguments":{"device":"n", "name":"n2", "bitmap":"b2"}}
{"error": {"class": "GenericError", "desc": "Enabled bitmap 'b2' incompatible with readonly export"}}
{"execute":"nbd-server-add", "arguments":{"device":"n", "name":"n2", "bitmap":"b3"}}
@@ -126,7 +126,7 @@ exports available: 0
{"execute":"nbd-server-add", "arguments":{"device":"nosuch"}}
{"error": {"class": "GenericError", "desc": "Cannot find device=nosuch nor node_name=nosuch"}}
{"execute":"nbd-server-add", "arguments":{"device":"n"}}
-{"error": {"class": "GenericError", "desc": "NBD server already has export named 'n'"}}
+{"error": {"class": "GenericError", "desc": "Block export id 'n' is already in use"}}
{"execute":"nbd-server-add", "arguments":{"device":"n", "name":"n2", "bitmap":"b2"}}
{"error": {"class": "GenericError", "desc": "Enabled bitmap 'b2' incompatible with readonly export"}}
{"execute":"nbd-server-add", "arguments":{"device":"n", "name":"n2", "bitmap":"b3"}}
--
2.25.4
^ permalink raw reply related [flat|nested] 44+ messages in thread
* [PATCH v2 19/31] block/export: Move strong user reference to block_exports
2020-09-24 15:26 [PATCH v2 00/31] block/export: Add infrastructure and QAPI for block exports Kevin Wolf
` (17 preceding siblings ...)
2020-09-24 15:27 ` [PATCH v2 18/31] block/export: Add 'id' option to block-export-add Kevin Wolf
@ 2020-09-24 15:27 ` Kevin Wolf
2020-09-24 15:27 ` [PATCH v2 20/31] block/export: Add block-export-del Kevin Wolf
` (14 subsequent siblings)
33 siblings, 0 replies; 44+ messages in thread
From: Kevin Wolf @ 2020-09-24 15:27 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, qemu-devel, stefanha, mreitz
The reference owned by the user/monitor that is created when adding the
export and dropped when removing it was tied to the 'exports' list in
nbd/server.c. Every block export will have a user reference, so move it
to the block export level and tie it to the 'block_exports' list in
block/export/export.c instead. This is necessary for introducing a QMP
command for removing exports.
Note that exports are present in block_exports even after the user has
requested shutdown. This is different from NBD's exports where exports
are immediately removed on a shutdown request, even if they are still in
the process of shutting down. In order to avoid that the user still
interacts with an export that is shutting down (and possibly removes it
a second time), we need to remember if the user actually still owns it.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
---
include/block/export.h | 8 ++++++++
block/export/export.c | 6 ++++++
blockdev-nbd.c | 5 -----
nbd/server.c | 2 --
4 files changed, 14 insertions(+), 7 deletions(-)
diff --git a/include/block/export.h b/include/block/export.h
index cdc6e161ea..4833947e89 100644
--- a/include/block/export.h
+++ b/include/block/export.h
@@ -60,6 +60,14 @@ struct BlockExport {
*/
int refcount;
+ /*
+ * True if one of the references in refcount belongs to the user. After the
+ * user has dropped their reference, they may not e.g. remove the same
+ * export a second time (which would decrease the refcount without having
+ * it incremented first).
+ */
+ bool user_owned;
+
/* The AioContext whose lock protects this BlockExport object. */
AioContext *ctx;
diff --git a/block/export/export.c b/block/export/export.c
index 7a4a78449a..62699dfa05 100644
--- a/block/export/export.c
+++ b/block/export/export.c
@@ -80,6 +80,7 @@ BlockExport *blk_exp_add(BlockExportOptions *export, Error **errp)
*exp = (BlockExport) {
.drv = drv,
.refcount = 1,
+ .user_owned = true,
.id = g_strdup(export->id),
};
@@ -143,6 +144,11 @@ void blk_exp_request_shutdown(BlockExport *exp)
aio_context_acquire(aio_context);
exp->drv->request_shutdown(exp);
+
+ assert(exp->user_owned);
+ exp->user_owned = false;
+ blk_exp_unref(exp);
+
aio_context_release(aio_context);
}
diff --git a/blockdev-nbd.c b/blockdev-nbd.c
index 814554dd90..9efbaef8f7 100644
--- a/blockdev-nbd.c
+++ b/blockdev-nbd.c
@@ -235,11 +235,6 @@ int nbd_export_create(BlockExport *exp, BlockExportOptions *exp_args,
goto out;
}
- /* The list of named exports has a strong reference to this export now and
- * our only way of accessing it is through nbd_export_find(), so we can drop
- * the strong reference that is @exp. */
- blk_exp_unref(exp);
-
ret = 0;
out:
aio_context_release(aio_context);
diff --git a/nbd/server.c b/nbd/server.c
index 32147e4871..22a1d66168 100644
--- a/nbd/server.c
+++ b/nbd/server.c
@@ -1616,7 +1616,6 @@ int nbd_export_new(BlockExport *blk_exp, BlockDriverState *bs,
blk_add_aio_context_notifier(blk, blk_aio_attached, blk_aio_detach, exp);
- blk_exp_ref(&exp->common);
QTAILQ_INSERT_TAIL(&exports, exp, next);
return 0;
@@ -1663,7 +1662,6 @@ static void nbd_export_request_shutdown(BlockExport *blk_exp)
client_close(client, true);
}
if (exp->name) {
- blk_exp_unref(&exp->common);
g_free(exp->name);
exp->name = NULL;
QTAILQ_REMOVE(&exports, exp, next);
--
2.25.4
^ permalink raw reply related [flat|nested] 44+ messages in thread
* [PATCH v2 20/31] block/export: Add block-export-del
2020-09-24 15:26 [PATCH v2 00/31] block/export: Add infrastructure and QAPI for block exports Kevin Wolf
` (18 preceding siblings ...)
2020-09-24 15:27 ` [PATCH v2 19/31] block/export: Move strong user reference to block_exports Kevin Wolf
@ 2020-09-24 15:27 ` Kevin Wolf
2020-09-24 15:27 ` [PATCH v2 21/31] block/export: Add BLOCK_EXPORT_DELETED event Kevin Wolf
` (13 subsequent siblings)
33 siblings, 0 replies; 44+ messages in thread
From: Kevin Wolf @ 2020-09-24 15:27 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, qemu-devel, stefanha, mreitz
Implement a new QMP command block-export-del and make nbd-server-remove
a wrapper around it.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
---
qapi/block-export.json | 32 ++++++++++++++++++++-----
include/block/export.h | 1 +
include/block/nbd.h | 1 -
block/export/export.c | 43 +++++++++++++++++++++++++++++++++-
block/monitor/block-hmp-cmds.c | 4 ++--
blockdev-nbd.c | 25 +++++---------------
nbd/server.c | 14 -----------
7 files changed, 77 insertions(+), 43 deletions(-)
diff --git a/qapi/block-export.json b/qapi/block-export.json
index 47c75a1794..0ba4d84c97 100644
--- a/qapi/block-export.json
+++ b/qapi/block-export.json
@@ -116,9 +116,9 @@
'data': 'NbdServerAddOptions', 'boxed': true }
##
-# @NbdServerRemoveMode:
+# @BlockExportRemoveMode:
#
-# Mode for removing an NBD export.
+# Mode for removing a block export.
#
# @safe: Remove export if there are no existing connections, fail otherwise.
#
@@ -134,16 +134,16 @@
#
# Since: 2.12
##
-{'enum': 'NbdServerRemoveMode', 'data': ['safe', 'hard']}
+{'enum': 'BlockExportRemoveMode', 'data': ['safe', 'hard']}
##
# @nbd-server-remove:
#
# Remove NBD export by name.
#
-# @name: Export name.
+# @name: Block export id.
#
-# @mode: Mode of command operation. See @NbdServerRemoveMode description.
+# @mode: Mode of command operation. See @BlockExportRemoveMode description.
# Default is 'safe'.
#
# Returns: error if
@@ -154,7 +154,7 @@
# Since: 2.12
##
{ 'command': 'nbd-server-remove',
- 'data': {'name': 'str', '*mode': 'NbdServerRemoveMode'} }
+ 'data': {'name': 'str', '*mode': 'BlockExportRemoveMode'} }
##
# @nbd-server-stop:
@@ -213,3 +213,23 @@
##
{ 'command': 'block-export-add',
'data': 'BlockExportOptions', 'boxed': true }
+
+##
+# @block-export-del:
+#
+# Request to remove a block export. This drops the user's reference to the
+# export, but the export may still stay around after this command returns until
+# the shutdown of the export has completed.
+#
+# @id: Block export id.
+#
+# @mode: Mode of command operation. See @BlockExportRemoveMode description.
+# Default is 'safe'.
+#
+# Returns: Error if the export is not found or @mode is 'safe' and the export
+# is still in use (e.g. by existing client connections)
+#
+# Since: 5.2
+##
+{ 'command': 'block-export-del',
+ 'data': { 'id': 'str', '*mode': 'BlockExportRemoveMode' } }
diff --git a/include/block/export.h b/include/block/export.h
index 4833947e89..ff54d35872 100644
--- a/include/block/export.h
+++ b/include/block/export.h
@@ -76,6 +76,7 @@ struct BlockExport {
};
BlockExport *blk_exp_add(BlockExportOptions *export, Error **errp);
+BlockExport *blk_exp_find(const char *id);
void blk_exp_ref(BlockExport *exp);
void blk_exp_unref(BlockExport *exp);
void blk_exp_request_shutdown(BlockExport *exp);
diff --git a/include/block/nbd.h b/include/block/nbd.h
index 0b9f3e5d4e..a4dc1f9e54 100644
--- a/include/block/nbd.h
+++ b/include/block/nbd.h
@@ -337,7 +337,6 @@ int nbd_export_new(BlockExport *blk_exp, BlockDriverState *bs,
const char *bitmap, bool readonly, bool shared,
bool writethrough, Error **errp);
void nbd_export_set_on_eject_blk(BlockExport *exp, BlockBackend *blk);
-void nbd_export_remove(NBDExport *exp, NbdServerRemoveMode mode, Error **errp);
AioContext *nbd_export_aio_context(NBDExport *exp);
NBDExport *nbd_export_find(const char *name);
diff --git a/block/export/export.c b/block/export/export.c
index 62699dfa05..d186beffe9 100644
--- a/block/export/export.c
+++ b/block/export/export.c
@@ -29,7 +29,7 @@ static const BlockExportDriver *blk_exp_drivers[] = {
static QLIST_HEAD(, BlockExport) block_exports =
QLIST_HEAD_INITIALIZER(block_exports);
-static BlockExport *blk_exp_find(const char *id)
+BlockExport *blk_exp_find(const char *id)
{
BlockExport *exp;
@@ -143,12 +143,23 @@ void blk_exp_request_shutdown(BlockExport *exp)
AioContext *aio_context = exp->ctx;
aio_context_acquire(aio_context);
+
+ /*
+ * If the user doesn't own the export any more, it is already shutting
+ * down. We must not call .request_shutdown and decrease the refcount a
+ * second time.
+ */
+ if (!exp->user_owned) {
+ goto out;
+ }
+
exp->drv->request_shutdown(exp);
assert(exp->user_owned);
exp->user_owned = false;
blk_exp_unref(exp);
+out:
aio_context_release(aio_context);
}
@@ -199,3 +210,33 @@ void qmp_block_export_add(BlockExportOptions *export, Error **errp)
{
blk_exp_add(export, errp);
}
+
+void qmp_block_export_del(const char *id,
+ bool has_mode, BlockExportRemoveMode mode,
+ Error **errp)
+{
+ ERRP_GUARD();
+ BlockExport *exp;
+
+ exp = blk_exp_find(id);
+ if (exp == NULL) {
+ error_setg(errp, "Export '%s' is not found", id);
+ return;
+ }
+ if (!exp->user_owned) {
+ error_setg(errp, "Export '%s' is already shutting down", id);
+ return;
+ }
+
+ if (!has_mode) {
+ mode = BLOCK_EXPORT_REMOVE_MODE_SAFE;
+ }
+ if (mode == BLOCK_EXPORT_REMOVE_MODE_SAFE && exp->refcount > 1) {
+ error_setg(errp, "export '%s' still in use", exp->id);
+ error_append_hint(errp, "Use mode='hard' to force client "
+ "disconnect\n");
+ return;
+ }
+
+ blk_exp_request_shutdown(exp);
+}
diff --git a/block/monitor/block-hmp-cmds.c b/block/monitor/block-hmp-cmds.c
index db357cafcb..d15a2be827 100644
--- a/block/monitor/block-hmp-cmds.c
+++ b/block/monitor/block-hmp-cmds.c
@@ -476,8 +476,8 @@ void hmp_nbd_server_remove(Monitor *mon, const QDict *qdict)
bool force = qdict_get_try_bool(qdict, "force", false);
Error *err = NULL;
- /* Rely on NBD_SERVER_REMOVE_MODE_SAFE being the default */
- qmp_nbd_server_remove(name, force, NBD_SERVER_REMOVE_MODE_HARD, &err);
+ /* Rely on BLOCK_EXPORT_REMOVE_MODE_SAFE being the default */
+ qmp_nbd_server_remove(name, force, BLOCK_EXPORT_REMOVE_MODE_HARD, &err);
hmp_handle_error(mon, err);
}
diff --git a/blockdev-nbd.c b/blockdev-nbd.c
index 9efbaef8f7..4a9a1be571 100644
--- a/blockdev-nbd.c
+++ b/blockdev-nbd.c
@@ -307,31 +307,18 @@ fail:
}
void qmp_nbd_server_remove(const char *name,
- bool has_mode, NbdServerRemoveMode mode,
+ bool has_mode, BlockExportRemoveMode mode,
Error **errp)
{
- NBDExport *exp;
- AioContext *aio_context;
-
- if (!nbd_server) {
- error_setg(errp, "NBD server not running");
- return;
- }
+ BlockExport *exp;
- exp = nbd_export_find(name);
- if (exp == NULL) {
- error_setg(errp, "Export '%s' is not found", name);
+ exp = blk_exp_find(name);
+ if (exp && exp->drv->type != BLOCK_EXPORT_TYPE_NBD) {
+ error_setg(errp, "Block export '%s' is not an NBD export", name);
return;
}
- if (!has_mode) {
- mode = NBD_SERVER_REMOVE_MODE_SAFE;
- }
-
- aio_context = nbd_export_aio_context(exp);
- aio_context_acquire(aio_context);
- nbd_export_remove(exp, mode, errp);
- aio_context_release(aio_context);
+ qmp_block_export_del(name, has_mode, mode, errp);
}
void qmp_nbd_server_stop(Error **errp)
diff --git a/nbd/server.c b/nbd/server.c
index 22a1d66168..1a0e1db401 100644
--- a/nbd/server.c
+++ b/nbd/server.c
@@ -1669,20 +1669,6 @@ static void nbd_export_request_shutdown(BlockExport *blk_exp)
blk_exp_unref(&exp->common);
}
-void nbd_export_remove(NBDExport *exp, NbdServerRemoveMode mode, Error **errp)
-{
- ERRP_GUARD();
- if (mode == NBD_SERVER_REMOVE_MODE_HARD || QTAILQ_EMPTY(&exp->clients)) {
- nbd_export_request_shutdown(&exp->common);
- return;
- }
-
- assert(mode == NBD_SERVER_REMOVE_MODE_SAFE);
-
- error_setg(errp, "export '%s' still in use", exp->name);
- error_append_hint(errp, "Use mode='hard' to force client disconnect\n");
-}
-
static void nbd_export_delete(BlockExport *blk_exp)
{
NBDExport *exp = container_of(blk_exp, NBDExport, common);
--
2.25.4
^ permalink raw reply related [flat|nested] 44+ messages in thread
* [PATCH v2 21/31] block/export: Add BLOCK_EXPORT_DELETED event
2020-09-24 15:26 [PATCH v2 00/31] block/export: Add infrastructure and QAPI for block exports Kevin Wolf
` (19 preceding siblings ...)
2020-09-24 15:27 ` [PATCH v2 20/31] block/export: Add block-export-del Kevin Wolf
@ 2020-09-24 15:27 ` Kevin Wolf
2020-09-25 12:34 ` Max Reitz
2020-09-24 15:27 ` [PATCH v2 22/31] block/export: Move blk to BlockExport Kevin Wolf
` (12 subsequent siblings)
33 siblings, 1 reply; 44+ messages in thread
From: Kevin Wolf @ 2020-09-24 15:27 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, qemu-devel, stefanha, mreitz
Clients may want to know when an export has finally disappeard
(block-export-del returns earlier than that in the general case), so add
a QAPI event for it.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
qapi/block-export.json | 12 ++++++++++++
block/export/export.c | 2 ++
tests/qemu-iotests/140 | 9 ++++++++-
tests/qemu-iotests/140.out | 2 +-
tests/qemu-iotests/223.out | 4 ++++
5 files changed, 27 insertions(+), 2 deletions(-)
diff --git a/qapi/block-export.json b/qapi/block-export.json
index 0ba4d84c97..3551c2e748 100644
--- a/qapi/block-export.json
+++ b/qapi/block-export.json
@@ -233,3 +233,15 @@
##
{ 'command': 'block-export-del',
'data': { 'id': 'str', '*mode': 'BlockExportRemoveMode' } }
+
+##
+# @BLOCK_EXPORT_DELETED:
+#
+# Emitted when a block export is removed and its id can be reused.
+#
+# @id: Block export id.
+#
+# Since: 5.2
+##
+{ 'event': 'BLOCK_EXPORT_DELETED',
+ 'data': { 'id': 'str' } }
diff --git a/block/export/export.c b/block/export/export.c
index d186beffe9..87940d5c40 100644
--- a/block/export/export.c
+++ b/block/export/export.c
@@ -19,6 +19,7 @@
#include "block/nbd.h"
#include "qapi/error.h"
#include "qapi/qapi-commands-block-export.h"
+#include "qapi/qapi-events-block-export.h"
#include "qemu/id.h"
static const BlockExportDriver *blk_exp_drivers[] = {
@@ -113,6 +114,7 @@ static void blk_exp_delete_bh(void *opaque)
assert(exp->refcount == 0);
QLIST_REMOVE(exp, next);
exp->drv->delete(exp);
+ qapi_event_send_block_export_deleted(exp->id);
g_free(exp->id);
g_free(exp);
diff --git a/tests/qemu-iotests/140 b/tests/qemu-iotests/140
index 8d2ce5d9e3..309b177e77 100755
--- a/tests/qemu-iotests/140
+++ b/tests/qemu-iotests/140
@@ -81,10 +81,17 @@ $QEMU_IO_PROG -f raw -r -c 'read -P 42 0 64k' \
"nbd+unix:///drv?socket=$SOCK_DIR/nbd" 2>&1 \
| _filter_qemu_io | _filter_nbd
+# The order of 'return' and the BLOCK_EXPORT_DELETED event is undefined. Just
+# wait until we've twice seen one of them. Filter the 'return' line out so that
+# the output is defined.
_send_qemu_cmd $QEMU_HANDLE \
"{ 'execute': 'eject',
'arguments': { 'device': 'drv' }}" \
- 'return'
+ 'return\|BLOCK_EXPORT_DELETED' |
+ grep -v 'return'
+
+_send_qemu_cmd $QEMU_HANDLE '' 'return\|BLOCK_EXPORT_DELETED' |
+ grep -v 'return'
$QEMU_IO_PROG -f raw -r -c close \
"nbd+unix:///drv?socket=$SOCK_DIR/nbd" 2>&1 \
diff --git a/tests/qemu-iotests/140.out b/tests/qemu-iotests/140.out
index 86b985da75..62d9c3ab3c 100644
--- a/tests/qemu-iotests/140.out
+++ b/tests/qemu-iotests/140.out
@@ -11,7 +11,7 @@ wrote 65536/65536 bytes at offset 0
read 65536/65536 bytes at offset 0
64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
{ 'execute': 'eject', 'arguments': { 'device': 'drv' }}
-{"return": {}}
+{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_EXPORT_DELETED", "data": {"id": "drv"}}
qemu-io: can't open device nbd+unix:///drv?socket=SOCK_DIR/nbd: Requested export not available
server reported: export 'drv' not present
{ 'execute': 'quit' }
diff --git a/tests/qemu-iotests/223.out b/tests/qemu-iotests/223.out
index 31ce9e6fe0..f6eac23f04 100644
--- a/tests/qemu-iotests/223.out
+++ b/tests/qemu-iotests/223.out
@@ -102,8 +102,10 @@ read 2097152/2097152 bytes at offset 2097152
{"execute":"nbd-server-remove", "arguments":{"name":"n"}}
{"return": {}}
{"execute":"nbd-server-remove", "arguments":{"name":"n2"}}
+{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_EXPORT_DELETED", "data": {"id": "n"}}
{"return": {}}
{"execute":"nbd-server-remove", "arguments":{"name":"n2"}}
+{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_EXPORT_DELETED", "data": {"id": "n2"}}
{"error": {"class": "GenericError", "desc": "Export 'n2' is not found"}}
{"execute":"nbd-server-stop"}
{"return": {}}
@@ -183,8 +185,10 @@ read 2097152/2097152 bytes at offset 2097152
{"execute":"nbd-server-remove", "arguments":{"name":"n"}}
{"return": {}}
{"execute":"nbd-server-remove", "arguments":{"name":"n2"}}
+{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_EXPORT_DELETED", "data": {"id": "n"}}
{"return": {}}
{"execute":"nbd-server-remove", "arguments":{"name":"n2"}}
+{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_EXPORT_DELETED", "data": {"id": "n2"}}
{"error": {"class": "GenericError", "desc": "Export 'n2' is not found"}}
{"execute":"nbd-server-stop"}
{"return": {}}
--
2.25.4
^ permalink raw reply related [flat|nested] 44+ messages in thread
* [PATCH v2 22/31] block/export: Move blk to BlockExport
2020-09-24 15:26 [PATCH v2 00/31] block/export: Add infrastructure and QAPI for block exports Kevin Wolf
` (20 preceding siblings ...)
2020-09-24 15:27 ` [PATCH v2 21/31] block/export: Add BLOCK_EXPORT_DELETED event Kevin Wolf
@ 2020-09-24 15:27 ` Kevin Wolf
2020-09-24 15:27 ` [PATCH v2 23/31] block/export: Create BlockBackend in blk_exp_add() Kevin Wolf
` (11 subsequent siblings)
33 siblings, 0 replies; 44+ messages in thread
From: Kevin Wolf @ 2020-09-24 15:27 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, qemu-devel, stefanha, mreitz
Every block export has a BlockBackend representing the disk that is
exported. It should live in BlockExport therefore.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
---
include/block/export.h | 3 +++
block/export/export.c | 3 +++
nbd/server.c | 43 +++++++++++++++++++++---------------------
3 files changed, 28 insertions(+), 21 deletions(-)
diff --git a/include/block/export.h b/include/block/export.h
index ff54d35872..7feb02e10d 100644
--- a/include/block/export.h
+++ b/include/block/export.h
@@ -71,6 +71,9 @@ struct BlockExport {
/* The AioContext whose lock protects this BlockExport object. */
AioContext *ctx;
+ /* The block device to export */
+ BlockBackend *blk;
+
/* List entry for block_exports */
QLIST_ENTRY(BlockExport) next;
};
diff --git a/block/export/export.c b/block/export/export.c
index 87940d5c40..ad374a6649 100644
--- a/block/export/export.c
+++ b/block/export/export.c
@@ -92,6 +92,8 @@ BlockExport *blk_exp_add(BlockExportOptions *export, Error **errp)
return NULL;
}
+ assert(exp->blk != NULL);
+
QLIST_INSERT_HEAD(&block_exports, exp, next);
return exp;
}
@@ -114,6 +116,7 @@ static void blk_exp_delete_bh(void *opaque)
assert(exp->refcount == 0);
QLIST_REMOVE(exp, next);
exp->drv->delete(exp);
+ blk_unref(exp->blk);
qapi_event_send_block_export_deleted(exp->id);
g_free(exp->id);
g_free(exp);
diff --git a/nbd/server.c b/nbd/server.c
index 1a0e1db401..f9af45c480 100644
--- a/nbd/server.c
+++ b/nbd/server.c
@@ -84,7 +84,6 @@ struct NBDRequestData {
struct NBDExport {
BlockExport common;
- BlockBackend *blk;
char *name;
char *description;
uint64_t size;
@@ -643,7 +642,7 @@ static int nbd_negotiate_handle_info(NBDClient *client, Error **errp)
* whether this is OPT_INFO or OPT_GO. */
/* minimum - 1 for back-compat, or actual if client will obey it. */
if (client->opt == NBD_OPT_INFO || blocksize) {
- check_align = sizes[0] = blk_get_request_alignment(exp->blk);
+ check_align = sizes[0] = blk_get_request_alignment(exp->common.blk);
} else {
sizes[0] = 1;
}
@@ -652,7 +651,7 @@ static int nbd_negotiate_handle_info(NBDClient *client, Error **errp)
* TODO: is blk_bs(blk)->bl.opt_transfer appropriate? */
sizes[1] = MAX(4096, sizes[0]);
/* maximum - At most 32M, but smaller as appropriate. */
- sizes[2] = MIN(blk_get_max_transfer(exp->blk), NBD_MAX_BUFFER_SIZE);
+ sizes[2] = MIN(blk_get_max_transfer(exp->common.blk), NBD_MAX_BUFFER_SIZE);
trace_nbd_negotiate_handle_info_block_size(sizes[0], sizes[1], sizes[2]);
sizes[0] = cpu_to_be32(sizes[0]);
sizes[1] = cpu_to_be32(sizes[1]);
@@ -684,7 +683,7 @@ static int nbd_negotiate_handle_info(NBDClient *client, Error **errp)
* tolerate all clients, regardless of alignments.
*/
if (client->opt == NBD_OPT_INFO && !blocksize &&
- blk_get_request_alignment(exp->blk) > 1) {
+ blk_get_request_alignment(exp->common.blk) > 1) {
return nbd_negotiate_send_rep_err(client,
NBD_REP_ERR_BLOCK_SIZE_REQD,
errp,
@@ -1557,7 +1556,7 @@ int nbd_export_new(BlockExport *blk_exp, BlockDriverState *bs,
blk_set_allow_aio_context_change(blk, true);
QTAILQ_INIT(&exp->clients);
- exp->blk = blk;
+ exp->common.blk = blk;
exp->name = g_strdup(name);
assert(!desc || strlen(desc) <= NBD_MAX_STRING_SIZE);
exp->description = g_strdup(desc);
@@ -1679,15 +1678,13 @@ static void nbd_export_delete(BlockExport *blk_exp)
g_free(exp->description);
exp->description = NULL;
- if (exp->blk) {
+ if (exp->common.blk) {
if (exp->eject_notifier_blk) {
notifier_remove(&exp->eject_notifier);
blk_unref(exp->eject_notifier_blk);
}
- blk_remove_aio_context_notifier(exp->blk, blk_aio_attached,
+ blk_remove_aio_context_notifier(exp->common.blk, blk_aio_attached,
blk_aio_detach, exp);
- blk_unref(exp->blk);
- exp->blk = NULL;
}
if (exp->export_bitmap) {
@@ -1840,7 +1837,7 @@ static int coroutine_fn nbd_co_send_sparse_read(NBDClient *client,
while (progress < size) {
int64_t pnum;
- int status = bdrv_block_status_above(blk_bs(exp->blk), NULL,
+ int status = bdrv_block_status_above(blk_bs(exp->common.blk), NULL,
offset + progress,
size - progress, &pnum, NULL,
NULL);
@@ -1872,7 +1869,8 @@ static int coroutine_fn nbd_co_send_sparse_read(NBDClient *client,
stl_be_p(&chunk.length, pnum);
ret = nbd_co_send_iov(client, iov, 1, errp);
} else {
- ret = blk_pread(exp->blk, offset + progress, data + progress, pnum);
+ ret = blk_pread(exp->common.blk, offset + progress,
+ data + progress, pnum);
if (ret < 0) {
error_setg_errno(errp, -ret, "reading from file failed");
break;
@@ -2136,7 +2134,8 @@ static int nbd_co_receive_request(NBDRequestData *req, NBDRequest *request,
}
if (request->type != NBD_CMD_CACHE) {
- req->data = blk_try_blockalign(client->exp->blk, request->len);
+ req->data = blk_try_blockalign(client->exp->common.blk,
+ request->len);
if (req->data == NULL) {
error_setg(errp, "No memory");
return -ENOMEM;
@@ -2232,7 +2231,7 @@ static coroutine_fn int nbd_do_cmd_read(NBDClient *client, NBDRequest *request,
/* XXX: NBD Protocol only documents use of FUA with WRITE */
if (request->flags & NBD_CMD_FLAG_FUA) {
- ret = blk_co_flush(exp->blk);
+ ret = blk_co_flush(exp->common.blk);
if (ret < 0) {
return nbd_send_generic_reply(client, request->handle, ret,
"flush failed", errp);
@@ -2246,7 +2245,7 @@ static coroutine_fn int nbd_do_cmd_read(NBDClient *client, NBDRequest *request,
data, request->len, errp);
}
- ret = blk_pread(exp->blk, request->from, data, request->len);
+ ret = blk_pread(exp->common.blk, request->from, data, request->len);
if (ret < 0) {
return nbd_send_generic_reply(client, request->handle, ret,
"reading from file failed", errp);
@@ -2281,7 +2280,7 @@ static coroutine_fn int nbd_do_cmd_cache(NBDClient *client, NBDRequest *request,
assert(request->type == NBD_CMD_CACHE);
- ret = blk_co_preadv(exp->blk, request->from, request->len,
+ ret = blk_co_preadv(exp->common.blk, request->from, request->len,
NULL, BDRV_REQ_COPY_ON_READ | BDRV_REQ_PREFETCH);
return nbd_send_generic_reply(client, request->handle, ret,
@@ -2312,7 +2311,8 @@ static coroutine_fn int nbd_handle_request(NBDClient *client,
if (request->flags & NBD_CMD_FLAG_FUA) {
flags |= BDRV_REQ_FUA;
}
- ret = blk_pwrite(exp->blk, request->from, data, request->len, flags);
+ ret = blk_pwrite(exp->common.blk, request->from, data, request->len,
+ flags);
return nbd_send_generic_reply(client, request->handle, ret,
"writing to file failed", errp);
@@ -2333,7 +2333,7 @@ static coroutine_fn int nbd_handle_request(NBDClient *client,
int align = client->check_align ?: 1;
int len = MIN(request->len, QEMU_ALIGN_DOWN(BDRV_REQUEST_MAX_BYTES,
align));
- ret = blk_pwrite_zeroes(exp->blk, request->from, len, flags);
+ ret = blk_pwrite_zeroes(exp->common.blk, request->from, len, flags);
request->len -= len;
request->from += len;
}
@@ -2345,7 +2345,7 @@ static coroutine_fn int nbd_handle_request(NBDClient *client,
abort();
case NBD_CMD_FLUSH:
- ret = blk_co_flush(exp->blk);
+ ret = blk_co_flush(exp->common.blk);
return nbd_send_generic_reply(client, request->handle, ret,
"flush failed", errp);
@@ -2356,12 +2356,12 @@ static coroutine_fn int nbd_handle_request(NBDClient *client,
int align = client->check_align ?: 1;
int len = MIN(request->len, QEMU_ALIGN_DOWN(BDRV_REQUEST_MAX_BYTES,
align));
- ret = blk_co_pdiscard(exp->blk, request->from, len);
+ ret = blk_co_pdiscard(exp->common.blk, request->from, len);
request->len -= len;
request->from += len;
}
if (ret >= 0 && request->flags & NBD_CMD_FLAG_FUA) {
- ret = blk_co_flush(exp->blk);
+ ret = blk_co_flush(exp->common.blk);
}
return nbd_send_generic_reply(client, request->handle, ret,
"discard failed", errp);
@@ -2379,7 +2379,8 @@ static coroutine_fn int nbd_handle_request(NBDClient *client,
if (client->export_meta.base_allocation) {
ret = nbd_co_send_block_status(client, request->handle,
- blk_bs(exp->blk), request->from,
+ blk_bs(exp->common.blk),
+ request->from,
request->len, dont_fragment,
!client->export_meta.bitmap,
NBD_META_ID_BASE_ALLOCATION,
--
2.25.4
^ permalink raw reply related [flat|nested] 44+ messages in thread
* [PATCH v2 23/31] block/export: Create BlockBackend in blk_exp_add()
2020-09-24 15:26 [PATCH v2 00/31] block/export: Add infrastructure and QAPI for block exports Kevin Wolf
` (21 preceding siblings ...)
2020-09-24 15:27 ` [PATCH v2 22/31] block/export: Move blk to BlockExport Kevin Wolf
@ 2020-09-24 15:27 ` Kevin Wolf
2020-09-24 15:27 ` [PATCH v2 24/31] block/export: Add query-block-exports Kevin Wolf
` (10 subsequent siblings)
33 siblings, 0 replies; 44+ messages in thread
From: Kevin Wolf @ 2020-09-24 15:27 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, qemu-devel, stefanha, mreitz
Every export type will need a BlockBackend, so creating it centrally in
blk_exp_add() instead of the .create driver callback avoids duplication.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
---
include/block/nbd.h | 4 ++--
block/export/export.c | 49 +++++++++++++++++++++++++++++++++++++++----
blockdev-nbd.c | 33 ++++-------------------------
nbd/server.c | 38 +++++++++++----------------------
4 files changed, 63 insertions(+), 61 deletions(-)
diff --git a/include/block/nbd.h b/include/block/nbd.h
index a4dc1f9e54..5270b7eadd 100644
--- a/include/block/nbd.h
+++ b/include/block/nbd.h
@@ -332,10 +332,10 @@ typedef struct NBDClient NBDClient;
int nbd_export_create(BlockExport *exp, BlockExportOptions *exp_args,
Error **errp);
-int nbd_export_new(BlockExport *blk_exp, BlockDriverState *bs,
+int nbd_export_new(BlockExport *blk_exp,
const char *name, const char *desc,
const char *bitmap, bool readonly, bool shared,
- bool writethrough, Error **errp);
+ Error **errp);
void nbd_export_set_on_eject_blk(BlockExport *exp, BlockBackend *blk);
AioContext *nbd_export_aio_context(NBDExport *exp);
diff --git a/block/export/export.c b/block/export/export.c
index ad374a6649..8702c233f3 100644
--- a/block/export/export.c
+++ b/block/export/export.c
@@ -58,7 +58,10 @@ static const BlockExportDriver *blk_exp_find_driver(BlockExportType type)
BlockExport *blk_exp_add(BlockExportOptions *export, Error **errp)
{
const BlockExportDriver *drv;
- BlockExport *exp;
+ BlockExport *exp = NULL;
+ BlockDriverState *bs;
+ BlockBackend *blk;
+ AioContext *ctx;
int ret;
if (!id_wellformed(export->id)) {
@@ -76,6 +79,33 @@ BlockExport *blk_exp_add(BlockExportOptions *export, Error **errp)
return NULL;
}
+ bs = bdrv_lookup_bs(NULL, export->node_name, errp);
+ if (!bs) {
+ return NULL;
+ }
+
+ ctx = bdrv_get_aio_context(bs);
+ aio_context_acquire(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
+ * access since the export could be available before migration handover.
+ * ctx was acquired in the caller.
+ */
+ bdrv_invalidate_cache(bs, NULL);
+
+ blk = blk_new(ctx, BLK_PERM_CONSISTENT_READ, BLK_PERM_ALL);
+ ret = blk_insert_bs(blk, bs, errp);
+ if (ret < 0) {
+ goto fail;
+ }
+
+ if (!export->has_writethrough) {
+ export->writethrough = false;
+ }
+ blk_set_enable_write_cache(blk, !export->writethrough);
+
assert(drv->instance_size >= sizeof(BlockExport));
exp = g_malloc0(drv->instance_size);
*exp = (BlockExport) {
@@ -83,19 +113,30 @@ BlockExport *blk_exp_add(BlockExportOptions *export, Error **errp)
.refcount = 1,
.user_owned = true,
.id = g_strdup(export->id),
+ .ctx = ctx,
+ .blk = blk,
};
ret = drv->create(exp, export, errp);
if (ret < 0) {
- g_free(exp->id);
- g_free(exp);
- return NULL;
+ goto fail;
}
assert(exp->blk != NULL);
QLIST_INSERT_HEAD(&block_exports, exp, next);
+
+ aio_context_release(ctx);
return exp;
+
+fail:
+ blk_unref(blk);
+ aio_context_release(ctx);
+ if (exp) {
+ g_free(exp->id);
+ g_free(exp);
+ }
+ return NULL;
}
/* Callers must hold exp->ctx lock */
diff --git a/blockdev-nbd.c b/blockdev-nbd.c
index 4a9a1be571..cdbbcdb958 100644
--- a/blockdev-nbd.c
+++ b/blockdev-nbd.c
@@ -177,9 +177,6 @@ int nbd_export_create(BlockExport *exp, BlockExportOptions *exp_args,
Error **errp)
{
BlockExportOptionsNbd *arg = &exp_args->u.nbd;
- BlockDriverState *bs = NULL;
- AioContext *aio_context;
- int ret;
assert(exp_args->type == BLOCK_EXPORT_TYPE_NBD);
@@ -207,38 +204,16 @@ int nbd_export_create(BlockExport *exp, BlockExportOptions *exp_args,
return -EEXIST;
}
- bs = bdrv_lookup_bs(NULL, exp_args->node_name, errp);
- if (!bs) {
- return -ENOENT;
- }
-
- aio_context = bdrv_get_aio_context(bs);
- aio_context_acquire(aio_context);
-
if (!arg->has_writable) {
arg->writable = false;
}
- if (bdrv_is_read_only(bs) && arg->writable) {
- ret = -EINVAL;
+ if (blk_is_read_only(exp->blk) && arg->writable) {
error_setg(errp, "Cannot export read-only node as writable");
- goto out;
- }
-
- if (!exp_args->has_writethrough) {
- exp_args->writethrough = false;
- }
-
- ret = nbd_export_new(exp, bs, arg->name, arg->description, arg->bitmap,
- !arg->writable, !arg->writable,
- exp_args->writethrough, errp);
- if (ret < 0) {
- goto out;
+ return -EINVAL;
}
- ret = 0;
- out:
- aio_context_release(aio_context);
- return ret;
+ return nbd_export_new(exp, arg->name, arg->description, arg->bitmap,
+ !arg->writable, !arg->writable, errp);
}
void qmp_nbd_server_add(NbdServerAddOptions *arg, Error **errp)
diff --git a/nbd/server.c b/nbd/server.c
index f9af45c480..6c8532de23 100644
--- a/nbd/server.c
+++ b/nbd/server.c
@@ -1507,56 +1507,42 @@ void nbd_export_set_on_eject_blk(BlockExport *exp, BlockBackend *blk)
blk_add_remove_bs_notifier(blk, &nbd_exp->eject_notifier);
}
-int nbd_export_new(BlockExport *blk_exp, BlockDriverState *bs,
+int nbd_export_new(BlockExport *blk_exp,
const char *name, const char *desc,
const char *bitmap, bool readonly, bool shared,
- bool writethrough, Error **errp)
+ Error **errp)
{
NBDExport *exp = container_of(blk_exp, NBDExport, common);
- AioContext *ctx;
- BlockBackend *blk;
+ BlockBackend *blk = blk_exp->blk;
int64_t size;
- uint64_t perm;
+ uint64_t perm, shared_perm;
int ret;
- size = bdrv_getlength(bs);
+ size = blk_getlength(blk);
if (size < 0) {
error_setg_errno(errp, -size,
"Failed to determine the NBD export's length");
return size;
}
- ctx = bdrv_get_aio_context(bs);
- blk_exp->ctx = ctx;
-
- /*
- * NBD exports are used for non-shared storage migration. Make sure
- * that BDRV_O_INACTIVE is cleared and the image is ready for write
- * access since the export could be available before migration handover.
- * ctx was acquired in the caller.
- */
assert(name && strlen(name) <= NBD_MAX_STRING_SIZE);
- bdrv_invalidate_cache(bs, NULL);
-
/* Don't allow resize while the NBD server is running, otherwise we don't
* care what happens with the node. */
- perm = BLK_PERM_CONSISTENT_READ;
+ blk_get_perm(blk, &perm, &shared_perm);
+
if (!readonly) {
perm |= BLK_PERM_WRITE;
}
- blk = blk_new(ctx, perm,
- BLK_PERM_CONSISTENT_READ | BLK_PERM_WRITE_UNCHANGED |
- BLK_PERM_WRITE | BLK_PERM_GRAPH_MOD);
- ret = blk_insert_bs(blk, bs, errp);
+
+ ret = blk_set_perm(blk, perm, shared_perm & ~BLK_PERM_RESIZE, errp);
if (ret < 0) {
- goto fail;
+ return ret;
}
- blk_set_enable_write_cache(blk, !writethrough);
+
blk_set_allow_aio_context_change(blk, true);
QTAILQ_INIT(&exp->clients);
- exp->common.blk = blk;
exp->name = g_strdup(name);
assert(!desc || strlen(desc) <= NBD_MAX_STRING_SIZE);
exp->description = g_strdup(desc);
@@ -1574,6 +1560,7 @@ int nbd_export_new(BlockExport *blk_exp, BlockDriverState *bs,
exp->size = QEMU_ALIGN_DOWN(size, BDRV_SECTOR_SIZE);
if (bitmap) {
+ BlockDriverState *bs = blk_bs(blk);
BdrvDirtyBitmap *bm = NULL;
while (bs) {
@@ -1620,7 +1607,6 @@ int nbd_export_new(BlockExport *blk_exp, BlockDriverState *bs,
return 0;
fail:
- blk_unref(blk);
g_free(exp->name);
g_free(exp->description);
return ret;
--
2.25.4
^ permalink raw reply related [flat|nested] 44+ messages in thread
* [PATCH v2 24/31] block/export: Add query-block-exports
2020-09-24 15:26 [PATCH v2 00/31] block/export: Add infrastructure and QAPI for block exports Kevin Wolf
` (22 preceding siblings ...)
2020-09-24 15:27 ` [PATCH v2 23/31] block/export: Create BlockBackend in blk_exp_add() Kevin Wolf
@ 2020-09-24 15:27 ` Kevin Wolf
2020-09-24 15:27 ` [PATCH v2 25/31] block/export: Move writable to BlockExportOptions Kevin Wolf
` (9 subsequent siblings)
33 siblings, 0 replies; 44+ messages in thread
From: Kevin Wolf @ 2020-09-24 15:27 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, qemu-devel, stefanha, mreitz
This adds a simple QMP command to query the list of block exports.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
---
qapi/block-export.json | 32 ++++++++++++++++++++++++++++++++
block/export/export.c | 23 +++++++++++++++++++++++
2 files changed, 55 insertions(+)
diff --git a/qapi/block-export.json b/qapi/block-export.json
index 3551c2e748..28d4766215 100644
--- a/qapi/block-export.json
+++ b/qapi/block-export.json
@@ -245,3 +245,35 @@
##
{ 'event': 'BLOCK_EXPORT_DELETED',
'data': { 'id': 'str' } }
+
+##
+# @BlockExportInfo:
+#
+# Information about a single block export.
+#
+# @id: The unique identifier for the block export
+#
+# @type: The block export type
+#
+# @node-name: The node name of the block node that is exported
+#
+# @shutting-down: True if the export is shutting down (e.g. after a
+# block-export-del command, but before the shutdown has
+# completed)
+#
+# Since: 5.2
+##
+{ 'struct': 'BlockExportInfo',
+ 'data': { 'id': 'str',
+ 'type': 'BlockExportType',
+ 'node-name': 'str',
+ 'shutting-down': 'bool' } }
+
+##
+# @query-block-exports:
+#
+# Returns: A list of BlockExportInfo describing all block exports
+#
+# Since: 5.2
+##
+{ 'command': 'query-block-exports', 'returns': ['BlockExportInfo'] }
diff --git a/block/export/export.c b/block/export/export.c
index 8702c233f3..657bb58b51 100644
--- a/block/export/export.c
+++ b/block/export/export.c
@@ -286,3 +286,26 @@ void qmp_block_export_del(const char *id,
blk_exp_request_shutdown(exp);
}
+
+BlockExportInfoList *qmp_query_block_exports(Error **errp)
+{
+ BlockExportInfoList *head = NULL, **p_next = &head;
+ BlockExport *exp;
+
+ QLIST_FOREACH(exp, &block_exports, next) {
+ BlockExportInfoList *entry = g_new0(BlockExportInfoList, 1);
+ BlockExportInfo *info = g_new(BlockExportInfo, 1);
+ *info = (BlockExportInfo) {
+ .id = g_strdup(exp->id),
+ .type = exp->drv->type,
+ .node_name = g_strdup(bdrv_get_node_name(blk_bs(exp->blk))),
+ .shutting_down = !exp->user_owned,
+ };
+
+ entry->value = info;
+ *p_next = entry;
+ p_next = &entry->next;
+ }
+
+ return head;
+}
--
2.25.4
^ permalink raw reply related [flat|nested] 44+ messages in thread
* [PATCH v2 25/31] block/export: Move writable to BlockExportOptions
2020-09-24 15:26 [PATCH v2 00/31] block/export: Add infrastructure and QAPI for block exports Kevin Wolf
` (23 preceding siblings ...)
2020-09-24 15:27 ` [PATCH v2 24/31] block/export: Add query-block-exports Kevin Wolf
@ 2020-09-24 15:27 ` Kevin Wolf
2020-09-24 15:27 ` [PATCH v2 26/31] nbd: Merge nbd_export_new() and nbd_export_create() Kevin Wolf
` (8 subsequent siblings)
33 siblings, 0 replies; 44+ messages in thread
From: Kevin Wolf @ 2020-09-24 15:27 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, qemu-devel, stefanha, mreitz
The 'writable' option is a basic option that will probably be applicable
to most if not all export types that we will implement. Move it from NBD
to the generic BlockExport layer.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
---
qapi/block-export.json | 15 ++++++++++-----
block/export/export.c | 16 +++++++++++++++-
blockdev-nbd.c | 18 +++++-------------
nbd/server.c | 5 -----
qemu-nbd.c | 4 ++--
5 files changed, 32 insertions(+), 26 deletions(-)
diff --git a/qapi/block-export.json b/qapi/block-export.json
index 28d4766215..ad01e9f8aa 100644
--- a/qapi/block-export.json
+++ b/qapi/block-export.json
@@ -74,9 +74,6 @@
# @description: Free-form description of the export, up to 4096 bytes.
# (Since 5.0)
#
-# @writable: Whether clients should be able to write to the device via the
-# NBD connection (default false).
-#
# @bitmap: Also export the dirty bitmap reachable from @device, so the
# NBD client can use NBD_OPT_SET_META_CONTEXT with
# "qemu:dirty-bitmap:NAME" to inspect the bitmap. (since 4.0)
@@ -85,7 +82,7 @@
##
{ 'struct': 'BlockExportOptionsNbd',
'data': { '*name': 'str', '*description': 'str',
- '*writable': 'bool', '*bitmap': 'str' } }
+ '*bitmap': 'str' } }
##
# @NbdServerAddOptions:
@@ -94,11 +91,15 @@
#
# @device: The device name or node name of the node to be exported
#
+# @writable: Whether clients should be able to write to the device via the
+# NBD connection (default false).
+#
# Since: 5.0
##
{ 'struct': 'NbdServerAddOptions',
'base': 'BlockExportOptionsNbd',
- 'data': { 'device': 'str' } }
+ 'data': { 'device': 'str',
+ '*writable': 'bool' } }
##
# @nbd-server-add:
@@ -188,6 +189,9 @@
#
# @node-name: The node name of the block node to be exported (since: 5.2)
#
+# @writable: True if clients should be able to write to the export
+# (default false)
+#
# @writethrough: If true, caches are flushed after every write request to the
# export before completion is signalled. (since: 5.2;
# default: false)
@@ -198,6 +202,7 @@
'base': { 'type': 'BlockExportType',
'id': 'str',
'node-name': 'str',
+ '*writable': 'bool',
'*writethrough': 'bool' },
'discriminator': 'type',
'data': {
diff --git a/block/export/export.c b/block/export/export.c
index 657bb58b51..f2c00d13bf 100644
--- a/block/export/export.c
+++ b/block/export/export.c
@@ -62,6 +62,7 @@ BlockExport *blk_exp_add(BlockExportOptions *export, Error **errp)
BlockDriverState *bs;
BlockBackend *blk;
AioContext *ctx;
+ uint64_t perm;
int ret;
if (!id_wellformed(export->id)) {
@@ -84,6 +85,14 @@ BlockExport *blk_exp_add(BlockExportOptions *export, Error **errp)
return NULL;
}
+ if (!export->has_writable) {
+ export->writable = false;
+ }
+ if (bdrv_is_read_only(bs) && export->writable) {
+ error_setg(errp, "Cannot export read-only node as writable");
+ return NULL;
+ }
+
ctx = bdrv_get_aio_context(bs);
aio_context_acquire(ctx);
@@ -95,7 +104,12 @@ BlockExport *blk_exp_add(BlockExportOptions *export, Error **errp)
*/
bdrv_invalidate_cache(bs, NULL);
- blk = blk_new(ctx, BLK_PERM_CONSISTENT_READ, BLK_PERM_ALL);
+ perm = BLK_PERM_CONSISTENT_READ;
+ if (export->writable) {
+ perm |= BLK_PERM_WRITE;
+ }
+
+ blk = blk_new(ctx, perm, BLK_PERM_ALL);
ret = blk_insert_bs(blk, bs, errp);
if (ret < 0) {
goto fail;
diff --git a/blockdev-nbd.c b/blockdev-nbd.c
index cdbbcdb958..30e165c23f 100644
--- a/blockdev-nbd.c
+++ b/blockdev-nbd.c
@@ -204,16 +204,8 @@ int nbd_export_create(BlockExport *exp, BlockExportOptions *exp_args,
return -EEXIST;
}
- if (!arg->has_writable) {
- arg->writable = false;
- }
- if (blk_is_read_only(exp->blk) && arg->writable) {
- error_setg(errp, "Cannot export read-only node as writable");
- return -EINVAL;
- }
-
return nbd_export_new(exp, arg->name, arg->description, arg->bitmap,
- !arg->writable, !arg->writable, errp);
+ !exp_args->writable, !exp_args->writable, errp);
}
void qmp_nbd_server_add(NbdServerAddOptions *arg, Error **errp)
@@ -241,13 +233,13 @@ void qmp_nbd_server_add(NbdServerAddOptions *arg, Error **errp)
.type = BLOCK_EXPORT_TYPE_NBD,
.id = g_strdup(arg->name),
.node_name = g_strdup(bdrv_get_node_name(bs)),
+ .has_writable = arg->has_writable,
+ .writable = arg->writable,
.u.nbd = {
.has_name = true,
.name = g_strdup(arg->name),
.has_description = arg->has_description,
.description = g_strdup(arg->description),
- .has_writable = arg->has_writable,
- .writable = arg->writable,
.has_bitmap = arg->has_bitmap,
.bitmap = g_strdup(arg->bitmap),
},
@@ -259,8 +251,8 @@ void qmp_nbd_server_add(NbdServerAddOptions *arg, Error **errp)
* block-export-add.
*/
if (bdrv_is_read_only(bs)) {
- export_opts->u.nbd.has_writable = true;
- export_opts->u.nbd.writable = false;
+ export_opts->has_writable = true;
+ export_opts->writable = false;
}
export = blk_exp_add(export_opts, errp);
diff --git a/nbd/server.c b/nbd/server.c
index 6c8532de23..465ec9e762 100644
--- a/nbd/server.c
+++ b/nbd/server.c
@@ -1530,11 +1530,6 @@ int nbd_export_new(BlockExport *blk_exp,
/* Don't allow resize while the NBD server is running, otherwise we don't
* care what happens with the node. */
blk_get_perm(blk, &perm, &shared_perm);
-
- if (!readonly) {
- perm |= BLK_PERM_WRITE;
- }
-
ret = blk_set_perm(blk, perm, shared_perm & ~BLK_PERM_RESIZE, errp);
if (ret < 0) {
return ret;
diff --git a/qemu-nbd.c b/qemu-nbd.c
index ac82acb4ac..6d7ac7490f 100644
--- a/qemu-nbd.c
+++ b/qemu-nbd.c
@@ -1068,13 +1068,13 @@ int main(int argc, char **argv)
.node_name = g_strdup(bdrv_get_node_name(bs)),
.has_writethrough = true,
.writethrough = writethrough,
+ .has_writable = true,
+ .writable = !readonly,
.u.nbd = {
.has_name = true,
.name = g_strdup(export_name),
.has_description = !!export_description,
.description = g_strdup(export_description),
- .has_writable = true,
- .writable = !readonly,
.has_bitmap = !!bitmap,
.bitmap = g_strdup(bitmap),
},
--
2.25.4
^ permalink raw reply related [flat|nested] 44+ messages in thread
* [PATCH v2 26/31] nbd: Merge nbd_export_new() and nbd_export_create()
2020-09-24 15:26 [PATCH v2 00/31] block/export: Add infrastructure and QAPI for block exports Kevin Wolf
` (24 preceding siblings ...)
2020-09-24 15:27 ` [PATCH v2 25/31] block/export: Move writable to BlockExportOptions Kevin Wolf
@ 2020-09-24 15:27 ` Kevin Wolf
2020-09-25 12:42 ` Max Reitz
2020-09-24 15:27 ` [PATCH v2 27/31] nbd: Deprecate nbd-server-add/remove Kevin Wolf
` (7 subsequent siblings)
33 siblings, 1 reply; 44+ messages in thread
From: Kevin Wolf @ 2020-09-24 15:27 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, qemu-devel, stefanha, mreitz
There is no real reason any more why nbd_export_new() and
nbd_export_create() should be separate functions. The latter only
performs a few checks before it calls the former.
What makes the current state stand out is that it's the only function in
BlockExportDriver that is not a static function inside nbd/server.c, but
a small wrapper in blockdev-nbd.c that then calls back into nbd/server.c
for the real functionality.
Move all the checks to nbd/server.c and make the resulting function
static to improve readability.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
---
include/block/nbd.h | 7 +-----
blockdev-nbd.c | 40 +++++----------------------------
nbd/server.c | 54 ++++++++++++++++++++++++++++++++-------------
3 files changed, 45 insertions(+), 56 deletions(-)
diff --git a/include/block/nbd.h b/include/block/nbd.h
index 5270b7eadd..3dd9a04546 100644
--- a/include/block/nbd.h
+++ b/include/block/nbd.h
@@ -330,12 +330,6 @@ int nbd_errno_to_system_errno(int err);
typedef struct NBDExport NBDExport;
typedef struct NBDClient NBDClient;
-int nbd_export_create(BlockExport *exp, BlockExportOptions *exp_args,
- Error **errp);
-int nbd_export_new(BlockExport *blk_exp,
- const char *name, const char *desc,
- const char *bitmap, bool readonly, bool shared,
- Error **errp);
void nbd_export_set_on_eject_blk(BlockExport *exp, BlockBackend *blk);
AioContext *nbd_export_aio_context(NBDExport *exp);
@@ -349,6 +343,7 @@ void nbd_client_get(NBDClient *client);
void nbd_client_put(NBDClient *client);
void nbd_server_is_qemu_nbd(bool value);
+bool nbd_server_is_running(void);
void nbd_server_start(SocketAddress *addr, const char *tls_creds,
const char *tls_authz, uint32_t max_connections,
Error **errp);
diff --git a/blockdev-nbd.c b/blockdev-nbd.c
index 30e165c23f..8174023e5c 100644
--- a/blockdev-nbd.c
+++ b/blockdev-nbd.c
@@ -37,6 +37,11 @@ void nbd_server_is_qemu_nbd(bool value)
is_qemu_nbd = value;
}
+bool nbd_server_is_running(void)
+{
+ return nbd_server || is_qemu_nbd;
+}
+
static void nbd_blockdev_client_closed(NBDClient *client, bool ignored)
{
nbd_client_put(client);
@@ -173,41 +178,6 @@ void qmp_nbd_server_start(SocketAddressLegacy *addr,
qapi_free_SocketAddress(addr_flat);
}
-int nbd_export_create(BlockExport *exp, BlockExportOptions *exp_args,
- Error **errp)
-{
- BlockExportOptionsNbd *arg = &exp_args->u.nbd;
-
- assert(exp_args->type == BLOCK_EXPORT_TYPE_NBD);
-
- if (!nbd_server && !is_qemu_nbd) {
- error_setg(errp, "NBD server not running");
- return -EINVAL;
- }
-
- if (!arg->has_name) {
- arg->name = exp_args->node_name;
- }
-
- if (strlen(arg->name) > NBD_MAX_STRING_SIZE) {
- error_setg(errp, "export name '%s' too long", arg->name);
- return -EINVAL;
- }
-
- if (arg->description && strlen(arg->description) > NBD_MAX_STRING_SIZE) {
- error_setg(errp, "description '%s' too long", arg->description);
- return -EINVAL;
- }
-
- if (nbd_export_find(arg->name)) {
- error_setg(errp, "NBD server already has export named '%s'", arg->name);
- return -EEXIST;
- }
-
- return nbd_export_new(exp, arg->name, arg->description, arg->bitmap,
- !exp_args->writable, !exp_args->writable, errp);
-}
-
void qmp_nbd_server_add(NbdServerAddOptions *arg, Error **errp)
{
BlockExport *export;
diff --git a/nbd/server.c b/nbd/server.c
index 465ec9e762..f74766add7 100644
--- a/nbd/server.c
+++ b/nbd/server.c
@@ -1507,17 +1507,44 @@ void nbd_export_set_on_eject_blk(BlockExport *exp, BlockBackend *blk)
blk_add_remove_bs_notifier(blk, &nbd_exp->eject_notifier);
}
-int nbd_export_new(BlockExport *blk_exp,
- const char *name, const char *desc,
- const char *bitmap, bool readonly, bool shared,
- Error **errp)
+static int nbd_export_create(BlockExport *blk_exp, BlockExportOptions *exp_args,
+ Error **errp)
{
NBDExport *exp = container_of(blk_exp, NBDExport, common);
+ BlockExportOptionsNbd *arg = &exp_args->u.nbd;
BlockBackend *blk = blk_exp->blk;
int64_t size;
uint64_t perm, shared_perm;
+ bool readonly = !exp_args->writable;
+ bool shared = !exp_args->writable;
int ret;
+ assert(exp_args->type == BLOCK_EXPORT_TYPE_NBD);
+
+ if (!nbd_server_is_running()) {
+ error_setg(errp, "NBD server not running");
+ return -EINVAL;
+ }
+
+ if (!arg->has_name) {
+ arg->name = exp_args->node_name;
+ }
+
+ if (strlen(arg->name) > NBD_MAX_STRING_SIZE) {
+ error_setg(errp, "export name '%s' too long", arg->name);
+ return -EINVAL;
+ }
+
+ if (arg->description && strlen(arg->description) > NBD_MAX_STRING_SIZE) {
+ error_setg(errp, "description '%s' too long", arg->description);
+ return -EINVAL;
+ }
+
+ if (nbd_export_find(arg->name)) {
+ error_setg(errp, "NBD server already has export named '%s'", arg->name);
+ return -EEXIST;
+ }
+
size = blk_getlength(blk);
if (size < 0) {
error_setg_errno(errp, -size,
@@ -1525,8 +1552,6 @@ int nbd_export_new(BlockExport *blk_exp,
return size;
}
- assert(name && strlen(name) <= NBD_MAX_STRING_SIZE);
-
/* Don't allow resize while the NBD server is running, otherwise we don't
* care what happens with the node. */
blk_get_perm(blk, &perm, &shared_perm);
@@ -1538,9 +1563,8 @@ int nbd_export_new(BlockExport *blk_exp,
blk_set_allow_aio_context_change(blk, true);
QTAILQ_INIT(&exp->clients);
- exp->name = g_strdup(name);
- assert(!desc || strlen(desc) <= NBD_MAX_STRING_SIZE);
- exp->description = g_strdup(desc);
+ exp->name = g_strdup(arg->name);
+ exp->description = g_strdup(arg->description);
exp->nbdflags = (NBD_FLAG_HAS_FLAGS | NBD_FLAG_SEND_FLUSH |
NBD_FLAG_SEND_FUA | NBD_FLAG_SEND_CACHE);
if (readonly) {
@@ -1554,12 +1578,12 @@ int nbd_export_new(BlockExport *blk_exp,
}
exp->size = QEMU_ALIGN_DOWN(size, BDRV_SECTOR_SIZE);
- if (bitmap) {
+ if (arg->bitmap) {
BlockDriverState *bs = blk_bs(blk);
BdrvDirtyBitmap *bm = NULL;
while (bs) {
- bm = bdrv_find_dirty_bitmap(bs, bitmap);
+ bm = bdrv_find_dirty_bitmap(bs, arg->bitmap);
if (bm != NULL) {
break;
}
@@ -1569,7 +1593,7 @@ int nbd_export_new(BlockExport *blk_exp,
if (bm == NULL) {
ret = -ENOENT;
- error_setg(errp, "Bitmap '%s' is not found", bitmap);
+ error_setg(errp, "Bitmap '%s' is not found", arg->bitmap);
goto fail;
}
@@ -1583,15 +1607,15 @@ int nbd_export_new(BlockExport *blk_exp,
ret = -EINVAL;
error_setg(errp,
"Enabled bitmap '%s' incompatible with readonly export",
- bitmap);
+ arg->bitmap);
goto fail;
}
bdrv_dirty_bitmap_set_busy(bm, true);
exp->export_bitmap = bm;
- assert(strlen(bitmap) <= BDRV_BITMAP_MAX_NAME_SIZE);
+ assert(strlen(arg->bitmap) <= BDRV_BITMAP_MAX_NAME_SIZE);
exp->export_bitmap_context = g_strdup_printf("qemu:dirty-bitmap:%s",
- bitmap);
+ arg->bitmap);
assert(strlen(exp->export_bitmap_context) < NBD_MAX_STRING_SIZE);
}
--
2.25.4
^ permalink raw reply related [flat|nested] 44+ messages in thread
* [PATCH v2 27/31] nbd: Deprecate nbd-server-add/remove
2020-09-24 15:26 [PATCH v2 00/31] block/export: Add infrastructure and QAPI for block exports Kevin Wolf
` (25 preceding siblings ...)
2020-09-24 15:27 ` [PATCH v2 26/31] nbd: Merge nbd_export_new() and nbd_export_create() Kevin Wolf
@ 2020-09-24 15:27 ` Kevin Wolf
2020-09-25 12:43 ` Max Reitz
2020-09-24 15:27 ` [PATCH v2 28/31] iotests: Factor out qemu_tool_pipe_and_status() Kevin Wolf
` (6 subsequent siblings)
33 siblings, 1 reply; 44+ messages in thread
From: Kevin Wolf @ 2020-09-24 15:27 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, qemu-devel, stefanha, mreitz
These QMP commands are replaced by block-export-add/del.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
qapi/block-export.json | 11 +++++++++--
docs/system/deprecated.rst | 6 ++++++
2 files changed, 15 insertions(+), 2 deletions(-)
diff --git a/qapi/block-export.json b/qapi/block-export.json
index ad01e9f8aa..2291d6cb0c 100644
--- a/qapi/block-export.json
+++ b/qapi/block-export.json
@@ -108,13 +108,16 @@
#
# The export name will be used as the id for the resulting block export.
#
+# Features:
+# @deprecated: This command is deprecated. Use @block-export-add instead.
+#
# Returns: error if the server is not running, or export with the same name
# already exists.
#
# Since: 1.3.0
##
{ 'command': 'nbd-server-add',
- 'data': 'NbdServerAddOptions', 'boxed': true }
+ 'data': 'NbdServerAddOptions', 'boxed': true, 'features': ['deprecated'] }
##
# @BlockExportRemoveMode:
@@ -147,6 +150,9 @@
# @mode: Mode of command operation. See @BlockExportRemoveMode description.
# Default is 'safe'.
#
+# Features:
+# @deprecated: This command is deprecated. Use @block-export-del instead.
+#
# Returns: error if
# - the server is not running
# - export is not found
@@ -155,7 +161,8 @@
# Since: 2.12
##
{ 'command': 'nbd-server-remove',
- 'data': {'name': 'str', '*mode': 'BlockExportRemoveMode'} }
+ 'data': {'name': 'str', '*mode': 'BlockExportRemoveMode'},
+ 'features': ['deprecated'] }
##
# @nbd-server-stop:
diff --git a/docs/system/deprecated.rst b/docs/system/deprecated.rst
index 808c334fe7..71276c08f3 100644
--- a/docs/system/deprecated.rst
+++ b/docs/system/deprecated.rst
@@ -303,6 +303,12 @@ chardev client socket with ``wait`` option (since 4.0)
Character devices creating sockets in client mode should not specify
the 'wait' field, which is only applicable to sockets in server mode
+``nbd-server-add`` and ``nbd-server-remove`` (since 5.2)
+''''''''''''''''''''''''''''''''''''''''''''''''''''''''
+
+Use the more generic commands ``block-export-add`` and ``block-export-del``
+instead.
+
Human Monitor Protocol (HMP) commands
-------------------------------------
--
2.25.4
^ permalink raw reply related [flat|nested] 44+ messages in thread
* [PATCH v2 28/31] iotests: Factor out qemu_tool_pipe_and_status()
2020-09-24 15:26 [PATCH v2 00/31] block/export: Add infrastructure and QAPI for block exports Kevin Wolf
` (26 preceding siblings ...)
2020-09-24 15:27 ` [PATCH v2 27/31] nbd: Deprecate nbd-server-add/remove Kevin Wolf
@ 2020-09-24 15:27 ` Kevin Wolf
2020-09-25 12:48 ` Max Reitz
2020-09-24 15:27 ` [PATCH v2 29/31] iotests: Introduce qemu_nbd_list_log() Kevin Wolf
` (5 subsequent siblings)
33 siblings, 1 reply; 44+ messages in thread
From: Kevin Wolf @ 2020-09-24 15:27 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, qemu-devel, stefanha, mreitz
We have three almost identical functions that call an external process
and return its output and return code. Refactor them into small wrappers
around a common function.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
tests/qemu-iotests/iotests.py | 49 ++++++++++++++++-------------------
1 file changed, 23 insertions(+), 26 deletions(-)
diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index 91e4a57126..81edf8adbf 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -90,21 +90,30 @@ luks_default_secret_object = 'secret,id=keysec0,data=' + \
luks_default_key_secret_opt = 'key-secret=keysec0'
-def qemu_img_pipe_and_status(*args: str) -> Tuple[str, int]:
+def qemu_tool_pipe_and_status(tool: str, args: Sequence[str],
+ connect_stderr: bool = True) -> Tuple[str, int]:
"""
- Run qemu-img and return both its output and its exit code
+ Run a tool and return both its output and its exit code
"""
- subp = subprocess.Popen(qemu_img_args + list(args),
+ stderr = subprocess.STDOUT if connect_stderr else None
+ subp = subprocess.Popen(args,
stdout=subprocess.PIPE,
- stderr=subprocess.STDOUT,
+ stderr=stderr,
universal_newlines=True)
output = subp.communicate()[0]
if subp.returncode < 0:
- sys.stderr.write('qemu-img received signal %i: %s\n'
- % (-subp.returncode,
+ sys.stderr.write('%s received signal %i: %s\n'
+ % (tool, -subp.returncode,
' '.join(qemu_img_args + list(args))))
return (output, subp.returncode)
+def qemu_img_pipe_and_status(*args: str) -> Tuple[str, int]:
+ """
+ Run qemu-img and return both its output and its exit code
+ """
+ full_args = qemu_img_args + list(args)
+ return qemu_tool_pipe_and_status('qemu-img', full_args)
+
def qemu_img(*args: str) -> int:
'''Run qemu-img and return the exit code'''
return qemu_img_pipe_and_status(*args)[1]
@@ -265,19 +274,13 @@ def qemu_nbd(*args):
'''Run qemu-nbd in daemon mode and return the parent's exit code'''
return subprocess.call(qemu_nbd_args + ['--fork'] + list(args))
-def qemu_nbd_early_pipe(*args):
+def qemu_nbd_early_pipe(*args: str) -> Tuple[int, str]:
'''Run qemu-nbd in daemon mode and return both the parent's exit code
and its output in case of an error'''
- subp = subprocess.Popen(qemu_nbd_args + ['--fork'] + list(args),
- stdout=subprocess.PIPE,
- universal_newlines=True)
- output = subp.communicate()[0]
- if subp.returncode < 0:
- sys.stderr.write('qemu-nbd received signal %i: %s\n' %
- (-subp.returncode,
- ' '.join(qemu_nbd_args + ['--fork'] + list(args))))
-
- return subp.returncode, output if subp.returncode else ''
+ full_args = qemu_nbd_args + ['--fork'] + list(args)
+ output, returncode = qemu_tool_pipe_and_status('qemu-nbd', full_args,
+ connect_stderr=False)
+ return returncode, output if returncode else ''
@contextmanager
def qemu_nbd_popen(*args):
@@ -1143,20 +1146,14 @@ def verify_working_luks():
if not working:
notrun(reason)
-def qemu_pipe(*args):
+def qemu_pipe(*args: str) -> str:
"""
Run qemu with an option to print something and exit (e.g. a help option).
:return: QEMU's stdout output.
"""
- args = [qemu_prog] + qemu_opts + list(args)
- subp = subprocess.Popen(args, stdout=subprocess.PIPE,
- stderr=subprocess.STDOUT,
- universal_newlines=True)
- output = subp.communicate()[0]
- if subp.returncode < 0:
- sys.stderr.write('qemu received signal %i: %s\n' %
- (-subp.returncode, ' '.join(args)))
+ full_args = [qemu_prog] + qemu_opts + list(args)
+ output, _ = qemu_tool_pipe_and_status('qemu', full_args)
return output
def supported_formats(read_only=False):
--
2.25.4
^ permalink raw reply related [flat|nested] 44+ messages in thread
* [PATCH v2 29/31] iotests: Introduce qemu_nbd_list_log()
2020-09-24 15:26 [PATCH v2 00/31] block/export: Add infrastructure and QAPI for block exports Kevin Wolf
` (27 preceding siblings ...)
2020-09-24 15:27 ` [PATCH v2 28/31] iotests: Factor out qemu_tool_pipe_and_status() Kevin Wolf
@ 2020-09-24 15:27 ` Kevin Wolf
2020-09-25 12:55 ` Max Reitz
2020-09-24 15:27 ` [PATCH v2 30/31] iotests: Allow supported and unsupported formats at the same time Kevin Wolf
` (4 subsequent siblings)
33 siblings, 1 reply; 44+ messages in thread
From: Kevin Wolf @ 2020-09-24 15:27 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, qemu-devel, stefanha, mreitz
Add a function to list the NBD exports offered by an NBD server.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
tests/qemu-iotests/iotests.py | 13 ++++++++++++-
1 file changed, 12 insertions(+), 1 deletion(-)
diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index 81edf8adbf..cb9f90f737 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -67,7 +67,8 @@ if os.environ.get('QEMU_IO_OPTIONS_NO_FMT'):
qemu_io_args_no_fmt += \
os.environ['QEMU_IO_OPTIONS_NO_FMT'].strip().split(' ')
-qemu_nbd_args = [os.environ.get('QEMU_NBD_PROG', 'qemu-nbd')]
+qemu_nbd_prog = os.environ.get('QEMU_NBD_PROG', 'qemu-nbd')
+qemu_nbd_args = [qemu_nbd_prog]
if os.environ.get('QEMU_NBD_OPTIONS'):
qemu_nbd_args += os.environ['QEMU_NBD_OPTIONS'].strip().split(' ')
@@ -282,6 +283,13 @@ def qemu_nbd_early_pipe(*args: str) -> Tuple[int, str]:
connect_stderr=False)
return returncode, output if returncode else ''
+def qemu_nbd_list_log(*args: str) -> str:
+ '''Run qemu-nbd to list remote exports'''
+ full_args = [qemu_nbd_prog, '-L'] + list(args)
+ output, _ = qemu_tool_pipe_and_status('qemu-nbd', full_args)
+ log(output, filters=[filter_testfiles, filter_nbd_exports])
+ return output
+
@contextmanager
def qemu_nbd_popen(*args):
'''Context manager running qemu-nbd within the context'''
@@ -415,6 +423,9 @@ def filter_qmp_imgfmt(qmsg):
return value
return filter_qmp(qmsg, _filter)
+def filter_nbd_exports(output: str) -> str:
+ return re.sub(r'((min|opt|max) block): [0-9]+', r'\1: XXX', output)
+
Msg = TypeVar('Msg', Dict[str, Any], List[Any], str)
--
2.25.4
^ permalink raw reply related [flat|nested] 44+ messages in thread
* [PATCH v2 30/31] iotests: Allow supported and unsupported formats at the same time
2020-09-24 15:26 [PATCH v2 00/31] block/export: Add infrastructure and QAPI for block exports Kevin Wolf
` (28 preceding siblings ...)
2020-09-24 15:27 ` [PATCH v2 29/31] iotests: Introduce qemu_nbd_list_log() Kevin Wolf
@ 2020-09-24 15:27 ` Kevin Wolf
2020-09-25 12:59 ` Max Reitz
2020-09-24 15:27 ` [PATCH v2 31/31] iotests: Test block-export-* QMP interface Kevin Wolf
` (3 subsequent siblings)
33 siblings, 1 reply; 44+ messages in thread
From: Kevin Wolf @ 2020-09-24 15:27 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, qemu-devel, stefanha, mreitz
This is useful for specifying 'generic' as supported (which includes
only writable image formats), but still excluding some incompatible
writable formats.
It also removes more lines than it adds.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
tests/qemu-iotests/iotests.py | 6 +-----
1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index cb9f90f737..9c33689cba 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -1064,16 +1064,12 @@ def case_notrun(reason):
def _verify_image_format(supported_fmts: Sequence[str] = (),
unsupported_fmts: Sequence[str] = ()) -> None:
- assert not (supported_fmts and unsupported_fmts)
-
if 'generic' in supported_fmts and \
os.environ.get('IMGFMT_GENERIC', 'true') == 'true':
# similar to
# _supported_fmt generic
# for bash tests
- if imgfmt == 'luks':
- verify_working_luks()
- return
+ supported_fmts = ()
not_sup = supported_fmts and (imgfmt not in supported_fmts)
if not_sup or (imgfmt in unsupported_fmts):
--
2.25.4
^ permalink raw reply related [flat|nested] 44+ messages in thread
* [PATCH v2 31/31] iotests: Test block-export-* QMP interface
2020-09-24 15:26 [PATCH v2 00/31] block/export: Add infrastructure and QAPI for block exports Kevin Wolf
` (29 preceding siblings ...)
2020-09-24 15:27 ` [PATCH v2 30/31] iotests: Allow supported and unsupported formats at the same time Kevin Wolf
@ 2020-09-24 15:27 ` Kevin Wolf
2020-09-25 13:02 ` Max Reitz
2020-09-24 17:21 ` [PATCH v2 00/31] block/export: Add infrastructure and QAPI for block exports no-reply
` (2 subsequent siblings)
33 siblings, 1 reply; 44+ messages in thread
From: Kevin Wolf @ 2020-09-24 15:27 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, qemu-devel, stefanha, mreitz
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
tests/qemu-iotests/307 | 132 +++++++++++++++++++++++++++++++++++++
tests/qemu-iotests/307.out | 124 ++++++++++++++++++++++++++++++++++
tests/qemu-iotests/group | 1 +
3 files changed, 257 insertions(+)
create mode 100755 tests/qemu-iotests/307
create mode 100644 tests/qemu-iotests/307.out
diff --git a/tests/qemu-iotests/307 b/tests/qemu-iotests/307
new file mode 100755
index 0000000000..de7c25fcfc
--- /dev/null
+++ b/tests/qemu-iotests/307
@@ -0,0 +1,132 @@
+#!/usr/bin/env python3
+#
+# Copyright (C) 2020 Red Hat, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program 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 General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program. If not, see <http://www.gnu.org/licenses/>.
+#
+# Creator/Owner: Kevin Wolf <kwolf@redhat.com>
+#
+# Test the block export QAPI interfaces
+
+import iotests
+import os
+
+# Need a writable image format (but not vpc, which rounds the image size, nor
+# luks which requires special command lines)
+iotests.script_initialize(
+ supported_fmts=['generic'],
+ unsupported_fmts=['luks', 'vpc'],
+ supported_platforms=['linux'],
+)
+
+with iotests.FilePath('image') as img, \
+ iotests.FilePath('socket', base_dir=iotests.sock_dir) as socket, \
+ iotests.VM() as vm:
+
+ iotests.qemu_img('create', '-f', iotests.imgfmt, img, '64M')
+ iotests.qemu_io_log('-f', iotests.imgfmt, '-c', 'write -P 0x11 0 4k', img)
+
+ iotests.log('=== Launch VM ===')
+
+ virtio_scsi_device = iotests.get_virtio_scsi_device()
+
+ vm.add_object('iothread,id=iothread0')
+ vm.add_blockdev(f'file,filename={img},node-name=file')
+ vm.add_blockdev(f'{iotests.imgfmt},file=file,node-name=fmt')
+ vm.add_blockdev('raw,file=file,node-name=ro,read-only=on')
+ vm.add_device(f'id=scsi0,driver={virtio_scsi_device},iothread=iothread0')
+ vm.launch()
+
+ vm.qmp_log('nbd-server-start',
+ addr={'type': 'unix', 'data': {'path': socket}},
+ filters=(iotests.filter_qmp_testfiles, ))
+ vm.qmp_log('query-block-exports')
+
+ iotests.log('\n=== Create a read-only NBD export ===')
+
+ vm.qmp_log('block-export-add', id='export0', type='nbd', node_name='fmt')
+ vm.qmp_log('query-block-exports')
+
+ iotests.qemu_nbd_list_log('-k', socket)
+
+ iotests.log('\n=== Try a few invalid things ===')
+
+ vm.qmp_log('block-export-add', id='#invalid', type='nbd', node_name='fmt')
+ vm.qmp_log('block-export-add', id='export0', type='nbd', node_name='fmt')
+ vm.qmp_log('block-export-add', id='export1', type='nbd', node_name='ro',
+ writable=True)
+ vm.qmp_log('block-export-del', id='export1')
+ vm.qmp_log('query-block-exports')
+
+ iotests.log('\n=== Move export to an iothread ===')
+
+ vm.qmp_log('device_add', id='sda', driver='scsi-hd', drive='fmt')
+ vm.qmp_log('query-block-exports')
+ iotests.qemu_nbd_list_log('-k', socket)
+
+ iotests.log('\n=== Add a writable export ===')
+
+ # This fails because share-rw=off
+ vm.qmp_log('block-export-add', id='export1', type='nbd', node_name='fmt',
+ name='export1', writable=True, writethrough=True,
+ description='This is the writable second export')
+
+ vm.qmp_log('device_del', id='sda')
+ event = vm.event_wait(name="DEVICE_DELETED",
+ match={'data': {'device': 'sda'}})
+ iotests.log(event, filters=[iotests.filter_qmp_event])
+ vm.qmp_log('device_add', id='sda', driver='scsi-hd', drive='fmt',
+ share_rw=True)
+
+ # Now it should work
+ vm.qmp_log('block-export-add', id='export1', type='nbd', node_name='fmt',
+ name='export1', writable=True, writethrough=True,
+ description='This is the writable second export')
+
+ vm.qmp_log('query-block-exports')
+ iotests.qemu_nbd_list_log('-k', socket)
+
+ iotests.log('\n=== Connect qemu-io to export1, try removing exports ===')
+
+ nbd_url = f'nbd+unix:///export1?socket={socket}'
+ qemu_io = iotests.QemuIoInteractive('-f', 'raw', nbd_url)
+
+ iotests.log(qemu_io.cmd('read -P 0x11 0 4k'),
+ filters=[iotests.filter_qemu_io])
+ iotests.log(qemu_io.cmd('write -P 0x22 4k 4k'),
+ filters=[iotests.filter_qemu_io])
+
+ vm.qmp_log('block-export-del', id='export1')
+ vm.qmp_log('block-export-del', id='export0')
+ iotests.log(vm.get_qmp_events_filtered())
+ qemu_io.close()
+
+ vm.qmp_log('query-block-exports')
+ iotests.qemu_nbd_list_log('-k', socket)
+
+ iotests.log('\n=== Connect qemu-io again, try force removing ===')
+
+ qemu_io = iotests.QemuIoInteractive('-f', 'raw', nbd_url)
+ vm.qmp_log('block-export-del', id='export1')
+ vm.qmp_log('block-export-del', id='export1', mode='hard')
+
+ # This should fail now
+ iotests.log(qemu_io.cmd('read -P 0x11 0 4k'))
+ qemu_io.close()
+
+ vm.qmp_log('query-block-exports')
+ iotests.qemu_nbd_list_log('-k', socket)
+
+ iotests.log('\n=== Shut down QEMU ===')
+ vm.shutdown()
diff --git a/tests/qemu-iotests/307.out b/tests/qemu-iotests/307.out
new file mode 100644
index 0000000000..daa8ad2da0
--- /dev/null
+++ b/tests/qemu-iotests/307.out
@@ -0,0 +1,124 @@
+wrote 4096/4096 bytes at offset 0
+4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+=== Launch VM ===
+{"execute": "nbd-server-start", "arguments": {"addr": {"data": {"path": "SOCK_DIR/PID-socket"}, "type": "unix"}}}
+{"return": {}}
+{"execute": "query-block-exports", "arguments": {}}
+{"return": []}
+
+=== Create a read-only NBD export ===
+{"execute": "block-export-add", "arguments": {"id": "export0", "node-name": "fmt", "type": "nbd"}}
+{"return": {}}
+{"execute": "query-block-exports", "arguments": {}}
+{"return": [{"id": "export0", "node-name": "fmt", "shutting-down": false, "type": "nbd"}]}
+exports available: 1
+ export: 'fmt'
+ size: 67108864
+ flags: 0x58f ( readonly flush fua df multi cache )
+ min block: XXX
+ opt block: XXX
+ max block: XXX
+ available meta contexts: 1
+ base:allocation
+
+
+=== Try a few invalid things ===
+{"execute": "block-export-add", "arguments": {"id": "#invalid", "node-name": "fmt", "type": "nbd"}}
+{"error": {"class": "GenericError", "desc": "Invalid block export id"}}
+{"execute": "block-export-add", "arguments": {"id": "export0", "node-name": "fmt", "type": "nbd"}}
+{"error": {"class": "GenericError", "desc": "Block export id 'export0' is already in use"}}
+{"execute": "block-export-add", "arguments": {"id": "export1", "node-name": "ro", "type": "nbd", "writable": true}}
+{"error": {"class": "GenericError", "desc": "Cannot export read-only node as writable"}}
+{"execute": "block-export-del", "arguments": {"id": "export1"}}
+{"error": {"class": "GenericError", "desc": "Export 'export1' is not found"}}
+{"execute": "query-block-exports", "arguments": {}}
+{"return": [{"id": "export0", "node-name": "fmt", "shutting-down": false, "type": "nbd"}]}
+
+=== Move export to an iothread ===
+{"execute": "device_add", "arguments": {"drive": "fmt", "driver": "scsi-hd", "id": "sda"}}
+{"return": {}}
+{"execute": "query-block-exports", "arguments": {}}
+{"return": [{"id": "export0", "node-name": "fmt", "shutting-down": false, "type": "nbd"}]}
+exports available: 1
+ export: 'fmt'
+ size: 67108864
+ flags: 0x58f ( readonly flush fua df multi cache )
+ min block: XXX
+ opt block: XXX
+ max block: XXX
+ available meta contexts: 1
+ base:allocation
+
+
+=== Add a writable export ===
+{"execute": "block-export-add", "arguments": {"description": "This is the writable second export", "id": "export1", "name": "export1", "node-name": "fmt", "type": "nbd", "writable": true, "writethrough": true}}
+{"error": {"class": "GenericError", "desc": "Conflicts with use by sda as 'root', which does not allow 'write' on fmt"}}
+{"execute": "device_del", "arguments": {"id": "sda"}}
+{"return": {}}
+{"data": {"device": "sda", "path": "/machine/peripheral/sda"}, "event": "DEVICE_DELETED", "timestamp": {"microseconds": "USECS", "seconds": "SECS"}}
+{"execute": "device_add", "arguments": {"drive": "fmt", "driver": "scsi-hd", "id": "sda", "share-rw": true}}
+{"return": {}}
+{"execute": "block-export-add", "arguments": {"description": "This is the writable second export", "id": "export1", "name": "export1", "node-name": "fmt", "type": "nbd", "writable": true, "writethrough": true}}
+{"return": {}}
+{"execute": "query-block-exports", "arguments": {}}
+{"return": [{"id": "export1", "node-name": "fmt", "shutting-down": false, "type": "nbd"}, {"id": "export0", "node-name": "fmt", "shutting-down": false, "type": "nbd"}]}
+exports available: 2
+ export: 'fmt'
+ size: 67108864
+ flags: 0x58f ( readonly flush fua df multi cache )
+ min block: XXX
+ opt block: XXX
+ max block: XXX
+ available meta contexts: 1
+ base:allocation
+ export: 'export1'
+ description: This is the writable second export
+ size: 67108864
+ flags: 0xced ( flush fua trim zeroes df cache fast-zero )
+ min block: XXX
+ opt block: XXX
+ max block: XXX
+ available meta contexts: 1
+ base:allocation
+
+
+=== Connect qemu-io to export1, try removing exports ===
+read 4096/4096 bytes at offset 0
+4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+wrote 4096/4096 bytes at offset 4096
+4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+{"execute": "block-export-del", "arguments": {"id": "export1"}}
+{"error": {"class": "GenericError", "desc": "export 'export1' still in use"}}
+{"execute": "block-export-del", "arguments": {"id": "export0"}}
+{"return": {}}
+[{"data": {"id": "export0"}, "event": "BLOCK_EXPORT_DELETED", "timestamp": {"microseconds": "USECS", "seconds": "SECS"}}]
+{"execute": "query-block-exports", "arguments": {}}
+{"return": [{"id": "export1", "node-name": "fmt", "shutting-down": false, "type": "nbd"}]}
+exports available: 1
+ export: 'export1'
+ description: This is the writable second export
+ size: 67108864
+ flags: 0xced ( flush fua trim zeroes df cache fast-zero )
+ min block: XXX
+ opt block: XXX
+ max block: XXX
+ available meta contexts: 1
+ base:allocation
+
+
+=== Connect qemu-io again, try force removing ===
+{"execute": "block-export-del", "arguments": {"id": "export1"}}
+{"error": {"class": "GenericError", "desc": "export 'export1' still in use"}}
+{"execute": "block-export-del", "arguments": {"id": "export1", "mode": "hard"}}
+{"return": {}}
+read failed: Input/output error
+
+{"execute": "query-block-exports", "arguments": {}}
+{"return": []}
+exports available: 0
+
+
+=== Shut down QEMU ===
diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
index ff59cfd2d4..9e4f7c0153 100644
--- a/tests/qemu-iotests/group
+++ b/tests/qemu-iotests/group
@@ -314,3 +314,4 @@
303 rw quick
304 rw quick
305 rw quick
+307 rw quick export
--
2.25.4
^ permalink raw reply related [flat|nested] 44+ messages in thread
* Re: [PATCH v2 00/31] block/export: Add infrastructure and QAPI for block exports
2020-09-24 15:26 [PATCH v2 00/31] block/export: Add infrastructure and QAPI for block exports Kevin Wolf
` (30 preceding siblings ...)
2020-09-24 15:27 ` [PATCH v2 31/31] iotests: Test block-export-* QMP interface Kevin Wolf
@ 2020-09-24 17:21 ` no-reply
2020-09-25 8:45 ` Stefan Hajnoczi
2020-09-25 8:51 ` Stefan Hajnoczi
2020-09-28 15:29 ` Kevin Wolf
33 siblings, 1 reply; 44+ messages in thread
From: no-reply @ 2020-09-24 17:21 UTC (permalink / raw)
To: kwolf; +Cc: kwolf, stefanha, qemu-devel, qemu-block, mreitz
Patchew URL: https://patchew.org/QEMU/20200924152717.287415-1-kwolf@redhat.com/
Hi,
This series seems to have some coding style problems. See output below for
more information:
Type: series
Message-id: 20200924152717.287415-1-kwolf@redhat.com
Subject: [PATCH v2 00/31] block/export: Add infrastructure and QAPI for block exports
=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===
From https://github.com/patchew-project/qemu
c122bca..f7f1d91 master -> master
From https://github.com/patchew-project/qemu
* [new tag] patchew/20200924152717.287415-1-kwolf@redhat.com -> patchew/20200924152717.287415-1-kwolf@redhat.com
Switched to a new branch 'test'
772bb89 iotests: Test block-export-* QMP interface
a89a8b7 iotests: Allow supported and unsupported formats at the same time
fd15df6 iotests: Introduce qemu_nbd_list_log()
7fe6af5 iotests: Factor out qemu_tool_pipe_and_status()
67144d8 nbd: Deprecate nbd-server-add/remove
81170eb nbd: Merge nbd_export_new() and nbd_export_create()
2c8869b block/export: Move writable to BlockExportOptions
63951c4 block/export: Add query-block-exports
7f1940a block/export: Create BlockBackend in blk_exp_add()
c20f5f0 block/export: Move blk to BlockExport
46c45ed block/export: Add BLOCK_EXPORT_DELETED event
549525b block/export: Add block-export-del
deb98e0 block/export: Move strong user reference to block_exports
ad75b22 block/export: Add 'id' option to block-export-add
ffadf38 block/export: Add blk_exp_close_all(_type)
1da2124 block/export: Allocate BlockExport in blk_exp_add()
602df28 block/export: Add node-name to BlockExportOptions
b1fc3ff block/export: Move AioContext from NBDExport to BlockExport
a5d275f block/export: Move refcount from NBDExport to BlockExport
650f2eb nbd/server: Simplify export shutdown
634dc2f qemu-nbd: Use blk_exp_add() to create the export
3e1f7cf nbd: Remove NBDExport.close callback
3a41880 nbd: Add writethrough to block-export-add
f990833 nbd: Add max-connections to nbd-server-start
de1c397 block/export: Remove magic from block-export-add
9bc13b8 qemu-nbd: Use raw block driver for --offset
8eb933e qemu-storage-daemon: Use qmp_block_export_add()
38a4b01 block/export: Add BlockExport infrastructure and block-export-add
66aff14 qapi: Rename BlockExport to BlockExportOptions
026ac5d qapi: Create block-export module
b1715f6 nbd: Remove unused nbd_export_get_blockdev()
=== OUTPUT BEGIN ===
1/31 Checking commit b1715f628199 (nbd: Remove unused nbd_export_get_blockdev())
2/31 Checking commit 026ac5dda678 (qapi: Create block-export module)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#231:
new file mode 100644
total: 0 errors, 1 warnings, 410 lines checked
Patch 2/31 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
3/31 Checking commit 66aff14308cb (qapi: Rename BlockExport to BlockExportOptions)
4/31 Checking commit 38a4b017ec72 (block/export: Add BlockExport infrastructure and block-export-add)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#21:
new file mode 100644
ERROR: "(foo*)" should be "(foo *)"
#156: FILE: blockdev-nbd.c:224:
+ return (BlockExport*) exp;
total: 1 errors, 1 warnings, 247 lines checked
Patch 4/31 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
5/31 Checking commit 8eb933e1e3f7 (qemu-storage-daemon: Use qmp_block_export_add())
6/31 Checking commit 9bc13b8dba46 (qemu-nbd: Use raw block driver for --offset)
7/31 Checking commit de1c39724d9e (block/export: Remove magic from block-export-add)
8/31 Checking commit f99083319892 (nbd: Add max-connections to nbd-server-start)
9/31 Checking commit 3a418807fc14 (nbd: Add writethrough to block-export-add)
10/31 Checking commit 3e1f7cf80488 (nbd: Remove NBDExport.close callback)
11/31 Checking commit 634dc2f6f2ec (qemu-nbd: Use blk_exp_add() to create the export)
12/31 Checking commit 650f2eba0db8 (nbd/server: Simplify export shutdown)
13/31 Checking commit a5d275f4c538 (block/export: Move refcount from NBDExport to BlockExport)
ERROR: "(foo*)" should be "(foo *)"
#49: FILE: blockdev-nbd.c:239:
+ blk_exp_unref((BlockExport*) exp);
total: 1 errors, 0 warnings, 214 lines checked
Patch 13/31 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
14/31 Checking commit b1fc3ffc0ccd (block/export: Move AioContext from NBDExport to BlockExport)
15/31 Checking commit 602df286881c (block/export: Add node-name to BlockExportOptions)
16/31 Checking commit 1da2124e164e (block/export: Allocate BlockExport in blk_exp_add())
17/31 Checking commit ffadf38dd6c3 (block/export: Add blk_exp_close_all(_type))
18/31 Checking commit ad75b22688f5 (block/export: Add 'id' option to block-export-add)
19/31 Checking commit deb98e00621d (block/export: Move strong user reference to block_exports)
20/31 Checking commit 549525bb9497 (block/export: Add block-export-del)
21/31 Checking commit 46c45ed62a8d (block/export: Add BLOCK_EXPORT_DELETED event)
22/31 Checking commit c20f5f0cba91 (block/export: Move blk to BlockExport)
23/31 Checking commit 7f1940ac70c5 (block/export: Create BlockBackend in blk_exp_add())
24/31 Checking commit 63951c4d2fbd (block/export: Add query-block-exports)
25/31 Checking commit 2c8869bd86fb (block/export: Move writable to BlockExportOptions)
26/31 Checking commit 81170eb030be (nbd: Merge nbd_export_new() and nbd_export_create())
27/31 Checking commit 67144d8ca22d (nbd: Deprecate nbd-server-add/remove)
28/31 Checking commit 7fe6af5d135c (iotests: Factor out qemu_tool_pipe_and_status())
29/31 Checking commit fd15df623a84 (iotests: Introduce qemu_nbd_list_log())
30/31 Checking commit a89a8b774b81 (iotests: Allow supported and unsupported formats at the same time)
31/31 Checking commit 772bb8914d39 (iotests: Test block-export-* QMP interface)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#11:
new file mode 100755
total: 0 errors, 1 warnings, 260 lines checked
Patch 31/31 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
=== OUTPUT END ===
Test command exited with code: 1
The full log is available at
http://patchew.org/logs/20200924152717.287415-1-kwolf@redhat.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH v2 00/31] block/export: Add infrastructure and QAPI for block exports
2020-09-24 17:21 ` [PATCH v2 00/31] block/export: Add infrastructure and QAPI for block exports no-reply
@ 2020-09-25 8:45 ` Stefan Hajnoczi
0 siblings, 0 replies; 44+ messages in thread
From: Stefan Hajnoczi @ 2020-09-25 8:45 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, qemu-block, mreitz
[-- Attachment #1: Type: text/plain, Size: 349 bytes --]
On Thu, Sep 24, 2020 at 10:21:56AM -0700, no-reply@patchew.org wrote:
> ERROR: "(foo*)" should be "(foo *)"
> #156: FILE: blockdev-nbd.c:224:
> + return (BlockExport*) exp;
[...]
> ERROR: "(foo*)" should be "(foo *)"
> #49: FILE: blockdev-nbd.c:239:
> + blk_exp_unref((BlockExport*) exp);
These can be fixed when merging the patches.
Stefan
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH v2 00/31] block/export: Add infrastructure and QAPI for block exports
2020-09-24 15:26 [PATCH v2 00/31] block/export: Add infrastructure and QAPI for block exports Kevin Wolf
` (31 preceding siblings ...)
2020-09-24 17:21 ` [PATCH v2 00/31] block/export: Add infrastructure and QAPI for block exports no-reply
@ 2020-09-25 8:51 ` Stefan Hajnoczi
2020-09-28 15:29 ` Kevin Wolf
33 siblings, 0 replies; 44+ messages in thread
From: Stefan Hajnoczi @ 2020-09-25 8:51 UTC (permalink / raw)
To: Kevin Wolf; +Cc: qemu-devel, qemu-block, mreitz
[-- Attachment #1: Type: text/plain, Size: 1490 bytes --]
On Thu, Sep 24, 2020 at 05:26:46PM +0200, Kevin Wolf wrote:
> We are planning to add more block export types than just NBD in the near
> future (e.g. vhost-user-blk and FUSE). This series lays the ground for
> this with some generic block export infrastructure and QAPI interfaces
> that will allow managing all of them (for now add/remove/query).
>
> As a side effect, qemu-storage-daemon can now map --export directly to
> the block-export-add QMP command, similar to other command line options.
> The built-in NBD servers also gains new options that bring it at least a
> little closer to feature parity with qemu-nbd.
>
> v2:
> - Rebased on current master
> - Fixed assumption of &blk_exp_nbd instead of drv in generic code [Max]
> - Documented blk_exp_request_shutdown() better [Max]
> - iotests 140: Fixed race between QMP return value and event [Max]
> - Improved the commit message for patch 26
> - Removed copy&paste error in deprecated.rst [Max]
> - iotests: Take Sequence instead of positional arguments in
> qemu_tool_pipe_and_status() [Max]
> - iotests: Separate patch for qemu_nbd_list_log [Max]
> - iotests 307: [Max]
> * Allow more image formats
> * Use sock_dir for the socket
> * Use f-strings instead of % operator
> * Log events after deleting an export
> * Test force removing an export
I rebased and retested the vhost-user-blk-server block exports series on
top of this:
Acked-by: Stefan Hajnoczi <stefanha@redhat.com>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH v2 21/31] block/export: Add BLOCK_EXPORT_DELETED event
2020-09-24 15:27 ` [PATCH v2 21/31] block/export: Add BLOCK_EXPORT_DELETED event Kevin Wolf
@ 2020-09-25 12:34 ` Max Reitz
0 siblings, 0 replies; 44+ messages in thread
From: Max Reitz @ 2020-09-25 12:34 UTC (permalink / raw)
To: Kevin Wolf, qemu-block; +Cc: qemu-devel, stefanha
[-- Attachment #1.1: Type: text/plain, Size: 1746 bytes --]
On 24.09.20 17:27, Kevin Wolf wrote:
> Clients may want to know when an export has finally disappeard
> (block-export-del returns earlier than that in the general case), so add
> a QAPI event for it.
>
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
> qapi/block-export.json | 12 ++++++++++++
> block/export/export.c | 2 ++
> tests/qemu-iotests/140 | 9 ++++++++-
> tests/qemu-iotests/140.out | 2 +-
> tests/qemu-iotests/223.out | 4 ++++
> 5 files changed, 27 insertions(+), 2 deletions(-)
[...]
> diff --git a/tests/qemu-iotests/140 b/tests/qemu-iotests/140
> index 8d2ce5d9e3..309b177e77 100755
> --- a/tests/qemu-iotests/140
> +++ b/tests/qemu-iotests/140
> @@ -81,10 +81,17 @@ $QEMU_IO_PROG -f raw -r -c 'read -P 42 0 64k' \
> "nbd+unix:///drv?socket=$SOCK_DIR/nbd" 2>&1 \
> | _filter_qemu_io | _filter_nbd
>
> +# The order of 'return' and the BLOCK_EXPORT_DELETED event is undefined. Just
> +# wait until we've twice seen one of them. Filter the 'return' line out so that
> +# the output is defined.
> _send_qemu_cmd $QEMU_HANDLE \
> "{ 'execute': 'eject',
> 'arguments': { 'device': 'drv' }}" \
> - 'return'
> + 'return\|BLOCK_EXPORT_DELETED' |
> + grep -v 'return'
> +
> +_send_qemu_cmd $QEMU_HANDLE '' 'return\|BLOCK_EXPORT_DELETED' |
> + grep -v 'return'
Funny. I did basically the same thing (only I filtered the event, not
the return):
https://git.xanclic.moe/XanClic/qemu/commit/e6f7510149a4a26c868013639ec89d28f16857d8#diff-3
and considered it kind of a hack.
Oh well. :)
Reviewed-by: Max Reitz <mreitz@redhat.com>
> $QEMU_IO_PROG -f raw -r -c close \
> "nbd+unix:///drv?socket=$SOCK_DIR/nbd" 2>&1 \
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH v2 26/31] nbd: Merge nbd_export_new() and nbd_export_create()
2020-09-24 15:27 ` [PATCH v2 26/31] nbd: Merge nbd_export_new() and nbd_export_create() Kevin Wolf
@ 2020-09-25 12:42 ` Max Reitz
0 siblings, 0 replies; 44+ messages in thread
From: Max Reitz @ 2020-09-25 12:42 UTC (permalink / raw)
To: Kevin Wolf, qemu-block; +Cc: qemu-devel, stefanha
[-- Attachment #1.1: Type: text/plain, Size: 499 bytes --]
On 24.09.20 17:27, Kevin Wolf wrote:
> There is no real reason any more why nbd_export_new() and
> nbd_export_create() should be separate functions. The latter only
> performs a few checks before it calls the former.
>
> What makes the current state stand out is that it's the only function in
> BlockExportDriver that is not a static function inside nbd/server.c, but
> a small wrapper in blockdev-nbd.c that then calls back into nbd/server.c
> for the real functionality.
Thanks :)
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH v2 27/31] nbd: Deprecate nbd-server-add/remove
2020-09-24 15:27 ` [PATCH v2 27/31] nbd: Deprecate nbd-server-add/remove Kevin Wolf
@ 2020-09-25 12:43 ` Max Reitz
0 siblings, 0 replies; 44+ messages in thread
From: Max Reitz @ 2020-09-25 12:43 UTC (permalink / raw)
To: Kevin Wolf, qemu-block; +Cc: qemu-devel, stefanha
[-- Attachment #1.1: Type: text/plain, Size: 350 bytes --]
On 24.09.20 17:27, Kevin Wolf wrote:
> These QMP commands are replaced by block-export-add/del.
>
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
> qapi/block-export.json | 11 +++++++++--
> docs/system/deprecated.rst | 6 ++++++
> 2 files changed, 15 insertions(+), 2 deletions(-)
Reviewed-by: Max Reitz <mreitz@redhat.com>
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH v2 28/31] iotests: Factor out qemu_tool_pipe_and_status()
2020-09-24 15:27 ` [PATCH v2 28/31] iotests: Factor out qemu_tool_pipe_and_status() Kevin Wolf
@ 2020-09-25 12:48 ` Max Reitz
0 siblings, 0 replies; 44+ messages in thread
From: Max Reitz @ 2020-09-25 12:48 UTC (permalink / raw)
To: Kevin Wolf, qemu-block; +Cc: qemu-devel, stefanha
[-- Attachment #1.1: Type: text/plain, Size: 453 bytes --]
On 24.09.20 17:27, Kevin Wolf wrote:
> We have three almost identical functions that call an external process
> and return its output and return code. Refactor them into small wrappers
> around a common function.
>
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
> tests/qemu-iotests/iotests.py | 49 ++++++++++++++++-------------------
> 1 file changed, 23 insertions(+), 26 deletions(-)
Reviewed-by: Max Reitz <mreitz@redhat.com>
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH v2 29/31] iotests: Introduce qemu_nbd_list_log()
2020-09-24 15:27 ` [PATCH v2 29/31] iotests: Introduce qemu_nbd_list_log() Kevin Wolf
@ 2020-09-25 12:55 ` Max Reitz
0 siblings, 0 replies; 44+ messages in thread
From: Max Reitz @ 2020-09-25 12:55 UTC (permalink / raw)
To: Kevin Wolf, qemu-block; +Cc: qemu-devel, stefanha
[-- Attachment #1.1: Type: text/plain, Size: 318 bytes --]
On 24.09.20 17:27, Kevin Wolf wrote:
> Add a function to list the NBD exports offered by an NBD server.
>
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
> tests/qemu-iotests/iotests.py | 13 ++++++++++++-
> 1 file changed, 12 insertions(+), 1 deletion(-)
Reviewed-by: Max Reitz <mreitz@redhat.com>
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH v2 30/31] iotests: Allow supported and unsupported formats at the same time
2020-09-24 15:27 ` [PATCH v2 30/31] iotests: Allow supported and unsupported formats at the same time Kevin Wolf
@ 2020-09-25 12:59 ` Max Reitz
0 siblings, 0 replies; 44+ messages in thread
From: Max Reitz @ 2020-09-25 12:59 UTC (permalink / raw)
To: Kevin Wolf, qemu-block; +Cc: qemu-devel, stefanha
[-- Attachment #1.1: Type: text/plain, Size: 453 bytes --]
On 24.09.20 17:27, Kevin Wolf wrote:
> This is useful for specifying 'generic' as supported (which includes
> only writable image formats), but still excluding some incompatible
> writable formats.
>
> It also removes more lines than it adds.
>
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
> tests/qemu-iotests/iotests.py | 6 +-----
> 1 file changed, 1 insertion(+), 5 deletions(-)
Reviewed-by: Max Reitz <mreitz@redhat.com>
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH v2 31/31] iotests: Test block-export-* QMP interface
2020-09-24 15:27 ` [PATCH v2 31/31] iotests: Test block-export-* QMP interface Kevin Wolf
@ 2020-09-25 13:02 ` Max Reitz
0 siblings, 0 replies; 44+ messages in thread
From: Max Reitz @ 2020-09-25 13:02 UTC (permalink / raw)
To: Kevin Wolf, qemu-block; +Cc: qemu-devel, stefanha
[-- Attachment #1.1: Type: text/plain, Size: 462 bytes --]
On 24.09.20 17:27, Kevin Wolf wrote:
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
> tests/qemu-iotests/307 | 132 +++++++++++++++++++++++++++++++++++++
> tests/qemu-iotests/307.out | 124 ++++++++++++++++++++++++++++++++++
> tests/qemu-iotests/group | 1 +
> 3 files changed, 257 insertions(+)
> create mode 100755 tests/qemu-iotests/307
> create mode 100644 tests/qemu-iotests/307.out
Reviewed-by: Max Reitz <mreitz@redhat.com>
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH v2 04/31] block/export: Add BlockExport infrastructure and block-export-add
2020-09-24 15:26 ` [PATCH v2 04/31] block/export: Add BlockExport infrastructure and block-export-add Kevin Wolf
@ 2020-09-25 13:18 ` Eric Blake
0 siblings, 0 replies; 44+ messages in thread
From: Eric Blake @ 2020-09-25 13:18 UTC (permalink / raw)
To: Kevin Wolf, qemu-block; +Cc: qemu-devel, stefanha, mreitz
On 9/24/20 10:26 AM, Kevin Wolf wrote:
> We want to have a common set of commands for all types of block exports.
> Currently, this is only NBD, but we're going to add more types.
>
> This patch adds the basic BlockExport and BlockExportDriver structs and
> a QMP command block-export-add that creates a new export based on the
> given BlockExportOptions.
>
> qmp_nbd_server_add() becomes a wrapper around qmp_block_export_add().
>
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> Reviewed-by: Max Reitz <mreitz@redhat.com>
> ---
Reviewed-by: Eric Blake <eblake@redhat.com>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH v2 00/31] block/export: Add infrastructure and QAPI for block exports
2020-09-24 15:26 [PATCH v2 00/31] block/export: Add infrastructure and QAPI for block exports Kevin Wolf
` (32 preceding siblings ...)
2020-09-25 8:51 ` Stefan Hajnoczi
@ 2020-09-28 15:29 ` Kevin Wolf
33 siblings, 0 replies; 44+ messages in thread
From: Kevin Wolf @ 2020-09-28 15:29 UTC (permalink / raw)
To: qemu-block; +Cc: qemu-devel, stefanha, mreitz
Am 24.09.2020 um 17:26 hat Kevin Wolf geschrieben:
> We are planning to add more block export types than just NBD in the near
> future (e.g. vhost-user-blk and FUSE). This series lays the ground for
> this with some generic block export infrastructure and QAPI interfaces
> that will allow managing all of them (for now add/remove/query).
>
> As a side effect, qemu-storage-daemon can now map --export directly to
> the block-export-add QMP command, similar to other command line options.
> The built-in NBD servers also gains new options that bring it at least a
> little closer to feature parity with qemu-nbd.
Thanks for the review, applied to the block branch.
Kevin
^ permalink raw reply [flat|nested] 44+ messages in thread
end of thread, other threads:[~2020-09-28 15:31 UTC | newest]
Thread overview: 44+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-24 15:26 [PATCH v2 00/31] block/export: Add infrastructure and QAPI for block exports Kevin Wolf
2020-09-24 15:26 ` [PATCH v2 01/31] nbd: Remove unused nbd_export_get_blockdev() Kevin Wolf
2020-09-24 15:26 ` [PATCH v2 02/31] qapi: Create block-export module Kevin Wolf
2020-09-24 15:26 ` [PATCH v2 03/31] qapi: Rename BlockExport to BlockExportOptions Kevin Wolf
2020-09-24 15:26 ` [PATCH v2 04/31] block/export: Add BlockExport infrastructure and block-export-add Kevin Wolf
2020-09-25 13:18 ` Eric Blake
2020-09-24 15:26 ` [PATCH v2 05/31] qemu-storage-daemon: Use qmp_block_export_add() Kevin Wolf
2020-09-24 15:26 ` [PATCH v2 06/31] qemu-nbd: Use raw block driver for --offset Kevin Wolf
2020-09-24 15:26 ` [PATCH v2 07/31] block/export: Remove magic from block-export-add Kevin Wolf
2020-09-24 15:26 ` [PATCH v2 08/31] nbd: Add max-connections to nbd-server-start Kevin Wolf
2020-09-24 15:26 ` [PATCH v2 09/31] nbd: Add writethrough to block-export-add Kevin Wolf
2020-09-24 15:26 ` [PATCH v2 10/31] nbd: Remove NBDExport.close callback Kevin Wolf
2020-09-24 15:26 ` [PATCH v2 11/31] qemu-nbd: Use blk_exp_add() to create the export Kevin Wolf
2020-09-24 15:26 ` [PATCH v2 12/31] nbd/server: Simplify export shutdown Kevin Wolf
2020-09-24 15:26 ` [PATCH v2 13/31] block/export: Move refcount from NBDExport to BlockExport Kevin Wolf
2020-09-24 15:27 ` [PATCH v2 14/31] block/export: Move AioContext " Kevin Wolf
2020-09-24 15:27 ` [PATCH v2 15/31] block/export: Add node-name to BlockExportOptions Kevin Wolf
2020-09-24 15:27 ` [PATCH v2 16/31] block/export: Allocate BlockExport in blk_exp_add() Kevin Wolf
2020-09-24 15:27 ` [PATCH v2 17/31] block/export: Add blk_exp_close_all(_type) Kevin Wolf
2020-09-24 15:27 ` [PATCH v2 18/31] block/export: Add 'id' option to block-export-add Kevin Wolf
2020-09-24 15:27 ` [PATCH v2 19/31] block/export: Move strong user reference to block_exports Kevin Wolf
2020-09-24 15:27 ` [PATCH v2 20/31] block/export: Add block-export-del Kevin Wolf
2020-09-24 15:27 ` [PATCH v2 21/31] block/export: Add BLOCK_EXPORT_DELETED event Kevin Wolf
2020-09-25 12:34 ` Max Reitz
2020-09-24 15:27 ` [PATCH v2 22/31] block/export: Move blk to BlockExport Kevin Wolf
2020-09-24 15:27 ` [PATCH v2 23/31] block/export: Create BlockBackend in blk_exp_add() Kevin Wolf
2020-09-24 15:27 ` [PATCH v2 24/31] block/export: Add query-block-exports Kevin Wolf
2020-09-24 15:27 ` [PATCH v2 25/31] block/export: Move writable to BlockExportOptions Kevin Wolf
2020-09-24 15:27 ` [PATCH v2 26/31] nbd: Merge nbd_export_new() and nbd_export_create() Kevin Wolf
2020-09-25 12:42 ` Max Reitz
2020-09-24 15:27 ` [PATCH v2 27/31] nbd: Deprecate nbd-server-add/remove Kevin Wolf
2020-09-25 12:43 ` Max Reitz
2020-09-24 15:27 ` [PATCH v2 28/31] iotests: Factor out qemu_tool_pipe_and_status() Kevin Wolf
2020-09-25 12:48 ` Max Reitz
2020-09-24 15:27 ` [PATCH v2 29/31] iotests: Introduce qemu_nbd_list_log() Kevin Wolf
2020-09-25 12:55 ` Max Reitz
2020-09-24 15:27 ` [PATCH v2 30/31] iotests: Allow supported and unsupported formats at the same time Kevin Wolf
2020-09-25 12:59 ` Max Reitz
2020-09-24 15:27 ` [PATCH v2 31/31] iotests: Test block-export-* QMP interface Kevin Wolf
2020-09-25 13:02 ` Max Reitz
2020-09-24 17:21 ` [PATCH v2 00/31] block/export: Add infrastructure and QAPI for block exports no-reply
2020-09-25 8:45 ` Stefan Hajnoczi
2020-09-25 8:51 ` Stefan Hajnoczi
2020-09-28 15:29 ` Kevin Wolf
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.