Re: deadlock when using iothread during backup_clean()

2023-09-05 Thread Fiona Ebner
Am 05.09.23 um 12:01 schrieb Fiona Ebner: > > Can we assume block_job_remove_all_bdrv() to always hold the job's > AioContext? And if yes, can we just tell bdrv_graph_wrlock() that it > needs to release that before polling to fix the deadlock? > I tried a doing something sim

[PATCH v2 1/2] hw/ide: reset: cancel async DMA operation before resetting state

2023-09-06 Thread Fiona Ebner
io_complete (acb=0x7f6460005b10) at ../block/block-backend.c:1524 > #9 blk_aio_write_entry (opaque=0x7f6460005b10) at > ../block/block-backend.c:1594 > #10 0x5595ad258cfb in coroutine_trampoline (i0=, > i1=) at ../util/coroutine-ucontext.c:177 Signed-off-by: Fiona Ebner Reviewed-

[PATCH v2 2/2] tests/qtest: ahci-test: add test exposing reset issue with pending callback

2023-09-06 Thread Fiona Ebner
d verifies that it is still intact after a reset with a pending operation. It also checks that the pending operation actually completes correctly. Signed-off-by: Fiona Ebner --- Changes in v2: * Move variable declarations to the beginning of the function. * Use g_autofree for malloced buffers.

Re: strace showing QEMU process doing >99% ppoll

2023-09-06 Thread Fiona Ebner
Am 10.07.23 um 14:34 schrieb Fiona Ebner: > Hi, > since a while we have about a dozen people reporting [0] VMs rarely > getting stuck with the QEMU process looping and just doing ppoll() and > not much else (example strace [1] output and stacktrace [2]). > > Just wanted to ask i

deadlock when using iothread during backup_clean()

