* [PULL 01/37] block/sheepdog: Replace magic val by NANOSECONDS_PER_SECOND definition
2020-10-02 14:43 [PULL 00/37] Block layer patches Kevin Wolf
@ 2020-10-02 14:43 ` Kevin Wolf
2020-10-02 14:43 ` [PULL 02/37] tests/check-block: Do not run the iotests with old versions of bash Kevin Wolf
` (36 subsequent siblings)
37 siblings, 0 replies; 39+ messages in thread
From: Kevin Wolf @ 2020-10-02 14:43 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, peter.maydell, qemu-devel
From: Philippe Mathieu-Daudé <philmd@redhat.com>
Use self-explicit NANOSECONDS_PER_SECOND definition instead
of magic value.
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-Id: <20200921110145.520944-1-philmd@redhat.com>
Reviewed-by: Alberto Garcia <berto@igalia.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
block/sheepdog.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/block/sheepdog.c b/block/sheepdog.c
index 2f5c0eb376..25111d5a70 100644
--- a/block/sheepdog.c
+++ b/block/sheepdog.c
@@ -740,7 +740,7 @@ static coroutine_fn void reconnect_to_sdog(void *opaque)
if (s->fd < 0) {
trace_sheepdog_reconnect_to_sdog();
error_report_err(local_err);
- qemu_co_sleep_ns(QEMU_CLOCK_REALTIME, 1000000000ULL);
+ qemu_co_sleep_ns(QEMU_CLOCK_REALTIME, NANOSECONDS_PER_SECOND);
}
};
--
2.25.4
^ permalink raw reply related [flat|nested] 39+ messages in thread* [PULL 02/37] tests/check-block: Do not run the iotests with old versions of bash
2020-10-02 14:43 [PULL 00/37] Block layer patches Kevin Wolf
2020-10-02 14:43 ` [PULL 01/37] block/sheepdog: Replace magic val by NANOSECONDS_PER_SECOND definition Kevin Wolf
@ 2020-10-02 14:43 ` Kevin Wolf
2020-10-02 14:43 ` [PULL 03/37] docs: Document the throttle block filter Kevin Wolf
` (35 subsequent siblings)
37 siblings, 0 replies; 39+ messages in thread
From: Kevin Wolf @ 2020-10-02 14:43 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, peter.maydell, qemu-devel
From: Thomas Huth <thuth@redhat.com>
macOS is shipped with a very old version of the bash (3.2), which
is currently not suitable for running the iotests anymore (e.g.
it is missing support for "readarray" which is used in the file
tests/qemu-iotests/common.filter). Add a check to skip the iotests
in this case - if someone still wants to run the iotests on macOS,
they can install a newer version from homebrew, for example.
Signed-off-by: Thomas Huth <thuth@redhat.com>
Message-Id: <20200918153514.330705-1-thuth@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
tests/check-block.sh | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/tests/check-block.sh b/tests/check-block.sh
index a5a69060e1..f6b1bda7b9 100755
--- a/tests/check-block.sh
+++ b/tests/check-block.sh
@@ -46,6 +46,11 @@ if ! command -v bash >/dev/null 2>&1 ; then
exit 0
fi
+if LANG=C bash --version | grep -q 'GNU bash, version [123]' ; then
+ echo "bash version too old ==> Not running the qemu-iotests."
+ exit 0
+fi
+
if ! (sed --version | grep 'GNU sed') > /dev/null 2>&1 ; then
if ! command -v gsed >/dev/null 2>&1; then
echo "GNU sed not available ==> Not running the qemu-iotests."
--
2.25.4
^ permalink raw reply related [flat|nested] 39+ messages in thread* [PULL 03/37] docs: Document the throttle block filter
2020-10-02 14:43 [PULL 00/37] Block layer patches Kevin Wolf
2020-10-02 14:43 ` [PULL 01/37] block/sheepdog: Replace magic val by NANOSECONDS_PER_SECOND definition Kevin Wolf
2020-10-02 14:43 ` [PULL 02/37] tests/check-block: Do not run the iotests with old versions of bash Kevin Wolf
@ 2020-10-02 14:43 ` Kevin Wolf
2020-10-02 14:43 ` [PULL 04/37] qemu-io-cmds: Simplify help_oneline Kevin Wolf
` (34 subsequent siblings)
37 siblings, 0 replies; 39+ messages in thread
From: Kevin Wolf @ 2020-10-02 14:43 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, peter.maydell, qemu-devel
From: Alberto Garcia <berto@igalia.com>
This filter was added back in 2017 for QEMU 2.11 but it was never
properly documented, so let's explain how it works and add a couple of
examples.
Signed-off-by: Alberto Garcia <berto@igalia.com>
Message-Id: <20200921173016.27935-1-berto@igalia.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
docs/throttle.txt | 108 +++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 107 insertions(+), 1 deletion(-)
diff --git a/docs/throttle.txt b/docs/throttle.txt
index cd4e109d39..b5b78b7326 100644
--- a/docs/throttle.txt
+++ b/docs/throttle.txt
@@ -1,6 +1,6 @@
The QEMU throttling infrastructure
==================================
-Copyright (C) 2016 Igalia, S.L.
+Copyright (C) 2016,2020 Igalia, S.L.
Author: Alberto Garcia <berto@igalia.com>
This work is licensed under the terms of the GNU GPL, version 2 or
@@ -253,3 +253,109 @@ up. After those 60 seconds the bucket will have leaked 60 x 100 =
Also, due to the way the algorithm works, longer burst can be done at
a lower I/O rate, e.g. 1000 IOPS during 120 seconds.
+
+
+The 'throttle' block filter
+---------------------------
+Since QEMU 2.11 it is possible to configure the I/O limits using a
+'throttle' block filter. This filter uses the exact same throttling
+infrastructure described above but can be used anywhere in the node
+graph, allowing for more flexibility.
+
+The user can create an arbitrary number of filters and each one of
+them must be assigned to a group that contains the actual I/O limits.
+Different filters can use the same group so the limits are shared as
+described earlier in "Applying I/O limits to groups of disks".
+
+A group can be created using the object-add QMP function:
+
+ { "execute": "object-add",
+ "arguments": {
+ "qom-type": "throttle-group",
+ "id": "group0",
+ "props": {
+ "limits" : {
+ "iops-total": 1000
+ "bps-write": 2097152
+ }
+ }
+ }
+ }
+
+throttle-group has a 'limits' property (of type ThrottleLimits as
+defined in qapi/block-core.json) which can be set on creation or later
+with 'qom-set'.
+
+A throttle-group can also be created with the -object command line
+option but at the moment there is no way to pass a 'limits' parameter
+that contains a ThrottleLimits structure. The solution is to set the
+individual values directly, like in this example:
+
+ -object throttle-group,id=group0,x-iops-total=1000,x-bps-write=2097152
+
+Note however that this is not a stable API (hence the 'x-' prefixes) and
+will disappear when -object gains support for structured options and
+enables use of 'limits'.
+
+Once we have a throttle-group we can use the throttle block filter,
+where the 'file' property must be set to the block device that we want
+to filter:
+
+ { "execute": "blockdev-add",
+ "arguments": {
+ "options": {
+ "driver": "qcow2",
+ "node-name": "disk0",
+ "file": {
+ "driver": "file",
+ "filename": "/path/to/disk.qcow2"
+ }
+ }
+ }
+ }
+
+ { "execute": "blockdev-add",
+ "arguments": {
+ "driver": "throttle",
+ "node-name": "throttle0",
+ "throttle-group": "group0",
+ "file": "disk0"
+ }
+ }
+
+A similar setup can also be done with the command line, for example:
+
+ -drive driver=throttle,throttle-group=group0,
+ file.driver=qcow2,file.file.filename=/path/to/disk.qcow2
+
+The scenario described so far is very simple but the throttle block
+filter allows for more complex configurations. For example, let's say
+that we have three different drives and we want to set I/O limits for
+each one of them and an additional set of limits for the combined I/O
+of all three drives.
+
+First we would define all throttle groups, one for each one of the
+drives and one that would apply to all of them:
+
+ -object throttle-group,id=limits0,x-iops-total=2000
+ -object throttle-group,id=limits1,x-iops-total=2500
+ -object throttle-group,id=limits2,x-iops-total=3000
+ -object throttle-group,id=limits012,x-iops-total=4000
+
+Now we can define the drives, and for each one of them we use two
+chained throttle filters: the drive's own filter and the combined
+filter.
+
+ -drive driver=throttle,throttle-group=limits012,
+ file.driver=throttle,file.throttle-group=limits0
+ file.file.driver=qcow2,file.file.file.filename=/path/to/disk0.qcow2
+ -drive driver=throttle,throttle-group=limits012,
+ file.driver=throttle,file.throttle-group=limits1
+ file.file.driver=qcow2,file.file.file.filename=/path/to/disk1.qcow2
+ -drive driver=throttle,throttle-group=limits012,
+ file.driver=throttle,file.throttle-group=limits2
+ file.file.driver=qcow2,file.file.file.filename=/path/to/disk2.qcow2
+
+In this example the individual drives have IOPS limits of 2000, 2500
+and 3000 respectively but the total combined I/O can never exceed 4000
+IOPS.
--
2.25.4
^ permalink raw reply related [flat|nested] 39+ messages in thread* [PULL 04/37] qemu-io-cmds: Simplify help_oneline
2020-10-02 14:43 [PULL 00/37] Block layer patches Kevin Wolf
` (2 preceding siblings ...)
2020-10-02 14:43 ` [PULL 03/37] docs: Document the throttle block filter Kevin Wolf
@ 2020-10-02 14:43 ` Kevin Wolf
2020-10-02 14:43 ` [PULL 05/37] nbd: Remove unused nbd_export_get_blockdev() Kevin Wolf
` (33 subsequent siblings)
37 siblings, 0 replies; 39+ messages in thread
From: Kevin Wolf @ 2020-10-02 14:43 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, peter.maydell, qemu-devel
From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
help_oneline is declared and starts as:
static void help_oneline(const char *cmd, const cmdinfo_t *ct)
{
if (cmd) {
printf("%s ", cmd);
} else {
printf("%s ", ct->name);
if (ct->altname) {
printf("(or %s) ", ct->altname);
}
}
However, there are only two routes to help_oneline being called:
help_f -> help_all -> help_oneline(ct->name, ct)
help_f -> help_onecmd(argv[1], ct)
In the first case, 'cmd' and 'ct->name' are the same thing,
so it's impossible for the if (cmd) to be false and then validly
print ct->name - this is upsetting gcc
( https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96739 )
In the second case, cmd is argv[1] and we know we've got argv[1]
so again (cmd) is non-NULL.
Simplify help_oneline by just printing cmd.
(Also strengthen argc check just to be pedantic)
Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Message-Id: <20200824102914.105619-1-dgilbert@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
qemu-io-cmds.c | 11 ++---------
1 file changed, 2 insertions(+), 9 deletions(-)
diff --git a/qemu-io-cmds.c b/qemu-io-cmds.c
index baeae86d8c..4153f1c0b0 100644
--- a/qemu-io-cmds.c
+++ b/qemu-io-cmds.c
@@ -2383,14 +2383,7 @@ static const cmdinfo_t sleep_cmd = {
static void help_oneline(const char *cmd, const cmdinfo_t *ct)
{
- if (cmd) {
- printf("%s ", cmd);
- } else {
- printf("%s ", ct->name);
- if (ct->altname) {
- printf("(or %s) ", ct->altname);
- }
- }
+ printf("%s ", cmd);
if (ct->args) {
printf("%s ", ct->args);
@@ -2420,7 +2413,7 @@ static int help_f(BlockBackend *blk, int argc, char **argv)
{
const cmdinfo_t *ct;
- if (argc == 1) {
+ if (argc < 2) {
help_all();
return 0;
}
--
2.25.4
^ permalink raw reply related [flat|nested] 39+ messages in thread* [PULL 05/37] nbd: Remove unused nbd_export_get_blockdev()
2020-10-02 14:43 [PULL 00/37] Block layer patches Kevin Wolf
` (3 preceding siblings ...)
2020-10-02 14:43 ` [PULL 04/37] qemu-io-cmds: Simplify help_oneline Kevin Wolf
@ 2020-10-02 14:43 ` Kevin Wolf
2020-10-02 14:43 ` [PULL 06/37] qapi: Create block-export module Kevin Wolf
` (32 subsequent siblings)
37 siblings, 0 replies; 39+ messages in thread
From: Kevin Wolf @ 2020-10-02 14:43 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, peter.maydell, qemu-devel
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20200924152717.287415-2-kwolf@redhat.com>
Acked-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@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] 39+ messages in thread* [PULL 06/37] qapi: Create block-export module
2020-10-02 14:43 [PULL 00/37] Block layer patches Kevin Wolf
` (4 preceding siblings ...)
2020-10-02 14:43 ` [PULL 05/37] nbd: Remove unused nbd_export_get_blockdev() Kevin Wolf
@ 2020-10-02 14:43 ` Kevin Wolf
2020-10-02 14:43 ` [PULL 07/37] qapi: Rename BlockExport to BlockExportOptions Kevin Wolf
` (31 subsequent siblings)
37 siblings, 0 replies; 39+ messages in thread
From: Kevin Wolf @ 2020-10-02 14:43 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, peter.maydell, qemu-devel
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>
Message-Id: <20200924152717.287415-3-kwolf@redhat.com>
Acked-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
qapi/block-core.json | 166 -------------------------
qapi/block-export.json | 174 +++++++++++++++++++++++++++
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, 183 insertions(+), 170 deletions(-)
create mode 100644 qapi/block-export.json
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 86ed72ef9f..12a24772b5 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -5194,172 +5194,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..b21b8e3f4d
--- /dev/null
+++ b/qapi/block-export.json
@@ -0,0 +1,174 @@
+# -*- 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".
+#
+# 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'
+ } }
diff --git a/qapi/qapi-schema.json b/qapi/qapi-schema.json
index 0c6ca5c000..8d567e1386 100644
--- a/qapi/qapi-schema.json
+++ b/qapi/qapi-schema.json
@@ -66,6 +66,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 7c4a89a882..ea359a0148 100644
--- a/qapi/meson.build
+++ b/qapi/meson.build
@@ -17,8 +17,9 @@ qapi_all_modules = [
'acpi',
'audio',
'authz',
- 'block-core',
'block',
+ 'block-core',
+ 'block-export',
'char',
'common',
'control',
@@ -49,6 +50,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] 39+ messages in thread* [PULL 07/37] qapi: Rename BlockExport to BlockExportOptions
2020-10-02 14:43 [PULL 00/37] Block layer patches Kevin Wolf
` (5 preceding siblings ...)
2020-10-02 14:43 ` [PULL 06/37] qapi: Create block-export module Kevin Wolf
@ 2020-10-02 14:43 ` Kevin Wolf
2020-10-02 14:43 ` [PULL 08/37] block/export: Add BlockExport infrastructure and block-export-add Kevin Wolf
` (30 subsequent siblings)
37 siblings, 0 replies; 39+ messages in thread
From: Kevin Wolf @ 2020-10-02 14:43 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, peter.maydell, qemu-devel
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>
Message-Id: <20200924152717.287415-4-kwolf@redhat.com>
Acked-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@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 b21b8e3f4d..6ac3a63123 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,16 +159,16 @@
'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] 39+ messages in thread* [PULL 08/37] block/export: Add BlockExport infrastructure and block-export-add
2020-10-02 14:43 [PULL 00/37] Block layer patches Kevin Wolf
` (6 preceding siblings ...)
2020-10-02 14:43 ` [PULL 07/37] qapi: Rename BlockExport to BlockExportOptions Kevin Wolf
@ 2020-10-02 14:43 ` Kevin Wolf
2020-10-02 14:43 ` [PULL 09/37] qemu-storage-daemon: Use qmp_block_export_add() Kevin Wolf
` (29 subsequent siblings)
37 siblings, 0 replies; 39+ messages in thread
From: Kevin Wolf @ 2020-10-02 14:43 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, peter.maydell, qemu-devel
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>
Message-Id: <20200924152717.287415-5-kwolf@redhat.com>
Acked-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
qapi/block-export.json | 10 +++++++++
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, 134 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 6ac3a63123..5890a94219 100644
--- a/qapi/block-export.json
+++ b/qapi/block-export.json
@@ -172,3 +172,13 @@
'data': {
'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 3161c1f037..0f0cc21d16 100644
--- a/meson.build
+++ b/meson.build
@@ -970,6 +970,7 @@ subdir('dump')
block_ss.add(files(
'block.c',
+ 'blockdev-nbd.c',
'blockjob.c',
'job.c',
'qemu-io-cmds.c',
@@ -982,7 +983,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] 39+ messages in thread* [PULL 09/37] qemu-storage-daemon: Use qmp_block_export_add()
2020-10-02 14:43 [PULL 00/37] Block layer patches Kevin Wolf
` (7 preceding siblings ...)
2020-10-02 14:43 ` [PULL 08/37] block/export: Add BlockExport infrastructure and block-export-add Kevin Wolf
@ 2020-10-02 14:43 ` Kevin Wolf
2020-10-02 14:43 ` [PULL 10/37] qemu-nbd: Use raw block driver for --offset Kevin Wolf
` (28 subsequent siblings)
37 siblings, 0 replies; 39+ messages in thread
From: Kevin Wolf @ 2020-10-02 14:43 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, peter.maydell, qemu-devel
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>
Message-Id: <20200924152717.287415-6-kwolf@redhat.com>
Acked-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@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] 39+ messages in thread* [PULL 10/37] qemu-nbd: Use raw block driver for --offset
2020-10-02 14:43 [PULL 00/37] Block layer patches Kevin Wolf
` (8 preceding siblings ...)
2020-10-02 14:43 ` [PULL 09/37] qemu-storage-daemon: Use qmp_block_export_add() Kevin Wolf
@ 2020-10-02 14:43 ` Kevin Wolf
2020-10-02 14:43 ` [PULL 11/37] block/export: Remove magic from block-export-add Kevin Wolf
` (27 subsequent siblings)
37 siblings, 0 replies; 39+ messages in thread
From: Kevin Wolf @ 2020-10-02 14:43 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, peter.maydell, qemu-devel
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>
Message-Id: <20200924152717.287415-7-kwolf@redhat.com>
Acked-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@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 e39d3b23c1..16473d809c 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] 39+ messages in thread* [PULL 11/37] block/export: Remove magic from block-export-add
2020-10-02 14:43 [PULL 00/37] Block layer patches Kevin Wolf
` (9 preceding siblings ...)
2020-10-02 14:43 ` [PULL 10/37] qemu-nbd: Use raw block driver for --offset Kevin Wolf
@ 2020-10-02 14:43 ` Kevin Wolf
2020-10-02 14:43 ` [PULL 12/37] nbd: Add max-connections to nbd-server-start Kevin Wolf
` (26 subsequent siblings)
37 siblings, 0 replies; 39+ messages in thread
From: Kevin Wolf @ 2020-10-02 14:43 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, peter.maydell, qemu-devel
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>
Message-Id: <20200924152717.287415-8-kwolf@redhat.com>
Acked-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@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 16473d809c..b2a0ea6c5e 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] 39+ messages in thread* [PULL 12/37] nbd: Add max-connections to nbd-server-start
2020-10-02 14:43 [PULL 00/37] Block layer patches Kevin Wolf
` (10 preceding siblings ...)
2020-10-02 14:43 ` [PULL 11/37] block/export: Remove magic from block-export-add Kevin Wolf
@ 2020-10-02 14:43 ` Kevin Wolf
2020-10-02 14:43 ` [PULL 13/37] nbd: Add writethrough to block-export-add Kevin Wolf
` (25 subsequent siblings)
37 siblings, 0 replies; 39+ messages in thread
From: Kevin Wolf @ 2020-10-02 14:43 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, peter.maydell, qemu-devel
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>
Message-Id: <20200924152717.287415-9-kwolf@redhat.com>
Acked-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@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 5890a94219..8aa8a01fa6 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:
@@ -46,6 +49,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] 39+ messages in thread* [PULL 13/37] nbd: Add writethrough to block-export-add
2020-10-02 14:43 [PULL 00/37] Block layer patches Kevin Wolf
` (11 preceding siblings ...)
2020-10-02 14:43 ` [PULL 12/37] nbd: Add max-connections to nbd-server-start Kevin Wolf
@ 2020-10-02 14:43 ` Kevin Wolf
2020-10-02 14:43 ` [PULL 14/37] nbd: Remove NBDExport.close callback Kevin Wolf
` (24 subsequent siblings)
37 siblings, 0 replies; 39+ messages in thread
From: Kevin Wolf @ 2020-10-02 14:43 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, peter.maydell, qemu-devel
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>
Message-Id: <20200924152717.287415-10-kwolf@redhat.com>
Acked-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@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 8aa8a01fa6..ce66f278b2 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] 39+ messages in thread* [PULL 14/37] nbd: Remove NBDExport.close callback
2020-10-02 14:43 [PULL 00/37] Block layer patches Kevin Wolf
` (12 preceding siblings ...)
2020-10-02 14:43 ` [PULL 13/37] nbd: Add writethrough to block-export-add Kevin Wolf
@ 2020-10-02 14:43 ` Kevin Wolf
2020-10-02 14:43 ` [PULL 15/37] qemu-nbd: Use blk_exp_add() to create the export Kevin Wolf
` (23 subsequent siblings)
37 siblings, 0 replies; 39+ messages in thread
From: Kevin Wolf @ 2020-10-02 14:43 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, peter.maydell, qemu-devel
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>
Message-Id: <20200924152717.287415-11-kwolf@redhat.com>
Acked-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@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 b2a0ea6c5e..abbed8f87e 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] 39+ messages in thread* [PULL 15/37] qemu-nbd: Use blk_exp_add() to create the export
2020-10-02 14:43 [PULL 00/37] Block layer patches Kevin Wolf
` (13 preceding siblings ...)
2020-10-02 14:43 ` [PULL 14/37] nbd: Remove NBDExport.close callback Kevin Wolf
@ 2020-10-02 14:43 ` Kevin Wolf
2020-10-02 14:43 ` [PULL 16/37] nbd/server: Simplify export shutdown Kevin Wolf
` (22 subsequent siblings)
37 siblings, 0 replies; 39+ messages in thread
From: Kevin Wolf @ 2020-10-02 14:43 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, peter.maydell, qemu-devel
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>
Message-Id: <20200924152717.287415-12-kwolf@redhat.com>
Acked-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@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 abbed8f87e..fc6e68a797 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] 39+ messages in thread* [PULL 16/37] nbd/server: Simplify export shutdown
2020-10-02 14:43 [PULL 00/37] Block layer patches Kevin Wolf
` (14 preceding siblings ...)
2020-10-02 14:43 ` [PULL 15/37] qemu-nbd: Use blk_exp_add() to create the export Kevin Wolf
@ 2020-10-02 14:43 ` Kevin Wolf
2020-10-02 14:43 ` [PULL 17/37] block/export: Move refcount from NBDExport to BlockExport Kevin Wolf
` (21 subsequent siblings)
37 siblings, 0 replies; 39+ messages in thread
From: Kevin Wolf @ 2020-10-02 14:43 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, peter.maydell, qemu-devel
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>
Message-Id: <20200924152717.287415-13-kwolf@redhat.com>
Acked-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@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] 39+ messages in thread* [PULL 17/37] block/export: Move refcount from NBDExport to BlockExport
2020-10-02 14:43 [PULL 00/37] Block layer patches Kevin Wolf
` (15 preceding siblings ...)
2020-10-02 14:43 ` [PULL 16/37] nbd/server: Simplify export shutdown Kevin Wolf
@ 2020-10-02 14:43 ` Kevin Wolf
2020-10-02 14:43 ` [PULL 18/37] block/export: Move AioContext " Kevin Wolf
` (20 subsequent siblings)
37 siblings, 0 replies; 39+ messages in thread
From: Kevin Wolf @ 2020-10-02 14:43 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, peter.maydell, qemu-devel
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>
Message-Id: <20200924152717.287415-14-kwolf@redhat.com>
Acked-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@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] 39+ messages in thread* [PULL 18/37] block/export: Move AioContext from NBDExport to BlockExport
2020-10-02 14:43 [PULL 00/37] Block layer patches Kevin Wolf
` (16 preceding siblings ...)
2020-10-02 14:43 ` [PULL 17/37] block/export: Move refcount from NBDExport to BlockExport Kevin Wolf
@ 2020-10-02 14:43 ` Kevin Wolf
2020-10-02 14:43 ` [PULL 19/37] block/export: Add node-name to BlockExportOptions Kevin Wolf
` (19 subsequent siblings)
37 siblings, 0 replies; 39+ messages in thread
From: Kevin Wolf @ 2020-10-02 14:43 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, peter.maydell, qemu-devel
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Message-Id: <20200924152717.287415-15-kwolf@redhat.com>
Acked-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@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] 39+ messages in thread* [PULL 19/37] block/export: Add node-name to BlockExportOptions
2020-10-02 14:43 [PULL 00/37] Block layer patches Kevin Wolf
` (17 preceding siblings ...)
2020-10-02 14:43 ` [PULL 18/37] block/export: Move AioContext " Kevin Wolf
@ 2020-10-02 14:43 ` Kevin Wolf
2020-10-02 14:43 ` [PULL 20/37] block/export: Allocate BlockExport in blk_exp_add() Kevin Wolf
` (18 subsequent siblings)
37 siblings, 0 replies; 39+ messages in thread
From: Kevin Wolf @ 2020-10-02 14:43 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, peter.maydell, qemu-devel
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>
Message-Id: <20200924152717.287415-16-kwolf@redhat.com>
Acked-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@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 ce66f278b2..1091c97f6f 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 fc6e68a797..fe0d1928c3 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] 39+ messages in thread* [PULL 20/37] block/export: Allocate BlockExport in blk_exp_add()
2020-10-02 14:43 [PULL 00/37] Block layer patches Kevin Wolf
` (18 preceding siblings ...)
2020-10-02 14:43 ` [PULL 19/37] block/export: Add node-name to BlockExportOptions Kevin Wolf
@ 2020-10-02 14:43 ` Kevin Wolf
2020-10-02 14:43 ` [PULL 21/37] block/export: Add blk_exp_close_all(_type) Kevin Wolf
` (17 subsequent siblings)
37 siblings, 0 replies; 39+ messages in thread
From: Kevin Wolf @ 2020-10-02 14:43 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, peter.maydell, qemu-devel
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>
Message-Id: <20200924152717.287415-17-kwolf@redhat.com>
Acked-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@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] 39+ messages in thread* [PULL 21/37] block/export: Add blk_exp_close_all(_type)
2020-10-02 14:43 [PULL 00/37] Block layer patches Kevin Wolf
` (19 preceding siblings ...)
2020-10-02 14:43 ` [PULL 20/37] block/export: Allocate BlockExport in blk_exp_add() Kevin Wolf
@ 2020-10-02 14:43 ` Kevin Wolf
2020-10-02 14:43 ` [PULL 22/37] block/export: Add 'id' option to block-export-add Kevin Wolf
` (16 subsequent siblings)
37 siblings, 0 replies; 39+ messages in thread
From: Kevin Wolf @ 2020-10-02 14:43 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, peter.maydell, qemu-devel
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>
Message-Id: <20200924152717.287415-18-kwolf@redhat.com>
Acked-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@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 f72a2e26e8..f4b6dd5d3d 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 fe0d1928c3..cfa5b78b1a 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] 39+ messages in thread* [PULL 22/37] block/export: Add 'id' option to block-export-add
2020-10-02 14:43 [PULL 00/37] Block layer patches Kevin Wolf
` (20 preceding siblings ...)
2020-10-02 14:43 ` [PULL 21/37] block/export: Add blk_exp_close_all(_type) Kevin Wolf
@ 2020-10-02 14:43 ` Kevin Wolf
2020-10-02 14:43 ` [PULL 23/37] block/export: Move strong user reference to block_exports Kevin Wolf
` (15 subsequent siblings)
37 siblings, 0 replies; 39+ messages in thread
From: Kevin Wolf @ 2020-10-02 14:43 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, peter.maydell, qemu-devel
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>
Message-Id: <20200924152717.287415-19-kwolf@redhat.com>
Acked-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@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 1091c97f6f..658bdf05e1 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 cfa5b78b1a..66cb8f91b1 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] 39+ messages in thread* [PULL 23/37] block/export: Move strong user reference to block_exports
2020-10-02 14:43 [PULL 00/37] Block layer patches Kevin Wolf
` (21 preceding siblings ...)
2020-10-02 14:43 ` [PULL 22/37] block/export: Add 'id' option to block-export-add Kevin Wolf
@ 2020-10-02 14:43 ` Kevin Wolf
2020-10-02 14:43 ` [PULL 24/37] block/export: Add block-export-del Kevin Wolf
` (14 subsequent siblings)
37 siblings, 0 replies; 39+ messages in thread
From: Kevin Wolf @ 2020-10-02 14:43 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, peter.maydell, qemu-devel
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>
Message-Id: <20200924152717.287415-20-kwolf@redhat.com>
Acked-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@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] 39+ messages in thread* [PULL 24/37] block/export: Add block-export-del
2020-10-02 14:43 [PULL 00/37] Block layer patches Kevin Wolf
` (22 preceding siblings ...)
2020-10-02 14:43 ` [PULL 23/37] block/export: Move strong user reference to block_exports Kevin Wolf
@ 2020-10-02 14:43 ` Kevin Wolf
2020-10-02 14:43 ` [PULL 25/37] block/export: Add BLOCK_EXPORT_DELETED event Kevin Wolf
` (13 subsequent siblings)
37 siblings, 0 replies; 39+ messages in thread
From: Kevin Wolf @ 2020-10-02 14:43 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, peter.maydell, qemu-devel
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>
Message-Id: <20200924152717.287415-21-kwolf@redhat.com>
Acked-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@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 658bdf05e1..3d22527c84 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] 39+ messages in thread* [PULL 25/37] block/export: Add BLOCK_EXPORT_DELETED event
2020-10-02 14:43 [PULL 00/37] Block layer patches Kevin Wolf
` (23 preceding siblings ...)
2020-10-02 14:43 ` [PULL 24/37] block/export: Add block-export-del Kevin Wolf
@ 2020-10-02 14:43 ` Kevin Wolf
2020-10-02 14:43 ` [PULL 26/37] block/export: Move blk to BlockExport Kevin Wolf
` (12 subsequent siblings)
37 siblings, 0 replies; 39+ messages in thread
From: Kevin Wolf @ 2020-10-02 14:43 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, peter.maydell, qemu-devel
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>
Message-Id: <20200924152717.287415-22-kwolf@redhat.com>
Acked-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
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 3d22527c84..76e014c406 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] 39+ messages in thread* [PULL 26/37] block/export: Move blk to BlockExport
2020-10-02 14:43 [PULL 00/37] Block layer patches Kevin Wolf
` (24 preceding siblings ...)
2020-10-02 14:43 ` [PULL 25/37] block/export: Add BLOCK_EXPORT_DELETED event Kevin Wolf
@ 2020-10-02 14:43 ` Kevin Wolf
2020-10-02 14:43 ` [PULL 27/37] block/export: Create BlockBackend in blk_exp_add() Kevin Wolf
` (11 subsequent siblings)
37 siblings, 0 replies; 39+ messages in thread
From: Kevin Wolf @ 2020-10-02 14:43 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, peter.maydell, qemu-devel
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>
Message-Id: <20200924152717.287415-23-kwolf@redhat.com>
Acked-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@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] 39+ messages in thread* [PULL 27/37] block/export: Create BlockBackend in blk_exp_add()
2020-10-02 14:43 [PULL 00/37] Block layer patches Kevin Wolf
` (25 preceding siblings ...)
2020-10-02 14:43 ` [PULL 26/37] block/export: Move blk to BlockExport Kevin Wolf
@ 2020-10-02 14:43 ` Kevin Wolf
2020-10-02 14:43 ` [PULL 28/37] block/export: Add query-block-exports Kevin Wolf
` (10 subsequent siblings)
37 siblings, 0 replies; 39+ messages in thread
From: Kevin Wolf @ 2020-10-02 14:43 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, peter.maydell, qemu-devel
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>
Message-Id: <20200924152717.287415-24-kwolf@redhat.com>
Acked-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@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] 39+ messages in thread* [PULL 28/37] block/export: Add query-block-exports
2020-10-02 14:43 [PULL 00/37] Block layer patches Kevin Wolf
` (26 preceding siblings ...)
2020-10-02 14:43 ` [PULL 27/37] block/export: Create BlockBackend in blk_exp_add() Kevin Wolf
@ 2020-10-02 14:43 ` Kevin Wolf
2020-10-02 14:43 ` [PULL 29/37] block/export: Move writable to BlockExportOptions Kevin Wolf
` (9 subsequent siblings)
37 siblings, 0 replies; 39+ messages in thread
From: Kevin Wolf @ 2020-10-02 14:43 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, peter.maydell, qemu-devel
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>
Message-Id: <20200924152717.287415-25-kwolf@redhat.com>
Acked-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@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 76e014c406..91b042b453 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] 39+ messages in thread* [PULL 29/37] block/export: Move writable to BlockExportOptions
2020-10-02 14:43 [PULL 00/37] Block layer patches Kevin Wolf
` (27 preceding siblings ...)
2020-10-02 14:43 ` [PULL 28/37] block/export: Add query-block-exports Kevin Wolf
@ 2020-10-02 14:43 ` Kevin Wolf
2020-10-02 14:43 ` [PULL 30/37] nbd: Merge nbd_export_new() and nbd_export_create() Kevin Wolf
` (8 subsequent siblings)
37 siblings, 0 replies; 39+ messages in thread
From: Kevin Wolf @ 2020-10-02 14:43 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, peter.maydell, qemu-devel
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>
Message-Id: <20200924152717.287415-26-kwolf@redhat.com>
Acked-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@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 91b042b453..3ce4d6276b 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 66cb8f91b1..bacb69b089 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] 39+ messages in thread* [PULL 30/37] nbd: Merge nbd_export_new() and nbd_export_create()
2020-10-02 14:43 [PULL 00/37] Block layer patches Kevin Wolf
` (28 preceding siblings ...)
2020-10-02 14:43 ` [PULL 29/37] block/export: Move writable to BlockExportOptions Kevin Wolf
@ 2020-10-02 14:43 ` Kevin Wolf
2020-10-02 14:43 ` [PULL 31/37] nbd: Deprecate nbd-server-add/remove Kevin Wolf
` (7 subsequent siblings)
37 siblings, 0 replies; 39+ messages in thread
From: Kevin Wolf @ 2020-10-02 14:43 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, peter.maydell, qemu-devel
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>
Message-Id: <20200924152717.287415-27-kwolf@redhat.com>
Acked-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@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] 39+ messages in thread* [PULL 31/37] nbd: Deprecate nbd-server-add/remove
2020-10-02 14:43 [PULL 00/37] Block layer patches Kevin Wolf
` (29 preceding siblings ...)
2020-10-02 14:43 ` [PULL 30/37] nbd: Merge nbd_export_new() and nbd_export_create() Kevin Wolf
@ 2020-10-02 14:43 ` Kevin Wolf
2020-10-02 14:43 ` [PULL 32/37] iotests: Factor out qemu_tool_pipe_and_status() Kevin Wolf
` (6 subsequent siblings)
37 siblings, 0 replies; 39+ messages in thread
From: Kevin Wolf @ 2020-10-02 14:43 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, peter.maydell, qemu-devel
These QMP commands are replaced by block-export-add/del.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Message-Id: <20200924152717.287415-28-kwolf@redhat.com>
Acked-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
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 3ce4d6276b..65804834d9 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 da862201ba..5e8346f7bf 100644
--- a/docs/system/deprecated.rst
+++ b/docs/system/deprecated.rst
@@ -264,6 +264,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] 39+ messages in thread* [PULL 32/37] iotests: Factor out qemu_tool_pipe_and_status()
2020-10-02 14:43 [PULL 00/37] Block layer patches Kevin Wolf
` (30 preceding siblings ...)
2020-10-02 14:43 ` [PULL 31/37] nbd: Deprecate nbd-server-add/remove Kevin Wolf
@ 2020-10-02 14:43 ` Kevin Wolf
2020-10-02 14:43 ` [PULL 33/37] iotests: Introduce qemu_nbd_list_log() Kevin Wolf
` (5 subsequent siblings)
37 siblings, 0 replies; 39+ messages in thread
From: Kevin Wolf @ 2020-10-02 14:43 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, peter.maydell, qemu-devel
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>
Message-Id: <20200924152717.287415-29-kwolf@redhat.com>
Acked-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
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 f48460480a..f7ad0c1395 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -88,21 +88,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]
@@ -263,19 +272,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):
@@ -1141,20 +1144,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] 39+ messages in thread* [PULL 33/37] iotests: Introduce qemu_nbd_list_log()
2020-10-02 14:43 [PULL 00/37] Block layer patches Kevin Wolf
` (31 preceding siblings ...)
2020-10-02 14:43 ` [PULL 32/37] iotests: Factor out qemu_tool_pipe_and_status() Kevin Wolf
@ 2020-10-02 14:43 ` Kevin Wolf
2020-10-02 14:43 ` [PULL 34/37] iotests: Allow supported and unsupported formats at the same time Kevin Wolf
` (4 subsequent siblings)
37 siblings, 0 replies; 39+ messages in thread
From: Kevin Wolf @ 2020-10-02 14:43 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, peter.maydell, qemu-devel
Add a function to list the NBD exports offered by an NBD server.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Message-Id: <20200924152717.287415-30-kwolf@redhat.com>
Acked-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
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 f7ad0c1395..9695c917e4 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -65,7 +65,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(' ')
@@ -280,6 +281,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'''
@@ -413,6 +421,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] 39+ messages in thread* [PULL 34/37] iotests: Allow supported and unsupported formats at the same time
2020-10-02 14:43 [PULL 00/37] Block layer patches Kevin Wolf
` (32 preceding siblings ...)
2020-10-02 14:43 ` [PULL 33/37] iotests: Introduce qemu_nbd_list_log() Kevin Wolf
@ 2020-10-02 14:43 ` Kevin Wolf
2020-10-02 14:43 ` [PULL 35/37] iotests: Test block-export-* QMP interface Kevin Wolf
` (3 subsequent siblings)
37 siblings, 0 replies; 39+ messages in thread
From: Kevin Wolf @ 2020-10-02 14:43 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, peter.maydell, qemu-devel
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>
Message-Id: <20200924152717.287415-31-kwolf@redhat.com>
Acked-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
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 9695c917e4..f212cec446 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -1062,16 +1062,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] 39+ messages in thread* [PULL 35/37] iotests: Test block-export-* QMP interface
2020-10-02 14:43 [PULL 00/37] Block layer patches Kevin Wolf
` (33 preceding siblings ...)
2020-10-02 14:43 ` [PULL 34/37] iotests: Allow supported and unsupported formats at the same time Kevin Wolf
@ 2020-10-02 14:43 ` Kevin Wolf
2020-10-02 14:43 ` [PULL 36/37] qemu-storage-daemon: Fix help line for --export Kevin Wolf
` (2 subsequent siblings)
37 siblings, 0 replies; 39+ messages in thread
From: Kevin Wolf @ 2020-10-02 14:43 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, peter.maydell, qemu-devel
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Message-Id: <20200924152717.287415-32-kwolf@redhat.com>
Acked-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
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] 39+ messages in thread* [PULL 36/37] qemu-storage-daemon: Fix help line for --export
2020-10-02 14:43 [PULL 00/37] Block layer patches Kevin Wolf
` (34 preceding siblings ...)
2020-10-02 14:43 ` [PULL 35/37] iotests: Test block-export-* QMP interface Kevin Wolf
@ 2020-10-02 14:43 ` Kevin Wolf
2020-10-02 14:43 ` [PULL 37/37] qcow2: Use L1E_SIZE in qcow2_write_l1_entry() Kevin Wolf
2020-10-02 18:11 ` [PULL 00/37] Block layer patches Peter Maydell
37 siblings, 0 replies; 39+ messages in thread
From: Kevin Wolf @ 2020-10-02 14:43 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, peter.maydell, qemu-devel
Commit 5f479a8d renamed the 'device' option of --export into
'node-name', but forgot to update the help in qemu-storage-daemon.
Fixes: 5f479a8dc086bfa42c9f94e9ab69962f256e207f
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Message-Id: <20200930133909.58820-1-kwolf@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
storage-daemon/qemu-storage-daemon.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/storage-daemon/qemu-storage-daemon.c b/storage-daemon/qemu-storage-daemon.c
index e6157ff518..1ae1cda481 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>,id=<id>,[,name=<export-name>]\n"
+" --export [type=]nbd,id=<id>,node-name=<node-name>[,name=<export-name>]\n"
" [,writable=on|off][,bitmap=<name>]\n"
" export the specified block node over NBD\n"
" (requires --nbd-server)\n"
--
2.25.4
^ permalink raw reply related [flat|nested] 39+ messages in thread* [PULL 37/37] qcow2: Use L1E_SIZE in qcow2_write_l1_entry()
2020-10-02 14:43 [PULL 00/37] Block layer patches Kevin Wolf
` (35 preceding siblings ...)
2020-10-02 14:43 ` [PULL 36/37] qemu-storage-daemon: Fix help line for --export Kevin Wolf
@ 2020-10-02 14:43 ` Kevin Wolf
2020-10-02 18:11 ` [PULL 00/37] Block layer patches Peter Maydell
37 siblings, 0 replies; 39+ messages in thread
From: Kevin Wolf @ 2020-10-02 14:43 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, peter.maydell, qemu-devel
From: Alberto Garcia <berto@igalia.com>
We overlooked these in 02b1ecfa100e7ecc2306560cd27a4a2622bfeb04
Signed-off-by: Alberto Garcia <berto@igalia.com>
Message-Id: <20200928162333.14998-1-berto@igalia.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
block/qcow2-cluster.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index 9acc6ce4ae..aa87d3e99b 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -240,14 +240,14 @@ int qcow2_write_l1_entry(BlockDriverState *bs, int l1_index)
}
ret = qcow2_pre_write_overlap_check(bs, QCOW2_OL_ACTIVE_L1,
- s->l1_table_offset + 8 * l1_start_index, bufsize, false);
+ s->l1_table_offset + L1E_SIZE * l1_start_index, bufsize, false);
if (ret < 0) {
return ret;
}
BLKDBG_EVENT(bs->file, BLKDBG_L1_UPDATE);
ret = bdrv_pwrite_sync(bs->file,
- s->l1_table_offset + 8 * l1_start_index,
+ s->l1_table_offset + L1E_SIZE * l1_start_index,
buf, bufsize);
if (ret < 0) {
return ret;
--
2.25.4
^ permalink raw reply related [flat|nested] 39+ messages in thread* Re: [PULL 00/37] Block layer patches
2020-10-02 14:43 [PULL 00/37] Block layer patches Kevin Wolf
` (36 preceding siblings ...)
2020-10-02 14:43 ` [PULL 37/37] qcow2: Use L1E_SIZE in qcow2_write_l1_entry() Kevin Wolf
@ 2020-10-02 18:11 ` Peter Maydell
37 siblings, 0 replies; 39+ messages in thread
From: Peter Maydell @ 2020-10-02 18:11 UTC (permalink / raw)
To: Kevin Wolf; +Cc: QEMU Developers, Qemu-block
On Fri, 2 Oct 2020 at 15:44, Kevin Wolf <kwolf@redhat.com> wrote:
>
> The following changes since commit 0d2a4545bf7e763984d3ee3e802617544cb7fc7a:
>
> Merge remote-tracking branch 'remotes/stsquad/tags/pull-testing-and-python-021020-1' into staging (2020-10-02 13:39:20 +0100)
>
> are available in the Git repository at:
>
> git://repo.or.cz/qemu/kevin.git tags/for-upstream
>
> for you to fetch changes up to c508c73dca636cc0fc7413d1e4a43fcfe4a5698c:
>
> qcow2: Use L1E_SIZE in qcow2_write_l1_entry() (2020-10-02 15:46:40 +0200)
>
> ----------------------------------------------------------------
> Block layer patches:
>
> - Add block export infrastructure
> - iotests improvements
> - Document the throttle block filter
> - Misc code cleanups
Applied, thanks.
Please update the changelog at https://wiki.qemu.org/ChangeLog/5.2
for any user-visible changes.
-- PMM
^ permalink raw reply [flat|nested] 39+ messages in thread