| Message ID | 20240322095009.346989-1-f.ebner@proxmox.com |
|---|---|
| Headers | show |
| Series | fix two edge cases related to stream block jobs | expand |
On Fri, Mar 22, 2024 at 10:50:05AM +0100, Fiona Ebner wrote: > Changes in v3: > * Also deal with edge case in bdrv_next_cleanup(). Haven't run > into an actual issue there, but at least the caller in > migration/block.c uses bdrv_nb_sectors() which, while not a > coroutine wrapper itself (it's written manually), may call > bdrv_refresh_total_sectors(), which is a generated coroutine > wrapper, so AFAIU, the block graph can change during that call. > And even without that, it's just better to be more consistent > with bdrv_next(). > > Changes in v2: > * Ran into another issue while writing the IO test Stefan wanted > to have (good call :)), so include a fix for that and add the > test. I didn't notice during manual testing, because I hadn't > used a scripted QMP 'quit', so there was no race. > > Fiona Ebner (3): > block-backend: fix edge case in bdrv_next() where BDS associated to BB > changes > block-backend: fix edge case in bdrv_next_cleanup() where BDS > associated to BB changes > iotests: add test for stream job with an unaligned prefetch read > > Stefan Reiter (1): > block/io: accept NULL qiov in bdrv_pad_request > > block/block-backend.c | 18 ++-- > block/io.c | 31 ++++--- > .../tests/stream-unaligned-prefetch | 86 +++++++++++++++++++ > .../tests/stream-unaligned-prefetch.out | 5 ++ > 4 files changed, 117 insertions(+), 23 deletions(-) > create mode 100755 tests/qemu-iotests/tests/stream-unaligned-prefetch > create mode 100644 tests/qemu-iotests/tests/stream-unaligned-prefetch.out Looks good to me. I will wait until Thursday before merging in case Hanna, Vladimir, or Kevin have comments. Thanks! Stefan
Am 25.03.2024 um 21:11 hat Stefan Hajnoczi geschrieben: > On Fri, Mar 22, 2024 at 10:50:05AM +0100, Fiona Ebner wrote: > > Changes in v3: > > * Also deal with edge case in bdrv_next_cleanup(). Haven't run > > into an actual issue there, but at least the caller in > > migration/block.c uses bdrv_nb_sectors() which, while not a > > coroutine wrapper itself (it's written manually), may call > > bdrv_refresh_total_sectors(), which is a generated coroutine > > wrapper, so AFAIU, the block graph can change during that call. > > And even without that, it's just better to be more consistent > > with bdrv_next(). > > > > Changes in v2: > > * Ran into another issue while writing the IO test Stefan wanted > > to have (good call :)), so include a fix for that and add the > > test. I didn't notice during manual testing, because I hadn't > > used a scripted QMP 'quit', so there was no race. > > > > Fiona Ebner (3): > > block-backend: fix edge case in bdrv_next() where BDS associated to BB > > changes > > block-backend: fix edge case in bdrv_next_cleanup() where BDS > > associated to BB changes > > iotests: add test for stream job with an unaligned prefetch read > > > > Stefan Reiter (1): > > block/io: accept NULL qiov in bdrv_pad_request > > > > block/block-backend.c | 18 ++-- > > block/io.c | 31 ++++--- > > .../tests/stream-unaligned-prefetch | 86 +++++++++++++++++++ > > .../tests/stream-unaligned-prefetch.out | 5 ++ > > 4 files changed, 117 insertions(+), 23 deletions(-) > > create mode 100755 tests/qemu-iotests/tests/stream-unaligned-prefetch > > create mode 100644 tests/qemu-iotests/tests/stream-unaligned-prefetch.out > > Looks good to me. I will wait until Thursday before merging in case > Hanna, Vladimir, or Kevin have comments. Thanks! Let's not delay it to -rc2. If something turns out to be wrong with it, we can still revert it, but I think getting fixes in earlier is better during freeze. Thanks, applied to the block branch. Kevin
Changes in v3: * Also deal with edge case in bdrv_next_cleanup(). Haven't run into an actual issue there, but at least the caller in migration/block.c uses bdrv_nb_sectors() which, while not a coroutine wrapper itself (it's written manually), may call bdrv_refresh_total_sectors(), which is a generated coroutine wrapper, so AFAIU, the block graph can change during that call. And even without that, it's just better to be more consistent with bdrv_next(). Changes in v2: * Ran into another issue while writing the IO test Stefan wanted to have (good call :)), so include a fix for that and add the test. I didn't notice during manual testing, because I hadn't used a scripted QMP 'quit', so there was no race. Fiona Ebner (3): block-backend: fix edge case in bdrv_next() where BDS associated to BB changes block-backend: fix edge case in bdrv_next_cleanup() where BDS associated to BB changes iotests: add test for stream job with an unaligned prefetch read Stefan Reiter (1): block/io: accept NULL qiov in bdrv_pad_request block/block-backend.c | 18 ++-- block/io.c | 31 ++++--- .../tests/stream-unaligned-prefetch | 86 +++++++++++++++++++ .../tests/stream-unaligned-prefetch.out | 5 ++ 4 files changed, 117 insertions(+), 23 deletions(-) create mode 100755 tests/qemu-iotests/tests/stream-unaligned-prefetch create mode 100644 tests/qemu-iotests/tests/stream-unaligned-prefetch.out