Re: [PULL 00/28] Block layer patches

2023-09-19 Thread Stefan Hajnoczi
On Tue, 19 Sept 2023 at 06:26, Kevin Wolf wrote: > Am 18.09.2023 um 20:56 hat Stefan Hajnoczi geschrieben: > If we could fully get rid of the AioContext lock (as we originally > stated as a goal), that would automatically solve this kind of > deadlocks. Grepping for "ctx locked", "context

Re: [PULL 00/28] Block layer patches

2023-09-19 Thread Stefan Hajnoczi
On Tue, 19 Sept 2023 at 06:26, Kevin Wolf wrote: > > Am 18.09.2023 um 20:56 hat Stefan Hajnoczi geschrieben: > > Hi Kevin, > > I believe that my own commit "block-coroutine-wrapper: use > > qemu_get_current_aio_context()" breaks this test. The failure is > > non-deterministic (happens about 1 out

Re: [PULL 00/28] Block layer patches

2023-09-19 Thread Stefan Hajnoczi
On Tue, 19 Sept 2023 at 06:26, Kevin Wolf wrote: > > Am 18.09.2023 um 20:56 hat Stefan Hajnoczi geschrieben: > > Hi Kevin, > > I believe that my own commit "block-coroutine-wrapper: use > > qemu_get_current_aio_context()" breaks this test. The failure is > > non-deterministic (happens about 1 out

[PATCH v3 4/8] qemu-img: add chunk size parameter to compare_buffers()

2023-09-19 Thread Andrey Drobyshev via
Add @chsize param to the function which, if non-zero, would represent the chunk size to be used for comparison. If it's zero, then BDRV_SECTOR_SIZE is used as default chunk size, which is the previous behaviour. In particular, we're going to use this param in img_rebase() to make the write

[PATCH v3 1/8] qemu-img: rebase: stop when reaching EOF of old backing file

2023-09-19 Thread Andrey Drobyshev via
In case when we're rebasing within one backing chain, and when target image is larger than old backing file, bdrv_is_allocated_above() ends up setting *pnum = 0. As a result, target offset isn't getting incremented, and we get stuck in an infinite for loop. Let's detect this case and proceed

[PATCH v3 8/8] iotests: add tests for "qemu-img rebase" with compression

2023-09-19 Thread Andrey Drobyshev via
The test cases considered so far: 314 (new test suite): 1. Check that compression mode isn't compatible with "-f raw" (raw format doesn't support compression). 2. Check that rebasing an image onto no backing file preserves the data and writes the copied clusters actually compressed. 3.

[PATCH v3 5/8] qemu-img: rebase: avoid unnecessary COW operations

2023-09-19 Thread Andrey Drobyshev via
When rebasing an image from one backing file to another, we need to compare data from old and new backings. If the diff between that data happens to be unaligned to the target cluster size, we might end up doing partial writes, which would lead to copy-on-write and additional IO. Consider the

[PATCH v3 7/8] qemu-img: add compression option to rebase subcommand

2023-09-19 Thread Andrey Drobyshev via
If we rebase an image whose backing file has compressed clusters, we might end up wasting disk space since the copied clusters are now uncompressed. In order to have better control over this, let's add "--compress" option to the "qemu-img rebase" command. Note that this option affects only the

[PATCH v3 3/8] qemu-img: rebase: use backing files' BlockBackend for buffer alignment

2023-09-19 Thread Andrey Drobyshev via
Since commit bb1c05973cf ("qemu-img: Use qemu_blockalign"), buffers for the data read from the old and new backing files are aligned using BlockDriverState (or BlockBackend later on) referring to the target image. However, this isn't quite right, because buf_new is only being used for reading from

[PATCH v3 6/8] iotests/{024, 271}: add testcases for qemu-img rebase

2023-09-19 Thread Andrey Drobyshev via
As the previous commit changes the logic of "qemu-img rebase" (it's using write alignment now), let's add a couple more test cases which would ensure it works correctly. In particular, the following scenarios: 024: add test case for rebase within one backing chain when the overlay cluster

[PATCH v3 2/8] qemu-iotests: 024: add rebasing test case for overlay_size > backing_size

2023-09-19 Thread Andrey Drobyshev via
Before previous commit, rebase was getting infitely stuck in case of rebasing within the same backing chain and when overlay_size > backing_size. Let's add this case to the rebasing test 024 to make sure it doesn't break again. Signed-off-by: Andrey Drobyshev Reviewed-by: Denis V. Lunev

[PATCH v3 0/8] qemu-img: rebase: add compression support

2023-09-19 Thread Andrey Drobyshev via
v2 --> v3: * Patch 3/8: fixed logic in the if statement, so that we align on blk when blk_old_backing == NULL; * Patch 4/8: comment fix; * Patch 5/8: comment fix; dropped redundant "if (blk_new_backing)" statements. v2:

Re: [PATCH v2 5/8] qemu-img: rebase: avoid unnecessary COW operations

2023-09-19 Thread Andrey Drobyshev
On 9/19/23 13:46, Hanna Czenczek wrote: > On 15.09.23 18:20, Andrey Drobyshev wrote: >> When rebasing an image from one backing file to another, we need to >> compare data from old and new backings.  If the diff between that data >> happens to be unaligned to the target cluster size, we might end

Re: [PATCH] block: mark aio_poll as non-coroutine

2023-09-19 Thread Kevin Wolf
Am 08.09.2023 um 09:54 hat Paolo Bonzini geschrieben: > It is forbidden to block on the event loop during a coroutine, as that > can cause deadlocks due to recursive locking. > > Signed-off-by: Paolo Bonzini Thanks, applied to the block branch. Kevin

Re: Deadlock with SATA CD I/O and eject

2023-09-19 Thread Kevin Wolf
Am 19.09.2023 um 14:57 hat John Levon geschrieben: > On Tue, Sep 19, 2023 at 12:54:59PM +0200, Kevin Wolf wrote: > > > > In the meantime, we start processing the blk_drain() code, so by the time > > > this > > > blk_pread() actually gets handled, quiesce is set, and we get stuck in the > > >

Re: [PATCH v2] block-backend: Add new bds_io_in_flight counter

2023-09-19 Thread Kevin Wolf
Am 31.03.2023 um 18:23 hat Hanna Czenczek geschrieben: > IDE TRIM is a BB user that wants to elevate its BB's in-flight counter > for a "macro" operation that consists of several actual I/O operations. > Each of those operations is individually started and awaited. It does > this so that

Re: Deadlock with SATA CD I/O and eject

2023-09-19 Thread John Levon
On Tue, Sep 19, 2023 at 12:54:59PM +0200, Kevin Wolf wrote: > > In the meantime, we start processing the blk_drain() code, so by the time > > this > > blk_pread() actually gets handled, quiesce is set, and we get stuck in the > > blk_wait_while_drained(). > > > > I don't know the qemu block

Re: [PATCH v2 8/8] iotests: add tests for "qemu-img rebase" with compression

2023-09-19 Thread Hanna Czenczek
On 15.09.23 18:20, Andrey Drobyshev wrote: The test cases considered so far: 314 (new test suite): 1. Check that compression mode isn't compatible with "-f raw" (raw format doesn't support compression). 2. Check that rebasing an image onto no backing file preserves the data and writes

Re: [PATCH 1/1] block: improve alignment detection and fix 271 test

2023-09-19 Thread Denis V. Lunev
On 9/7/23 23:53, Denis V. Lunev wrote: Unfortunately 271 IO test is broken if started in non-cached mode. Commits commit a6b257a08e3d72219f03e461a52152672fec0612 Author: Nir Soffer Date: Tue Aug 13 21:21:03 2019 +0300 file-posix: Handle undetectable alignment and

[PATCH v5 5/5] vhost-user: fix lost reconnect

2023-09-19 Thread Li Feng
When the vhost-user is reconnecting to the backend, and if the vhost-user fails at the get_features in vhost_dev_init(), then the reconnect will fail and it will not be retriggered forever. The reason is: When the vhost-user fails at get_features, the vhost_dev_cleanup will be called immediately.

[PATCH v5 0/5] Implement reconnect for vhost-user-scsi

2023-09-19 Thread Li Feng
Ping ... Could anyone review this series and merge them? Changes for v5: - No logic has been changed, just move part of the code from patch 4 to patch 5. Changes for v4: - Merge https://lore.kernel.org/all/20230830045722.611224-1-fen...@smartx.com/ to this series. - Add ERRP_GUARD in

[PATCH v5 2/5] vhost: move and rename the conn retry times

2023-09-19 Thread Li Feng
Multiple devices need this macro, move it to a common header. Signed-off-by: Li Feng Reviewed-by: Raphael Norwitz --- hw/block/vhost-user-blk.c | 4 +--- hw/virtio/vhost-user-gpio.c | 3 +-- include/hw/virtio/vhost.h | 2 ++ 3 files changed, 4 insertions(+), 5 deletions(-) diff --git

[PATCH v5 1/5] vhost-user-common: send get_inflight_fd once

2023-09-19 Thread Li Feng
Currently the get_inflight_fd will be sent every time the device is started, and the backend will allocate shared memory to save the inflight state. If the backend finds that it receives the second get_inflight_fd, it will release the previous shared memory, which breaks inflight working logic.

[PATCH v5 3/5] vhost-user-scsi: support reconnect to backend

2023-09-19 Thread Li Feng
If the backend crashes and restarts, the device is broken. This patch adds reconnect for vhost-user-scsi. This patch also improves the error messages, and reports some silent errors. Tested with spdk backend. Signed-off-by: Li Feng --- hw/scsi/vhost-scsi-common.c | 16 +-

[PATCH v5 4/5] vhost-user-scsi: start vhost when guest kicks

2023-09-19 Thread Li Feng
Let's keep the same behavior as vhost-user-blk. Some old guests kick virtqueue before setting VIRTIO_CONFIG_S_DRIVER_OK. Signed-off-by: Li Feng --- hw/scsi/vhost-user-scsi.c | 48 +++ 1 file changed, 44 insertions(+), 4 deletions(-) diff --git

Re: [PATCH v2 6/8] iotests/{024, 271}: add testcases for qemu-img rebase

2023-09-19 Thread Hanna Czenczek
On 15.09.23 18:20, Andrey Drobyshev wrote: As the previous commit changes the logic of "qemu-img rebase" (it's using write alignment now), let's add a couple more test cases which would ensure it works correctly. In particular, the following scenarios: 024: add test case for rebase within one

Re: Deadlock with SATA CD I/O and eject

2023-09-19 Thread Hanna Czenczek
On 19.09.23 12:54, Kevin Wolf wrote: Am 18.09.2023 um 19:28 hat John Levon geschrieben: Observed with base of qemu 6.2.0, but from code inspection it looks to me like it's still current in upstream master. Apologies if I have missed a fix in this area. Symptom: run a UEFI-booted SATA CD

Re: Deadlock with SATA CD I/O and eject

2023-09-19 Thread Kevin Wolf
Am 18.09.2023 um 19:28 hat John Levon geschrieben: > > Observed with base of qemu 6.2.0, but from code inspection it looks to me like > it's still current in upstream master. Apologies if I have missed a fix in > this > area. > > Symptom: run a UEFI-booted SATA CD Windows installer. When it

Re: [PATCH v2 5/8] qemu-img: rebase: avoid unnecessary COW operations

2023-09-19 Thread Hanna Czenczek
On 15.09.23 18:20, Andrey Drobyshev wrote: When rebasing an image from one backing file to another, we need to compare data from old and new backings. If the diff between that data happens to be unaligned to the target cluster size, we might end up doing partial writes, which would lead to

Re: [PULL 00/28] Block layer patches

2023-09-19 Thread Kevin Wolf
Am 18.09.2023 um 20:56 hat Stefan Hajnoczi geschrieben: > Hi Kevin, > I believe that my own commit "block-coroutine-wrapper: use > qemu_get_current_aio_context()" breaks this test. The failure is > non-deterministic (happens about 1 out of 4 runs). > > It seems the job hangs and the test times

Re: [PATCH 1/2] blockdev: qmp_transaction: harden transaction properties for bitmaps

2023-09-19 Thread Andrey Zhadchenko
Hi! Thanks for the review On 9/12/23 21:29, Vladimir Sementsov-Ogievskiy wrote: On 04.09.23 11:31, Andrey Zhadchenko wrote: Unlike other transaction commands, bitmap operations do not drain target bds. If we have an IOThread, this may result in some inconsistencies, as bitmap content may

Re: [PATCH 20/22] tests: extend test 131 to cover availability of the discard operation

2023-09-19 Thread Alexander Ivanov
On 9/18/23 20:00, Denis V. Lunev wrote: This patch contains test which minimally tests discard and new cluster allocation logic. The following checks are added: * write 2 clusters, discard the first allocated * write another cluster, check that the hole is filled * write 2 clusters, discard the

Re: [PATCH 4/7] block/dirty-bitmap: Clean up local variable shadowing

2023-09-19 Thread Kevin Wolf
Am 19.09.2023 um 07:48 hat Markus Armbruster geschrieben: > Kevin Wolf writes: > > > Am 31.08.2023 um 15:25 hat Markus Armbruster geschrieben: > >> Local variables shadowing other local variables or parameters make the > >> code needlessly hard to understand. Tracked down with -Wshadow=local. >

Re: [PATCH 19/22] parallels: naive implementation of parallels_co_pdiscard

2023-09-19 Thread Alexander Ivanov
On 9/18/23 20:00, Denis V. Lunev wrote: * Discarding with backing stores is not supported by the format. * There is no buffering/queueing of the discard operation. * Only operations aligned to the cluster are supported. Signed-off-by: Denis V. Lunev --- block/parallels.c | 46

Re: [PATCH 04/22] parallels: invent parallels_opts_prealloc() helper to parse prealloc opts

2023-09-19 Thread Alexander Ivanov
On 9/18/23 20:00, Denis V. Lunev wrote: This patch creates above mentioned helper and moves its usage to the beginning of parallels_open(). This simplifies parallels_open() a bit. The patch also ensures that we store prealloc_size on block driver state always in sectors. This makes code cleaner

Re: [PATCH 03/22] parallels: fix memory leak in parallels_open()

2023-09-19 Thread Alexander Ivanov
On 9/18/23 20:00, Denis V. Lunev wrote: We should free opts allocated through qemu_opts_create() at the end. Signed-off-by: Denis V. Lunev --- block/parallels.c | 1 + 1 file changed, 1 insertion(+) diff --git a/block/parallels.c b/block/parallels.c index 428f72de1c..af7be427c9 100644 ---

Re: [PATCH v2 3/8] qemu-img: rebase: use backing files' BlockBackend for buffer alignment

2023-09-19 Thread Andrey Drobyshev
On 9/19/23 11:18, Hanna Czenczek wrote: > On 15.09.23 18:20, Andrey Drobyshev wrote: >> Since commit bb1c05973cf ("qemu-img: Use qemu_blockalign"), buffers for >> the data read from the old and new backing files are aligned using >> BlockDriverState (or BlockBackend later on) referring to the

Re: [PATCH v2 4/8] qemu-img: add chunk size parameter to compare_buffers()

2023-09-19 Thread Hanna Czenczek
On 15.09.23 18:20, Andrey Drobyshev wrote: Add @chsize param to the function which, if non-zero, would represent the chunk size to be used for comparison. If it's zero, then BDRV_SECTOR_SIZE is used as default chunk size, which is the previous behaviour. In particular, we're going to use this

Re: [PATCH v2 3/8] qemu-img: rebase: use backing files' BlockBackend for buffer alignment

2023-09-19 Thread Hanna Czenczek
On 15.09.23 18:20, Andrey Drobyshev wrote: Since commit bb1c05973cf ("qemu-img: Use qemu_blockalign"), buffers for the data read from the old and new backing files are aligned using BlockDriverState (or BlockBackend later on) referring to the target image. However, this isn't quite right,

Re: [PATCH 7/7] qobject atomics osdep: Make a few macros more hygienic

2023-09-19 Thread Markus Armbruster
Eric Blake writes: > On Fri, Sep 01, 2023 at 10:48:26AM +0200, Markus Armbruster wrote: >> > Indeed, not fully understanding the preprocessor makes for some >> > interesting death traps. >> >> We use ALL_CAPS for macros to signal "watch out for traps". >> > >> >> -#define QOBJECT(obj) ({