diff mbox series

[3/7] block: protect parallel jobs from overlapping

Message ID 1587407806-109784-4-git-send-email-andrey.shinkevich@virtuozzo.com
State New
Headers show
Series Apply COR-filter to the block-stream permanently | expand

Commit Message

Andrey Shinkevich April 20, 2020, 6:36 p.m. UTC
When it comes to the check for the blocked operations, the node may be
a filter linked to blk. In that case, do not miss to set blocked
operations for the underlying node.

Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
---
 blockjob.c | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

Comments

Vladimir Sementsov-Ogievskiy April 21, 2020, 12:33 p.m. UTC | #1
20.04.2020 21:36, Andrey Shinkevich wrote:
> When it comes to the check for the blocked operations, the node may be
> a filter linked to blk.

"blk" commonly refers to BlockBackend, which is unrelated here. You mean just a filter.

> In that case, do not miss to set blocked
> operations for the underlying node.
> 
> Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
> ---
>   blockjob.c | 15 ++++++++++++++-
>   1 file changed, 14 insertions(+), 1 deletion(-)
> 
> diff --git a/blockjob.c b/blockjob.c
> index 73d9f1b..2898929 100644
> --- a/blockjob.c
> +++ b/blockjob.c
> @@ -189,7 +189,14 @@ void block_job_remove_all_bdrv(BlockJob *job)
>       GSList *l;
>       for (l = job->nodes; l; l = l->next) {
>           BdrvChild *c = l->data;
> -        bdrv_op_unblock_all(c->bs, job->blocker);
> +        BlockDriverState *bs = c->bs;
> +        bdrv_op_unblock_all(bs, job->blocker);
> +        if (bs->drv && bs->drv->is_filter) {
> +            bs = bdrv_filtered_bs(bs);
> +            if (bs) {
> +                bdrv_op_unblock_all(bs, job->blocker);
> +            }
> +        }
>           bdrv_root_unref_child(c);
>       }
>       g_slist_free(job->nodes);
> @@ -230,6 +237,12 @@ int block_job_add_bdrv(BlockJob *job, const char *name, BlockDriverState *bs,
>   
>       job->nodes = g_slist_prepend(job->nodes, c);
>       bdrv_op_block_all(bs, job->blocker);
> +    if (bs->drv && bs->drv->is_filter) {
> +        bs = bdrv_filtered_bs(bs);
> +        if (bs) {
> +            bdrv_op_block_all(bs, job->blocker);

This will lead to setting op blocker twice, if there are filters inside backing chain. Is it safe?

Still, I don't think it's correct thing. Job should add all it's nodes by hand. If it add some
filter node, but don't add it's filtered node, it is definitely doing something wrong (see my
answer to 1/7).


> +        }
> +    }
>   
>       return 0;
>   }
>
diff mbox series

Patch

diff --git a/blockjob.c b/blockjob.c
index 73d9f1b..2898929 100644
--- a/blockjob.c
+++ b/blockjob.c
@@ -189,7 +189,14 @@  void block_job_remove_all_bdrv(BlockJob *job)
     GSList *l;
     for (l = job->nodes; l; l = l->next) {
         BdrvChild *c = l->data;
-        bdrv_op_unblock_all(c->bs, job->blocker);
+        BlockDriverState *bs = c->bs;
+        bdrv_op_unblock_all(bs, job->blocker);
+        if (bs->drv && bs->drv->is_filter) {
+            bs = bdrv_filtered_bs(bs);
+            if (bs) {
+                bdrv_op_unblock_all(bs, job->blocker);
+            }
+        }
         bdrv_root_unref_child(c);
     }
     g_slist_free(job->nodes);
@@ -230,6 +237,12 @@  int block_job_add_bdrv(BlockJob *job, const char *name, BlockDriverState *bs,
 
     job->nodes = g_slist_prepend(job->nodes, c);
     bdrv_op_block_all(bs, job->blocker);
+    if (bs->drv && bs->drv->is_filter) {
+        bs = bdrv_filtered_bs(bs);
+        if (bs) {
+            bdrv_op_block_all(bs, job->blocker);
+        }
+    }
 
     return 0;
 }