37bac4b8e6
Allow overlapping request by removing the assert that made it
impossible. There are only two callers:
1. block_copy_task_create()
It already asserts the very same condition before calling
reqlist_init_req().
2. cbw_snapshot_read_lock()
There is no need to have read requests be non-overlapping in
copy-before-write when used for snapshot-access. In fact, there was no
protection against two callers of cbw_snapshot_read_lock() calling
reqlist_init_req() with overlapping ranges and this could lead to an
assertion failure [1].
In particular, with the reproducer script below [0], two
cbw_co_snapshot_block_status() callers could race, with the second
calling reqlist_init_req() before the first one finishes and removes
its conflicting request.
[0]:
> #!/bin/bash -e
> dd if=/dev/urandom of=/tmp/disk.raw bs=1M count=1024
> ./qemu-img create /tmp/fleecing.raw -f raw 1G
> (
> ./qemu-system-x86_64 --qmp stdio \
> --blockdev raw,node-name=node0,file.driver=file,file.filename=/tmp/disk.raw \
> --blockdev raw,node-name=node1,file.driver=file,file.filename=/tmp/fleecing.raw \
> <<EOF
> {"execute": "qmp_capabilities"}
> {"execute": "blockdev-add", "arguments": { "driver": "copy-before-write", "file": "node0", "target": "node1", "node-name": "node3" } }
> {"execute": "blockdev-add", "arguments": { "driver": "snapshot-access", "file": "node3", "node-name": "snap0" } }
> {"execute": "nbd-server-start", "arguments": {"addr": { "type": "unix", "data": { "path": "/tmp/nbd.socket" } } } }
> {"execute": "block-export-add", "arguments": {"id": "exp0", "node-name": "snap0", "type": "nbd", "name": "exp0"}}
> EOF
> ) &
> sleep 5
> while true; do
> ./qemu-nbd -d /dev/nbd0
> ./qemu-nbd -c /dev/nbd0 nbd:unix:/tmp/nbd.socket:exportname=exp0 -f raw -r
> nbdinfo --map 'nbd+unix:///exp0?socket=/tmp/nbd.socket'
> done
[1]:
> #5 0x000071e5f0088eb2 in __GI___assert_fail (...) at ./assert/assert.c:101
> #6 0x0000615285438017 in reqlist_init_req (...) at ../block/reqlist.c:23
> #7 0x00006152853e2d98 in cbw_snapshot_read_lock (...) at ../block/copy-before-write.c:237
> #8 0x00006152853e3068 in cbw_co_snapshot_block_status (...) at ../block/copy-before-write.c:304
> #9 0x00006152853f4d22 in bdrv_co_snapshot_block_status (...) at ../block/io.c:3726
> #10 0x000061528543a63e in snapshot_access_co_block_status (...) at ../block/snapshot-access.c:48
> #11 0x00006152853f1a0a in bdrv_co_do_block_status (...) at ../block/io.c:2474
> #12 0x00006152853f2016 in bdrv_co_common_block_status_above (...) at ../block/io.c:2652
> #13 0x00006152853f22cf in bdrv_co_block_status_above (...) at ../block/io.c:2732
> #14 0x00006152853d9a86 in blk_co_block_status_above (...) at ../block/block-backend.c:1473
> #15 0x000061528538da6c in blockstatus_to_extents (...) at ../nbd/server.c:2374
> #16 0x000061528538deb1 in nbd_co_send_block_status (...) at ../nbd/server.c:2481
> #17 0x000061528538f424 in nbd_handle_request (...) at ../nbd/server.c:2978
> #18 0x000061528538f906 in nbd_trip (...) at ../nbd/server.c:3121
> #19 0x00006152855a7caf in coroutine_trampoline (...) at ../util/coroutine-ucontext.c:175
Cc: qemu-stable@nongnu.org
Suggested-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
Message-Id: <20240712140716.517911-1-f.ebner@proxmox.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
(cherry picked from commit 6475155d51)
Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
84 lines
1.9 KiB
C
84 lines
1.9 KiB
C
/*
|
|
* reqlist API
|
|
*
|
|
* Copyright (C) 2013 Proxmox Server Solutions
|
|
* Copyright (c) 2021 Virtuozzo International GmbH.
|
|
*
|
|
* Authors:
|
|
* Dietmar Maurer (dietmar@proxmox.com)
|
|
* Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.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 "qemu/range.h"
|
|
|
|
#include "block/reqlist.h"
|
|
|
|
void reqlist_init_req(BlockReqList *reqs, BlockReq *req, int64_t offset,
|
|
int64_t bytes)
|
|
{
|
|
*req = (BlockReq) {
|
|
.offset = offset,
|
|
.bytes = bytes,
|
|
};
|
|
qemu_co_queue_init(&req->wait_queue);
|
|
QLIST_INSERT_HEAD(reqs, req, list);
|
|
}
|
|
|
|
BlockReq *reqlist_find_conflict(BlockReqList *reqs, int64_t offset,
|
|
int64_t bytes)
|
|
{
|
|
BlockReq *r;
|
|
|
|
QLIST_FOREACH(r, reqs, list) {
|
|
if (ranges_overlap(offset, bytes, r->offset, r->bytes)) {
|
|
return r;
|
|
}
|
|
}
|
|
|
|
return NULL;
|
|
}
|
|
|
|
bool coroutine_fn reqlist_wait_one(BlockReqList *reqs, int64_t offset,
|
|
int64_t bytes, CoMutex *lock)
|
|
{
|
|
BlockReq *r = reqlist_find_conflict(reqs, offset, bytes);
|
|
|
|
if (!r) {
|
|
return false;
|
|
}
|
|
|
|
qemu_co_queue_wait(&r->wait_queue, lock);
|
|
|
|
return true;
|
|
}
|
|
|
|
void coroutine_fn reqlist_wait_all(BlockReqList *reqs, int64_t offset,
|
|
int64_t bytes, CoMutex *lock)
|
|
{
|
|
while (reqlist_wait_one(reqs, offset, bytes, lock)) {
|
|
/* continue */
|
|
}
|
|
}
|
|
|
|
void coroutine_fn reqlist_shrink_req(BlockReq *req, int64_t new_bytes)
|
|
{
|
|
if (new_bytes == req->bytes) {
|
|
return;
|
|
}
|
|
|
|
assert(new_bytes > 0 && new_bytes < req->bytes);
|
|
|
|
req->bytes = new_bytes;
|
|
qemu_co_queue_restart_all(&req->wait_queue);
|
|
}
|
|
|
|
void coroutine_fn reqlist_remove_req(BlockReq *req)
|
|
{
|
|
QLIST_REMOVE(req, list);
|
|
qemu_co_queue_restart_all(&req->wait_queue);
|
|
}
|