2023-09-05 Thread Fiona Ebner
Hi, as the title says, a deadlock is possible in backup_clean() when using a drive with an IO thread. The main thread keeps waiting in bdrv_graph_wrlock() for reader_count() to become 0, while the IO thread waits for its AioContext lock, which the main thread holds (because it also is the backup

[POC 2/2] add test exposing AHCI reset issue

2023-08-24 Thread Fiona Ebner
her sector not covered by this test. Signed-off-by: Fiona Ebner --- tests/qtest/ahci-test.c | 81 + 1 file changed, 81 insertions(+) diff --git a/tests/qtest/ahci-test.c b/tests/qtest/ahci-test.c index abab761c26..3ebeb4e255 100644 --- a/tests/qtest/ahci-test.c +

[PATCH 1/2] hw/ide: reset: cancel async DMA operation before reseting state

2023-08-24 Thread Fiona Ebner
io_complete (acb=0x7f6460005b10) at ../block/block-backend.c:1524 > #9 blk_aio_write_entry (opaque=0x7f6460005b10) at > ../block/block-backend.c:1594 > #10 0x5595ad258cfb in coroutine_trampoline (i0=, > i1=) at ../util/coroutine-ucontext.c:177 Signed-off-by: Fiona Ebner --- hw

[PATCH v3 6/9] qapi/block-core: use JobType for BlockJobInfo's type

2023-10-13 Thread Fiona Ebner
In preparation to turn BlockJobInfo into a union with @type as the discriminator. That requires it to be an enum. No functional change is intended. Signed-off-by: Fiona Ebner Reviewed-by: Vladimir Sementsov-Ogievskiy --- No changes in v3. block/monitor/block-hmp-cmds.c | 4 ++-- blockjob.c

[PATCH v3 7/9] qapi/block-core: turn BlockJobInfo into a union

2023-10-13 Thread Fiona Ebner
In preparation to additionally return job-type-specific information. Signed-off-by: Fiona Ebner Reviewed-by: Vladimir Sementsov-Ogievskiy --- No changes in v3. qapi/block-core.json | 8 +--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/qapi/block-core.json b/qapi/block

[PATCH v3 5/9] mirror: implement mirror_change method

2023-10-13 Thread Fiona Ebner
, the copy_mode member is now shared between the iothread and the main thread, so turn accesses to it atomic. Signed-off-by: Fiona Ebner --- Changes in v3: * turn accesses to copy_mode atomic and... * ...slightly adapt error handling in mirror_change as a consequence block/mirror.c

[PATCH v5] migration: hold the BQL during setup

2023-10-13 Thread Fiona Ebner
he comment in ram_init_bitmaps() was introduced by 4987783400 ("migration: fix incorrect memory_global_dirty_log_start outside BQL") and is removed, because it referred to the qemu_mutex_lock_iothread() call. Signed-off-by: Fiona Ebner Reviewed-by: Fabiano Rosas Reviewed-by: Juan Quintela

Re: [PATCH v4] migration: hold the BQL during setup

2023-10-13 Thread Fiona Ebner
Am 12.10.23 um 22:40 schrieb Fabiano Rosas: > Fiona Ebner writes: > >> This is intended to be a semantic revert of commit 9b09503752 >> ("migration: run setup callbacks out of big lock"). There have been so >> many changes since that commit (e.g. a new setup c

[PATCH v3 4/9] block/mirror: determine copy_to_target only once

2023-10-13 Thread Fiona Ebner
whether copy_to_target is true or false. Avoid that scenario by determining copy_to_target only once and passing it to bdrv_mirror_top_do_write() as an argument. Signed-off-by: Fiona Ebner Reviewed-by: Vladimir Sementsov-Ogievskiy --- No changes in v3. block/mirror.c | 41

[PATCH v3 2/9] block/mirror: set actively_synced even after the job is ready

2023-10-13 Thread Fiona Ebner
In preparation to allow switching from background to active mode. This ensures that setting actively_synced will not be missed when the switch happens after the job is ready. Signed-off-by: Fiona Ebner Reviewed-by: Vladimir Sementsov-Ogievskiy --- No changes in v3. block/mirror.c | 6

[PATCH v3 0/9] mirror: allow switching from background to active mode

2023-10-13 Thread Fiona Ebner
ng is whether the target is actively synced, the total data sent, and the remaining dirty bytes. Initially, I tried to go for a more general 'job-change' command, but to avoid mutual inclusion of block-core.json and job.json, more preparation would be required. Fiona Ebner (9): blockjob: introd

[PATCH v3 8/9] blockjob: query driver-specific info via a new 'query' driver method

2023-10-13 Thread Fiona Ebner
Signed-off-by: Fiona Ebner --- Changes in v3: * Unlock job mutex while calling the driver's handler for query. blockjob.c | 6 ++ include/block/blockjob_int.h | 5 + 2 files changed, 11 insertions(+) diff --git a/blockjob.c b/blockjob.c index f8cf6e58e2

[PATCH v3 1/9] blockjob: introduce block-job-change QMP command

2023-10-13 Thread Fiona Ebner
which will allow changing job-type-specific options after job creation. In the JobVerbTable, the same allow bits as for set-speed are used, because set-speed can be considered an existing change command. Signed-off-by: Fiona Ebner Reviewed-by: Vladimir Sementsov-Ogievskiy --- No changes in v3

[PATCH v3 3/9] block/mirror: move dirty bitmap to filter

2023-10-13 Thread Fiona Ebner
-by: Fiona Ebner Reviewed-by: Vladimir Sementsov-Ogievskiy --- No changes in v3. block/mirror.c | 16 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/block/mirror.c b/block/mirror.c index b764ad5108..0ed54754e2 100644 --- a/block/mirror.c +++ b/block/mirror.c

[PATCH v3 9/9] mirror: return mirror-specific information upon query

2023-10-13 Thread Fiona Ebner
of an iothread, the actively_synced member is now shared between the iothread and the main thread, so turn accesses to it atomic. Requires to adapt the output for iotest 109. Signed-off-by: Fiona Ebner --- Changes in v3: * squash with patch adapting iotest output * turn accesses

Re: [PULL 1/1] virtio-blk: don't start dataplane during the stop of dataplane

2023-10-17 Thread Fiona Ebner
Am 16.10.23 um 21:40 schrieb Stefan Hajnoczi: > diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c > index 39e7f23fab..c2d59389cb 100644 > --- a/hw/block/virtio-blk.c > +++ b/hw/block/virtio-blk.c > @@ -1166,7 +1166,7 @@ static void virtio_blk_handle_output(VirtIODevice > *vdev, VirtQueue

Re: deadlock when using iothread during backup_clean()

2023-10-17 Thread Fiona Ebner
Am 06.10.23 um 14:18 schrieb Fiona Ebner: > Am 04.10.23 um 19:08 schrieb Vladimir Sementsov-Ogievskiy: >> On 28.09.23 11:06, Fiona Ebner wrote: >>> For fixing the backup cancel deadlock, I tried the following: >>> >>>> diff --git a/blockjob.c b/blockjob.c &

Re: deadlock when using iothread during backup_clean()

2023-10-17 Thread Fiona Ebner
Am 17.10.23 um 14:12 schrieb Kevin Wolf: > Am 17.10.2023 um 12:18 hat Fiona Ebner geschrieben: >> I ran into similar issues now with mirror, (both deadlocks and stuck >> guest IO at other times), and interestingly, also during job start. >> >> Also had a backtrace sim

Re: [PATCH v3 0/9] mirror: allow switching from background to active mode

2023-10-25 Thread Fiona Ebner
Am 23.10.23 um 13:39 schrieb Fiona Ebner: > Am 19.10.23 um 15:36 schrieb Kevin Wolf: >> Most of this series looks good to me. Apart from the comments I made in >> the individual patches, I would like to see iotests coverage of changing >> the mirroring mode. At the least t

Re: [PATCH v2 1/2] hw/ide: reset: cancel async DMA operation before resetting state

2023-10-27 Thread Fiona Ebner
Am 26.10.23 um 08:17 schrieb Michael Tokarev: > 06.09.2023 16:09, Fiona Ebner wrote: >> If there is a pending DMA operation during ide_bus_reset(), the fact >> that the IDEState is already reset before the operation is canceled >> can be problematic. In particular, ide_dma

Re: deadlock when using iothread during backup_clean()

2023-11-03 Thread Fiona Ebner
Am 20.10.23 um 15:52 schrieb Fiona Ebner: > And I found that with drive-mirror, the issue during starting seems to > manifest with the bdrv_open() call. Adding a return before it, the guest > IO didn't get stuck in my testing, but adding a return after it, it can > get stuck. I'll try

Re: [PULL 29/32] virtio-blk: implement BlockDevOps->drained_begin()

2023-11-03 Thread Fiona Ebner
Hi, Am 30.05.23 um 18:32 schrieb Kevin Wolf: > From: Stefan Hajnoczi > > Detach ioeventfds during drained sections to stop I/O submission from > the guest. virtio-blk is no longer reliant on aio_disable_external() > after this patch. This will allow us to remove the > aio_disable_external() API

[PATCH v4 08/10] blockjob: query driver-specific info via a new 'query' driver method

2023-10-31 Thread Fiona Ebner
Signed-off-by: Fiona Ebner --- No changes in v4. blockjob.c | 6 ++ include/block/blockjob_int.h | 5 + 2 files changed, 11 insertions(+) diff --git a/blockjob.c b/blockjob.c index 9665b02627..41719dcf7d 100644 --- a/blockjob.c +++ b/blockjob.c @@ -376,6 +376,7

[PATCH v4 09/10] mirror: return mirror-specific information upon query

2023-10-31 Thread Fiona Ebner
of an iothread, the actively_synced member is now shared between the iothread and the main thread, so turn accesses to it atomic. Requires to adapt the output for iotest 109. Signed-off-by: Fiona Ebner --- Changes in v4: * mention that actively_synced should be accessed with atomics above

[PATCH v4 07/10] qapi/block-core: turn BlockJobInfo into a union

2023-10-31 Thread Fiona Ebner
In preparation to additionally return job-type-specific information. Signed-off-by: Fiona Ebner Reviewed-by: Vladimir Sementsov-Ogievskiy --- No changes in v4. qapi/block-core.json | 8 +--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/qapi/block-core.json b/qapi/block

[PATCH v4 06/10] qapi/block-core: use JobType for BlockJobInfo's type

2023-10-31 Thread Fiona Ebner
In preparation to turn BlockJobInfo into a union with @type as the discriminator. That requires it to be an enum. Even without that requirement, it's nicer to have an enum instead of a str here. No functional change is intended. Signed-off-by: Fiona Ebner Reviewed-by: Vladimir Sementsov

[PATCH v4 01/10] blockjob: introduce block-job-change QMP command

2023-10-31 Thread Fiona Ebner
which will allow changing job-type-specific options after job creation. In the JobVerbTable, the same allow bits as for set-speed are used, because set-speed can be considered an existing change command. Signed-off-by: Fiona Ebner Reviewed-by: Vladimir Sementsov-Ogievskiy --- Changes in v4

[PATCH v4 10/10] iotests: add test for changing mirror's copy_mode

2023-10-31 Thread Fiona Ebner
to active mode actually happened. This is done by hitting the throttling limit, issuing a synchronous write and then immediately verifying the target side. QSD is used, because otherwise, a synchronous write would hang there. Signed-off-by: Fiona Ebner --- New in v4. .../tests/mirror-change-copy-mode

[PATCH v4 00/10] mirror: allow switching from background to active mode

2023-10-31 Thread Fiona Ebner
ml/qemu-devel/2023-10/msg02993.html Fiona Ebner (10): blockjob: introduce block-job-change QMP command block/mirror: set actively_synced even after the job is ready block/mirror: move dirty bitmap to filter block/mirror: determine copy_to_target only once mirror: implement mirror_change

[PATCH v4 02/10] block/mirror: set actively_synced even after the job is ready

2023-10-31 Thread Fiona Ebner
In preparation to allow switching from background to active mode. This ensures that setting actively_synced will not be missed when the switch happens after the job is ready. Signed-off-by: Fiona Ebner Reviewed-by: Vladimir Sementsov-Ogievskiy --- No changes in v4. block/mirror.c | 6

[PATCH v4 05/10] mirror: implement mirror_change method

2023-10-31 Thread Fiona Ebner
, the copy_mode member is now shared between the iothread and the main thread, so turn accesses to it atomic. Signed-off-by: Fiona Ebner --- Changes in v4: * add comment about how copy_mode should be accessed * add GLOBAL_STATE_CODE annotation for mirror_change * use two spaces in between

[PATCH v4 04/10] block/mirror: determine copy_to_target only once

2023-10-31 Thread Fiona Ebner
whether copy_to_target is true or false. Avoid that scenario by determining copy_to_target only once and passing it to bdrv_mirror_top_do_write() as an argument. Signed-off-by: Fiona Ebner Reviewed-by: Vladimir Sementsov-Ogievskiy --- No changes in v4. block/mirror.c | 41

[PATCH v4 03/10] block/mirror: move dirty bitmap to filter

2023-10-31 Thread Fiona Ebner
-by: Fiona Ebner --- Changes in v4: * Also set actively_synced to false when setting the bitmap in bdrv_mirror_top_do_write. block/mirror.c | 17 + 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/block/mirror.c b/block/mirror.c index 1c2c00ee1d

Re: deadlock when using iothread during backup_clean()

2023-09-28 Thread Fiona Ebner
Am 05.09.23 um 13:42 schrieb Paolo Bonzini: > On 9/5/23 12:01, Fiona Ebner wrote: >> Can we assume block_job_remove_all_bdrv() to always hold the job's >> AioContext? > > I think so, see job_unref_locked(), job_prepare_locked() and > job_finalize_single_locked(). 

Re: [PATCH 1/1] hw/ide/core: terminate in-flight DMA on IDE bus reset

2023-09-26 Thread Fiona Ebner
Am 25.09.23 um 21:53 schrieb John Snow: > On Thu, Sep 21, 2023 at 12:07 PM Simon Rowe wrote: >> >> When an IDE controller is reset, its internal state is being cleared >> before any outstanding I/O is cancelled. If a response to DMA is >> received in this window, the aio callback will incorrectly

[PATCH] util/log: re-allow switching away from stderr log file

2023-10-04 Thread Fiona Ebner
ation like > ./qemu-system-x86_64 -trace 'qemu_mutex_lock,file=log' from opening the specified log file. Fixes: 59bde21374 ("util/log: do not close and reopen log files when flags are turned off") Signed-off-by: Fiona Ebner --- util/log.c | 2 ++ 1 file changed, 2 insertions(+) diff --

Re: [PATCH 1/1] hw/ide/core: terminate in-flight DMA on IDE bus reset

2023-09-28 Thread Fiona Ebner
Am 26.09.23 um 16:45 schrieb John Snow: > > > On Tue, Sep 26, 2023, 3:11 AM Fiona Ebner <mailto:f.eb...@proxmox.com>> wrote: > > Am 25.09.23 um 21:53 schrieb John Snow: > > On Thu, Sep 21, 2023 at 12:07 PM Simon Rowe > mailto:simon.r...@nutanix.co

Re: [RFC] migration/block-dirty-bitmap: make loading bitmap for device with iothread future-proof

2023-10-05 Thread Fiona Ebner
ly one aio > context and therefore we are already in it? > Yes, I noticed that myself a bit later ;) Am 01.08.23 um 09:57 schrieb Fiona Ebner: > And the patch itself also got an issue. AFAICT, when > qcow2_co_load_vmstate() is called, we already have acquired the context > fo

[PATCH v2 10/10] iotests: adapt test output for new mirror query property

2023-10-09 Thread Fiona Ebner
Signed-off-by: Fiona Ebner --- New in v2. tests/qemu-iotests/109.out | 24 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/tests/qemu-iotests/109.out b/tests/qemu-iotests/109.out index 2611d6a40f..965c9a6a0a 100644 --- a/tests/qemu-iotests/109.out +++ b

[PATCH v2 09/10] mirror: return mirror-specific information upon query

2023-10-09 Thread Fiona Ebner
-by: Fiona Ebner --- Changes in v2: * udpate QEMU version in QAPI * update indentation in QAPI (like in a937b6aa73 ("qapi: Reformat doc comments to conform to current conventions")) block/mirror.c | 10 ++ qapi/block-core.json | 16 +++- 2 files c

[PATCH v2 03/10] block/mirror: move dirty bitmap to filter

2023-10-09 Thread Fiona Ebner
-by: Fiona Ebner --- New in v2. block/mirror.c | 16 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/block/mirror.c b/block/mirror.c index b764ad5108..0ed54754e2 100644 --- a/block/mirror.c +++ b/block/mirror.c @@ -1500,6 +1500,10 @@ bdrv_mirror_top_do_write

[PATCH v4] migration: hold the BQL during setup

2023-10-12 Thread Fiona Ebner
it_bitmaps() was introduced by 4987783400 ("migration: fix incorrect memory_global_dirty_log_start outside BQL") and is removed, because it referred to the qemu_mutex_lock_iothread() call. Signed-off-by: Fiona Ebner --- Changes in v4: * Rebase on current master (save_prepare handler got ad

Re: deadlock when using iothread during backup_clean()

2023-10-18 Thread Fiona Ebner
Am 17.10.23 um 16:20 schrieb Kevin Wolf: > Am 17.10.2023 um 15:37 hat Fiona Ebner geschrieben: >> Am 17.10.23 um 14:12 schrieb Kevin Wolf: >>> Am 17.10.2023 um 12:18 hat Fiona Ebner geschrieben: >>>> I ran into similar issues now with mirror, (both deadlocks and stuc

Re: [PATCH v3 0/9] mirror: allow switching from background to active mode

2023-10-18 Thread Fiona Ebner
Am 18.10.23 um 11:41 schrieb Markus Armbruster: > Fiona Ebner writes: >> >> Initially, I tried to go for a more general 'job-change' command, but >> to avoid mutual inclusion of block-core.json and job.json, more >> preparation would be required. > > Can you

[PATCH 2/3] block: avoid potential deadlock during bdrv_graph_wrlock() in bdrv_close()

2023-10-19 Thread Fiona Ebner
or calling that the BlockDriverState's AioContext lock is supposed to be held. Signed-off-by: Fiona Ebner --- Is it safe to expect callers of bdrv_unref() to hold the appropriate AioContext lock? block.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/block.c b/block.c index

[PATCH 1/3] blockjob: drop AioContext lock before calling bdrv_graph_wrlock()

2023-10-19 Thread Fiona Ebner
e_all_bdrv(), that is for the main thread's AioContext and does not need to be dropped (bdrv_graph_wrlock(bs) also skips dropping the lock if bdrv_get_aio_context(bs) == qemu_get_aio_context()). Suggested-by: Paolo Bonzini Signed-off-by: Fiona Ebner --- blockjob.c | 2 ++ 1 file changed, 2

[PATCH 3/3] blockdev: mirror: avoid potential deadlock when using iothread

2023-10-19 Thread Fiona Ebner
on top of the source node. Signed-off-by: Fiona Ebner --- blockdev.c | 14 -- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/blockdev.c b/blockdev.c index a01c62596b..877e3a26d4 100644 --- a/blockdev.c +++ b/blockdev.c @@ -2968,6 +2968,7 @@ static void

[PATCH 0/3] fix a few blockjob-related deadlocks when using iothread

2023-10-19 Thread Fiona Ebner
The discussion leading up to this series can be found here: https://lists.nongnu.org/archive/html/qemu-devel/2023-09/msg00729.html Fiona Ebner (3): blockjob: drop AioContext lock before calling bdrv_graph_wrlock() block: avoid deadlock during bdrv_graph_wrlock() in bdrv_close() blockdev

Re: deadlock when using iothread during backup_clean()

2023-10-19 Thread Fiona Ebner
Am 19.10.23 um 14:14 schrieb Kevin Wolf: > Am 18.10.2023 um 11:42 hat Fiona Ebner geschrieben: >> Am 17.10.23 um 16:20 schrieb Kevin Wolf: >>> Am 17.10.2023 um 15:37 hat Fiona Ebner geschrieben: >>>> Am 17.10.23 um 14:12 schrieb Kevin Wolf: >>>>> Am 17

[PATCH v2 01/10] blockjob: introduce block-job-change QMP command

2023-10-09 Thread Fiona Ebner
which will allow changing job-type-specific options after job creation. In the JobVerbTable, the same allow bits as for set-speed are used, because set-speed can be considered an existing change command. Signed-off-by: Fiona Ebner --- Changes in v2: * update QEMU version in QAPI * fix

[PATCH v2 00/10] mirror: allow switching from background to active mode

2023-10-09 Thread Fiona Ebner
s. Initially, I tried to go for a more general 'job-change' command, but I couldn't figure out a way to avoid mutual inclusion between block-core.json and job.json. Fiona Ebner (10): blockjob: introduce block-job-change QMP command block/mirror: set actively_synced even after the job is

[PATCH v2 06/10] qapi/block-core: use JobType for BlockJobInfo's type

2023-10-09 Thread Fiona Ebner
In preparation to turn BlockJobInfo into a union with @type as the discriminator. That requires it to be an enum. No functional change is intended. Signed-off-by: Fiona Ebner Reviewed-by: Vladimir Sementsov-Ogievskiy --- No changes in v2. block/monitor/block-hmp-cmds.c | 4 ++-- blockjob.c

[PATCH v2 04/10] block/mirror: determine copy_to_target only once

2023-10-09 Thread Fiona Ebner
whether copy_to_target is true or false. Avoid that scenario by determining copy_to_target only once and passing it to bdrv_mirror_top_do_write() as an argument. Signed-off-by: Fiona Ebner --- New in v2. block/mirror.c | 41 ++--- 1 file changed, 18 insertions

[PATCH v2 07/10] qapi/block-core: turn BlockJobInfo into a union

2023-10-09 Thread Fiona Ebner
In preparation to additionally return job-type-specific information. Signed-off-by: Fiona Ebner Reviewed-by: Vladimir Sementsov-Ogievskiy --- No changes in v2. qapi/block-core.json | 8 +--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/qapi/block-core.json b/qapi/block

[PATCH v2 05/10] mirror: implement mirror_change method

2023-10-09 Thread Fiona Ebner
which allows switching the @copy-mode from 'background' to 'write-blocking'. This is useful for management applications, so they can start out in background mode to avoid limiting guest write speed and switch to active mode when certain criteria are fulfilled. Signed-off-by: Fiona Ebner

[PATCH v2 02/10] block/mirror: set actively_synced even after the job is ready

2023-10-09 Thread Fiona Ebner
In preparation to allow switching from background to active mode. This ensures that setting actively_synced will not be missed when the switch happens after the job is ready. Signed-off-by: Fiona Ebner Reviewed-by: Vladimir Sementsov-Ogievskiy --- No changes in v2. block/mirror.c | 6

[PATCH v2 08/10] blockjob: query driver-specific info via a new 'query' driver method

2023-10-09 Thread Fiona Ebner
Signed-off-by: Fiona Ebner --- No changes in v2. blockjob.c | 4 include/block/blockjob_int.h | 5 + 2 files changed, 9 insertions(+) diff --git a/blockjob.c b/blockjob.c index f8cf6e58e2..7e8cfad0fd 100644 --- a/blockjob.c +++ b/blockjob.c @@ -376,6 +376,7

Re: [PATCH v2 05/10] mirror: implement mirror_change method

2023-10-11 Thread Fiona Ebner
Am 10.10.23 um 21:37 schrieb Vladimir Sementsov-Ogievskiy: > On 09.10.23 12:46, Fiona Ebner wrote: >>   +static void mirror_change(BlockJob *job, BlockJobChangeOptions *opts, >> +  Error **errp) >> +{ >> +    MirrorBlockJob *s = container_of(

Re: [PATCH v2 00/10] mirror: allow switching from background to active mode

2023-10-11 Thread Fiona Ebner
Am 10.10.23 um 19:55 schrieb Vladimir Sementsov-Ogievskiy: > On 09.10.23 12:46, Fiona Ebner wrote: >> >> Initially, I tried to go for a more general 'job-change' command, but >> I couldn't figure out a way to avoid mutual inclusion between >> block-core.json

Re: [PATCH 3/3] blockdev: mirror: avoid potential deadlock when using iothread

2023-10-20 Thread Fiona Ebner
Am 19.10.23 um 15:19 schrieb Fiona Ebner: > The bdrv_getlength() function is a generated co-wrapper and uses > AIO_WAIT_WHILE() to wait for the spawned coroutine. AIO_WAIT_WHILE() > expects the lock to be acquired exactly once. > > This can happen when the source node is expli

Re: deadlock when using iothread during backup_clean()

2023-10-20 Thread Fiona Ebner
Am 19.10.23 um 15:53 schrieb Fiona Ebner: > Am 19.10.23 um 14:14 schrieb Kevin Wolf: >> Am 18.10.2023 um 11:42 hat Fiona Ebner geschrieben: >>> Am 17.10.23 um 16:20 schrieb Kevin Wolf: >>>> Am 17.10.2023 um 15:37 hat Fiona Ebner geschrieben: >>>>> Am 17

Re: [PATCH v3 1/9] blockjob: introduce block-job-change QMP command

2023-10-23 Thread Fiona Ebner
Am 18.10.23 um 17:52 schrieb Kevin Wolf: > Am 13.10.2023 um 11:21 hat Fiona Ebner geschrieben: >> which will allow changing job-type-specific options after job >> creation. >> >> In the JobVerbTable, the same allow bits as for set-speed are used, >> because set-sp

Re: [PATCH v3 0/9] mirror: allow switching from background to active mode

2023-10-23 Thread Fiona Ebner
Am 19.10.23 um 15:36 schrieb Kevin Wolf: > Most of this series looks good to me. Apart from the comments I made in > the individual patches, I would like to see iotests coverage of changing > the mirroring mode. At the least to show that the query result changes, > but ideally also that requests

Re: [PATCH v3 5/9] mirror: implement mirror_change method

2023-10-23 Thread Fiona Ebner
Am 18.10.23 um 18:59 schrieb Kevin Wolf: > Am 13.10.2023 um 11:21 hat Fiona Ebner geschrieben: >> which allows switching the @copy-mode from 'background' to >> 'write-blocking'. >> >> This is useful for management applications, so they can start out in >> backgr

Re: [PATCH v3 5/9] mirror: implement mirror_change method

2023-10-23 Thread Fiona Ebner
Am 23.10.23 um 14:59 schrieb Kevin Wolf: > Am 23.10.2023 um 13:37 hat Fiona Ebner geschrieben: >>>> +current = qatomic_cmpxchg(>copy_mode, MIRROR_COPY_MODE_BACKGROUND, >>>> + change_opts->copy_mode); >>>> +

Re: deadlock when using iothread during backup_clean()

2023-10-06 Thread Fiona Ebner
Am 04.10.23 um 19:08 schrieb Vladimir Sementsov-Ogievskiy: > On 28.09.23 11:06, Fiona Ebner wrote: >> For fixing the backup cancel deadlock, I tried the following: >> >>> diff --git a/blockjob.c b/blockjob.c >>> index 58c5d64539..fd6132ebfe 100644 >>

Re: [PATCH v2 1/2] hw/ide: reset: cancel async DMA operation before resetting state

2023-10-24 Thread Fiona Ebner
Ping Am 06.09.23 um 15:09 schrieb Fiona Ebner: > If there is a pending DMA operation during ide_bus_reset(), the fact > that the IDEState is already reset before the operation is canceled > can be problematic. In particular, ide_dma_cb() might be called and > then use the reset ID

Re: Lost partition tables on ide-hd + ahci drive

2023-08-23 Thread Fiona Ebner
Am 23.08.23 um 10:47 schrieb Fiona Ebner: > Am 17.02.23 um 22:22 schrieb Mike Maslenkin: >> I can not tell anything about dma-reentracy issues, but yes, i would >> start to look at check_cmd() function call sequence. >> The most interesting is why Sector Count = 1. I thought a

Re: Lost partition tables on ide-hd + ahci drive

2023-08-23 Thread Fiona Ebner
Am 17.02.23 um 22:22 schrieb Mike Maslenkin: > I can not tell anything about dma-reentracy issues, but yes, i would > start to look at check_cmd() function call sequence. > The most interesting is why Sector Count = 1. I thought about race > with IDE reset where registers initialized with > value

[PATCH] iotests: adapt test output for new qemu_cleanup() behavior

2023-08-17 Thread Fiona Ebner
nts before the BLOCK_JOB_CANCELLED event. Reported-by: Kevin Wolf Signed-off-by: Fiona Ebner --- tests/qemu-iotests/109.out | 24 tests/qemu-iotests/185 | 2 ++ tests/qemu-iotests/185.out | 4 3 files changed, 30 insertions(+) diff --git a/tests/qemu-iotests/109.out b/tests/qe

Re: [PATCH] qemu_cleanup: begin drained section after vm_shutdown()

2023-08-17 Thread Fiona Ebner
Am 14.08.23 um 15:53 schrieb Kevin Wolf: > Am 06.07.2023 um 16:43 hat Paolo Bonzini geschrieben: >> Queued, thanks. > > This patch broke qemu-iotests 109 (for raw images), some block jobs get > now paused once more. This is probably okay, but please double check and > fix either the reference

Re: [PATCH v3] migration: hold the BQL during setup

2023-08-28 Thread Fiona Ebner
Ping Am 14.07.23 um 10:20 schrieb Fiona Ebner: > Ping > > Am 30.06.23 um 16:18 schrieb Fiona Ebner: >> This is intended to be a semantic revert of commit 9b09503752 >> ("migration: run setup callbacks out of big lock"). There have been so >> many change

Re: [POC 2/2] add test exposing AHCI reset issue

2023-08-25 Thread Fiona Ebner
Am 24.08.23 um 15:38 schrieb Fiona Ebner: > Fails without the previous commit "hw/ide: reset: cancel async DMA > operation before reseting state". > > I haven't ever written such a test before, but I wanted something to > expose the problem more easily.

Re: Guest reboot issues since QEMU 6.0 and Linux 5.11

2022-07-22 Thread Fiona Ebner
Am 21.07.22 um 17:51 schrieb Maxim Levitsky: > On Thu, 2022-07-21 at 14:49 +0200, Fabian Ebner wrote: >> Hi, >> since about half a year ago, we're getting user reports about guest >> reboot issues with KVM/QEMU[0]. >> >> The most common scenario is a Windows Server VM (2012R2/2016/2019, >>

Re: Guest reboot issues since QEMU 6.0 and Linux 5.11

2022-08-02 Thread Fiona Ebner
Am 28.07.22 um 12:13 schrieb Yan Vugenfirer: > Hi Fabian, > > Can you save the dump file with QEMU monitor using dump-guest-memory or with > virsh dump? > Then you can use elf2dmp (compiled with QEMU and is found in “contrib” > folder) to covert the dump file to WinDbg format and examine the

[PATCH v2] hw/net/vmxnet3: allow VMXNET3_MAX_MTU itself as a value

2022-08-25 Thread Fiona Ebner
et: vmxnet3: validate configuration values during activate (CVE-2021-20203)") Signed-off-by: Fiona Ebner --- Feel free to adapt the commit message as you see fit. v1 -> v2: * Add commit message with some rationale. hw/net/vmxnet3.c | 2 +- 1 file changed, 1 insertion(+), 1 dele

[RFC] hw/net/vmxnet3: allow VMXNET3_MAX_MTU itself as a value

2022-08-24 Thread Fiona Ebner
Fixes: d05dcd94ae ("net: vmxnet3: validate configuration values during activate (CVE-2021-20203)") Signed-off-by: Fiona Ebner --- I'm not familiar with this code, so really I'm asking: is the change justified? I tested the change and it seems to work, but I only have some rough

Question regarding live-migration with drive-mirror

2022-09-28 Thread Fiona Ebner
Hi, recently one of our users provided a backtrace[0] for the following assertion failure during a live migration that uses drive-mirror to sync a local disk: > bdrv_co_write_req_prepare: Assertion `!(bs->open_flags & BDRV_O_INACTIVE)' > failed The way we do migration with a local disk is

Re: Question regarding live-migration with drive-mirror

2022-09-29 Thread Fiona Ebner
Am 28.09.22 um 20:53 schrieb Dr. David Alan Gilbert: > * Fiona Ebner (f.eb...@proxmox.com) wrote: >> Hi, >> recently one of our users provided a backtrace[0] for the following >> assertion failure during a live migration that uses drive-mirror to sync >> a local disk: &g

Re: [PATCH v2] migration/channel-block: fix return value for qio_channel_block_{readv, writev}

2022-10-18 Thread Fiona Ebner
Am 17.10.22 um 11:54 schrieb Zhang, Chen: > > >> -Original Message- >> From: Qemu-devel >> On Behalf Of Fiona Ebner >> Sent: Thursday, October 13, 2022 4:41 PM >> To: qemu-devel@nongnu.org >> Cc: dgilb...@redhat.com; quint...@redhat.com;

Re: [PATCH] vl: change PID file path resolve error to warning

2022-10-28 Thread Fiona Ebner
Am 28.10.22 um 09:26 schrieb Thomas Lamprecht: > On 28/10/2022 09:11, Fiona Ebner wrote: >> Am 27.10.22 um 14:17 schrieb Daniel P. Berrangé: >>> On Thu, Oct 27, 2022 at 12:14:43PM +0200, Fiona Ebner wrote: >>>> +warn_report("not removing PID file on

Re: [PATCH] vl: change PID file path resolve error to warning

2022-10-28 Thread Fiona Ebner
Am 27.10.22 um 14:17 schrieb Daniel P. Berrangé: > On Thu, Oct 27, 2022 at 12:14:43PM +0200, Fiona Ebner wrote: >> +warn_report("not removing PID file on exit: cannot resolve PID >> file" >> +" path: %s: %s", pid_file,

[PATCH] vl: change PID file path resolve error to warning

2022-10-27 Thread Fiona Ebner
by: Thomas Lamprecht Signed-off-by: Fiona Ebner --- For completeness, here is a reproducer based on our actual invocation written in Rust (depends on the "nix" crate). It works fine with QEMU 7.0, but not anymore with 7.1. use std::fs::File; use std::io::Read; use std::os::unix::io:

[PATCH] migration/channel-block: fix return value for qio_channel_block_{readv, writev}

2022-10-12 Thread Fiona Ebner
it is. Signed-off-by: Fiona Ebner --- migration/channel-block.c | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/migration/channel-block.c b/migration/channel-block.c index c55c8c93ce..aabc4634a4 100644 --- a/migration/channel-block.c +++ b/migration/channel-block.c @@ -62,7

Re: [PATCH] migration/channel-block: fix return value for qio_channel_block_{readv, writev}

2022-10-12 Thread Fiona Ebner
Am 12.10.22 um 13:19 schrieb Fiona Ebner: > in the error case. The documentation in include/io/channel.h states > that -1 or QIO_CHANNEL_ERR_BLOCK should be returned upon error. Simply > passing along the return value from the bdrv-functions has the > potential to confuse the cal

[PATCH v2] migration/channel-block: fix return value for qio_channel_block_{readv, writev}

2022-10-13 Thread Fiona Ebner
it is. Signed-off-by: Fiona Ebner --- v1 -> v2: * Use error_setg_errno() instead of error_setg(). * Use "failed" instead of "returned error" in error message. Now that no numerical error code is used, this sounds a bit nicer IMHO. migration/channel-blo

[PATCH v2] vl: defuse PID file path resolve error

2022-10-31 Thread Fiona Ebner
quot;vl: Unlink absolute PID file path") Reported-by: Dominik Csapak Suggested-by: Thomas Lamprecht Signed-off-by: Fiona Ebner --- v1 -> v2: * Ignore error if errno == ENOENT. softmmu/vl.c | 9 + 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/softmmu/vl

[PATCH] migration/rdma: fix return value for qio_channel_rdma_{readv, writev}

2022-12-09 Thread Fiona Ebner
Signed-off-by: Fiona Ebner --- I did only compile-test the patch, because I have no experience with RDMA. @Zhang Chen: In [0], besides qio_channel_rdma_writev/readv, you mentioned qio_channel_buffer_writev/readv and 'etc.', but I did only find the issue in the RDMA-specific implementation.

[PATCH] block/mirror: add 'write-blocking-after-ready' copy mode

2022-12-07 Thread Fiona Ebner
is limited by the synchronous writes to the mirror target. The newly introduced copy mode reduces the time that limit is in effect. Signed-off-by: Fiona Ebner --- See [0] for a bit more context. While the new copy mode doesn't fundamentally improve the downside of 2) (especially when multiple drives

Re: [PATCH v2] hw/net/vmxnet3: allow VMXNET3_MAX_MTU itself as a value

2022-12-14 Thread Fiona Ebner
Am 25.08.22 um 11:29 schrieb Fiona Ebner: > Currently, VMXNET3_MAX_MTU itself (being 9000) is not considered a > valid value for the MTU, but a guest running ESXi 7.0 might try to > set it and fail the assert [0]. > > In the Linux kernel, dev->max_mtu itself is a valid

Re: [PATCH] block/mirror: add 'write-blocking-after-ready' copy mode

2023-01-24 Thread Fiona Ebner
Am 07.12.22 um 14:27 schrieb Fiona Ebner: > The new copy mode starts out in 'background' mode and switches to > 'write-blocking' mode once the job transitions to ready. > > Before switching to active mode and indicating that the drives are > actively synced, it is necessa

Re: [PATCH v2] vl: defuse PID file path resolve error

2023-01-24 Thread Fiona Ebner
Am 31.10.22 um 10:47 schrieb Fiona Ebner: > Commit 85c4bf8aa6 ("vl: Unlink absolute PID file path") introduced a > critical error when the PID file path cannot be resolved. Before this > commit, it was possible to invoke QEMU when the PID file was a file > created with mk

Re: [PATCH 1/4] block: fix detect-zeroes= with BDRV_REQ_REGISTERED_BUF

2023-01-27 Thread Fiona Ebner
ared because bdrv_co_do_pwrite_zeroes() fails with > -EINVAL when it's set. > > Fiona Ebner bisected and diagnosed this QEMU 7.2 > regression where writes containing zeroes to a blockdev with > discard=unmap,detect-zeroes=unmap fail. > > Buglink: https://gitlab.com/qemu-p

Re: [PATCH] block/mirror: add 'write-blocking-after-ready' copy mode

2023-02-02 Thread Fiona Ebner
Am 31.01.23 um 19:18 schrieb Denis V. Lunev: > On 1/31/23 18:44, Vladimir Sementsov-Ogievskiy wrote: >> + Den >> >> Den, I remember we thought about that, and probably had a solution? >> >> Another possible approach to get benefits from both modes is to switch >> to blocking mode after first loop

Re: [PATCH] block/mirror: add 'write-blocking-after-ready' copy mode

2023-02-02 Thread Fiona Ebner
Am 31.01.23 um 18:44 schrieb Vladimir Sementsov-Ogievskiy: >> @@ -1035,10 +1036,31 @@ static int coroutine_fn mirror_run(Job *job, >> Error **errp) >>   if (s->in_flight == 0 && cnt == 0) { >>   trace_mirror_before_flush(s); >>   if (!job_is_ready(>common.job)) { >>

Re: Deadlock with ide_issue_trim and draining

2023-03-08 Thread Fiona Ebner
Am 07.03.23 um 15:27 schrieb Hanna Czenczek: > On 07.03.23 14:44, Hanna Czenczek wrote: >> On 07.03.23 13:22, Fiona Ebner wrote: >>> Hi, >>> I am suspecting that commit 7e5cdb345f ("ide: Increment BB in-flight >>> counter for TRIM BH") in

  1   2   3   >