Re: [PATCH v3 0/5] block-copy: make helper APIs thread safe

2021-06-14 Thread Emanuele Giuseppe Esposito
Please discard this thread, I had an issue with git send-email and patch 3-5 are missing. Thank you, Emanuele On 14/06/2021 10:08, Emanuele Giuseppe Esposito wrote: This serie of patches bring thread safety to the smaller APIs used by block-copy, namely ratelimit, progressmeter, co-shared

[PATCH v5 0/6] blkdebug: fix racing condition when iterating on

2021-06-14 Thread Emanuele Giuseppe Esposito
smaller granularity locks to allow multiple iothread execution in the same block device. Signed-off-by: Emanuele Giuseppe Esposito --- v5: * Add comment in patch 1 to explain why we don't need _SAFE in for loop * Move the state update (s->state = new_state) in patch 5, to maintain the s

[PATCH v5 2/6] blkdebug: move post-resume handling to resume_req_by_tag

2021-06-14 Thread Emanuele Giuseppe Esposito
to handle the yielding. Co-developed-by: Paolo Bonzini Signed-off-by: Emanuele Giuseppe Esposito Reviewed-by: Vladimir Sementsov-Ogievskiy --- block/blkdebug.c | 31 ++- 1 file changed, 18 insertions(+), 13 deletions(-) diff --git a/block/blkdebug.c b/block/blkdebug.c

[PATCH v5 5/6] block/blkdebug: remove new_state field and instead use a local variable

2021-06-14 Thread Emanuele Giuseppe Esposito
-by: Emanuele Giuseppe Esposito --- block/blkdebug.c | 13 ++--- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/block/blkdebug.c b/block/blkdebug.c index dd82131d1e..b47c3fd97c 100644 --- a/block/blkdebug.c +++ b/block/blkdebug.c @@ -40,7 +40,6 @@ typedef struct

[PATCH v5 3/6] blkdebug: track all actions

2021-06-14 Thread Emanuele Giuseppe Esposito
Add a counter for each action that a rule can trigger. This is mainly used to keep track of how many coroutine_yield() we need to perform after processing all rules in the list. Co-developed-by: Paolo Bonzini Signed-off-by: Emanuele Giuseppe Esposito Reviewed-by: Vladimir Sementsov-Ogievskiy

[PATCH v5 1/6] blkdebug: refactor removal of a suspended request

2021-06-14 Thread Emanuele Giuseppe Esposito
lo Bonzini Signed-off-by: Emanuele Giuseppe Esposito Reviewed-by: Vladimir Sementsov-Ogievskiy Reviewed-by: Eric Blake --- block/blkdebug.c | 33 ++--- 1 file changed, 22 insertions(+), 11 deletions(-) diff --git a/block/blkdebug.c b/block/blkdebug.c index

[PATCH v5 6/6] blkdebug: protect rules and suspended_reqs with a lock

2021-06-14 Thread Emanuele Giuseppe Esposito
Giuseppe Esposito --- block/blkdebug.c | 49 ++-- 1 file changed, 39 insertions(+), 10 deletions(-) diff --git a/block/blkdebug.c b/block/blkdebug.c index b47c3fd97c..8b67554bec 100644 --- a/block/blkdebug.c +++ b/block/blkdebug.c @@ -38,24 +38,27

[PATCH v5 4/6] blkdebug: do not suspend in the middle of QLIST_FOREACH_SAFE

2021-06-14 Thread Emanuele Giuseppe Esposito
affect the existing testcases. Use actions_count to see how many yield to issue. Co-developed-by: Paolo Bonzini Signed-off-by: Emanuele Giuseppe Esposito Reviewed-by: Vladimir Sementsov-Ogievskiy --- block/blkdebug.c | 7 ++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/block

[PATCH v4 0/6] block-copy: protect block-copy internal structures

2021-06-14 Thread Emanuele Giuseppe Esposito
to true. Based-on: <20210518094058.25952-1-eespo...@redhat.com> Signed-off-by: Emanuele Giuseppe Esposito --- v4: * Introduce patch 1 (refactoring) * Adjust to Vladimir's style comments in patch 2 [Paolo, Vladimir] * Extend the lock to cover an additional race condition in block_copy_

[PATCH v4 1/6] block-copy: small refactor in block_copy_task_entry and block_copy_common

2021-06-14 Thread Emanuele Giuseppe Esposito
Use a local variable instead of referencing BlockCopyState through a BlockCopyCallState or BlockCopyTask every time. This is in preparation for next patches. No functional change intended. Signed-off-by: Emanuele Giuseppe Esposito --- block/block-copy.c | 14 -- 1 file changed, 8

[PATCH v4 3/6] block-copy: improve comments of BlockCopyTask and BlockCopyState types and functions

2021-06-14 Thread Emanuele Giuseppe Esposito
Signed-off-by: Emanuele Giuseppe Esposito --- block/block-copy.c | 49 +- 1 file changed, 31 insertions(+), 18 deletions(-) diff --git a/block/block-copy.c b/block/block-copy.c index 3f26be8ddc..5ff7764e87 100644 --- a/block/block-copy.c +++ b/block/block-copy.c

[PATCH v4 2/6] block-copy: streamline choice of copy_range vs. read/write

2021-06-14 Thread Emanuele Giuseppe Esposito
as an additional copy method. While at it, store the common computation of block_copy_max_transfer into a new field of BlockCopyState, and make sure that we always obey max_transfer; that's more efficient even for the COPY_RANGE_READ_WRITE case. Signed-off-by: Emanuele Giuseppe Esposito Signed-off

[PATCH v4 5/6] block-copy: add a CoMutex

