Add an option in BlockExportOptions to allow creating an export on an
inactive node without activating the node. This mode needs to be
explicitly supported by the export type (so that it doesn't perform any
operations that are forbidden for inactive nodes), so this patch alone
doesn't allow this option to be successfully used yet.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
qapi/block-export.json | 10 +++++++++-
include/block/block-global-state.h | 3 +++
include/block/export.h | 3 +++
block.c | 4 ++++
block/export/export.c | 31 ++++++++++++++++++++----------
5 files changed, 40 insertions(+), 11 deletions(-)
diff --git a/qapi/block-export.json b/qapi/block-export.json
index ce33fe378d..117b05d13c 100644
--- a/qapi/block-export.json
+++ b/qapi/block-export.json
@@ -372,6 +372,13 @@
# cannot be moved to the iothread. The default is false.
# (since: 5.2)
#
+# @allow-inactive: If true, the export allows the exported node to be inactive.
+# If it is created for an inactive block node, the node remains inactive. If
+# the export type doesn't support running on an inactive node, an error is
+# returned. If false, inactive block nodes are automatically activated before
+# creating the export and trying to inactivate them later fails.
+# (since: 10.0; default: false)
+#
# Since: 4.2
##
{ 'union': 'BlockExportOptions',
@@ -381,7 +388,8 @@
'*iothread': 'str',
'node-name': 'str',
'*writable': 'bool',
- '*writethrough': 'bool' },
+ '*writethrough': 'bool',
+ '*allow-inactive': 'bool' },
'discriminator': 'type',
'data': {
'nbd': 'BlockExportOptionsNbd',
diff --git a/include/block/block-global-state.h b/include/block/block-global-state.h
index 22ec21117d..9be34b3c99 100644
--- a/include/block/block-global-state.h
+++ b/include/block/block-global-state.h
@@ -175,6 +175,9 @@ BlockDriverState * GRAPH_RDLOCK
check_to_replace_node(BlockDriverState *parent_bs, const char *node_name,
Error **errp);
+
+bool GRAPH_RDLOCK bdrv_is_inactive(BlockDriverState *bs);
+
int no_coroutine_fn GRAPH_RDLOCK
bdrv_activate(BlockDriverState *bs, Error **errp);
diff --git a/include/block/export.h b/include/block/export.h
index f2fe0f8078..4bd9531d4d 100644
--- a/include/block/export.h
+++ b/include/block/export.h
@@ -29,6 +29,9 @@ typedef struct BlockExportDriver {
*/
size_t instance_size;
+ /* True if the export type supports running on an inactive node */
+ bool supports_inactive;
+
/* Creates and starts a new block export */
int (*create)(BlockExport *, BlockExportOptions *, Error **);
diff --git a/block.c b/block.c
index 61e131e71f..7eeb8d076e 100644
--- a/block.c
+++ b/block.c
@@ -6845,6 +6845,10 @@ void bdrv_init_with_whitelist(void)
bdrv_init();
}
+bool bdrv_is_inactive(BlockDriverState *bs) {
+ return bs->open_flags & BDRV_O_INACTIVE;
+}
+
int bdrv_activate(BlockDriverState *bs, Error **errp)
{
BdrvChild *child, *parent;
diff --git a/block/export/export.c b/block/export/export.c
index bac42b8608..f3bbf11070 100644
--- a/block/export/export.c
+++ b/block/export/export.c
@@ -75,6 +75,7 @@ static const BlockExportDriver *blk_exp_find_driver(BlockExportType type)
BlockExport *blk_exp_add(BlockExportOptions *export, Error **errp)
{
bool fixed_iothread = export->has_fixed_iothread && export->fixed_iothread;
+ bool allow_inactive = export->has_allow_inactive && export->allow_inactive;
const BlockExportDriver *drv;
BlockExport *exp = NULL;
BlockDriverState *bs;
@@ -138,17 +139,24 @@ BlockExport *blk_exp_add(BlockExportOptions *export, Error **errp)
}
}
- /*
- * Block exports are used for non-shared storage migration. Make sure
- * that BDRV_O_INACTIVE is cleared and the image is ready for write
- * access since the export could be available before migration handover.
- * ctx was acquired in the caller.
- */
bdrv_graph_rdlock_main_loop();
- ret = bdrv_activate(bs, errp);
- if (ret < 0) {
- bdrv_graph_rdunlock_main_loop();
- goto fail;
+ if (allow_inactive) {
+ if (!drv->supports_inactive) {
+ error_setg(errp, "Export type does not support inactive exports");
+ bdrv_graph_rdunlock_main_loop();
+ goto fail;
+ }
+ } else {
+ /*
+ * Block exports are used for non-shared storage migration. Make sure
+ * that BDRV_O_INACTIVE is cleared and the image is ready for write
+ * access since the export could be available before migration handover.
+ */
+ ret = bdrv_activate(bs, errp);
+ if (ret < 0) {
+ bdrv_graph_rdunlock_main_loop();
+ goto fail;
+ }
}
bdrv_graph_rdunlock_main_loop();
@@ -162,6 +170,9 @@ BlockExport *blk_exp_add(BlockExportOptions *export, Error **errp)
if (!fixed_iothread) {
blk_set_allow_aio_context_change(blk, true);
}
+ if (allow_inactive) {
+ blk_set_force_allow_inactivate(blk);
+ }
ret = blk_insert_bs(blk, bs, errp);
if (ret < 0) {
--
2.48.1On Fri, Jan 31, 2025 at 10:50:47AM +0100, Kevin Wolf wrote: > Add an option in BlockExportOptions to allow creating an export on an > inactive node without activating the node. This mode needs to be > explicitly supported by the export type (so that it doesn't perform any > operations that are forbidden for inactive nodes), so this patch alone > doesn't allow this option to be successfully used yet. > > Signed-off-by: Kevin Wolf <kwolf@redhat.com> > --- > qapi/block-export.json | 10 +++++++++- > include/block/block-global-state.h | 3 +++ > include/block/export.h | 3 +++ > block.c | 4 ++++ > block/export/export.c | 31 ++++++++++++++++++++---------- > 5 files changed, 40 insertions(+), 11 deletions(-) > > diff --git a/qapi/block-export.json b/qapi/block-export.json > index ce33fe378d..117b05d13c 100644 > --- a/qapi/block-export.json > +++ b/qapi/block-export.json > @@ -372,6 +372,13 @@ > # cannot be moved to the iothread. The default is false. > # (since: 5.2) > # > +# @allow-inactive: If true, the export allows the exported node to be inactive. > +# If it is created for an inactive block node, the node remains inactive. If > +# the export type doesn't support running on an inactive node, an error is > +# returned. If false, inactive block nodes are automatically activated before > +# creating the export and trying to inactivate them later fails. > +# (since: 10.0; default: false) Exposing activation in the API is ugly but I don't see a cleaner option given that we cannot change block-export-add's existing behavior of activating the node by default. :( Ideally block-export-add would not modify active/inactive and leave it up to user to provide a node in the desired state. Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
On Fri, Jan 31, 2025 at 10:50:47AM +0100, Kevin Wolf wrote: > Add an option in BlockExportOptions to allow creating an export on an > inactive node without activating the node. This mode needs to be > explicitly supported by the export type (so that it doesn't perform any > operations that are forbidden for inactive nodes), so this patch alone > doesn't allow this option to be successfully used yet. > > Signed-off-by: Kevin Wolf <kwolf@redhat.com> > --- > qapi/block-export.json | 10 +++++++++- > include/block/block-global-state.h | 3 +++ > include/block/export.h | 3 +++ > block.c | 4 ++++ > block/export/export.c | 31 ++++++++++++++++++++---------- > 5 files changed, 40 insertions(+), 11 deletions(-) > Reviewed-by: Eric Blake <eblake@redhat.com> -- Eric Blake, Principal Software Engineer Red Hat, Inc. Virtualization: qemu.org | libguestfs.org
Kevin Wolf <kwolf@redhat.com> writes:
> Add an option in BlockExportOptions to allow creating an export on an
> inactive node without activating the node. This mode needs to be
> explicitly supported by the export type (so that it doesn't perform any
> operations that are forbidden for inactive nodes), so this patch alone
> doesn't allow this option to be successfully used yet.
>
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
> qapi/block-export.json | 10 +++++++++-
> include/block/block-global-state.h | 3 +++
> include/block/export.h | 3 +++
> block.c | 4 ++++
> block/export/export.c | 31 ++++++++++++++++++++----------
> 5 files changed, 40 insertions(+), 11 deletions(-)
>
> diff --git a/qapi/block-export.json b/qapi/block-export.json
> index ce33fe378d..117b05d13c 100644
> --- a/qapi/block-export.json
> +++ b/qapi/block-export.json
> @@ -372,6 +372,13 @@
> # cannot be moved to the iothread. The default is false.
> # (since: 5.2)
> #
> +# @allow-inactive: If true, the export allows the exported node to be inactive.
> +# If it is created for an inactive block node, the node remains inactive. If
> +# the export type doesn't support running on an inactive node, an error is
> +# returned. If false, inactive block nodes are automatically activated before
> +# creating the export and trying to inactivate them later fails.
> +# (since: 10.0; default: false)
> +#
> # Since: 4.2
> ##
> { 'union': 'BlockExportOptions',
> @@ -381,7 +388,8 @@
> '*iothread': 'str',
> 'node-name': 'str',
> '*writable': 'bool',
> - '*writethrough': 'bool' },
> + '*writethrough': 'bool',
> + '*allow-inactive': 'bool' },
> 'discriminator': 'type',
> 'data': {
> 'nbd': 'BlockExportOptionsNbd',
> diff --git a/include/block/block-global-state.h b/include/block/block-global-state.h
> index 22ec21117d..9be34b3c99 100644
> --- a/include/block/block-global-state.h
> +++ b/include/block/block-global-state.h
> @@ -175,6 +175,9 @@ BlockDriverState * GRAPH_RDLOCK
> check_to_replace_node(BlockDriverState *parent_bs, const char *node_name,
> Error **errp);
>
> +
> +bool GRAPH_RDLOCK bdrv_is_inactive(BlockDriverState *bs);
> +
> int no_coroutine_fn GRAPH_RDLOCK
> bdrv_activate(BlockDriverState *bs, Error **errp);
>
> diff --git a/include/block/export.h b/include/block/export.h
> index f2fe0f8078..4bd9531d4d 100644
> --- a/include/block/export.h
> +++ b/include/block/export.h
> @@ -29,6 +29,9 @@ typedef struct BlockExportDriver {
> */
> size_t instance_size;
>
> + /* True if the export type supports running on an inactive node */
> + bool supports_inactive;
> +
> /* Creates and starts a new block export */
> int (*create)(BlockExport *, BlockExportOptions *, Error **);
>
> diff --git a/block.c b/block.c
> index 61e131e71f..7eeb8d076e 100644
> --- a/block.c
> +++ b/block.c
> @@ -6845,6 +6845,10 @@ void bdrv_init_with_whitelist(void)
> bdrv_init();
> }
>
> +bool bdrv_is_inactive(BlockDriverState *bs) {
> + return bs->open_flags & BDRV_O_INACTIVE;
> +}
This function is needed by patch 1/15.
> +
> int bdrv_activate(BlockDriverState *bs, Error **errp)
> {
> BdrvChild *child, *parent;
> diff --git a/block/export/export.c b/block/export/export.c
> index bac42b8608..f3bbf11070 100644
> --- a/block/export/export.c
> +++ b/block/export/export.c
> @@ -75,6 +75,7 @@ static const BlockExportDriver *blk_exp_find_driver(BlockExportType type)
> BlockExport *blk_exp_add(BlockExportOptions *export, Error **errp)
> {
> bool fixed_iothread = export->has_fixed_iothread && export->fixed_iothread;
> + bool allow_inactive = export->has_allow_inactive && export->allow_inactive;
> const BlockExportDriver *drv;
> BlockExport *exp = NULL;
> BlockDriverState *bs;
> @@ -138,17 +139,24 @@ BlockExport *blk_exp_add(BlockExportOptions *export, Error **errp)
> }
> }
>
> - /*
> - * Block exports are used for non-shared storage migration. Make sure
> - * that BDRV_O_INACTIVE is cleared and the image is ready for write
> - * access since the export could be available before migration handover.
> - * ctx was acquired in the caller.
> - */
> bdrv_graph_rdlock_main_loop();
> - ret = bdrv_activate(bs, errp);
> - if (ret < 0) {
> - bdrv_graph_rdunlock_main_loop();
> - goto fail;
> + if (allow_inactive) {
> + if (!drv->supports_inactive) {
> + error_setg(errp, "Export type does not support inactive exports");
> + bdrv_graph_rdunlock_main_loop();
> + goto fail;
> + }
> + } else {
> + /*
> + * Block exports are used for non-shared storage migration. Make sure
> + * that BDRV_O_INACTIVE is cleared and the image is ready for write
> + * access since the export could be available before migration handover.
> + */
> + ret = bdrv_activate(bs, errp);
> + if (ret < 0) {
> + bdrv_graph_rdunlock_main_loop();
> + goto fail;
> + }
> }
> bdrv_graph_rdunlock_main_loop();
>
> @@ -162,6 +170,9 @@ BlockExport *blk_exp_add(BlockExportOptions *export, Error **errp)
> if (!fixed_iothread) {
> blk_set_allow_aio_context_change(blk, true);
> }
> + if (allow_inactive) {
> + blk_set_force_allow_inactivate(blk);
> + }
>
> ret = blk_insert_bs(blk, bs, errp);
> if (ret < 0) {
Am 31.01.2025 um 14:41 hat Fabiano Rosas geschrieben:
> Kevin Wolf <kwolf@redhat.com> writes:
>
> > Add an option in BlockExportOptions to allow creating an export on an
> > inactive node without activating the node. This mode needs to be
> > explicitly supported by the export type (so that it doesn't perform any
> > operations that are forbidden for inactive nodes), so this patch alone
> > doesn't allow this option to be successfully used yet.
> >
> > Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> > diff --git a/block.c b/block.c
> > index 61e131e71f..7eeb8d076e 100644
> > --- a/block.c
> > +++ b/block.c
> > @@ -6845,6 +6845,10 @@ void bdrv_init_with_whitelist(void)
> > bdrv_init();
> > }
> >
> > +bool bdrv_is_inactive(BlockDriverState *bs) {
> > + return bs->open_flags & BDRV_O_INACTIVE;
> > +}
>
> This function is needed by patch 1/15.
Thanks, I'll move it there.
Kevin
© 2016 - 2025 Red Hat, Inc.