diff mbox series

[v12,02/14] block: add insert/remove node functions

Message ID 1603390423-980205-3-git-send-email-andrey.shinkevich@virtuozzo.com (mailing list archive)
State New, archived
Headers show
Series Apply COR-filter to the block-stream permanently | expand

Commit Message

Andrey Shinkevich Oct. 22, 2020, 6:13 p.m. UTC
Provide API for a node insertion to and removal from a backing chain.

Suggested-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
---
 block.c               | 49 +++++++++++++++++++++++++++++++++++++++++++++++++
 include/block/block.h |  3 +++
 2 files changed, 52 insertions(+)

Comments

Vladimir Sementsov-Ogievskiy Oct. 23, 2020, 2:24 p.m. UTC | #1
22.10.2020 21:13, Andrey Shinkevich wrote:
> Provide API for a node insertion to and removal from a backing chain.
> 
> Suggested-by: Max Reitz <mreitz@redhat.com>
> Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
> ---
>   block.c               | 49 +++++++++++++++++++++++++++++++++++++++++++++++++
>   include/block/block.h |  3 +++
>   2 files changed, 52 insertions(+)
> 
> diff --git a/block.c b/block.c
> index 430edf7..502b483 100644
> --- a/block.c
> +++ b/block.c
> @@ -4670,6 +4670,55 @@ static void bdrv_delete(BlockDriverState *bs)
>       g_free(bs);
>   }
>   
> +BlockDriverState *bdrv_insert_node(BlockDriverState *bs, QDict *node_options,
> +                                   int flags, Error **errp)
> +{
> +    BlockDriverState *new_node_bs;
> +    Error *local_err = NULL;
> +
> +    new_node_bs =  bdrv_open(NULL, NULL, node_options, flags, errp);
> +    if (new_node_bs == NULL) {
> +        error_prepend(errp, "Could not create node: ");
> +        return NULL;
> +    }
> +
> +    bdrv_drained_begin(bs);
> +    bdrv_replace_node(bs, new_node_bs, &local_err);
> +    bdrv_drained_end(bs);
> +
> +    if (local_err) {
> +        bdrv_unref(new_node_bs);
> +        error_propagate(errp, local_err);
> +        return NULL;
> +    }
> +
> +    return new_node_bs;
> +}
> +
> +void bdrv_remove_node(BlockDriverState *bs)
> +{
> +    BdrvChild *child;
> +    BlockDriverState *inferior_bs;
> +
> +    child = bdrv_filter_or_cow_child(bs);
> +    if (!child) {
> +        return;
> +    }
> +    inferior_bs = child->bs;
> +
> +    /* Retain the BDS until we complete the graph change. */
> +    bdrv_ref(inferior_bs);
> +    /* Hold a guest back from writing while permissions are being reset. */
> +    bdrv_drained_begin(inferior_bs);
> +    /* Refresh permissions before the graph change. */
> +    bdrv_child_refresh_perms(bs, child, &error_abort);
> +    bdrv_replace_node(bs, inferior_bs, &error_abort);
> +
> +    bdrv_drained_end(inferior_bs);
> +    bdrv_unref(inferior_bs);
> +    bdrv_unref(bs);
> +}

The function is unused, and as I understand can't work as intended without s->active handling. I think it should be dropped

> +
>   /*
>    * Run consistency checks on an image
>    *
> diff --git a/include/block/block.h b/include/block/block.h
> index d16c401..ae7612f 100644
> --- a/include/block/block.h
> +++ b/include/block/block.h
> @@ -350,6 +350,9 @@ void bdrv_append(BlockDriverState *bs_new, BlockDriverState *bs_top,
>                    Error **errp);
>   void bdrv_replace_node(BlockDriverState *from, BlockDriverState *to,
>                          Error **errp);
> +BlockDriverState *bdrv_insert_node(BlockDriverState *bs, QDict *node_options,
> +                                   int flags, Error **errp);
> +void bdrv_remove_node(BlockDriverState *bs);
>   
>   int bdrv_parse_aio(const char *mode, int *flags);
>   int bdrv_parse_cache_mode(const char *mode, int *flags, bool *writethrough);
>
Vladimir Sementsov-Ogievskiy Oct. 23, 2020, 2:32 p.m. UTC | #2
23.10.2020 17:24, Vladimir Sementsov-Ogievskiy wrote:
> 22.10.2020 21:13, Andrey Shinkevich wrote:
>> Provide API for a node insertion to and removal from a backing chain.
>>
>> Suggested-by: Max Reitz <mreitz@redhat.com>
>> Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
>> ---
>>   block.c               | 49 +++++++++++++++++++++++++++++++++++++++++++++++++
>>   include/block/block.h |  3 +++
>>   2 files changed, 52 insertions(+)
>>
>> diff --git a/block.c b/block.c
>> index 430edf7..502b483 100644
>> --- a/block.c
>> +++ b/block.c
>> @@ -4670,6 +4670,55 @@ static void bdrv_delete(BlockDriverState *bs)
>>       g_free(bs);
>>   }
>> +BlockDriverState *bdrv_insert_node(BlockDriverState *bs, QDict *node_options,
>> +                                   int flags, Error **errp)
>> +{
>> +    BlockDriverState *new_node_bs;
>> +    Error *local_err = NULL;
>> +
>> +    new_node_bs =  bdrv_open(NULL, NULL, node_options, flags, errp);
>> +    if (new_node_bs == NULL) {
>> +        error_prepend(errp, "Could not create node: ");
>> +        return NULL;
>> +    }
>> +
>> +    bdrv_drained_begin(bs);
>> +    bdrv_replace_node(bs, new_node_bs, &local_err);
>> +    bdrv_drained_end(bs);
>> +
>> +    if (local_err) {
>> +        bdrv_unref(new_node_bs);
>> +        error_propagate(errp, local_err);
>> +        return NULL;
>> +    }
>> +
>> +    return new_node_bs;
>> +}
>> +
>> +void bdrv_remove_node(BlockDriverState *bs)
>> +{
>> +    BdrvChild *child;
>> +    BlockDriverState *inferior_bs;
>> +
>> +    child = bdrv_filter_or_cow_child(bs);
>> +    if (!child) {
>> +        return;
>> +    }
>> +    inferior_bs = child->bs;
>> +
>> +    /* Retain the BDS until we complete the graph change. */
>> +    bdrv_ref(inferior_bs);
>> +    /* Hold a guest back from writing while permissions are being reset. */
>> +    bdrv_drained_begin(inferior_bs);
>> +    /* Refresh permissions before the graph change. */
>> +    bdrv_child_refresh_perms(bs, child, &error_abort);
>> +    bdrv_replace_node(bs, inferior_bs, &error_abort);
>> +
>> +    bdrv_drained_end(inferior_bs);
>> +    bdrv_unref(inferior_bs);
>> +    bdrv_unref(bs);
>> +}
> 
> The function is unused, and as I understand can't work as intended without s->active handling. I think it should be dropped