2021-06-14 Thread Emanuele Giuseppe Esposito
in the following patch, because are used also outside coroutines. Also set block_copy_task_create as coroutine_fn because: 1) it is static and only invoked by coroutine functions 2) this patch introduces and uses a CoMutex lock there Signed-off-by: Emanuele Giuseppe Esposito --- block/block-copy.c

[PATCH v4 6/6] block-copy: atomic .cancelled and .finished fields in BlockCopyCallState

2021-06-14 Thread Emanuele Giuseppe Esposito
By adding acquire/release pairs, we ensure that .ret and .error_is_read fields are written by block_copy_dirty_clusters before .finished is true. The atomic here are necessary because the fields are concurrently modified also outside coroutines. Signed-off-by: Emanuele Giuseppe Esposito

[PATCH v4 4/6] block-copy: move progress_set_remaining in block_copy_task_end

2021-06-14 Thread Emanuele Giuseppe Esposito
Moving this function in task_end ensures to update the progress anyways, even if there is an error. It also helps in next patch, allowing task_end to have only one critical section. Reviewed-by: Vladimir Sementsov-Ogievskiy Signed-off-by: Emanuele Giuseppe Esposito --- block/block-copy.c | 6

[PATCH v2] qemu-iotests: fix pylint 2.8 consider-using-with error

2021-05-10 Thread Emanuele Giuseppe Esposito
. Signed-off-by: Emanuele Giuseppe Esposito --- v1 -> v2: * fix indentation [Max] * explain why we disabled the check in QemuIoInteractive's __init__ [Max] tests/qemu-iotests/iotests.py| 65 tests/qemu-iotests/testrunner.py | 22 +-- 2 files changed,

Re: [PATCH v2 5/5] blkdebug: protect rules and suspended_reqs with a lock

2021-05-11 Thread Emanuele Giuseppe Esposito
On 11/05/2021 10:37, Paolo Bonzini wrote: On 07/05/21 17:29, Eric Blake wrote: +    qemu_mutex_lock(>lock);   QLIST_FOREACH(r, >suspended_reqs, next) {   if (!strcmp(r->tag, tag)) { +    qemu_mutex_unlock(>lock);   return true;   }   } +   

Re: [PATCH v3 01/15] python: qemu: add timer parameter for qmp.accept socket

2021-05-14 Thread Emanuele Giuseppe Esposito
On 13/05/2021 19:54, John Snow wrote: On 4/14/21 1:03 PM, Emanuele Giuseppe Esposito wrote: Add a new _qmp_timer field to the QEMUMachine class. The default timer is 15 sec, as per the default in the qmp accept() function. Fine enough for now. What's the exact need for this change

Re: [PATCH v3 10/15] qemu_iotests: extent QMP socket timeout when using valgrind

2021-05-14 Thread Emanuele Giuseppe Esposito
On 13/05/2021 20:47, John Snow wrote: On 4/14/21 1:03 PM, Emanuele Giuseppe Esposito wrote: As with gdbserver, valgrind delays the test execution, so the default QMP socket timeout timeout too soon. Signed-off-by: Emanuele Giuseppe Esposito ---   python/qemu/machine.py    | 2

Re: [PATCH 5/6] co-shared-resource: protect with a mutex

2021-05-14 Thread Emanuele Giuseppe Esposito
we want to get from shres here, after possible call to block_copy_task_shrink(), as task->bytes may be reduced. Ah right, I missed that. So I guess if we want the caller to protect co-shared-resource, get_from_shres stays where it is, and put_ instead can still go into task_end (with a

[PATCH v2 7/7] block-copy: protect BlockCopyState .method fields

2021-05-18 Thread Emanuele Giuseppe Esposito
With tasks and calls lock protecting all State fields, .method is the last BlockCopyState field left unprotected. Set it as atomic. Signed-off-by: Emanuele Giuseppe Esposito --- block/block-copy.c | 37 ++--- 1 file changed, 18 insertions(+), 19 deletions

Re: [PATCH v2 4/5] progressmeter: protect with a mutex

2021-05-18 Thread Emanuele Giuseppe Esposito
On 18/05/2021 12:00, Vladimir Sementsov-Ogievskiy wrote: 18.05.2021 12:40, Emanuele Giuseppe Esposito wrote: Progressmeter is protected by the AioContext mutex, which is taken by the block jobs and their caller (like blockdev). We would like to remove the dependency of block layer code

[PATCH v2 3/5] blockjob: let ratelimit handle a speed of 0

2021-05-18 Thread Emanuele Giuseppe Esposito
From: Paolo Bonzini Reviewed-by: Vladimir Sementsov-Ogievskiy Signed-off-by: Paolo Bonzini Signed-off-by: Emanuele Giuseppe Esposito --- blockjob.c | 13 +++-- 1 file changed, 3 insertions(+), 10 deletions(-) diff --git a/blockjob.c b/blockjob.c index dc1d9e0e46..22e5bb9b1f 100644

[PATCH v2 0/5] block-copy: make helper APIs thread safe

2021-05-18 Thread Emanuele Giuseppe Esposito
(they are also based on the first ratelimit patch sent by Paolo), 4 covers progressmeter and 5 co-shared-resources. Based-on: <20210503112550.478521-1-pbonz...@redhat.com> Based-on: <20210413125533.217440-1-pbonz...@redhat.com> Signed-off-by: Emanuele Giuseppe Esposito --- v1 ->

[PATCH v2 0/7] block-copy: protect block-copy internal structures

2021-05-18 Thread Emanuele Giuseppe Esposito
onz...@redhat.com> Based-on: <20210518094058.25952-1-eespo...@redhat.com> Signed-off-by: Emanuele Giuseppe Esposito --- v1 -> v2: * More field categorized as IN/State/OUT in the various struct, better documentation in the structs * Fix a couple of places where I missed locks [

[PATCH v2 2/7] block-copy: improve documentation of BlockCopyTask and BlockCopyState types and functions

2021-05-18 Thread Emanuele Giuseppe Esposito
in block_copy_task_entry. Also set block_copy_task_create as coroutine_fn because: 1) it is static and only invoked by coroutine functions 2) next patches will introduce and use a CoMutex lock there Signed-off-by: Emanuele Giuseppe Esposito --- block/block-copy.c | 49

