Re: [PATCH v12 02/14] block: add insert/remove node functions
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
Signed-off-by: Andrey Shinkevich
---
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, _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, _abort);
+ bdrv_replace_node(bs, inferior_bs, _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
+
/*
* 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);
--
Best regards,
Vladimir
Re: [PATCH v12 02/14] block: add insert/remove node functions
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
Signed-off-by: Andrey Shinkevich
---
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, _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, _abort);
+bdrv_replace_node(bs, inferior_bs, _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);
--
Best regards,
Vladimir
[PATCH v12 02/14] block: add insert/remove node functions
Provide API for a node insertion to and removal from a backing chain.
Suggested-by: Max Reitz
Signed-off-by: Andrey Shinkevich
---
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, _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, _abort);
+bdrv_replace_node(bs, inferior_bs, _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);
--
1.8.3.1
