qemu-block
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v2] block/reqlist: allow adding overlapping requests


From: Vladimir Sementsov-Ogievskiy
Subject: Re: [PATCH v2] block/reqlist: allow adding overlapping requests
Date: Wed, 28 Aug 2024 16:34:05 +0300
User-agent: Mozilla Thunderbird

On 11.08.24 20:55, Michael Tokarev wrote:
12.07.2024 17:07, Fiona Ebner wrote:
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>

Hi!

Has this been forgotten or is it not needed for 9.1?


My apologies, this is forgotten. I think rc4 is too late, I'll send Pull 
request as soon as 9.2 window open.

--
Best regards,
Vladimir




reply via email to

[Prev in Thread] Current Thread [Next in Thread]