with bdrv_remove_node dropped:

Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>

> 
>> +
>>   /*
>>    * Run consistency checks on an image
>>    *
>> diff --git a/include/block/block.h b/include/block/block.h
>> index d16c401..ae7612f 100644
>> --- a/include/block/block.h
>> +++ b/include/block/block.h
>> @@ -350,6 +350,9 @@ void bdrv_append(BlockDriverState *bs_new, BlockDriverState *bs_top,
>>                    Error **errp);
>>   void bdrv_replace_node(BlockDriverState *from, BlockDriverState *to,
>>                          Error **errp);
>> +BlockDriverState *bdrv_insert_node(BlockDriverState *bs, QDict *node_options,
>> +                                   int flags, Error **errp);
>> +void bdrv_remove_node(BlockDriverState *bs);
>>   int bdrv_parse_aio(const char *mode, int *flags);
>>   int bdrv_parse_cache_mode(const char *mode, int *flags, bool *writethrough);
>>
> 
>
diff mbox series

Patch

diff --git a/block.c b/block.c
index 430edf7..502b483 100644
--- a/block.c
+++ b/block.c
@@ -4670,6 +4670,55 @@  static void bdrv_delete(BlockDriverState *bs)
     g_free(bs);
 }
 
+BlockDriverState *bdrv_insert_node(BlockDriverState *bs, QDict *node_options,
+                                   int flags, Error **errp)
+{
+    BlockDriverState *new_node_bs;
+    Error *local_err = NULL;
+
+    new_node_bs =  bdrv_open(NULL, NULL, node_options, flags, errp);
+    if (new_node_bs == NULL) {
+        error_prepend(errp, "Could not create node: ");
+        return NULL;
+    }
+
+    bdrv_drained_begin(bs);
+    bdrv_replace_node(bs, new_node_bs, &local_err);
+    bdrv_drained_end(bs);
+
+    if (local_err) {
+        bdrv_unref(new_node_bs);
+        error_propagate(errp, local_err);
+        return NULL;
+    }
+
+    return new_node_bs;
+}
+
+void bdrv_remove_node(BlockDriverState *bs)
+{
+    BdrvChild *child;
+    BlockDriverState *inferior_bs;
+
+    child = bdrv_filter_or_cow_child(bs);
+    if (!child) {
+        return;
+    }
+    inferior_bs = child->bs;
+
+    /* Retain the BDS until we complete the graph change. */
+    bdrv_ref(inferior_bs);
+    /* Hold a guest back from writing while permissions are being reset. */
+    bdrv_drained_begin(inferior_bs);
+    /* Refresh permissions before the graph change. */
+    bdrv_child_refresh_perms(bs, child, &error_abort);
+    bdrv_replace_node(bs, inferior_bs, &error_abort);
+
+    bdrv_drained_end(inferior_bs);
+    bdrv_unref(inferior_bs);
+    bdrv_unref(bs);
+}
+
 /*
  * Run consistency checks on an image
  *
diff --git a/include/block/block.h b/include/block/block.h
index d16c401..ae7612f 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -350,6 +350,9 @@  void bdrv_append(BlockDriverState *bs_new, BlockDriverState *bs_top,
                  Error **errp);
 void bdrv_replace_node(BlockDriverState *from, BlockDriverState *to,
                        Error **errp);
+BlockDriverState *bdrv_insert_node(BlockDriverState *bs, QDict *node_options,
+                                   int flags, Error **errp);
+void bdrv_remove_node(BlockDriverState *bs);
 
 int bdrv_parse_aio(const char *mode, int *flags);
 int bdrv_parse_cache_mode(const char *mode, int *flags, bool *writethrough);