* Mon May 30 2022 Miroslav Rezanina <mrezanin@redhat.com> - 7.0.0-5

- kvm-qemu-nbd-Pass-max-connections-to-blockdev-layer.patch [bz#1708300]
- kvm-nbd-server-Allow-MULTI_CONN-for-shared-writable-expo.patch [bz#1708300]
- Resolves: bz#1708300
  (RFE: qemu-nbd vs NBD_FLAG_CAN_MULTI_CONN)
This commit is contained in:
Miroslav Rezanina 2022-05-30 07:32:17 -04:00
parent 550d33ded2
commit 4f42bcba8f
3 changed files with 484 additions and 1 deletions

View File

@ -0,0 +1,381 @@
From 4a9ddf42788d3f924bdad7746f7aca615f03d7c1 Mon Sep 17 00:00:00 2001
From: Eric Blake <eblake@redhat.com>
Date: Wed, 11 May 2022 19:49:24 -0500
Subject: [PATCH 2/2] nbd/server: Allow MULTI_CONN for shared writable exports
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
RH-Author: Eric Blake <eblake@redhat.com>
RH-MergeRequest: 90: Advertise MULTI_CONN on writeable NBD servers
RH-Commit: [2/2] 53f0e885a5ed7f6e4bb14e74fe8e7957e6afe90f (ebblake/centos-qemu-kvm)
RH-Bugzilla: 1708300
RH-Acked-by: Nir Soffer <nsoffer@redhat.com>
RH-Acked-by: Kevin Wolf <kwolf@redhat.com>
RH-Acked-by: Daniel P. Berrangé <berrange@redhat.com>
According to the NBD spec, a server that advertises
NBD_FLAG_CAN_MULTI_CONN promises that multiple client connections will
not see any cache inconsistencies: when properly separated by a single
flush, actions performed by one client will be visible to another
client, regardless of which client did the flush.
We always satisfy these conditions in qemu - even when we support
multiple clients, ALL clients go through a single point of reference
into the block layer, with no local caching. The effect of one client
is instantly visible to the next client. Even if our backend were a
network device, we argue that any multi-path caching effects that
would cause inconsistencies in back-to-back actions not seeing the
effect of previous actions would be a bug in that backend, and not the
fault of caching in qemu. As such, it is safe to unconditionally
advertise CAN_MULTI_CONN for any qemu NBD server situation that
supports parallel clients.
Note, however, that we don't want to advertise CAN_MULTI_CONN when we
know that a second client cannot connect (for historical reasons,
qemu-nbd defaults to a single connection while nbd-server-add and QMP
commands default to unlimited connections; but we already have
existing means to let either style of NBD server creation alter those
defaults). This is visible by no longer advertising MULTI_CONN for
'qemu-nbd -r' without -e, as in the iotest nbd-qemu-allocation.
The harder part of this patch is setting up an iotest to demonstrate
behavior of multiple NBD clients to a single server. It might be
possible with parallel qemu-io processes, but I found it easier to do
in python with the help of libnbd, and help from Nir and Vladimir in
writing the test.
Signed-off-by: Eric Blake <eblake@redhat.com>
Suggested-by: Nir Soffer <nsoffer@redhat.com>
Suggested-by: Vladimir Sementsov-Ogievskiy <v.sementsov-og@mail.ru>
Message-Id: <20220512004924.417153-3-eblake@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
(cherry picked from commit 58a6fdcc9efb2a7c1ef4893dca4aa5e8020ca3dc)
Conflicts:
nbd/server.c - context, e5fb29d5 not backported
Signed-off-by: Eric Blake <eblake@redhat.com>
---
MAINTAINERS | 1 +
blockdev-nbd.c | 5 +
docs/interop/nbd.txt | 1 +
docs/tools/qemu-nbd.rst | 3 +-
include/block/nbd.h | 3 +-
nbd/server.c | 10 +-
qapi/block-export.json | 8 +-
tests/qemu-iotests/tests/nbd-multiconn | 145 ++++++++++++++++++
tests/qemu-iotests/tests/nbd-multiconn.out | 5 +
.../tests/nbd-qemu-allocation.out | 2 +-
10 files changed, 172 insertions(+), 11 deletions(-)
create mode 100755 tests/qemu-iotests/tests/nbd-multiconn
create mode 100644 tests/qemu-iotests/tests/nbd-multiconn.out
diff --git a/MAINTAINERS b/MAINTAINERS
index 4ad2451e03..2fe20a49ab 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -3370,6 +3370,7 @@ F: qemu-nbd.*
F: blockdev-nbd.c
F: docs/interop/nbd.txt
F: docs/tools/qemu-nbd.rst
+F: tests/qemu-iotests/tests/*nbd*
T: git https://repo.or.cz/qemu/ericb.git nbd
T: git https://src.openvz.org/scm/~vsementsov/qemu.git nbd
diff --git a/blockdev-nbd.c b/blockdev-nbd.c
index add41a23af..c6d9b0324c 100644
--- a/blockdev-nbd.c
+++ b/blockdev-nbd.c
@@ -44,6 +44,11 @@ bool nbd_server_is_running(void)
return nbd_server || qemu_nbd_connections >= 0;
}
+int nbd_server_max_connections(void)
+{
+ return nbd_server ? nbd_server->max_connections : qemu_nbd_connections;
+}
+
static void nbd_blockdev_client_closed(NBDClient *client, bool ignored)
{
nbd_client_put(client);
diff --git a/docs/interop/nbd.txt b/docs/interop/nbd.txt
index bdb0f2a41a..f5ca25174a 100644
--- a/docs/interop/nbd.txt
+++ b/docs/interop/nbd.txt
@@ -68,3 +68,4 @@ NBD_CMD_BLOCK_STATUS for "qemu:dirty-bitmap:", NBD_CMD_CACHE
* 4.2: NBD_FLAG_CAN_MULTI_CONN for shareable read-only exports,
NBD_CMD_FLAG_FAST_ZERO
* 5.2: NBD_CMD_BLOCK_STATUS for "qemu:allocation-depth"
+* 7.1: NBD_FLAG_CAN_MULTI_CONN for shareable writable exports
diff --git a/docs/tools/qemu-nbd.rst b/docs/tools/qemu-nbd.rst
index 4c950f6199..8e08a29e89 100644
--- a/docs/tools/qemu-nbd.rst
+++ b/docs/tools/qemu-nbd.rst
@@ -139,8 +139,7 @@ driver options if :option:`--image-opts` is specified.
.. option:: -e, --shared=NUM
Allow up to *NUM* clients to share the device (default
- ``1``), 0 for unlimited. Safe for readers, but for now,
- consistency is not guaranteed between multiple writers.
+ ``1``), 0 for unlimited.
.. option:: -t, --persistent
diff --git a/include/block/nbd.h b/include/block/nbd.h
index c5a29ce1c6..c74b7a9d2e 100644
--- a/include/block/nbd.h
+++ b/include/block/nbd.h
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2016-2020 Red Hat, Inc.
+ * Copyright (C) 2016-2022 Red Hat, Inc.
* Copyright (C) 2005 Anthony Liguori <anthony@codemonkey.ws>
*
* Network Block Device
@@ -346,6 +346,7 @@ void nbd_client_put(NBDClient *client);
void nbd_server_is_qemu_nbd(int max_connections);
bool nbd_server_is_running(void);
+int nbd_server_max_connections(void);
void nbd_server_start(SocketAddress *addr, const char *tls_creds,
const char *tls_authz, uint32_t max_connections,
Error **errp);
diff --git a/nbd/server.c b/nbd/server.c
index c5644fd3f6..6e2157acfa 100644
--- a/nbd/server.c
+++ b/nbd/server.c
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2016-2021 Red Hat, Inc.
+ * Copyright (C) 2016-2022 Red Hat, Inc.
* Copyright (C) 2005 Anthony Liguori <anthony@codemonkey.ws>
*
* Network Block Device Server Side
@@ -1642,7 +1642,6 @@ static int nbd_export_create(BlockExport *blk_exp, BlockExportOptions *exp_args,
int64_t size;
uint64_t perm, shared_perm;
bool readonly = !exp_args->writable;
- bool shared = !exp_args->writable;
strList *bitmaps;
size_t i;
int ret;
@@ -1693,11 +1692,12 @@ static int nbd_export_create(BlockExport *blk_exp, BlockExportOptions *exp_args,
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 (nbd_server_max_connections() != 1) {
+ exp->nbdflags |= NBD_FLAG_CAN_MULTI_CONN;
+ }
if (readonly) {
exp->nbdflags |= NBD_FLAG_READ_ONLY;
- if (shared) {
- exp->nbdflags |= NBD_FLAG_CAN_MULTI_CONN;
- }
} else {
exp->nbdflags |= (NBD_FLAG_SEND_TRIM | NBD_FLAG_SEND_WRITE_ZEROES |
NBD_FLAG_SEND_FAST_ZERO);
diff --git a/qapi/block-export.json b/qapi/block-export.json
index 1e34927f85..755ccc89b1 100644
--- a/qapi/block-export.json
+++ b/qapi/block-export.json
@@ -21,7 +21,9 @@
# 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)
+# time, 0 for unlimited. Setting this to 1 also stops
+# the server from advertising multiple client support
+# (since 5.2; default: 0)
#
# Since: 4.2
##
@@ -50,7 +52,9 @@
# 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)
+# time, 0 for unlimited. Setting this to 1 also stops
+# the server from advertising multiple client support
+# (since 5.2; default: 0).
#
# Returns: error if the server is already running.
#
diff --git a/tests/qemu-iotests/tests/nbd-multiconn b/tests/qemu-iotests/tests/nbd-multiconn
new file mode 100755
index 0000000000..b121f2e363
--- /dev/null
+++ b/tests/qemu-iotests/tests/nbd-multiconn
@@ -0,0 +1,145 @@
+#!/usr/bin/env python3
+# group: rw auto quick
+#
+# Test cases for NBD multi-conn advertisement
+#
+# Copyright (C) 2022 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/>.
+
+import os
+from contextlib import contextmanager
+import iotests
+from iotests import qemu_img_create, qemu_io
+
+
+disk = os.path.join(iotests.test_dir, 'disk')
+size = '4M'
+nbd_sock = os.path.join(iotests.sock_dir, 'nbd_sock')
+nbd_uri = 'nbd+unix:///{}?socket=' + nbd_sock
+
+
+@contextmanager
+def open_nbd(export_name):
+ h = nbd.NBD()
+ try:
+ h.connect_uri(nbd_uri.format(export_name))
+ yield h
+ finally:
+ h.shutdown()
+
+class TestNbdMulticonn(iotests.QMPTestCase):
+ def setUp(self):
+ qemu_img_create('-f', iotests.imgfmt, disk, size)
+ qemu_io('-c', 'w -P 1 0 2M', '-c', 'w -P 2 2M 2M', disk)
+
+ self.vm = iotests.VM()
+ self.vm.launch()
+ result = self.vm.qmp('blockdev-add', {
+ 'driver': 'qcow2',
+ 'node-name': 'n',
+ 'file': {'driver': 'file', 'filename': disk}
+ })
+ self.assert_qmp(result, 'return', {})
+
+ def tearDown(self):
+ self.vm.shutdown()
+ os.remove(disk)
+ try:
+ os.remove(nbd_sock)
+ except OSError:
+ pass
+
+ @contextmanager
+ def run_server(self, max_connections=None):
+ args = {
+ 'addr': {
+ 'type': 'unix',
+ 'data': {'path': nbd_sock}
+ }
+ }
+ if max_connections is not None:
+ args['max-connections'] = max_connections
+
+ result = self.vm.qmp('nbd-server-start', args)
+ self.assert_qmp(result, 'return', {})
+ yield
+
+ result = self.vm.qmp('nbd-server-stop')
+ self.assert_qmp(result, 'return', {})
+
+ def add_export(self, name, writable=None):
+ args = {
+ 'type': 'nbd',
+ 'id': name,
+ 'node-name': 'n',
+ 'name': name,
+ }
+ if writable is not None:
+ args['writable'] = writable
+
+ result = self.vm.qmp('block-export-add', args)
+ self.assert_qmp(result, 'return', {})
+
+ def test_default_settings(self):
+ with self.run_server():
+ self.add_export('r')
+ self.add_export('w', writable=True)
+ with open_nbd('r') as h:
+ self.assertTrue(h.can_multi_conn())
+ with open_nbd('w') as h:
+ self.assertTrue(h.can_multi_conn())
+
+ def test_limited_connections(self):
+ with self.run_server(max_connections=1):
+ self.add_export('r')
+ self.add_export('w', writable=True)
+ with open_nbd('r') as h:
+ self.assertFalse(h.can_multi_conn())
+ with open_nbd('w') as h:
+ self.assertFalse(h.can_multi_conn())
+
+ def test_parallel_writes(self):
+ with self.run_server():
+ self.add_export('w', writable=True)
+
+ clients = [nbd.NBD() for _ in range(3)]
+ for c in clients:
+ c.connect_uri(nbd_uri.format('w'))
+ self.assertTrue(c.can_multi_conn())
+
+ initial_data = clients[0].pread(1024 * 1024, 0)
+ self.assertEqual(initial_data, b'\x01' * 1024 * 1024)
+
+ updated_data = b'\x03' * 1024 * 1024
+ clients[1].pwrite(updated_data, 0)
+ clients[2].flush()
+ current_data = clients[0].pread(1024 * 1024, 0)
+
+ self.assertEqual(updated_data, current_data)
+
+ for i in range(3):
+ clients[i].shutdown()
+
+
+if __name__ == '__main__':
+ try:
+ # Easier to use libnbd than to try and set up parallel
+ # 'qemu-nbd --list' or 'qemu-io' processes, but not all systems
+ # have libnbd installed.
+ import nbd # type: ignore
+
+ iotests.main(supported_fmts=['qcow2'])
+ except ImportError:
+ iotests.notrun('libnbd not installed')
diff --git a/tests/qemu-iotests/tests/nbd-multiconn.out b/tests/qemu-iotests/tests/nbd-multiconn.out
new file mode 100644
index 0000000000..8d7e996700
--- /dev/null
+++ b/tests/qemu-iotests/tests/nbd-multiconn.out
@@ -0,0 +1,5 @@
+...
+----------------------------------------------------------------------
+Ran 3 tests
+
+OK
diff --git a/tests/qemu-iotests/tests/nbd-qemu-allocation.out b/tests/qemu-iotests/tests/nbd-qemu-allocation.out
index 0bf1abb063..9d938db24e 100644
--- a/tests/qemu-iotests/tests/nbd-qemu-allocation.out
+++ b/tests/qemu-iotests/tests/nbd-qemu-allocation.out
@@ -17,7 +17,7 @@ wrote 2097152/2097152 bytes at offset 1048576
exports available: 1
export: ''
size: 4194304
- flags: 0x58f ( readonly flush fua df multi cache )
+ flags: 0x48f ( readonly flush fua df cache )
min block: 1
opt block: 4096
max block: 33554432
--
2.31.1

View File

@ -0,0 +1,92 @@
From e6aae1d0368a152924c38775e517f4e83c1d898b Mon Sep 17 00:00:00 2001
From: Eric Blake <eblake@redhat.com>
Date: Wed, 11 May 2022 19:49:23 -0500
Subject: [PATCH 1/2] qemu-nbd: Pass max connections to blockdev layer
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
RH-Author: Eric Blake <eblake@redhat.com>
RH-MergeRequest: 90: Advertise MULTI_CONN on writeable NBD servers
RH-Commit: [1/2] b0e33fd125bf3523b8b9a4dead3c8bb2342bfd4e (ebblake/centos-qemu-kvm)
RH-Bugzilla: 1708300
RH-Acked-by: Nir Soffer <None>
RH-Acked-by: Kevin Wolf <kwolf@redhat.com>
RH-Acked-by: Daniel P. Berrangé <berrange@redhat.com>
The next patch wants to adjust whether the NBD server code advertises
MULTI_CONN based on whether it is known if the server limits to
exactly one client. For a server started by QMP, this information is
obtained through nbd_server_start (which can support more than one
export); but for qemu-nbd (which supports exactly one export), it is
controlled only by the command-line option -e/--shared. Since we
already have a hook function used by qemu-nbd, it's easiest to just
alter its signature to fit our needs.
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <20220512004924.417153-2-eblake@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
(cherry picked from commit a5fced40212ed73c715ca298a2929dd4d99c9999)
Signed-off-by: Eric Blake <eblake@redhat.com>
---
blockdev-nbd.c | 8 ++++----
include/block/nbd.h | 2 +-
qemu-nbd.c | 2 +-
3 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/blockdev-nbd.c b/blockdev-nbd.c
index 9840d25a82..add41a23af 100644
--- a/blockdev-nbd.c
+++ b/blockdev-nbd.c
@@ -30,18 +30,18 @@ typedef struct NBDServerData {
} NBDServerData;
static NBDServerData *nbd_server;
-static bool is_qemu_nbd;
+static int qemu_nbd_connections = -1; /* Non-negative if this is qemu-nbd */
static void nbd_update_server_watch(NBDServerData *s);
-void nbd_server_is_qemu_nbd(bool value)
+void nbd_server_is_qemu_nbd(int max_connections)
{
- is_qemu_nbd = value;
+ qemu_nbd_connections = max_connections;
}
bool nbd_server_is_running(void)
{
- return nbd_server || is_qemu_nbd;
+ return nbd_server || qemu_nbd_connections >= 0;
}
static void nbd_blockdev_client_closed(NBDClient *client, bool ignored)
diff --git a/include/block/nbd.h b/include/block/nbd.h
index a98eb665da..c5a29ce1c6 100644
--- a/include/block/nbd.h
+++ b/include/block/nbd.h
@@ -344,7 +344,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_is_qemu_nbd(int max_connections);
bool nbd_server_is_running(void);
void nbd_server_start(SocketAddress *addr, const char *tls_creds,
const char *tls_authz, uint32_t max_connections,
diff --git a/qemu-nbd.c b/qemu-nbd.c
index 713e7557a9..8c25ae93df 100644
--- a/qemu-nbd.c
+++ b/qemu-nbd.c
@@ -1087,7 +1087,7 @@ int main(int argc, char **argv)
bs->detect_zeroes = detect_zeroes;
- nbd_server_is_qemu_nbd(true);
+ nbd_server_is_qemu_nbd(shared);
export_opts = g_new(BlockExportOptions, 1);
*export_opts = (BlockExportOptions) {
--
2.31.1

View File

@ -151,7 +151,7 @@ Obsoletes: %{name}-block-ssh <= %{epoch}:%{version} \
Summary: QEMU is a machine emulator and virtualizer
Name: qemu-kvm
Version: 7.0.0
Release: 4%{?rcrel}%{?dist}%{?cc_suffix}
Release: 5%{?rcrel}%{?dist}%{?cc_suffix}
# Epoch because we pushed a qemu-1.0 package. AIUI this can't ever be dropped
# Epoch 15 used for RHEL 8
# Epoch 17 used for RHEL 9 (due to release versioning offset in RHEL 8.5)
@ -240,6 +240,10 @@ Patch42: kvm-virtio-scsi-move-request-related-items-from-.h-to-.c.patch
Patch43: kvm-Revert-virtio-scsi-Reject-scsi-cd-if-data-plane-enab.patch
# For bz#2064530 - Rebuild qemu-kvm with clang-14
Patch44: kvm-migration-Fix-operator-type.patch
# For bz#1708300 - RFE: qemu-nbd vs NBD_FLAG_CAN_MULTI_CONN
Patch45: kvm-qemu-nbd-Pass-max-connections-to-blockdev-layer.patch
# For bz#1708300 - RFE: qemu-nbd vs NBD_FLAG_CAN_MULTI_CONN
Patch46: kvm-nbd-server-Allow-MULTI_CONN-for-shared-writable-expo.patch
# Source-git patches
@ -1275,6 +1279,12 @@ useradd -r -u 107 -g qemu -G kvm -d / -s /sbin/nologin \
%endif
%changelog
* Mon May 30 2022 Miroslav Rezanina <mrezanin@redhat.com> - 7.0.0-5
- kvm-qemu-nbd-Pass-max-connections-to-blockdev-layer.patch [bz#1708300]
- kvm-nbd-server-Allow-MULTI_CONN-for-shared-writable-expo.patch [bz#1708300]
- Resolves: bz#1708300
(RFE: qemu-nbd vs NBD_FLAG_CAN_MULTI_CONN)
* Thu May 19 2022 Miroslav Rezanina <mrezanin@redhat.com> - 7.0.0-4
- kvm-qapi-machine.json-Add-cluster-id.patch [bz#2041823]
- kvm-qtest-numa-test-Specify-CPU-topology-in-aarch64_numa.patch [bz#2041823]