[PATCH v6 27/33] block-backend-common.h: split function pointers in BlockDevOps

Assertions in the callers of the function pointrs are already added by previous patches. Signed-off-by: Emanuele Giuseppe Esposito Reviewed-by: Stefan Hajnoczi Reviewed-by: Philippe Mathieu-Daudé --- include/sysemu/block-backend-common.h | 28 ++- 1 file changed, 23

[PATCH v6 29/33] job.h: assertions in the callers of JobDriver funcion pointers

Signed-off-by: Emanuele Giuseppe Esposito --- job.c | 9 + 1 file changed, 9 insertions(+) diff --git a/job.c b/job.c index 54db80df66..39bf511949 100644 --- a/job.c +++ b/job.c @@ -381,6 +381,8 @@ void job_ref(Job *job) void job_unref(Job *job) { +assert(qemu_in_main_thread

[PATCH v6 20/33] block: rename bdrv_invalidate_cache_all, blk_invalidate_cache and test_sync_op_invalidate_cache

Following the bdrv_activate renaming, change also the name of the respective callers. bdrv_invalidate_cache_all -> bdrv_activate_all blk_invalidate_cache -> blk_activate test_sync_op_invalidate_cache -> test_sync_op_activate No functional change intended. Signed-off-by: Emanuele

[PATCH v6 13/33] include/block/blockjob.h: global state API

blockjob functions run always under the BQL lock. Signed-off-by: Emanuele Giuseppe Esposito Reviewed-by: Stefan Hajnoczi --- include/block/blockjob.h | 9 + 1 file changed, 9 insertions(+) diff --git a/include/block/blockjob.h b/include/block/blockjob.h index 87fbb3985f..2373dfeb07

[PATCH v6 28/33] job.h: split function pointers in JobDriver

The job API will be handled separately in another serie. Signed-off-by: Emanuele Giuseppe Esposito --- include/qemu/job.h | 22 ++ 1 file changed, 22 insertions(+) diff --git a/include/qemu/job.h b/include/qemu/job.h index 6e67b6977f..4ea7a4a0cd 100644 --- a/include/qemu

[PATCH v6 30/33] include/block/block_int-common.h: introduce bdrv_amend_pre_run and bdrv_amend_clean

ons are restored. Therefore bdrv_amend_pre_run() and bdrv_amend_clean() will take care of just temporarly setting the crypto-specific updating_keys flag. Note that at this stage, they are not yet invoked. Signed-off-by: Emanuele Giuseppe Esposito --- block/crypto.c |

[PATCH v6 33/33] block.c: assertions to the block layer permissions API

Now that we "covered" the three main cases where the permission API was being used under BQL (fuse, amend and invalidate_cache), we can safely assert for the permission functions implemented in block.c Signed-off-by: Emanuele Giuseppe Esposito --- block.c | 12 1 file c

[PATCH v6 16/33] assertions for blockdev.h global state API

Signed-off-by: Emanuele Giuseppe Esposito Reviewed-by: Paolo Bonzini Reviewed-by: Stefan Hajnoczi --- block/block-backend.c | 3 +++ blockdev.c| 16 2 files changed, 19 insertions(+) diff --git a/block/block-backend.c b/block/block-backend.c index fe56cc1cf4

[PATCH v6 18/33] block/copy-before-write.h: global state API + assertions

copy-before-write functions always run under BQL lock. Signed-off-by: Emanuele Giuseppe Esposito Reviewed-by: Paolo Bonzini Reviewed-by: Stefan Hajnoczi --- block/copy-before-write.c | 2 ++ block/copy-before-write.h | 7 +++ 2 files changed, 9 insertions(+) diff --git a/block/copy

Re: [PATCH v3 14/16] job.c: use job_get_aio_context()

On 21/01/2022 13:33, Emanuele Giuseppe Esposito wrote: On 19/01/2022 11:31, Paolo Bonzini wrote: diff --git a/blockjob.c b/blockjob.c index cf1f49f6c2..468ba735c5 100644 --- a/blockjob.c +++ b/blockjob.c @@ -155,14 +155,16 @@ static void child_job_set_aio_ctx(BdrvChild *c, AioContext *ctx

[PATCH v6 21/33] block: move BQL logic of bdrv_co_invalidate_cache in bdrv_activate

ect call to bdrv_invalidate_cache(). Signed-off-by: Emanuele Giuseppe Esposito --- block.c | 36 +--- 1 file changed, 21 insertions(+), 15 deletions(-) diff --git a/block.c b/block.c index 7ab5031027..bad834c86e 100644 --- a/block.c +++ b/block.c @@ -6550,23 +6550

[PATCH v6 02/33] include/block/block: split header into I/O and global state API

er view on what needs what kind of protection. block-common.h contains common structures shared by both headers. block.h is left there for legacy and to avoid changing all includes in all c files that use the block APIs. Assertions are added in the next patch. Signed-off-by: Emanuele Giusepp

[PATCH v6 09/33] block: introduce assert_bdrv_graph_writable

. Because adding drains requires additional discussions, they will be added in future series. Signed-off-by: Emanuele Giuseppe Esposito --- block.c| 4 block/io.c | 11 +++ include/block/block_int-global-state.h | 8

[PATCH v6 25/33] block_int-common.h: split function pointers in BdrvChildClass

Signed-off-by: Emanuele Giuseppe Esposito --- include/block/block_int-common.h | 67 +++- 1 file changed, 40 insertions(+), 27 deletions(-) diff --git a/include/block/block_int-common.h b/include/block/block_int-common.h index e007dbf768..cc8c8835ba 100644 --- a

[PATCH v6 12/33] block.c: add assertions to static functions

Following the assertion derived from the API split, propagate the assertion also in the static functions. Signed-off-by: Emanuele Giuseppe Esposito --- block.c | 47 ++- block/block-backend.c | 3 +++ 2 files changed, 49 insertions(+), 1

[PATCH v6 19/33] block: introduce bdrv_activate

-off-by: Emanuele Giuseppe Esposito --- block.c| 7 ++- block/block-backend.c | 2 +- block/export/export.c | 2 +- block/parallels.c | 2 +- include/block/block-global-state.h | 2 +- tests/unit/test-block-iothread.c | 2

[PATCH v6 31/33] include/qemu/job.h: introduce job->pre_run() and use it in amend

lse anyways. Signed-off-by: Emanuele Giuseppe Esposito --- block/amend.c | 33 + include/qemu/job.h | 9 + 2 files changed, 42 insertions(+) diff --git a/block/amend.c b/block/amend.c index 392df9ef83..1618fd05a6 100644 --- a/block/amend.c +++ b/bl

[PATCH v6 32/33] crypto: delegate permission functions to JobDriver .pre_run

t(), now it is the moment to use it. Signed-off-by: Emanuele Giuseppe Esposito --- block/crypto.c | 57 -- job.c | 13 2 files changed, 50 insertions(+), 20 deletions(-) diff --git a/block/crypto.c b/block/crypto.c in

Re: [PATCH v3 14/16] job.c: use job_get_aio_context()

On 24/01/2022 15:22, Paolo Bonzini wrote: > On 1/21/22 16:18, Emanuele Giuseppe Esposito wrote: >>>> >>> >>> Better to use aio_co_schedule here, too, and move it under the >>> previous WITH_JOB_LOCK_GUARD. >> >> Unfortunately th

Re: [PATCH v3 03/16] job.h: define locked functions

On 24/01/2022 15:26, Paolo Bonzini wrote: > On 1/21/22 17:04, Vladimir Sementsov-Ogievskiy wrote: >>> >>> The split was proposed in previous versions, but Vladimir did not >>> really like it and suggested to send it as a separate series: >> >> I didn't really like it as it seemed unusual and uno

Re: [PATCH v3 09/16] jobs: remove aiocontext locks since the functions are under BQL

On 19/01/2022 12:09, Paolo Bonzini wrote: >> @@ -3707,15 +3707,11 @@ BlockJobInfoList *qmp_query_block_jobs(Error >> **errp) >>     for (job = block_job_next(NULL); job; job = >> block_job_next(job)) { >>   BlockJobInfo *value; >> -    AioContext *aio_context; >>     if (

Re: [PATCH v6 20/33] block: rename bdrv_invalidate_cache_all, blk_invalidate_cache and test_sync_op_invalidate_cache

On 24/01/2022 11:44, Juan Quintela wrote: >> diff --git a/migration/migration.c b/migration/migration.c >> index 0652165610..1f06fd2d18 100644 >> --- a/migration/migration.c >> +++ b/migration/migration.c >> @@ -499,7 +499,7 @@ static void process_incoming_migration_bh(void *opaque) >>

[PATCH v4 02/19] job.h: categorize fields in struct Job

Categorize the fields in struct Job to understand which ones need to be protected by the job mutex and which don't. Signed-off-by: Emanuele Giuseppe Esposito --- include/qemu/job.h | 59 ++ 1 file changed, 34 insertions(+), 25 deletions(-) diff

[PATCH v4 03/19] job.c: make job_event_* functions static

job_event_* functions can all be static, as they are not used outside job.c. Signed-off-by: Emanuele Giuseppe Esposito Reviewed-by: Stefan Hajnoczi --- include/qemu/job.h | 6 -- job.c | 12 ++-- 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/include

[PATCH v4 04/19] job.c: move inner aiocontext lock in callbacks

. Signed-off-by: Emanuele Giuseppe Esposito --- job.c | 17 - 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/job.c b/job.c index e97b159396..2b88cef19d 100644 --- a/job.c +++ b/job.c @@ -149,7 +149,6 @@ static void job_txn_del_job(Job *job) static int job_txn_apply

[PATCH v4 05/19] aio-wait.h: introduce AIO_WAIT_WHILE_UNLOCKED

Same as AIO_WAIT_WHILE macro, but if we are in the Main loop do not release and then acquire ctx_ 's aiocontext. Once all Aiocontext locks go away, this macro will replace AIO_WAIT_WHILE. Signed-off-by: Emanuele Giuseppe Esposito --- include/block/aio-wait.h | 15 +++ 1

[PATCH v4 06/19] jobs: remove aiocontext locks since the functions are under BQL

/release. We don't want this, as job_lock must be taken inside the AioContext lock, and taking it outside would cause deadlocks. Signed-off-by: Emanuele Giuseppe Esposito --- blockdev.c | 4 job-qmp.c | 4 2 files changed, 8 deletions(-) diff --git a/blockdev.c b/blockdev.c index 800e9

[PATCH v4 01/19] job.c: make job_mutex and job_lock/unlock() public

e can change the nop into an actual mutex and remove the aiocontext lock. Since job_mutex is already being used, add static real_job_{lock/unlock} for the existing usage. Signed-off-by: Emanuele Giuseppe Esposito --- include/qemu/job.h | 24 job.c

[PATCH v4 00/19] job: replace AioContext lock with job_mutex

B_LOCK_GUARD/WITH_JOB_LOCK_GUARD to avoid deadlocks and simplify the reviewer job * move patch 11 (block_job_query: remove atomic read) as last Emanuele Giuseppe Esposito (18): job.c: make job_mutex and job_lock/unlock() public job.h: categorize fields in struct Job job.c: make job_event_* function

[PATCH v4 08/19] jobs: add job lock in find_* functions

*. Signed-off-by: Emanuele Giuseppe Esposito --- blockdev.c | 14 +- job-qmp.c | 13 - 2 files changed, 25 insertions(+), 2 deletions(-) diff --git a/blockdev.c b/blockdev.c index 2e522dd7fe..7a9e9b4a13 100644 --- a/blockdev.c +++ b/blockdev.c @@ -3311,7 +3311,10 @@ out

[PATCH v4 09/19] jobs: use job locks also in the unit tests

Add missing job synchronization in the unit tests, with explicit locks. Note: at this stage, job_{lock/unlock} and job lock guard macros are *nop*. Signed-off-by: Emanuele Giuseppe Esposito --- tests/unit/test-bdrv-drain.c | 76 - tests/unit/test-block-iothread.c

[PATCH v4 14/19] blockjob: block_job_get_aio_context is a GS function

Move block_job_get_aio_context under the GS category in blockjob.h, as it is always called with the BQL held. This is done also to simplify the coming job->aiocontext protection. For more infos about the GS API, see include/block/block-global-state.h Signed-off-by: Emanuele Giuseppe Espos

[PATCH v4 16/19] job: detect change of aiocontext within job coroutine

From: Paolo Bonzini We want to make sure access of job->aio_context is always done under either BQL or job_mutex. The problem is that using aio_co_enter(job->aiocontext, job->co) in job_start and job_enter_cond makes the coroutine immediately resume, so we can't hold the job lock. And caching it

[PATCH v4 19/19] block_job_query: remove atomic read

Not sure what the atomic here was supposed to do, since job.busy is protected by the job lock. Since the whole function is called under job_mutex, just remove the atomic. Signed-off-by: Emanuele Giuseppe Esposito --- blockjob.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a

[PATCH v4 11/19] jobs: document all static functions and add _locked() suffix

Now that we added the job_lock/unlock pairs, we can also rename all static functions in job.c that are called with the job mutex held as _locked(), and add a little comment on top. No functional change intended. Signed-off-by: Emanuele Giuseppe Esposito --- blockjob.c | 29

[PATCH v4 10/19] job.h: define locked functions

job lock guard macros are *nop*. Signed-off-by: Emanuele Giuseppe Esposito --- block.c | 5 +- block/backup.c | 4 +- block/replication.c | 4 +- blockdev.c | 44 + blockjob.c | 28

[PATCH v4 07/19] jobs: protect jobs with job_lock/unlock

macros are *nop*. Signed-off-by: Emanuele Giuseppe Esposito --- block.c | 18 +--- block/replication.c | 8 +++- blockdev.c | 17 +-- blockjob.c | 60 +--- job-qmp.c | 2 + job.c | 110

[PATCH v4 13/19] block/mirror.c: use of job helpers in drivers to avoid TOC/TOU

Once job lock is used and aiocontext is removed, mirror has to perform job operations under the same critical section, using the helpers prepared in previous commit. Note: at this stage, job_{lock/unlock} and job lock guard macros are *nop*. Signed-off-by: Emanuele Giuseppe Esposito --- block

[PATCH v4 18/19] job.c: enable job lock/unlock and remove Aiocontext locks

ned-off-by: Emanuele Giuseppe Esposito --- blockdev.c | 65 --- include/qemu/job.h | 22 - job-qmp.c| 41 - job.c| 76 +++- tests/unit/t

[PATCH v4 12/19] job.h: define unlocked functions

in job.c). Note: at this stage, job_{lock/unlock} and job lock guard macros are *nop*. Signed-off-by: Emanuele Giuseppe Esposito --- blockjob.c | 20 - include/qemu/job.h | 38 --- job.c | 56

[PATCH v4 17/19] jobs: protect job.aio_context with BQL and job_mutex

introduce job_set_aio_context and make sure that the context is set under BQL, job_mutex and drain. Also make sure all other places where the aiocontext is read are protected. Suggested-by: Paolo Bonzini Signed-off-by: Emanuele Giuseppe Esposito --- block/replication.c | 2 +- blockjob.c

[PATCH v4 15/19] commit and mirror: create new nodes using bdrv_get_aio_context, and not the job aiocontext

We are always using the given bs AioContext, so there is no need to take the job ones (which is identical anyways). This also reduces the point we need to check when protecting job.aio_context field. Signed-off-by: Emanuele Giuseppe Esposito --- block/commit.c | 4 ++-- block/mirror.c | 2 +- 2

Re: [PATCH 00/12] Removal of Aiocontext lock through drains: protect bdrv_replace_child_noperm.

On 27/01/2022 14:46, Paolo Bonzini wrote: > On 1/26/22 12:29, Stefan Hajnoczi wrote: >> Still, I'm a bit surprised I didn't notice any >> aio_context_acquire/release() removals in this patch series because I >> thought callers need that before they switch to >> BDRV_POLL_WHILE_UNLOCKED()? > > I

Re: [PATCH] block: bdrv_set_backing_hd(): use drained section

On 27/01/2022 15:13, Kevin Wolf wrote: > Am 25.01.2022 um 11:12 hat Vladimir Sementsov-Ogievskiy geschrieben: >> 25.01.2022 12:24, Paolo Bonzini wrote: >>> On 1/24/22 18:37, Vladimir Sementsov-Ogievskiy wrote: Graph modifications should be done in drained section. stream_prepare() hand

Re: [PATCH v6 04/33] block/export/fuse.c: allow writable exports to take RESIZE permission

On 25/01/2022 17:51, Hanna Reitz wrote: > On 21.01.22 18:05, Emanuele Giuseppe Esposito wrote: >> Allow writable exports to get BLK_PERM_RESIZE permission >> from creation, in fuse_export_create(). >> In this way, there is no need to give the permission in >> fuse_do

Re: [PATCH v6 29/33] job.h: assertions in the callers of JobDriver funcion pointers

On 26/01/2022 15:13, Hanna Reitz wrote: > On 21.01.22 18:05, Emanuele Giuseppe Esposito wrote: >> Signed-off-by: Emanuele Giuseppe Esposito >> --- >>   job.c | 9 + >>   1 file changed, 9 insertions(+) > > Just curious, why did you remove the assertion

Re: [PATCH v6 25/33] block_int-common.h: split function pointers in BdrvChildClass

On 26/01/2022 13:42, Hanna Reitz wrote: > On 21.01.22 18:05, Emanuele Giuseppe Esposito wrote: >> Signed-off-by: Emanuele Giuseppe Esposito >> --- >>   include/block/block_int-common.h | 67 +++- >>   1 file changed, 40 insertions(+), 27 de

Re: [PATCH v6 32/33] crypto: delegate permission functions to JobDriver .pre_run

On 26/01/2022 17:10, Hanna Reitz wrote: > On 21.01.22 18:05, Emanuele Giuseppe Esposito wrote: >> block_crypto_amend_options_generic_luks uses the block layer >> permission API, therefore it should be called with the BQL held. >> >> However, the same function is bein

Re: [PATCH v4 16/19] job: detect change of aiocontext within job coroutine

> As a small improvement, we don't need aio_co_enter() in job_start(), > we can simplify it with qemu_coroutine_enter(). If the aio_context > where it is running is wrong, job_do_yield_locked() will automatically > reschedule the coroutine where it belongs, once the first > job_pause_point is trigg

Re: [PATCH v6 01/33] main-loop.h: introduce qemu_in_main_thread()

On 27/01/2022 11:56, Kevin Wolf wrote: > Am 21.01.2022 um 18:05 hat Emanuele Giuseppe Esposito geschrieben: >> When invoked from the main loop, this function is the same >> as qemu_mutex_iothread_locked, and returns true if the BQL is held. > > So its name is misleadi

[PATCH] block.h: remove outdated comment

ff-by: Emanuele Giuseppe Esposito --- include/block/block.h | 1 - 1 file changed, 1 deletion(-) diff --git a/include/block/block.h b/include/block/block.h index 9d4050220b..e1713ee306 100644 --- a/include/block/block.h +++ b/include/block/block.h @@ -344,7 +344,6 @@ typedef unsigned int BdrvC

Re: [PATCH v6 02/33] include/block/block: split header into I/O and global state API

On 27/01/2022 12:01, Kevin Wolf wrote: >> +/* Common functions that are neither I/O nor Global State */ >> + >> +int bdrv_parse_aio(const char *mode, int *flags); > Makes sense to me to have this here, it is just a helper function that > parses stuff and doesn't touch any state. However, what is di

Re: [PATCH v6 01/33] main-loop.h: introduce qemu_in_main_thread()

On 31/01/2022 16:49, Paolo Bonzini wrote: > On 1/31/22 15:33, Kevin Wolf wrote: >> I feel "use this function if your code is going to be used by unit >> tests, but use qemu_mutex_iothread_locked() otherwise" is a very strange >> interface. I'm relatively sure that qemu_mutex_iothread_locked() is

Re: [PATCH v6 02/33] include/block/block: split header into I/O and global state API

On 31/01/2022 16:58, Paolo Bonzini wrote: > On 1/31/22 15:54, Kevin Wolf wrote: >> So I guess the decision depends on what you're going to use the >> categories in the future. I get the feeling that we have one more >> category than this series introduces: >> >> * Global State (must run from the

Re: [PATCH 10/12] block.c: add subtree_drains where needed

On 01/02/2022 15:47, Vladimir Sementsov-Ogievskiy wrote: > 18.01.2022 19:27, Emanuele Giuseppe Esposito wrote: >> Protect bdrv_replace_child_noperm, as it modifies the >> graph by adding/removing elements to .children and .parents >> list of a bs. Use

Re: [PATCH 10/12] block.c: add subtree_drains where needed

On 02/02/2022 18:38, Paolo Bonzini wrote: > On 2/2/22 16:37, Emanuele Giuseppe Esposito wrote: >> So we have disk B with backing file C, and new disk A that wants to have >> backing file C. >> >> I think I understand what you mean, so in theory the operation wou

Re: [PATCH 05/12] test-bdrv-drain.c: adapt test to the coming subtree drains

On 19/01/2022 10:18, Paolo Bonzini wrote: > On 1/18/22 17:27, Emanuele Giuseppe Esposito wrote: >> - First of all, inconsistency between block_job_create under >> aiocontext lock that internally calls blk_insert_bs and therefore >> bdrv_replace_child_noperm, and blk_insert_

Re: [PATCH 10/12] block.c: add subtree_drains where needed

On 19/01/2022 10:47, Paolo Bonzini wrote: > > About this: > >> + * TODO: this is called by job_unref with lock held, because >> + * afterwards it calls bdrv_try_set_aio_context. >> + * Once all of this is fixed, take care of removing >> + * the aiocontext lock a

Re: [PATCH 01/12] introduce BDRV_POLL_WHILE_UNLOCKED

On 26/01/2022 11:49, Stefan Hajnoczi wrote: > On Tue, Jan 18, 2022 at 11:27:27AM -0500, Emanuele Giuseppe Esposito wrote: >> Same as BDRV_POLL_WHILE, but uses AIO_WAIT_WHILE_UNLOCKED. >> >> Signed-off-by: Emanuele Giuseppe Esposito >> --- >> include/block/blo

Re: [PATCH 09/12] jobs: ensure sleep in job_sleep_ns is fully performed

On 26/01/2022 12:21, Stefan Hajnoczi wrote: > On Tue, Jan 18, 2022 at 11:27:35AM -0500, Emanuele Giuseppe Esposito wrote: >> If a drain happens while a job is sleeping, the timeout >> gets cancelled and the job continues once the drain ends. >> This is especially bad for t

Re: [PATCH 10/12] block.c: add subtree_drains where needed

>> >>  From the other thread: >>> So you mean that backing_hd is modified as its parent is changed, do >>> I understand correctly? >> >> Yes >> >>> >>> As we started to discuss in a thread started with my "[PATCH] block: >>> bdrv_set_backing_hd(): use drained section", I think draining the whole >>

Re: [PATCH v6 13/33] include/block/blockjob.h: global state API

On 07/02/2022 18:26, Kevin Wolf wrote: > Am 21.01.2022 um 18:05 hat Emanuele Giuseppe Esposito geschrieben: >> blockjob functions run always under the BQL lock. >> >> Signed-off-by: Emanuele Giuseppe Esposito >> Reviewed-by: Stefan Hajnoczi >> ---

Re: [PATCH v6 02/33] include/block/block: split header into I/O and global state API

On 07/02/2022 17:53, Kevin Wolf wrote: > Am 01.02.2022 um 11:30 hat Paolo Bonzini geschrieben: >> On 2/1/22 10:45, Emanuele Giuseppe Esposito wrote: >>>> That said, even if they are a different category, I think it makes sense >>>> to leave them in the same head

Re: [PATCH v6 31/33] include/qemu/job.h: introduce job->pre_run() and use it in amend

On 07/02/2022 19:14, Kevin Wolf wrote: > Am 21.01.2022 um 18:05 hat Emanuele Giuseppe Esposito geschrieben: >> Introduce .pre_run() job callback. This cb will run in job_start, >> before the coroutine is created and runs run() in the job aiocontext. >> >> Therefore

Re: [PATCH v6 00/33] block layer: split block APIs in global state and I/O

On 07/02/2022 19:30, Kevin Wolf wrote: > Am 21.01.2022 um 18:05 hat Emanuele Giuseppe Esposito geschrieben: >> Each function in the GS API will have an assertion, checking >> that it is always running under BQL. >> I/O functions are instead thread safe (or so should be),

[PATCH v5 00/20] job: replace AioContext lock with job_mutex

ITH_JOB_LOCK_GUARD * mu(u)ltiple typos in commit messages * job API split patches are sent separately in another series * use of empty job_{lock/unlock} and JOB_LOCK_GUARD/WITH_JOB_LOCK_GUARD to avoid deadlocks and simplify the reviewer job * move patch 11 (block_job_query: remove atomic read

[PATCH v5 07/20] job.h: add _locked duplicates for job API functions called with and without job_mutex

) job_cancel_requested (_locked version is static) Signed-off-by: Emanuele Giuseppe Esposito --- include/qemu/job.h | 25 +--- job.c | 48 -- 2 files changed, 64 insertions(+), 9 deletions(-) diff --git a/include/qemu/job.h

[PATCH v5 08/20] jobs: protect jobs with job_lock/unlock

macros are *nop*. Signed-off-by: Emanuele Giuseppe Esposito --- block.c | 18 --- block/replication.c | 8 ++- blockdev.c | 17 -- blockjob.c | 62 +++--- job-qmp.c | 2 + job.c | 123

[PATCH v5 19/20] job.c: enable job lock/unlock and remove Aiocontext locks

ned-off-by: Emanuele Giuseppe Esposito --- blockdev.c | 65 --- include/qemu/job.h | 19 job-qmp.c| 41 - job.c| 77 +++- tests/unit/t

[PATCH v5 13/20] job.h: rename job API functions called with job_mutex held

miss Note that "locked" refers to the *nop* job_lock/unlock, and not real_job_lock/unlock. No functional change intended. Signed-off-by: Emanuele Giuseppe Esposito --- block.c | 2 +- block/mirror.c | 2 +- block/replication.

[PATCH v2 0/3] job: split job API in driver and monitor

t;. Based-on: <20220208143513.1077229-1-eespo...@redhat.com> --- v2: * rebased on new version of job_api Emanuele Giuseppe Esposito (3): jobs: add job-common.h jobs: add job-monitor.h jobs: add job-driver.h include/qemu/job-common.h | 357 +++ include/qemu/job

[PATCH v5 06/20] jobs: remove aiocontext locks since the functions are under BQL

/release. We don't want this, as job_lock must be taken inside the AioContext lock, and taking it outside would cause deadlocks. Signed-off-by: Emanuele Giuseppe Esposito --- blockdev.c | 4 job-qmp.c | 4 2 files changed, 8 deletions(-) diff --git a/blockdev.c b/blockdev.c index 8cac3

[PATCH v2 1/3] jobs: add job-common.h

-by: Emanuele Giuseppe Esposito --- include/qemu/job-common.h | 357 ++ include/qemu/job.h| 328 +- 2 files changed, 358 insertions(+), 327 deletions(-) create mode 100644 include/qemu/job-common.h diff --git a/include

[PATCH v5 03/20] job.c: API functions not used outside should be static

job_event_* functions can all be static, as they are not used outside job.c. Same applies for job_txn_add_job(). Signed-off-by: Emanuele Giuseppe Esposito --- include/qemu/job.h | 18 -- job.c | 12 +--- 2 files changed, 9 insertions(+), 21 deletions

[PATCH v5 04/20] job.c: move inner aiocontext lock in callbacks

. Signed-off-by: Emanuele Giuseppe Esposito --- job.c | 17 - 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/job.c b/job.c index f13939d3c6..d8c13ac0d1 100644 --- a/job.c +++ b/job.c @@ -149,7 +149,6 @@ static void job_txn_del_job(Job *job) static int job_txn_apply

[PATCH v5 01/20] job.c: make job_mutex and job_lock/unlock() public

e can change the nop into an actual mutex and remove the aiocontext lock. Since job_mutex is already being used, add static real_job_{lock/unlock} for the existing usage. Signed-off-by: Emanuele Giuseppe Esposito --- include/qemu/job.h | 24 job.c

[PATCH v5 02/20] job.h: categorize fields in struct Job

Categorize the fields in struct Job to understand which ones need to be protected by the job mutex and which don't. Signed-off-by: Emanuele Giuseppe Esposito --- include/qemu/job.h | 59 ++ 1 file changed, 34 insertions(+), 25 deletions(-) diff

[PATCH v5 09/20] jobs: add job lock in find_* functions

*. Signed-off-by: Emanuele Giuseppe Esposito --- blockdev.c | 14 +- job-qmp.c | 13 - 2 files changed, 25 insertions(+), 2 deletions(-) diff --git a/blockdev.c b/blockdev.c index c5fba4d157..08408cd44b 100644 --- a/blockdev.c +++ b/blockdev.c @@ -3311,7 +3311,10 @@ out

[PATCH v5 05/20] aio-wait.h: introduce AIO_WAIT_WHILE_UNLOCKED

Same as AIO_WAIT_WHILE macro, but if we are in the Main loop do not release and then acquire ctx_ 's aiocontext. Once all Aiocontext locks go away, this macro will replace AIO_WAIT_WHILE. Signed-off-by: Emanuele Giuseppe Esposito --- include/block/aio-wait.h | 15 +++ 1

[PATCH v5 10/20] jobs: use job locks also in the unit tests

Add missing job synchronization in the unit tests, with explicit locks. Note: at this stage, job_{lock/unlock} and job lock guard macros are *nop*. Signed-off-by: Emanuele Giuseppe Esposito --- tests/unit/test-bdrv-drain.c | 76 - tests/unit/test-block-iothread.c

[PATCH v5 11/20] block/mirror.c: use of job helpers in drivers to avoid TOC/TOU

Once job lock is used and aiocontext is removed, mirror has to perform job operations under the same critical section, using the helpers prepared in previous commit. Note: at this stage, job_{lock/unlock} and job lock guard macros are *nop*. Signed-off-by: Emanuele Giuseppe Esposito --- block

[PATCH v5 12/20] jobs: rename static functions called with job_mutex held

lock. No functional change intended. Signed-off-by: Emanuele Giuseppe Esposito --- job.c | 247 +- 1 file changed, 141 insertions(+), 106 deletions(-) diff --git a/job.c b/job.c index 75f7c28147..56d2a8d98f 100644 --- a/job.c +++ b/jo

[PATCH v5 18/20] jobs: protect job.aio_context with BQL and job_mutex

introduce job_set_aio_context and make sure that the context is set under BQL, job_mutex and drain. Also make sure all other places where the aiocontext is read are protected. Suggested-by: Paolo Bonzini Signed-off-by: Emanuele Giuseppe Esposito --- block/replication.c | 2 +- blockjob.c

[PATCH v5 15/20] job.h: define unlocked functions

in job.c). Note: at this stage, job_{lock/unlock} and job lock guard macros are *nop*. No functional change intended. Signed-off-by: Emanuele Giuseppe Esposito --- blockjob.c | 20 include/qemu/job.h | 37 ++--- job.c

[PATCH 4/6] test-bdrv-drain.c: adapt test to the coming subtree drains

the original after it's been invoked. Signed-off-by: Emanuele Giuseppe Esposito --- tests/unit/test-bdrv-drain.c | 17 - 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/tests/unit/test-bdrv-drain.c b/tests/unit/test-bdrv-drain.c index 4924ceb562..c52ba2db4e 1

[PATCH 3/6] block.c: bdrv_replace_child_noperm: first call ->attach(), and then add child

ng the drain with apply_subtree_drain(), leaving the node undrained between the two operations. Signed-off-by: Emanuele Giuseppe Esposito --- block.c | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/block.c b/block.c index ec346a7e2e..08a6e3a4ef 100644 --- a/block.c

[PATCH v5 16/20] commit and mirror: create new nodes using bdrv_get_aio_context, and not the job aiocontext

We are always using the given bs AioContext, so there is no need to take the job ones (which is identical anyways). This also reduces the point we need to check when protecting job.aio_context field. Signed-off-by: Emanuele Giuseppe Esposito --- block/commit.c | 4 ++-- block/mirror.c | 2 +- 2

[PATCH v5 20/20] block_job_query: remove atomic read

Not sure what the atomic here was supposed to do, since job.busy is protected by the job lock. Since the whole function is called under job_mutex, just remove the atomic. Signed-off-by: Emanuele Giuseppe Esposito --- blockjob.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a

[PATCH v5 14/20] block_job: rename block_job functions called with job_mutex held

Just as for the job API, rename block_job functions that are always called under job lock. No functional change intended. Signed-off-by: Emanuele Giuseppe Esposito --- block.c | 3 ++- block/backup.c | 4 ++-- blockdev.c | 12 +++- blockjob.c

[PATCH v5 17/20] job: detect change of aiocontext within job coroutine

From: Paolo Bonzini We want to make sure access of job->aio_context is always done under either BQL or job_mutex. The problem is that using aio_co_enter(job->aiocontext, job->co) in job_start and job_enter_cond makes the coroutine immediately resume, so we can't hold the job lock. And caching it

[PATCH 0/6] block: bug fixes in preparation of AioContext removal

a work in progress and these fixes are pretty much independent, I split that in two separate series. This serie is based on "job: replace AioContext lock with job_mutex" Based-on: <20220208143513.1077229-1-eespo...@redhat.com> Emanuele Giuseppe Esposito (6): block/io.c: fix b

[PATCH 6/6] jobs: ensure sleep in job_sleep_ns is fully performed

sserts. Signed-off-by: Emanuele Giuseppe Esposito --- job.c | 19 +++ tests/qemu-iotests/030 | 2 +- tests/qemu-iotests/151 | 4 ++-- tests/unit/test-blockjob.c | 2 +- 4 files changed, 15 insertions(+), 12 deletions(-) diff --git a/job.c b/job

[PATCH v2 2/3] jobs: add job-monitor.h

job-monitor.h contains all functions of job.h that are used by the monitor and essentially all functions that do not define a JobDriver/Blockdriver. No functional change intended. Signed-off-by: Emanuele Giuseppe Esposito --- include/qemu/job-monitor.h | 249

[PATCH v2 3/3] jobs: add job-driver.h

job-driver.h contains all functions of job.h that are used by the drivers (JobDriver, BlockJobDriver). These functions are unaware of the job_mutex, so they all take and release the lock internally. No functional change intended. Signed-off-by: Emanuele Giuseppe Esposito --- include/qemu/job

[PATCH 5/6] test-bdrv-drain.c: remove test_detach_by_parent_cb()

uot; https://lists.nongnu.org/archive/html/qemu-block/2018-05/msg01132.html but as explained above I believe that it is not valid anymore, and can be discarded. Signed-off-by: Emanuele Giuseppe Esposito --- tests/unit/test-bdrv-drain.c | 46 +--- 1 file changed, 1

[PATCH 2/6] block.c: bdrv_replace_child_noperm: first remove the child, and then call ->detach()

hat assert_bdrv_graph_writable is not yet fully enabled. Signed-off-by: Emanuele Giuseppe Esposito --- block.c | 10 ++ 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/block.c b/block.c index 4551eba2aa..ec346a7e2e 100644 --- a/block.c +++ b/block.c @@ -2854,14 +2854,16 @@ static v

[PATCH 1/6] block/io.c: fix bdrv_child_cb_drained_begin invocations from a coroutine

and poll will accomplish the same thing (invoking bdrv_do_drained_begin_quiesce) but will firstly check if we are already in a coroutine, and exit from that via bdrv_co_yield_to_drain(). Signed-off-by: Emanuele Giuseppe Esposito --- block.c | 2 +- block/io.c | 7

[PATCH 1/5] crypto: perform permission checks under BQL

ff-by: Emanuele Giuseppe Esposito --- block/amend.c | 24 block/crypto.c| 27 +++ include/block/block_int.h | 14 ++ 3 files changed, 65 insertions(+) diff --git a/block/amend.c b/block/amend.c index 392df

[PATCH 2/5] crypto: distinguish between main loop and I/O in block_crypto_amend_options_generic_luks

check for permissions but delegate .bdrv_amend_pre_run() and .bdrv_amend_clean() to do it, performing these checks before and after the job runs in its aiocontext. Signed-off-by: Emanuele Giuseppe Esposito --- block/crypto.c | 35 +++ 1 file changed, 15 inserti

[PATCH 3/5] block: introduce bdrv_activate

-off-by: Emanuele Giuseppe Esposito Reviewed-by: Hanna Reitz --- block.c | 7 ++- block/block-backend.c| 2 +- block/export/export.c| 2 +- block/parallels.c| 2 +- include/block/block.h| 1 + tests/unit/test-block

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