nbd/server: Allow MULTI_CONN for shared writable exports
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>
This commit is contained in:
parent
a5fced4021
commit
58a6fdcc9e
|
@ -3367,6 +3367,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
|
||||
|
||||
|
|
|
@ -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);
|
||||
|
|
|
@ -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
|
||||
|
|
|
@ -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
|
||||
|
||||
|
|
|
@ -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);
|
||||
|
|
10
nbd/server.c
10
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;
|
||||
BlockDirtyBitmapOrStrList *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 (readonly) {
|
||||
exp->nbdflags |= NBD_FLAG_READ_ONLY;
|
||||
if (shared) {
|
||||
|
||||
if (nbd_server_max_connections() != 1) {
|
||||
exp->nbdflags |= NBD_FLAG_CAN_MULTI_CONN;
|
||||
}
|
||||
if (readonly) {
|
||||
exp->nbdflags |= NBD_FLAG_READ_ONLY;
|
||||
} else {
|
||||
exp->nbdflags |= (NBD_FLAG_SEND_TRIM | NBD_FLAG_SEND_WRITE_ZEROES |
|
||||
NBD_FLAG_SEND_FAST_ZERO);
|
||||
|
|
|
@ -22,7 +22,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
|
||||
##
|
||||
|
@ -51,7 +53,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.
|
||||
#
|
||||
|
|
|
@ -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')
|
|
@ -0,0 +1,5 @@
|
|||
...
|
||||
----------------------------------------------------------------------
|
||||
Ran 3 tests
|
||||
|
||||
OK
|
|
@ -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
|
||||
|
|
Loading…
Reference in New Issue