[PATCH v2 3/7] block-copy: move progress_set_remaining in block_copy_task_end

2021-05-18 Thread Emanuele Giuseppe Esposito
Moving this function in task_end ensures to update the progress anyways, even if there is an error. It also helps in next patch, allowing task_end to have only one critical section. Signed-off-by: Emanuele Giuseppe Esposito --- block/block-copy.c | 6 +++--- 1 file changed, 3 insertions(+), 3

[PATCH v2 6/7] block-copy: atomic .cancelled and .finished fields in BlockCopyCallState

2021-05-18 Thread Emanuele Giuseppe Esposito
By adding acquire/release pairs, we ensure that .ret and .error_is_read fields are written by block_copy_dirty_clusters before .finished is true. Signed-off-by: Emanuele Giuseppe Esposito --- block/block-copy.c | 33 ++--- 1 file changed, 18 insertions(+), 15

[PATCH v2 1/7] block-copy: streamline choice of copy_range vs. read/write

2021-05-18 Thread Emanuele Giuseppe Esposito
of BlockCopyState, and make sure that we always obey max_transfer; that's more efficient even for the COPY_RANGE_READ_WRITE case. Signed-off-by: Emanuele Giuseppe Esposito Signed-off-by: Paolo Bonzini --- block/block-copy.c | 59 ++ 1 file changed, 39

[PATCH v2 5/7] block-copy: add QemuMutex lock for BlockCopyCallState list

2021-05-18 Thread Emanuele Giuseppe Esposito
. .finished, .cancelled and reads to .ret and .error_is_read will be protected in the following patch. .sleep state is handled in the series "coroutine: new sleep/wake API" Signed-off-by: Emanuele Giuseppe Esposito --- block/block-copy.c | 27 +++ 1 file changed, 19

[PATCH v2 1/5] ratelimit: treat zero speed as unlimited

2021-05-18 Thread Emanuele Giuseppe Esposito
s provided. Reviewed-by: Vladimir Sementsov-Ogievskiy Signed-off-by: Paolo Bonzini Signed-off-by: Emanuele Giuseppe Esposito --- include/qemu/ratelimit.h | 12 ++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/include/qemu/ratelimit.h b/include/qemu/ratelimit.h index

[PATCH v2 5/5] co-shared-resource: protect with a mutex

2021-05-18 Thread Emanuele Giuseppe Esposito
co-shared-resource is currently not thread-safe, as also reported in co-shared-resource.h. Add a QemuMutex because co_try_get_from_shres can also be invoked from non-coroutine context. Signed-off-by: Emanuele Giuseppe Esposito --- include/qemu/co-shared-resource.h | 4 +--- util/qemu-co-shared

[PATCH v2 2/5] block-copy: let ratelimit handle a speed of 0

2021-05-18 Thread Emanuele Giuseppe Esposito
From: Paolo Bonzini Reviewed-by: Vladimir Sementsov-Ogievskiy Signed-off-by: Paolo Bonzini Signed-off-by: Emanuele Giuseppe Esposito --- block/block-copy.c | 28 +++- 1 file changed, 11 insertions(+), 17 deletions(-) diff --git a/block/block-copy.c b/block/block

[PATCH v2 4/5] progressmeter: protect with a mutex

2021-05-18 Thread Emanuele Giuseppe Esposito
to implement the ProgressMeter API, but keep the struct as public, to avoid forcing allocation on the heap. Also add a mutex to be able to provide an accurate snapshot of the progress values to the caller. Reviewed-by: Stefan Hajnoczi Signed-off-by: Emanuele Giuseppe Esposito --- block

[PATCH v2 4/7] block-copy: add a CoMutex to the BlockCopyTask list

2021-05-18 Thread Emanuele Giuseppe Esposito
Because the list of tasks is only modified by coroutine functions, add a CoMutex in order to protect them. Use the same mutex to protect also BlockCopyState in_flight_bytes field to avoid adding additional syncronization primitives. Signed-off-by: Emanuele Giuseppe Esposito --- block/block

Re: [PATCH v3 10/15] qemu_iotests: extent QMP socket timeout when using valgrind

2021-05-18 Thread Emanuele Giuseppe Esposito
So the current plan I have for _qmp_timer is: - As Max suggested, move it in __init__ and check there for the wrapper contents. If we need to block forever (gdb, valgrind), we set it to None. Otherwise to 15 seconds. I think setting it always to None is not ideal, because if you are

Re: [PATCH v3 10/15] qemu_iotests: extent QMP socket timeout when using valgrind

2021-05-18 Thread Emanuele Giuseppe Esposito
On 18/05/2021 16:26, John Snow wrote: On 5/18/21 9:58 AM, Emanuele Giuseppe Esposito wrote: So the current plan I have for _qmp_timer is: - As Max suggested, move it in __init__ and check there for the wrapper contents. If we need to block forever (gdb, valgrind), we set it to None

Re: [PATCH] qemu-iotests: fix pylint 2.8 consider-using-with error

2021-05-10 Thread Emanuele Giuseppe Esposito
On 07/05/2021 17:39, Max Reitz wrote: On 06.05.21 10:48, Emanuele Giuseppe Esposito wrote: pylint 2.8 introduces consider-using-with error, suggesting to use the 'with' block statement when possible. http://pylint.pycqa.org/en/latest/whatsnew/2.8.html Modify all subprocess.Popen calls

[PATCH 5/6] co-shared-resource: protect with a mutex

