nbd-client: Refuse read-only client with BDRV_O_RDWR
The NBD spec says that clients should not try to write/trim to an export advertised as read-only by the server. But we failed to check that, and would allow the block layer to use NBD with BDRV_O_RDWR even when the server is read-only, which meant we were depending on the server sending a proper EPERM failure for various commands, and also exposes a leaky abstraction: using qemu-io in read-write mode would succeed on 'w -z 0 0' because of local short-circuiting logic, but 'w 0 0' would send a request over the wire (where it then depends on the server, and fails at least for qemu-nbd but might pass for other NBD implementations). With this patch, a client MUST request read-only mode to access a server that is doing a read-only export, or else it will get a message like: can't open device nbd://localhost:10809/foo: request for write access conflicts with read-only export It is no longer possible to even attempt writes over the wire (including the corner case of 0-length writes), because the block layer enforces the explicit read-only request; this matches the behavior of qcow2 when backed by a read-only POSIX file. Fix several iotests to comply with the new behavior (since qemu-nbd of an internal snapshot, as well as nbd-server-add over QMP, default to a read-only export, we must tell blockdev-add/qemu-io to set up a read-only client). CC: qemu-stable@nongnu.org Signed-off-by: Eric Blake <eblake@redhat.com> Message-Id: <20171108215703.9295-3-eblake@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
This commit is contained in:
parent
e659fb3b99
commit
1104d83c72
@ -697,6 +697,7 @@ int nbd_client_co_pwritev(BlockDriverState *bs, uint64_t offset,
|
||||
.len = bytes,
|
||||
};
|
||||
|
||||
assert(!(client->info.flags & NBD_FLAG_READ_ONLY));
|
||||
if (flags & BDRV_REQ_FUA) {
|
||||
assert(client->info.flags & NBD_FLAG_SEND_FUA);
|
||||
request.flags |= NBD_CMD_FLAG_FUA;
|
||||
@ -717,6 +718,7 @@ int nbd_client_co_pwrite_zeroes(BlockDriverState *bs, int64_t offset,
|
||||
.len = bytes,
|
||||
};
|
||||
|
||||
assert(!(client->info.flags & NBD_FLAG_READ_ONLY));
|
||||
if (!(client->info.flags & NBD_FLAG_SEND_WRITE_ZEROES)) {
|
||||
return -ENOTSUP;
|
||||
}
|
||||
@ -756,6 +758,7 @@ int nbd_client_co_pdiscard(BlockDriverState *bs, int64_t offset, int bytes)
|
||||
.len = bytes,
|
||||
};
|
||||
|
||||
assert(!(client->info.flags & NBD_FLAG_READ_ONLY));
|
||||
if (!(client->info.flags & NBD_FLAG_SEND_TRIM)) {
|
||||
return 0;
|
||||
}
|
||||
@ -814,6 +817,12 @@ int nbd_client_init(BlockDriverState *bs,
|
||||
logout("Failed to negotiate with the NBD server\n");
|
||||
return ret;
|
||||
}
|
||||
if (client->info.flags & NBD_FLAG_READ_ONLY &&
|
||||
!bdrv_is_read_only(bs)) {
|
||||
error_setg(errp,
|
||||
"request for write access conflicts with read-only export");
|
||||
return -EACCES;
|
||||
}
|
||||
if (client->info.flags & NBD_FLAG_SEND_FUA) {
|
||||
bs->supported_write_flags = BDRV_REQ_FUA;
|
||||
bs->supported_zero_flags |= BDRV_REQ_FUA;
|
||||
|
@ -117,15 +117,15 @@ _export_nbd_snapshot sn1
|
||||
|
||||
echo
|
||||
echo "== verifying the exported snapshot with patterns, method 1 =="
|
||||
$QEMU_IO_NBD -c 'read -P 0xa 0x1000 0x1000' "$nbd_snapshot_img" | _filter_qemu_io
|
||||
$QEMU_IO_NBD -c 'read -P 0xb 0x2000 0x1000' "$nbd_snapshot_img" | _filter_qemu_io
|
||||
$QEMU_IO_NBD -r -c 'read -P 0xa 0x1000 0x1000' "$nbd_snapshot_img" | _filter_qemu_io
|
||||
$QEMU_IO_NBD -r -c 'read -P 0xb 0x2000 0x1000' "$nbd_snapshot_img" | _filter_qemu_io
|
||||
|
||||
_export_nbd_snapshot1 sn1
|
||||
|
||||
echo
|
||||
echo "== verifying the exported snapshot with patterns, method 2 =="
|
||||
$QEMU_IO_NBD -c 'read -P 0xa 0x1000 0x1000' "$nbd_snapshot_img" | _filter_qemu_io
|
||||
$QEMU_IO_NBD -c 'read -P 0xb 0x2000 0x1000' "$nbd_snapshot_img" | _filter_qemu_io
|
||||
$QEMU_IO_NBD -r -c 'read -P 0xa 0x1000 0x1000' "$nbd_snapshot_img" | _filter_qemu_io
|
||||
$QEMU_IO_NBD -r -c 'read -P 0xb 0x2000 0x1000' "$nbd_snapshot_img" | _filter_qemu_io
|
||||
|
||||
$QEMU_IMG convert "$TEST_IMG" -l sn1 -O qcow2 "$converted_image"
|
||||
|
||||
|
@ -78,7 +78,7 @@ _send_qemu_cmd $QEMU_HANDLE \
|
||||
'arguments': { 'device': 'drv' }}" \
|
||||
'return'
|
||||
|
||||
$QEMU_IO_PROG -f raw -c 'read -P 42 0 64k' \
|
||||
$QEMU_IO_PROG -f raw -r -c 'read -P 42 0 64k' \
|
||||
"nbd+unix:///drv?socket=$TEST_DIR/nbd" 2>&1 \
|
||||
| _filter_qemu_io | _filter_nbd
|
||||
|
||||
@ -87,7 +87,7 @@ _send_qemu_cmd $QEMU_HANDLE \
|
||||
'arguments': { 'device': 'drv' }}" \
|
||||
'return'
|
||||
|
||||
$QEMU_IO_PROG -f raw -c close \
|
||||
$QEMU_IO_PROG -f raw -r -c close \
|
||||
"nbd+unix:///drv?socket=$TEST_DIR/nbd" 2>&1 \
|
||||
| _filter_qemu_io | _filter_nbd
|
||||
|
||||
|
@ -43,6 +43,7 @@ class NBDBlockdevAddBase(iotests.QMPTestCase):
|
||||
'driver': 'raw',
|
||||
'file': {
|
||||
'driver': 'nbd',
|
||||
'read-only': True,
|
||||
'server': address
|
||||
} }
|
||||
if export is not None:
|
||||
|
Loading…
Reference in New Issue
Block a user