diff mbox series

[v2,05/36] block: add bdrv_parent_try_set_aio_context

Message ID 20201127144522.29991-6-vsementsov@virtuozzo.com (mailing list archive)
State New, archived
Headers show
Series block: update graph permissions update | expand

Commit Message

Vladimir Sementsov-Ogievskiy Nov. 27, 2020, 2:44 p.m. UTC
We already have bdrv_parent_can_set_aio_context(). Add corresponding
bdrv_parent_set_aio_context_ignore() and
bdrv_parent_try_set_aio_context() and use them instead of open-coding.

Make bdrv_parent_try_set_aio_context() public, as it will be used in
further commit.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 include/block/block.h |  2 ++
 block.c               | 51 +++++++++++++++++++++++++++++++++----------
 2 files changed, 41 insertions(+), 12 deletions(-)

Comments

Kevin Wolf Jan. 18, 2021, 3:08 p.m. UTC | #1
Am 27.11.2020 um 15:44 hat Vladimir Sementsov-Ogievskiy geschrieben:
> We already have bdrv_parent_can_set_aio_context(). Add corresponding
> bdrv_parent_set_aio_context_ignore() and
> bdrv_parent_try_set_aio_context() and use them instead of open-coding.
> 
> Make bdrv_parent_try_set_aio_context() public, as it will be used in
> further commit.
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
>  include/block/block.h |  2 ++
>  block.c               | 51 +++++++++++++++++++++++++++++++++----------
>  2 files changed, 41 insertions(+), 12 deletions(-)
> 
> diff --git a/include/block/block.h b/include/block/block.h
> index ee3f5a6cca..550c5a7513 100644
> --- a/include/block/block.h
> +++ b/include/block/block.h
> @@ -686,6 +686,8 @@ bool bdrv_child_can_set_aio_context(BdrvChild *c, AioContext *ctx,
>                                      GSList **ignore, Error **errp);
>  bool bdrv_can_set_aio_context(BlockDriverState *bs, AioContext *ctx,
>                                GSList **ignore, Error **errp);
> +int bdrv_parent_try_set_aio_context(BdrvChild *c, AioContext *ctx,
> +                                    Error **errp);
>  int bdrv_probe_blocksizes(BlockDriverState *bs, BlockSizes *bsz);
>  int bdrv_probe_geometry(BlockDriverState *bs, HDGeometry *geo);
>  
> diff --git a/block.c b/block.c
> index 916087ee1a..5d925c208d 100644
> --- a/block.c
> +++ b/block.c
> @@ -81,6 +81,9 @@ static BlockDriverState *bdrv_open_inherit(const char *filename,
>                                             BdrvChildRole child_role,
>                                             Error **errp);
>  
> +static void bdrv_parent_set_aio_context_ignore(BdrvChild *c, AioContext *ctx,
> +                                               GSList **ignore);
> +
>  /* If non-zero, use only whitelisted block drivers */
>  static int use_bdrv_whitelist;
>  
> @@ -2655,17 +2658,12 @@ BdrvChild *bdrv_root_attach_child(BlockDriverState *child_bs,
>       * try moving the parent into the AioContext of child_bs instead. */
>      if (bdrv_get_aio_context(child_bs) != ctx) {
>          ret = bdrv_try_set_aio_context(child_bs, ctx, &local_err);
> -        if (ret < 0 && child_class->can_set_aio_ctx) {
> -            GSList *ignore = g_slist_prepend(NULL, child);
> -            ctx = bdrv_get_aio_context(child_bs);

You are losing this line...

> -            if (child_class->can_set_aio_ctx(child, ctx, &ignore, NULL)) {
> -                error_free(local_err);
> +        if (ret < 0) {
> +            if (bdrv_parent_try_set_aio_context(child, ctx, NULL) == 0) {

...before this one, so I think the wrong context is passed now. Instead
of trying to move the parent to the AioContext of the child, we'll try
to move it to the AioContext in which it already is (and which doesn't
match the AioContext of the child).

Kevin

>                  ret = 0;
> -                g_slist_free(ignore);
> -                ignore = g_slist_prepend(NULL, child);
> -                child_class->set_aio_ctx(child, ctx, &ignore);
> +                error_free(local_err);
> +                local_err = NULL;
>              }
> -            g_slist_free(ignore);
>          }
>          if (ret < 0) {
>              error_propagate(errp, local_err);
> @@ -6452,9 +6450,7 @@ void bdrv_set_aio_context_ignore(BlockDriverState *bs,
>          if (g_slist_find(*ignore, child)) {
>              continue;
>          }
> -        assert(child->klass->set_aio_ctx);
> -        *ignore = g_slist_prepend(*ignore, child);
> -        child->klass->set_aio_ctx(child, new_context, ignore);
> +        bdrv_parent_set_aio_context_ignore(child, new_context, ignore);
>      }
>  
>      bdrv_detach_aio_context(bs);
> @@ -6511,6 +6507,37 @@ static bool bdrv_parent_can_set_aio_context(BdrvChild *c, AioContext *ctx,
>      return true;
>  }
>  
> +static void bdrv_parent_set_aio_context_ignore(BdrvChild *c, AioContext *ctx,
> +                                               GSList **ignore)
> +{
> +    if (g_slist_find(*ignore, c)) {
> +        return;
> +    }
> +    *ignore = g_slist_prepend(*ignore, c);
> +
> +    assert(c->klass->set_aio_ctx);
> +    c->klass->set_aio_ctx(c, ctx, ignore);
> +}
> +
> +int bdrv_parent_try_set_aio_context(BdrvChild *c, AioContext *ctx,
> +                                    Error **errp)
> +{
> +    GSList *ignore = NULL;
> +
> +    if (!bdrv_parent_can_set_aio_context(c, ctx, &ignore, errp)) {
> +        g_slist_free(ignore);
> +        return -EPERM;
> +    }
> +
> +    g_slist_free(ignore);
> +    ignore = NULL;
> +
> +    bdrv_parent_set_aio_context_ignore(c, ctx, &ignore);
> +    g_slist_free(ignore);
> +
> +    return 0;
> +}
> +
>  bool bdrv_child_can_set_aio_context(BdrvChild *c, AioContext *ctx,
>                                      GSList **ignore, Error **errp)
>  {
> -- 
> 2.21.3
>
Vladimir Sementsov-Ogievskiy Jan. 18, 2021, 5:26 p.m. UTC | #2
18.01.2021 18:08, Kevin Wolf wrote:
> Am 27.11.2020 um 15:44 hat Vladimir Sementsov-Ogievskiy geschrieben:
>> We already have bdrv_parent_can_set_aio_context(). Add corresponding
>> bdrv_parent_set_aio_context_ignore() and
>> bdrv_parent_try_set_aio_context() and use them instead of open-coding.
>>
>> Make bdrv_parent_try_set_aio_context() public, as it will be used in
>> further commit.
>>
>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>> ---
>>   include/block/block.h |  2 ++
>>   block.c               | 51 +++++++++++++++++++++++++++++++++----------
>>   2 files changed, 41 insertions(+), 12 deletions(-)
>>
>> diff --git a/include/block/block.h b/include/block/block.h
>> index ee3f5a6cca..550c5a7513 100644
>> --- a/include/block/block.h
>> +++ b/include/block/block.h
>> @@ -686,6 +686,8 @@ bool bdrv_child_can_set_aio_context(BdrvChild *c, AioContext *ctx,
>>                                       GSList **ignore, Error **errp);
>>   bool bdrv_can_set_aio_context(BlockDriverState *bs, AioContext *ctx,
>>                                 GSList **ignore, Error **errp);
>> +int bdrv_parent_try_set_aio_context(BdrvChild *c, AioContext *ctx,
>> +                                    Error **errp);
>>   int bdrv_probe_blocksizes(BlockDriverState *bs, BlockSizes *bsz);
>>   int bdrv_probe_geometry(BlockDriverState *bs, HDGeometry *geo);
>>   
>> diff --git a/block.c b/block.c
>> index 916087ee1a..5d925c208d 100644
>> --- a/block.c
>> +++ b/block.c
>> @@ -81,6 +81,9 @@ static BlockDriverState *bdrv_open_inherit(const char *filename,
>>                                              BdrvChildRole child_role,
>>                                              Error **errp);
>>   
>> +static void bdrv_parent_set_aio_context_ignore(BdrvChild *c, AioContext *ctx,
>> +                                               GSList **ignore);
>> +
>>   /* If non-zero, use only whitelisted block drivers */
>>   static int use_bdrv_whitelist;
>>   
>> @@ -2655,17 +2658,12 @@ BdrvChild *bdrv_root_attach_child(BlockDriverState *child_bs,
>>        * try moving the parent into the AioContext of child_bs instead. */
>>       if (bdrv_get_aio_context(child_bs) != ctx) {
>>           ret = bdrv_try_set_aio_context(child_bs, ctx, &local_err);
>> -        if (ret < 0 && child_class->can_set_aio_ctx) {
>> -            GSList *ignore = g_slist_prepend(NULL, child);
>> -            ctx = bdrv_get_aio_context(child_bs);
> 
> You are losing this line...
> 
>> -            if (child_class->can_set_aio_ctx(child, ctx, &ignore, NULL)) {
>> -                error_free(local_err);
>> +        if (ret < 0) {
>> +            if (bdrv_parent_try_set_aio_context(child, ctx, NULL) == 0) {
> 
> ...before this one, so I think the wrong context is passed now. Instead
> of trying to move the parent to the AioContext of the child, we'll try
> to move it to the AioContext in which it already is (and which doesn't
> match the AioContext of the child).
> 

Oops, right, will fix

> 
>>                   ret = 0;
>> -                g_slist_free(ignore);
>> -                ignore = g_slist_prepend(NULL, child);
>> -                child_class->set_aio_ctx(child, ctx, &ignore);
>> +                error_free(local_err);
>> +                local_err = NULL;
>>               }
>> -            g_slist_free(ignore);
>>           }
>>           if (ret < 0) {
>>               error_propagate(errp, local_err);
>> @@ -6452,9 +6450,7 @@ void bdrv_set_aio_context_ignore(BlockDriverState *bs,
>>           if (g_slist_find(*ignore, child)) {
>>               continue;
>>           }
>> -        assert(child->klass->set_aio_ctx);
>> -        *ignore = g_slist_prepend(*ignore, child);
>> -        child->klass->set_aio_ctx(child, new_context, ignore);
>> +        bdrv_parent_set_aio_context_ignore(child, new_context, ignore);
>>       }
>>   
>>       bdrv_detach_aio_context(bs);
>> @@ -6511,6 +6507,37 @@ static bool bdrv_parent_can_set_aio_context(BdrvChild *c, AioContext *ctx,
>>       return true;
>>   }
>>   
>> +static void bdrv_parent_set_aio_context_ignore(BdrvChild *c, AioContext *ctx,
>> +                                               GSList **ignore)
>> +{
>> +    if (g_slist_find(*ignore, c)) {
>> +        return;
>> +    }
>> +    *ignore = g_slist_prepend(*ignore, c);
>> +
>> +    assert(c->klass->set_aio_ctx);
>> +    c->klass->set_aio_ctx(c, ctx, ignore);
>> +}
>> +
>> +int bdrv_parent_try_set_aio_context(BdrvChild *c, AioContext *ctx,
>> +                                    Error **errp)
>> +{
>> +    GSList *ignore = NULL;
>> +
>> +    if (!bdrv_parent_can_set_aio_context(c, ctx, &ignore, errp)) {
>> +        g_slist_free(ignore);
>> +        return -EPERM;
>> +    }
>> +
>> +    g_slist_free(ignore);
>> +    ignore = NULL;
>> +
>> +    bdrv_parent_set_aio_context_ignore(c, ctx, &ignore);
>> +    g_slist_free(ignore);
>> +
>> +    return 0;
>> +}
>> +
>>   bool bdrv_child_can_set_aio_context(BdrvChild *c, AioContext *ctx,
>>                                       GSList **ignore, Error **errp)
>>   {
>> -- 
>> 2.21.3
>>
>
diff mbox series

Patch

diff --git a/include/block/block.h b/include/block/block.h
index ee3f5a6cca..550c5a7513 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -686,6 +686,8 @@  bool bdrv_child_can_set_aio_context(BdrvChild *c, AioContext *ctx,
                                     GSList **ignore, Error **errp);
 bool bdrv_can_set_aio_context(BlockDriverState *bs, AioContext *ctx,
                               GSList **ignore, Error **errp);
+int bdrv_parent_try_set_aio_context(BdrvChild *c, AioContext *ctx,
+                                    Error **errp);
 int bdrv_probe_blocksizes(BlockDriverState *bs, BlockSizes *bsz);
 int bdrv_probe_geometry(BlockDriverState *bs, HDGeometry *geo);
 
diff --git a/block.c b/block.c
index 916087ee1a..5d925c208d 100644
--- a/block.c
+++ b/block.c
@@ -81,6 +81,9 @@  static BlockDriverState *bdrv_open_inherit(const char *filename,
                                            BdrvChildRole child_role,
                                            Error **errp);
 
+static void bdrv_parent_set_aio_context_ignore(BdrvChild *c, AioContext *ctx,
+                                               GSList **ignore);
+
 /* If non-zero, use only whitelisted block drivers */
 static int use_bdrv_whitelist;
 
@@ -2655,17 +2658,12 @@  BdrvChild *bdrv_root_attach_child(BlockDriverState *child_bs,
      * try moving the parent into the AioContext of child_bs instead. */
     if (bdrv_get_aio_context(child_bs) != ctx) {
         ret = bdrv_try_set_aio_context(child_bs, ctx, &local_err);
-        if (ret < 0 && child_class->can_set_aio_ctx) {
-            GSList *ignore = g_slist_prepend(NULL, child);
-            ctx = bdrv_get_aio_context(child_bs);
-            if (child_class->can_set_aio_ctx(child, ctx, &ignore, NULL)) {
-                error_free(local_err);
+        if (ret < 0) {
+            if (bdrv_parent_try_set_aio_context(child, ctx, NULL) == 0) {
                 ret = 0;
-                g_slist_free(ignore);
-                ignore = g_slist_prepend(NULL, child);
-                child_class->set_aio_ctx(child, ctx, &ignore);
+                error_free(local_err);
+                local_err = NULL;
             }
-            g_slist_free(ignore);
         }
         if (ret < 0) {
             error_propagate(errp, local_err);
@@ -6452,9 +6450,7 @@  void bdrv_set_aio_context_ignore(BlockDriverState *bs,
         if (g_slist_find(*ignore, child)) {
             continue;
         }
-        assert(child->klass->set_aio_ctx);
-        *ignore = g_slist_prepend(*ignore, child);
-        child->klass->set_aio_ctx(child, new_context, ignore);
+        bdrv_parent_set_aio_context_ignore(child, new_context, ignore);
     }
 
     bdrv_detach_aio_context(bs);
@@ -6511,6 +6507,37 @@  static bool bdrv_parent_can_set_aio_context(BdrvChild *c, AioContext *ctx,
     return true;
 }
 
+static void bdrv_parent_set_aio_context_ignore(BdrvChild *c, AioContext *ctx,
+                                               GSList **ignore)
+{
+    if (g_slist_find(*ignore, c)) {
+        return;
+    }
+    *ignore = g_slist_prepend(*ignore, c);
+
+    assert(c->klass->set_aio_ctx);
+    c->klass->set_aio_ctx(c, ctx, ignore);
+}
+
+int bdrv_parent_try_set_aio_context(BdrvChild *c, AioContext *ctx,
+                                    Error **errp)
+{
+    GSList *ignore = NULL;
+
+    if (!bdrv_parent_can_set_aio_context(c, ctx, &ignore, errp)) {
+        g_slist_free(ignore);
+        return -EPERM;
+    }
+
+    g_slist_free(ignore);
+    ignore = NULL;
+
+    bdrv_parent_set_aio_context_ignore(c, ctx, &ignore);
+    g_slist_free(ignore);
+
+    return 0;
+}
+
 bool bdrv_child_can_set_aio_context(BdrvChild *c, AioContext *ctx,
                                     GSList **ignore, Error **errp)
 {