2021-05-10 Thread Emanuele Giuseppe Esposito
co-shared-resource is currently not thread-safe, as also reported in co-shared-resource.h. Add a QemuMutex because co_try_get_from_shres can also be invoked from non-coroutine context. Signed-off-by: Emanuele Giuseppe Esposito --- util/qemu-co-shared-resource.c | 26

[PATCH 2/6] block-copy: let ratelimit handle a speed of 0

2021-05-10 Thread Emanuele Giuseppe Esposito
From: Paolo Bonzini Signed-off-by: Paolo Bonzini Signed-off-by: Emanuele Giuseppe Esposito --- block/block-copy.c | 28 ++-- 1 file changed, 10 insertions(+), 18 deletions(-) diff --git a/block/block-copy.c b/block/block-copy.c index c2e5090412..7e9467d48a 100644

[PATCH 3/6] blockjob: let ratelimit handle a speed of 0

2021-05-10 Thread Emanuele Giuseppe Esposito
From: Paolo Bonzini Signed-off-by: Paolo Bonzini Signed-off-by: Emanuele Giuseppe Esposito --- blockjob.c | 12 +++- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/blockjob.c b/blockjob.c index dc1d9e0e46..046c1bcd66 100644 --- a/blockjob.c +++ b/blockjob.c @@ -300,10

[PATCH 4/6] progressmeter: protect with a mutex

2021-05-10 Thread Emanuele Giuseppe Esposito
to do is to add a mutex to be able to provide an accurate snapshot of the progress values to the caller. Signed-off-by: Emanuele Giuseppe Esposito --- blockjob.c| 33 + include/qemu/progress_meter.h | 31 +++ job-qmp.c

[PATCH 0/6] block-copy: make helper APIs thread safe

2021-05-10 Thread Emanuele Giuseppe Esposito
by: Emanuele Giuseppe Esposito --- v1 -> v2: * More field categorized as IN/State/OUT in the various struct * Fix a couple of places where I missed locks [Vladimir, Paolo] Emanuele Giuseppe Esposito (3): progressmeter: protect with a mutex co-shared-resource: protect with a mutex a

[PATCH 1/6] ratelimit: treat zero speed as unlimited

2021-05-10 Thread Emanuele Giuseppe Esposito
d. Signed-off-by: Paolo Bonzini Signed-off-by: Emanuele Giuseppe Esposito --- include/qemu/ratelimit.h | 12 ++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/include/qemu/ratelimit.h b/include/qemu/ratelimit.h index 003ea6d5a3..48bf59e857 100644 --- a/include/qemu/r

[PATCH 6/6] aiopool: protect with a mutex

2021-05-10 Thread Emanuele Giuseppe Esposito
Divide the fields in AioTaskPool in IN and Status, and introduce a CoQueue instead of .wait to take care of suspending and resuming the calling coroutine, and a lock to protect the busy_tasks counter accesses and the AioTask .ret field. Signed-off-by: Emanuele Giuseppe Esposito --- block

[PATCH v2 2/5] blkdebug: move post-resume handling to resume_req_by_tag

2021-05-07 Thread Emanuele Giuseppe Esposito
to handle the yielding. Co-developed-by: Paolo Bonzini Signed-off-by: Emanuele Giuseppe Esposito --- block/blkdebug.c | 31 ++- 1 file changed, 18 insertions(+), 13 deletions(-) diff --git a/block/blkdebug.c b/block/blkdebug.c index 8f19d991fa..e37f999254 100644

[PATCH v2 4/5] blkdebug: do not suspend in the middle of QLIST_FOREACH_SAFE

2021-05-07 Thread Emanuele Giuseppe Esposito
affect the existing testcases. Use actions_count to see how many yeld to issue. Co-developed-by: Paolo Bonzini Signed-off-by: Emanuele Giuseppe Esposito --- block/blkdebug.c | 7 ++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/block/blkdebug.c b/block/blkdebug.c index

[PATCH v2 3/5] blkdebug: track all actions

2021-05-07 Thread Emanuele Giuseppe Esposito
Add a counter for each action that a rule can trigger. This is mainly used to keep track of how many coroutine_yeld() we need to perform after processing all rules in the list. Co-developed-by: Paolo Bonzini Signed-off-by: Emanuele Giuseppe Esposito --- block/blkdebug.c | 17

[PATCH v2 0/5] blkdebug: fix racing condition when iterating on

2021-05-07 Thread Emanuele Giuseppe Esposito
execution in the same block device. Signed-off-by: Emanuele Giuseppe Esposito --- v1 -> v2 * Change commit message of patch 4 and cover letter Emanuele Giuseppe Esposito (5): blkdebug: refactor removal of a suspended request blkdebug: move post-resume handling to resume_req_by_tag blkde

[PATCH v2 1/5] blkdebug: refactor removal of a suspended request

2021-05-07 Thread Emanuele Giuseppe Esposito
lo Bonzini Signed-off-by: Emanuele Giuseppe Esposito --- block/blkdebug.c | 28 +--- 1 file changed, 17 insertions(+), 11 deletions(-) diff --git a/block/blkdebug.c b/block/blkdebug.c index 2c0b9b0ee8..8f19d991fa 100644 --- a/block/blkdebug.c +++ b/block/blkdebug.c @@ -79

[PATCH v2 5/5] blkdebug: protect rules and suspended_reqs with a lock

2021-05-07 Thread Emanuele Giuseppe Esposito
Co-developed-by: Paolo Bonzini Signed-off-by: Emanuele Giuseppe Esposito --- block/blkdebug.c | 32 +++- 1 file changed, 31 insertions(+), 1 deletion(-) diff --git a/block/blkdebug.c b/block/blkdebug.c index dffd869b32..e92a35ccbb 100644 --- a/block/blkdebug.c +++ b

[PATCH] qemu-iotests: fix pylint 2.8 consider-using-with error

2021-05-06 Thread Emanuele Giuseppe Esposito
is assigned to a class field and used in other methods. Signed-off-by: Emanuele Giuseppe Esposito --- tests/qemu-iotests/iotests.py| 63 tests/qemu-iotests/testrunner.py | 22 +-- 2 files changed, 42 insertions(+), 43 deletions(-) diff --git a/tests

[PATCH v3 0/5] blkdebug: fix racing condition when iterating on

2021-05-17 Thread Emanuele Giuseppe Esposito
execution in the same block device. Signed-off-by: Emanuele Giuseppe Esposito --- v2 -> v3 * Fix "yeld"->"yield" in patches 3-4 [Eric] * Use lock guard instead of lock/unlock in patch 5 [Eric] Emanuele Giuseppe Esposito (5): blkdebug: refactor removal of a suspended req

Re: [PATCH 00/10] Python: delint iotests, machine.py and console_socket.py

2021-05-17 Thread Emanuele Giuseppe Esposito
    | 11 ---   python/qemu/machine.py   | 28 ++--   tests/qemu-iotests/iotests.py    | 55 +++-   tests/qemu-iotests/testrunner.py |  1 +   4 files changed, 57 insertions(+), 38 deletions(-) The iotests stuff was handled by Emanuele Giuseppe

[PATCH v3 4/5] blkdebug: do not suspend in the middle of QLIST_FOREACH_SAFE

2021-05-17 Thread Emanuele Giuseppe Esposito
affect the existing testcases. Use actions_count to see how many yield to issue. Co-developed-by: Paolo Bonzini Signed-off-by: Emanuele Giuseppe Esposito --- block/blkdebug.c | 7 ++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/block/blkdebug.c b/block/blkdebug.c index

[PATCH v3 3/5] blkdebug: track all actions

2021-05-17 Thread Emanuele Giuseppe Esposito
Add a counter for each action that a rule can trigger. This is mainly used to keep track of how many coroutine_yield() we need to perform after processing all rules in the list. Co-developed-by: Paolo Bonzini Signed-off-by: Emanuele Giuseppe Esposito --- block/blkdebug.c | 17

[PATCH v3 5/5] blkdebug: protect rules and suspended_reqs with a lock

2021-05-17 Thread Emanuele Giuseppe Esposito
Co-developed-by: Paolo Bonzini Signed-off-by: Emanuele Giuseppe Esposito --- block/blkdebug.c | 53 1 file changed, 40 insertions(+), 13 deletions(-) diff --git a/block/blkdebug.c b/block/blkdebug.c index dffd869b32..cf8b088ce7 100644

[PATCH v3 2/5] blkdebug: move post-resume handling to resume_req_by_tag

2021-05-17 Thread Emanuele Giuseppe Esposito
to handle the yielding. Co-developed-by: Paolo Bonzini Signed-off-by: Emanuele Giuseppe Esposito --- block/blkdebug.c | 31 ++- 1 file changed, 18 insertions(+), 13 deletions(-) diff --git a/block/blkdebug.c b/block/blkdebug.c index 8f19d991fa..e37f999254 100644

[PATCH v3 1/5] blkdebug: refactor removal of a suspended request

2021-05-17 Thread Emanuele Giuseppe Esposito
lo Bonzini Signed-off-by: Emanuele Giuseppe Esposito --- block/blkdebug.c | 28 +--- 1 file changed, 17 insertions(+), 11 deletions(-) diff --git a/block/blkdebug.c b/block/blkdebug.c index 2c0b9b0ee8..8f19d991fa 100644 --- a/block/blkdebug.c +++ b/block/blkdebug.c @@ -79

[PATCH v4 11/15] qemu-iotests: allow valgrind to read/delete the generated log file

2021-05-20 Thread Emanuele Giuseppe Esposito
When using -valgrind on the script tests, it generates a log file in $TEST_DIR that is either read (if valgrind finds problems) or otherwise deleted. Provide the same exact behavior when using -valgrind on the python tests. Signed-off-by: Emanuele Giuseppe Esposito --- tests/qemu-iotests

[PATCH v4 13/15] docs/devel/testing: add -valgrind option to the debug section of QEMU iotests

2021-05-20 Thread Emanuele Giuseppe Esposito
Reviewed-by: Max Reitz Signed-off-by: Emanuele Giuseppe Esposito --- docs/devel/testing.rst | 7 +++ 1 file changed, 7 insertions(+) diff --git a/docs/devel/testing.rst b/docs/devel/testing.rst index a746cab745..d743e88746 100644 --- a/docs/devel/testing.rst +++ b/docs/devel/testing.rst

[PATCH v4 00/15] qemu_iotests: improve debugging options

2021-05-20 Thread Emanuele Giuseppe Esposito
. This series is tested on the previous serie "qemu-iotests: quality of life improvements" but independent from it, so it can be applied separately. Signed-off-by: Emanuele Giuseppe Esposito --- v4: * Rename environment variable from GDB_QEMU to GDB_OPTIONS * This time test 297 (pylint) p

[PATCH v4 02/15] python: qemu: pass the wrapper field from QEMUQtestmachine to QEMUMachine

2021-05-20 Thread Emanuele Giuseppe Esposito
Reviewed-by: John Snow Reviewed-by: Max Reitz Signed-off-by: Emanuele Giuseppe Esposito --- python/qemu/qtest.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/python/qemu/qtest.py b/python/qemu/qtest.py index afea210d9d..e6a8fb5984 100644 --- a/python/qemu/qtest.py

[PATCH v4 05/15] qemu-iotests: delay QMP socket timers

2021-05-20 Thread Emanuele Giuseppe Esposito
Attaching gdbserver implies that the qmp socket should wait indefinitely for an answer from QEMU. Signed-off-by: Emanuele Giuseppe Esposito --- tests/qemu-iotests/iotests.py | 10 ++ 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu

[PATCH v4 09/15] qemu-iotests: extend the check script to support valgrind for python tests

2021-05-20 Thread Emanuele Giuseppe Esposito
lgring PID to assign to the log file name, use the "%p" flag in valgrind log file name that automatically puts the process PID at runtime. Reviewed-by: Max Reitz Signed-off-by: Emanuele Giuseppe Esposito --- tests/qemu-iotests/check | 7 --- tests/qemu-iotests/iotests.py | 11 +++

[PATCH v4 15/15] docs/devel/testing: add -p option to the debug section of QEMU iotests

2021-05-20 Thread Emanuele Giuseppe Esposito
Signed-off-by: Emanuele Giuseppe Esposito --- docs/devel/testing.rst | 4 1 file changed, 4 insertions(+) diff --git a/docs/devel/testing.rst b/docs/devel/testing.rst index d743e88746..1192d6489e 100644 --- a/docs/devel/testing.rst +++ b/docs/devel/testing.rst @@ -250,6 +250,10

[PATCH v4 14/15] qemu-iotests: add option to show qemu binary logs on stdout

2021-05-20 Thread Emanuele Giuseppe Esposito
Using the flag -p, allow the qemu binary to print to stdout. Reviewed-by: Max Reitz Signed-off-by: Emanuele Giuseppe Esposito --- tests/qemu-iotests/check | 4 +++- tests/qemu-iotests/iotests.py | 9 + tests/qemu-iotests/testenv.py | 9 +++-- 3 files changed, 19 insertions

[PATCH v4 04/15] qemu-iotests: add option to attach gdbserver

2021-05-20 Thread Emanuele Giuseppe Esposito
-off-by: Emanuele Giuseppe Esposito --- tests/qemu-iotests/check | 6 +- tests/qemu-iotests/iotests.py | 5 + tests/qemu-iotests/testenv.py | 19 --- 3 files changed, 26 insertions(+), 4 deletions(-) diff --git a/tests/qemu-iotests/check b/tests/qemu-iotests/check

[PATCH v4 06/15] qemu_iotests: insert gdbserver command line as wrapper for qemu binary

2021-05-20 Thread Emanuele Giuseppe Esposito
Signed-off-by: Emanuele Giuseppe Esposito --- tests/qemu-iotests/iotests.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py index cf1ca60376..c9628e6828 100644 --- a/tests/qemu-iotests/iotests.py +++ b/tests/qemu

[PATCH v4 08/15] docs/devel/testing: add -gdb option to the debugging section of QEMU iotests

2021-05-20 Thread Emanuele Giuseppe Esposito
Signed-off-by: Emanuele Giuseppe Esposito --- docs/devel/testing.rst | 11 +++ 1 file changed, 11 insertions(+) diff --git a/docs/devel/testing.rst b/docs/devel/testing.rst index 8144e316a4..a746cab745 100644 --- a/docs/devel/testing.rst +++ b/docs/devel/testing.rst @@ -229,6 +229,17

[PATCH v4 12/15] qemu-iotests: insert valgrind command line as wrapper for qemu binary

2021-05-20 Thread Emanuele Giuseppe Esposito
The priority will be given to gdb command line, meaning if the -gdb parameter and -valgrind are given, gdb will be wrapped around the qemu binary. Signed-off-by: Emanuele Giuseppe Esposito --- tests/qemu-iotests/iotests.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git

[PATCH v4 01/15] python: qemu: add timer parameter for qmp.accept socket

2021-05-20 Thread Emanuele Giuseppe Esposito
to qemu-iotests execution. Signed-off-by: Emanuele Giuseppe Esposito --- python/qemu/machine.py| 7 +-- python/qemu/qtest.py | 5 +++-- tests/qemu-iotests/iotests.py | 3 ++- 3 files changed, 10 insertions(+), 5 deletions(-) diff --git a/python/qemu/machine.py b/python/qemu

[PATCH v4 03/15] docs/devel/testing: add debug section to the QEMU iotests chapter

2021-05-20 Thread Emanuele Giuseppe Esposito
Introduce the "Debugging a test case" section, in preparation to the additional flags that will be added in the next patches. Signed-off-by: Emanuele Giuseppe Esposito --- docs/devel/testing.rst | 8 1 file changed, 8 insertions(+) diff --git a/docs/devel/testing.rst b/

[PATCH v4 10/15] qemu-iotests: extent QMP socket timeout when using valgrind

2021-05-20 Thread Emanuele Giuseppe Esposito
As with gdbserver, valgrind delays the test execution, so the default QMP socket timeout timeout too soon. Signed-off-by: Emanuele Giuseppe Esposito --- tests/qemu-iotests/iotests.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/qemu-iotests/iotests.py b/tests

[PATCH v4 07/15] qemu-iotests: add gdbserver option to script tests too

2021-05-20 Thread Emanuele Giuseppe Esposito
The only limitation here is that running a script with gdbserver will make the test output mismatch with the expected results, making the test fail. Signed-off-by: Emanuele Giuseppe Esposito --- tests/qemu-iotests/common.rc | 8 +++- 1 file changed, 7 insertions(+), 1 deletion(-) diff

Re: [PATCH 5/6] co-shared-resource: protect with a mutex

2021-05-14 Thread Emanuele Giuseppe Esposito
On 12/05/2021 17:44, Stefan Hajnoczi wrote: On Mon, May 10, 2021 at 10:59:40AM +0200, Emanuele Giuseppe Esposito wrote: co-shared-resource is currently not thread-safe, as also reported in co-shared-resource.h. Add a QemuMutex because co_try_get_from_shres can also be invoked from non

Re: [PATCH v2] qemu-iotests: fix pylint 2.8 consider-using-with error

2021-05-14 Thread Emanuele Giuseppe Esposito
On 14/05/2021 16:09, Max Reitz wrote: On 12.05.21 19:04, Max Reitz wrote: On 10.05.21 21:04, Emanuele Giuseppe Esposito wrote: pylint 2.8 introduces consider-using-with error, suggesting to use the 'with' block statement when possible. Modify all subprocess.Popen call to use

Re: [PATCH 5/6] co-shared-resource: protect with a mutex

2021-05-14 Thread Emanuele Giuseppe Esposito
On 14/05/2021 16:26, Vladimir Sementsov-Ogievskiy wrote: 14.05.2021 17:10, Emanuele Giuseppe Esposito wrote: On 12/05/2021 17:44, Stefan Hajnoczi wrote: On Mon, May 10, 2021 at 10:59:40AM +0200, Emanuele Giuseppe Esposito wrote: co-shared-resource is currently not thread-safe, as also

Re: [PATCH 5/6] co-shared-resource: protect with a mutex

2021-05-14 Thread Emanuele Giuseppe Esposito
On 14/05/2021 17:30, Vladimir Sementsov-Ogievskiy wrote: 14.05.2021 17:32, Emanuele Giuseppe Esposito wrote: On 14/05/2021 16:26, Vladimir Sementsov-Ogievskiy wrote: 14.05.2021 17:10, Emanuele Giuseppe Esposito wrote: On 12/05/2021 17:44, Stefan Hajnoczi wrote: On Mon, May 10, 2021

Re: [PATCH 4/6] progressmeter: protect with a mutex

2021-05-10 Thread Emanuele Giuseppe Esposito
On 10/05/2021 13:28, Vladimir Sementsov-Ogievskiy wrote: 10.05.2021 11:59, Emanuele Giuseppe Esposito wrote: Progressmeter is protected by the AioContext mutex, which is taken by the block jobs and their caller (like blockdev). We would like to remove the dependency of block layer code

Re: [PATCH 4/6] progressmeter: protect with a mutex

2021-05-10 Thread Emanuele Giuseppe Esposito
We don't have caller that pass only one pointer. So we can just do *current = pm->current; *total = pm->total; implicitly requiring both pointers to be non NULL. Is it so performance critical that we need to skip these safety checks? IMHO we can keep it as it is. Not performance. It's

Re: [PATCH v4 00/15] qemu_iotests: improve debugging options

2021-05-26 Thread Emanuele Giuseppe Esposito
On 26/05/2021 13:32, Vladimir Sementsov-Ogievskiy wrote: 20.05.2021 10:52, Emanuele Giuseppe Esposito wrote: This series adds the option to attach gdbserver and valgrind to the QEMU binary running in qemu_iotests. It also allows to redirect QEMU binaries output of the python tests

Re: [PATCH v4 04/15] qemu-iotests: add option to attach gdbserver

2021-05-26 Thread Emanuele Giuseppe Esposito
to stick to that. If you want to send a patch changing all -options in --options, feel free to do it in a separate series that changes also -gdb :) Thank you, Emanuele if -gdb is not provided but $GDB_OPTIONS is set, ignore the environment variable. Signed-off-by: Emanuele Giuseppe

Re: [PATCH v2 4/7] block-copy: add a CoMutex to the BlockCopyTask list

2021-05-25 Thread Emanuele Giuseppe Esposito
On 20/05/2021 17:19, Vladimir Sementsov-Ogievskiy wrote: 18.05.2021 13:07, Emanuele Giuseppe Esposito wrote: Because the list of tasks is only modified by coroutine functions, add a CoMutex in order to protect them. Use the same mutex to protect also BlockCopyState in_flight_bytes field

Re: [PATCH v2 5/7] block-copy: add QemuMutex lock for BlockCopyCallState list

2021-05-25 Thread Emanuele Giuseppe Esposito
On 21/05/2021 17:01, Paolo Bonzini wrote: On 20/05/21 17:30, Vladimir Sementsov-Ogievskiy wrote: 18.05.2021 13:07, Emanuele Giuseppe Esposito wrote: As for BlockCopyTask, add a lock to protect BlockCopyCallState ret and sleep_state fields. Also move ret, finished and cancelled in the OUT

Re: [PATCH v2 7/7] block-copy: protect BlockCopyState .method fields

2021-05-25 Thread Emanuele Giuseppe Esposito
On 21/05/2021 19:10, Vladimir Sementsov-Ogievskiy wrote: 18.05.2021 13:07, Emanuele Giuseppe Esposito wrote: With tasks and calls lock protecting all State fields, .method is the last BlockCopyState field left unprotected. Set it as atomic. Signed-off-by: Emanuele Giuseppe Esposito OK

Re: [PATCH v4 08/15] docs/devel/testing: add -gdb option to the debugging section of QEMU iotests

2021-05-28 Thread Emanuele Giuseppe Esposito
On 28/05/2021 19:16, Vladimir Sementsov-Ogievskiy wrote: 20.05.2021 10:52, Emanuele Giuseppe Esposito wrote: Signed-off-by: Emanuele Giuseppe Esposito Reviewed-by: Vladimir Sementsov-Ogievskiy ---   docs/devel/testing.rst | 11 +++   1 file changed, 11 insertions(+) diff --git

Re: [PATCH v4 09/15] qemu-iotests: extend the check script to support valgrind for python tests

2021-05-28 Thread Emanuele Giuseppe Esposito
  sys.exit(os.EX_USAGE) +qemu_valgrind = [] +if os.environ.get('VALGRIND_QEMU') == "y" and \ +    os.environ.get('NO_VALGRIND') != "y": +    valgrind_logfile = "--log-file=" + test_dir.strip() a bit strange that you need to strip test_dir here.. Why? Yep, it's unnecessary

Re: [PATCH v4 14/15] qemu-iotests: add option to show qemu binary logs on stdout

2021-05-28 Thread Emanuele Giuseppe Esposito
+   imgfmt = os.environ.get('IMGFMT', 'raw')   imgproto = os.environ.get('IMGPROTO', 'file')   output_dir = os.environ.get('OUTPUT_DIR', '.') @@ -614,6 +616,13 @@ def _post_shutdown(self) -> None:   super()._post_shutdown()   self.subprocess_check_valgrind(qemu_valgrind) +   

Re: [PATCH v3 5/5] blkdebug: protect rules and suspended_reqs with a lock

2021-06-02 Thread Emanuele Giuseppe Esposito
On 01/06/2021 10:39, Vladimir Sementsov-Ogievskiy wrote: 17.05.2021 17:50, Emanuele Giuseppe Esposito wrote: Co-developed-by: Paolo Bonzini Signed-off-by: Emanuele Giuseppe Esposito ---   block/blkdebug.c | 53   1 file changed, 40 insertions

Re: [PATCH v2 1/7] block-copy: streamline choice of copy_range vs. read/write

2021-05-20 Thread Emanuele Giuseppe Esposito
On 20/05/2021 16:42, Vladimir Sementsov-Ogievskiy wrote: 18.05.2021 13:07, Emanuele Giuseppe Esposito wrote: From: Paolo Bonzini Put the logic to determine the copy size in a separate function, so that there is a simple state machine for the possible methods of copying data from one

Re: [PATCH v2 2/7] block-copy: improve documentation of BlockCopyTask and BlockCopyState types and functions

2021-05-20 Thread Emanuele Giuseppe Esposito
On 20/05/2021 17:00, Vladimir Sementsov-Ogievskiy wrote: 18.05.2021 13:07, Emanuele Giuseppe Esposito wrote: As done in BlockCopyCallState, categorize BlockCopyTask and BlockCopyState in IN, State and OUT fields. This is just to understand which field has to be protected with a lock

Re: [PATCH v2 0/7] block-copy: protect block-copy internal structures

2021-05-20 Thread Emanuele Giuseppe Esposito
On 20/05/2021 15:47, Vladimir Sementsov-Ogievskiy wrote: 18.05.2021 13:07, Emanuele Giuseppe Esposito wrote: This serie of patches aims to reduce the usage of the global AioContexlock in block-copy, by introducing smaller granularity locks thus on making the block layer thread safe

Re: [PATCH v4 04/15] qemu-iotests: add option to attach gdbserver

2021-05-27 Thread Emanuele Giuseppe Esposito
On 26/05/2021 21:15, Vladimir Sementsov-Ogievskiy wrote: 20.05.2021 10:52, Emanuele Giuseppe Esposito wrote: Define -gdb flag and GDB_OPTIONS environment variable to python tests to attach a gdbserver to each qemu instance. This patch only adds and parses this flag, it does not yet add

Re: [PATCH v5 10/16] qemu-iotests: extend the check script to prepare supporting valgrind for python tests

2021-06-05 Thread Emanuele Giuseppe Esposito
On 05/06/2021 15:28, Vladimir Sementsov-Ogievskiy wrote: 04.06.2021 12:17, Emanuele Giuseppe Esposito wrote: Currently, the check script only parses the option and sets the VALGRIND_QEMU environmental variable to "y". Add another local python variable that prepares the co

Re: [PATCH v4 6/6] blkdebug: protect rules and suspended_reqs with a lock

2021-06-05 Thread Emanuele Giuseppe Esposito
On 05/06/2021 17:15, Vladimir Sementsov-Ogievskiy wrote: 04.06.2021 13:07, Emanuele Giuseppe Esposito wrote: First, categorize the structure fields to identify what needs to be protected and what doesn't. We essentially need to protect only .state, and the 3 lists in BDRVBlkdebugState

Re: [PATCH v4 01/15] python: qemu: add timer parameter for qmp.accept socket

2021-06-03 Thread Emanuele Giuseppe Esposito
On 03/06/2021 01:23, John Snow wrote: On 5/20/21 3:52 AM, Emanuele Giuseppe Esposito wrote: Alsp add a new _qmp_timer field to the QEMUMachine class. Let's change the default socket timeout to None, so that if a subclass needs to add a timer, it can be done by modifying this private field

Re: [PATCH v4 05/15] qemu-iotests: delay QMP socket timers

2021-06-03 Thread Emanuele Giuseppe Esposito
So, you just make the class do nothing.. I'd prefer something like this: @contextmanager def NoTimeout:    yield if qemu_gdb:   Timeout = NoTimeout I am not sure I understand what you want to do here. I have a basic understanding of @contextmanager, but can you explain more what you

Re: [PATCH] iotests/check: move general long options to double dash

2021-06-03 Thread Emanuele Giuseppe Esposito
On 26/05/2021 20:16, Vladimir Sementsov-Ogievskiy wrote: So, the change: -makecheck -> --makecheck -valgrind -> --valgrind -nocache -> --nocache -misalign -> --misalign Motivation: 1. Several long options are already have double dash: --dry-run, --color, --groups,

[PATCH v5 10/16] qemu-iotests: extend the check script to prepare supporting valgrind for python tests

2021-06-04 Thread Emanuele Giuseppe Esposito
lgrind PID to assign to the log file name, use the "%p" flag in valgrind log file name that automatically puts the process PID at runtime. Signed-off-by: Emanuele Giuseppe Esposito Reviewed-by: Max Reitz --- tests/qemu-iotests/check | 7 --- tests/qemu-iotests/iotests.py | 11 +++

<    1   2   3   4   5   6   7   8   9   10   >