* [PATCH 01/14] block: refactor bdrv_remove_file_or_backing_child to bdrv_remove_child
2022-02-07 16:37 [PATCH 00/14] block: blockdev-del force=false Vladimir Sementsov-Ogievskiy
@ 2022-02-07 16:37 ` Vladimir Sementsov-Ogievskiy
2022-02-07 16:37 ` [PATCH 02/14] block: drop bdrv_detach_child() Vladimir Sementsov-Ogievskiy
` (12 subsequent siblings)
13 siblings, 0 replies; 15+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2022-02-07 16:37 UTC (permalink / raw)
To: qemu-block
Cc: qemu-devel, armbru, jsnow, vsementsov, eblake, hreitz, kwolf, den,
ktkhai, igor
Now the function can remove any child, so give it more common name.
Drop assertions and drop bs argument which becomes unused. Function
would be reused in a further commit.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
block.c | 22 ++++++++--------------
1 file changed, 8 insertions(+), 14 deletions(-)
diff --git a/block.c b/block.c
index 0fddb57e78..d852daf7c8 100644
--- a/block.c
+++ b/block.c
@@ -89,9 +89,7 @@ static bool bdrv_recurse_has_child(BlockDriverState *bs,
static void bdrv_replace_child_noperm(BdrvChild *child,
BlockDriverState *new_bs);
-static void bdrv_remove_file_or_backing_child(BlockDriverState *bs,
- BdrvChild *child,
- Transaction *tran);
+static void bdrv_remove_child(BdrvChild *child, Transaction *tran);
static void bdrv_remove_filter_or_cow_child(BlockDriverState *bs,
Transaction *tran);
@@ -3236,7 +3234,7 @@ static int bdrv_set_file_or_backing_noperm(BlockDriverState *parent_bs,
if (child) {
bdrv_unset_inherits_from(parent_bs, child, tran);
- bdrv_remove_file_or_backing_child(parent_bs, child, tran);
+ bdrv_remove_child(child, tran);
}
if (!child_bs) {
@@ -4891,25 +4889,21 @@ static bool should_update_child(BdrvChild *c, BlockDriverState *to)
return ret;
}
-static void bdrv_remove_filter_or_cow_child_commit(void *opaque)
+static void bdrv_remove_child_commit(void *opaque)
{
bdrv_child_free(opaque);
}
-static TransactionActionDrv bdrv_remove_filter_or_cow_child_drv = {
- .commit = bdrv_remove_filter_or_cow_child_commit,
+static TransactionActionDrv bdrv_remove_child_drv = {
+ .commit = bdrv_remove_child_commit,
};
/*
* A function to remove backing or file child of @bs.
* Function doesn't update permissions, caller is responsible for this.
*/
-static void bdrv_remove_file_or_backing_child(BlockDriverState *bs,
- BdrvChild *child,
- Transaction *tran)
+static void bdrv_remove_child(BdrvChild *child, Transaction *tran)
{
- assert(child == bs->backing || child == bs->file);
-
if (!child) {
return;
}
@@ -4918,7 +4912,7 @@ static void bdrv_remove_file_or_backing_child(BlockDriverState *bs,
bdrv_replace_child_tran(child, NULL, tran);
}
- tran_add(tran, &bdrv_remove_filter_or_cow_child_drv, child);
+ tran_add(tran, &bdrv_remove_child_drv, child);
}
/*
@@ -4929,7 +4923,7 @@ static void bdrv_remove_file_or_backing_child(BlockDriverState *bs,
static void bdrv_remove_filter_or_cow_child(BlockDriverState *bs,
Transaction *tran)
{
- bdrv_remove_file_or_backing_child(bs, bdrv_filter_or_cow_child(bs), tran);
+ bdrv_remove_child(bdrv_filter_or_cow_child(bs), tran);
}
static int bdrv_replace_node_noperm(BlockDriverState *from,
--
2.31.1
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH 02/14] block: drop bdrv_detach_child()
2022-02-07 16:37 [PATCH 00/14] block: blockdev-del force=false Vladimir Sementsov-Ogievskiy
2022-02-07 16:37 ` [PATCH 01/14] block: refactor bdrv_remove_file_or_backing_child to bdrv_remove_child Vladimir Sementsov-Ogievskiy
@ 2022-02-07 16:37 ` Vladimir Sementsov-Ogievskiy
2022-02-07 16:37 ` [PATCH 03/14] block: bdrv_refresh_perms(): allow external tran Vladimir Sementsov-Ogievskiy
` (11 subsequent siblings)
13 siblings, 0 replies; 15+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2022-02-07 16:37 UTC (permalink / raw)
To: qemu-block
Cc: qemu-devel, armbru, jsnow, vsementsov, eblake, hreitz, kwolf, den,
ktkhai, igor
The only caller is bdrv_root_unref_child(), let's just do the logic
directly in it. It simplifies further convertion of
bdrv_root_unref_child() to transaction action.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
block.c | 45 +++++++++++++++++++--------------------------
1 file changed, 19 insertions(+), 26 deletions(-)
diff --git a/block.c b/block.c
index d852daf7c8..378841a546 100644
--- a/block.c
+++ b/block.c
@@ -2948,29 +2948,6 @@ static int bdrv_attach_child_noperm(BlockDriverState *parent_bs,
return 0;
}
-static void bdrv_detach_child(BdrvChild *child)
-{
- BlockDriverState *old_bs = child->bs;
-
- bdrv_replace_child_noperm(child, NULL);
- bdrv_child_free(child);
-
- if (old_bs) {
- /*
- * Update permissions for old node. We're just taking a parent away, so
- * we're loosening restrictions. Errors of permission update are not
- * fatal in this case, ignore them.
- */
- bdrv_refresh_perms(old_bs, NULL);
-
- /*
- * When the parent requiring a non-default AioContext is removed, the
- * node moves back to the main AioContext
- */
- bdrv_try_set_aio_context(old_bs, qemu_get_aio_context(), NULL);
- }
-}
-
/*
* This function steals the reference to child_bs from the caller.
* That reference is later dropped by bdrv_root_unref_child().
@@ -3053,10 +3030,26 @@ out:
/* Callers must ensure that child->frozen is false. */
void bdrv_root_unref_child(BdrvChild *child)
{
- BlockDriverState *child_bs;
+ BlockDriverState *child_bs = child->bs;
+
+ bdrv_replace_child_noperm(child, NULL);
+ bdrv_child_free(child);
+
+ if (child_bs) {
+ /*
+ * Update permissions for old node. We're just taking a parent away, so
+ * we're loosening restrictions. Errors of permission update are not
+ * fatal in this case, ignore them.
+ */
+ bdrv_refresh_perms(child_bs, NULL);
+
+ /*
+ * When the parent requiring a non-default AioContext is removed, the
+ * node moves back to the main AioContext
+ */
+ bdrv_try_set_aio_context(child_bs, qemu_get_aio_context(), NULL);
+ }
- child_bs = child->bs;
- bdrv_detach_child(child);
bdrv_unref(child_bs);
}
--
2.31.1
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH 03/14] block: bdrv_refresh_perms(): allow external tran
2022-02-07 16:37 [PATCH 00/14] block: blockdev-del force=false Vladimir Sementsov-Ogievskiy
2022-02-07 16:37 ` [PATCH 01/14] block: refactor bdrv_remove_file_or_backing_child to bdrv_remove_child Vladimir Sementsov-Ogievskiy
2022-02-07 16:37 ` [PATCH 02/14] block: drop bdrv_detach_child() Vladimir Sementsov-Ogievskiy
@ 2022-02-07 16:37 ` Vladimir Sementsov-Ogievskiy
2022-02-07 16:37 ` [PATCH 04/14] block: add bdrv_try_set_aio_context_tran transaction action Vladimir Sementsov-Ogievskiy
` (10 subsequent siblings)
13 siblings, 0 replies; 15+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2022-02-07 16:37 UTC (permalink / raw)
To: qemu-block
Cc: qemu-devel, armbru, jsnow, vsementsov, eblake, hreitz, kwolf, den,
ktkhai, igor
Allow passing external Transaction pointer, stop creating extra
Transaction objects.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
block.c | 31 ++++++++++++++++++++-----------
1 file changed, 20 insertions(+), 11 deletions(-)
diff --git a/block.c b/block.c
index 378841a546..7b65c7c5c0 100644
--- a/block.c
+++ b/block.c
@@ -2475,14 +2475,23 @@ char *bdrv_perm_names(uint64_t perm)
}
-static int bdrv_refresh_perms(BlockDriverState *bs, Error **errp)
+/* @tran is allowed to be NULL. In this case no rollback is possible */
+static int bdrv_refresh_perms(BlockDriverState *bs, Transaction *tran,
+ Error **errp)
{
int ret;
- Transaction *tran = tran_new();
+ Transaction *local_tran = NULL;
g_autoptr(GSList) list = bdrv_topological_dfs(NULL, NULL, bs);
+ if (!tran) {
+ tran = local_tran = tran_new();
+ }
+
ret = bdrv_list_refresh_perms(list, NULL, tran, errp);
- tran_finalize(tran, ret);
+
+ if (local_tran) {
+ tran_finalize(local_tran, ret);
+ }
return ret;
}
@@ -2496,7 +2505,7 @@ int bdrv_child_try_set_perm(BdrvChild *c, uint64_t perm, uint64_t shared,
bdrv_child_set_perm(c, perm, shared, tran);
- ret = bdrv_refresh_perms(c->bs, &local_err);
+ ret = bdrv_refresh_perms(c->bs, tran, &local_err);
tran_finalize(tran, ret);
@@ -2976,7 +2985,7 @@ BdrvChild *bdrv_root_attach_child(BlockDriverState *child_bs,
goto out;
}
- ret = bdrv_refresh_perms(child_bs, errp);
+ ret = bdrv_refresh_perms(child_bs, tran, errp);
out:
tran_finalize(tran, ret);
@@ -3014,7 +3023,7 @@ BdrvChild *bdrv_attach_child(BlockDriverState *parent_bs,
goto out;
}
- ret = bdrv_refresh_perms(parent_bs, errp);
+ ret = bdrv_refresh_perms(parent_bs, tran, errp);
if (ret < 0) {
goto out;
}
@@ -3041,7 +3050,7 @@ void bdrv_root_unref_child(BdrvChild *child)
* we're loosening restrictions. Errors of permission update are not
* fatal in this case, ignore them.
*/
- bdrv_refresh_perms(child_bs, NULL);
+ bdrv_refresh_perms(child_bs, NULL, NULL);
/*
* When the parent requiring a non-default AioContext is removed, the
@@ -3277,7 +3286,7 @@ int bdrv_set_backing_hd(BlockDriverState *bs, BlockDriverState *backing_hd,
goto out;
}
- ret = bdrv_refresh_perms(bs, errp);
+ ret = bdrv_refresh_perms(bs, tran, errp);
out:
tran_finalize(tran, ret);
@@ -5067,7 +5076,7 @@ int bdrv_append(BlockDriverState *bs_new, BlockDriverState *bs_top,
goto out;
}
- ret = bdrv_refresh_perms(bs_new, errp);
+ ret = bdrv_refresh_perms(bs_new, tran, errp);
out:
tran_finalize(tran, ret);
@@ -6297,7 +6306,7 @@ int coroutine_fn bdrv_co_invalidate_cache(BlockDriverState *bs, Error **errp)
*/
if (bs->open_flags & BDRV_O_INACTIVE) {
bs->open_flags &= ~BDRV_O_INACTIVE;
- ret = bdrv_refresh_perms(bs, errp);
+ ret = bdrv_refresh_perms(bs, NULL, errp);
if (ret < 0) {
bs->open_flags |= BDRV_O_INACTIVE;
return ret;
@@ -6422,7 +6431,7 @@ static int bdrv_inactivate_recurse(BlockDriverState *bs)
* We only tried to loosen restrictions, so errors are not fatal, ignore
* them.
*/
- bdrv_refresh_perms(bs, NULL);
+ bdrv_refresh_perms(bs, NULL, NULL);
/* Recursively inactivate children */
QLIST_FOREACH(child, &bs->children, next) {
--
2.31.1
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH 04/14] block: add bdrv_try_set_aio_context_tran transaction action
2022-02-07 16:37 [PATCH 00/14] block: blockdev-del force=false Vladimir Sementsov-Ogievskiy
` (2 preceding siblings ...)
2022-02-07 16:37 ` [PATCH 03/14] block: bdrv_refresh_perms(): allow external tran Vladimir Sementsov-Ogievskiy
@ 2022-02-07 16:37 ` Vladimir Sementsov-Ogievskiy
2022-02-07 16:37 ` [PATCH 05/14] block: merge bdrv_delete and bdrv_close Vladimir Sementsov-Ogievskiy
` (9 subsequent siblings)
13 siblings, 0 replies; 15+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2022-02-07 16:37 UTC (permalink / raw)
To: qemu-block
Cc: qemu-devel, armbru, jsnow, vsementsov, eblake, hreitz, kwolf, den,
ktkhai, igor
To be used in further commit.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
block.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 48 insertions(+)
diff --git a/block.c b/block.c
index 7b65c7c5c0..82fbf81a3c 100644
--- a/block.c
+++ b/block.c
@@ -2791,6 +2791,54 @@ static void bdrv_child_free(BdrvChild *child)
g_free(child);
}
+typedef struct BdrvTrySetAioContextState {
+ BlockDriverState *bs;
+ AioContext *old_ctx;
+} BdrvTrySetAioContextState;
+
+static void bdrv_try_set_aio_context_abort(void *opaque)
+{
+ BdrvTrySetAioContextState *s = opaque;
+
+ if (bdrv_get_aio_context(s->bs) != s->old_ctx) {
+ bdrv_try_set_aio_context(s->bs, s->old_ctx, &error_abort);
+ }
+}
+
+static TransactionActionDrv bdrv_try_set_aio_context_drv = {
+ .abort = bdrv_try_set_aio_context_abort,
+ .clean = g_free,
+};
+
+__attribute__((unused))
+static int bdrv_try_set_aio_context_tran(BlockDriverState *bs,
+ AioContext *new_ctx,
+ Transaction *tran,
+ Error **errp)
+{
+ AioContext *old_ctx = bdrv_get_aio_context(bs);
+ BdrvTrySetAioContextState *s;
+ int ret;
+
+ if (old_ctx == new_ctx) {
+ return 0;
+ }
+
+ ret = bdrv_try_set_aio_context(bs, new_ctx, errp);
+ if (ret < 0) {
+ return ret;
+ }
+
+ s = g_new(BdrvTrySetAioContextState, 1);
+ *s = (BdrvTrySetAioContextState) {
+ .bs = bs,
+ .old_ctx = old_ctx,
+ };
+ tran_add(tran, &bdrv_try_set_aio_context_drv, s);
+
+ return 0;
+}
+
typedef struct BdrvAttachChildCommonState {
BdrvChild *child;
AioContext *old_parent_ctx;
--
2.31.1
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH 05/14] block: merge bdrv_delete and bdrv_close
2022-02-07 16:37 [PATCH 00/14] block: blockdev-del force=false Vladimir Sementsov-Ogievskiy
` (3 preceding siblings ...)
2022-02-07 16:37 ` [PATCH 04/14] block: add bdrv_try_set_aio_context_tran transaction action Vladimir Sementsov-Ogievskiy
@ 2022-02-07 16:37 ` Vladimir Sementsov-Ogievskiy
2022-02-07 16:37 ` [PATCH 06/14] block: bdrv_delete(): drop unnecessary zeroing Vladimir Sementsov-Ogievskiy
` (8 subsequent siblings)
13 siblings, 0 replies; 15+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2022-02-07 16:37 UTC (permalink / raw)
To: qemu-block
Cc: qemu-devel, armbru, jsnow, vsementsov, eblake, hreitz, kwolf, den,
ktkhai, igor
bdrv_delete() is the only caller of bdrv_close(). Let's merge them to
simplify further commits.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
block.c | 27 ++++++++++-----------------
1 file changed, 10 insertions(+), 17 deletions(-)
diff --git a/block.c b/block.c
index 82fbf81a3c..71a5aec24c 100644
--- a/block.c
+++ b/block.c
@@ -4785,12 +4785,19 @@ static void bdrv_reopen_abort(BDRVReopenState *reopen_state)
}
-static void bdrv_close(BlockDriverState *bs)
+static void bdrv_delete(BlockDriverState *bs)
{
BdrvAioNotifier *ban, *ban_next;
BdrvChild *child, *next;
assert(!bs->refcnt);
+ assert(bdrv_op_blocker_is_empty(bs));
+
+ /* remove from list, if necessary */
+ if (bs->node_name[0] != '\0') {
+ QTAILQ_REMOVE(&graph_bdrv_states, bs, node_list);
+ }
+ QTAILQ_REMOVE(&all_bdrv_states, bs, bs_list);
bdrv_drained_begin(bs); /* complete I/O */
bdrv_flush(bs);
@@ -4844,6 +4851,8 @@ static void bdrv_close(BlockDriverState *bs)
if (bs->quiesce_counter) {
bdrv_drain_all_end_quiesce(bs);
}
+
+ g_free(bs);
}
void bdrv_close_all(void)
@@ -5164,22 +5173,6 @@ int bdrv_replace_child_bs(BdrvChild *child, BlockDriverState *new_bs,
return ret;
}
-static void bdrv_delete(BlockDriverState *bs)
-{
- assert(bdrv_op_blocker_is_empty(bs));
- assert(!bs->refcnt);
-
- /* remove from list, if necessary */
- if (bs->node_name[0] != '\0') {
- QTAILQ_REMOVE(&graph_bdrv_states, bs, node_list);
- }
- QTAILQ_REMOVE(&all_bdrv_states, bs, bs_list);
-
- bdrv_close(bs);
-
- g_free(bs);
-}
-
/*
* Replace @bs by newly created block node.
--
2.31.1
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH 06/14] block: bdrv_delete(): drop unnecessary zeroing
2022-02-07 16:37 [PATCH 00/14] block: blockdev-del force=false Vladimir Sementsov-Ogievskiy
` (4 preceding siblings ...)
2022-02-07 16:37 ` [PATCH 05/14] block: merge bdrv_delete and bdrv_close Vladimir Sementsov-Ogievskiy
@ 2022-02-07 16:37 ` Vladimir Sementsov-Ogievskiy
2022-02-07 16:37 ` [PATCH 07/14] block: implemet bdrv_try_unref() Vladimir Sementsov-Ogievskiy
` (7 subsequent siblings)
13 siblings, 0 replies; 15+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2022-02-07 16:37 UTC (permalink / raw)
To: qemu-block
Cc: qemu-devel, armbru, jsnow, vsementsov, eblake, hreitz, kwolf, den,
ktkhai, igor
No need to zero all these things before g_free(bs). Move memory freeing
to the end of the function to simplify further conversion to
transaction action.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
block.c | 40 ++++++++++++++--------------------------
1 file changed, 14 insertions(+), 26 deletions(-)
diff --git a/block.c b/block.c
index 71a5aec24c..231d1fc3ea 100644
--- a/block.c
+++ b/block.c
@@ -4815,32 +4815,6 @@ static void bdrv_delete(BlockDriverState *bs)
bdrv_unref_child(bs, child);
}
- assert(!bs->backing);
- assert(!bs->file);
- g_free(bs->opaque);
- bs->opaque = NULL;
- qatomic_set(&bs->copy_on_read, 0);
- bs->backing_file[0] = '\0';
- bs->backing_format[0] = '\0';
- bs->total_sectors = 0;
- bs->encrypted = false;
- bs->sg = false;
- qobject_unref(bs->options);
- qobject_unref(bs->explicit_options);
- bs->options = NULL;
- bs->explicit_options = NULL;
- qobject_unref(bs->full_open_options);
- bs->full_open_options = NULL;
- g_free(bs->block_status_cache);
- bs->block_status_cache = NULL;
-
- bdrv_release_named_dirty_bitmaps(bs);
- assert(QLIST_EMPTY(&bs->dirty_bitmaps));
-
- QLIST_FOREACH_SAFE(ban, &bs->aio_notifiers, list, ban_next) {
- g_free(ban);
- }
- QLIST_INIT(&bs->aio_notifiers);
bdrv_drained_end(bs);
/*
@@ -4852,6 +4826,20 @@ static void bdrv_delete(BlockDriverState *bs)
bdrv_drain_all_end_quiesce(bs);
}
+ /* Free memory */
+ g_free(bs->opaque);
+ qobject_unref(bs->options);
+ qobject_unref(bs->explicit_options);
+ qobject_unref(bs->full_open_options);
+ g_free(bs->block_status_cache);
+
+ bdrv_release_named_dirty_bitmaps(bs);
+ assert(QLIST_EMPTY(&bs->dirty_bitmaps));
+
+ QLIST_FOREACH_SAFE(ban, &bs->aio_notifiers, list, ban_next) {
+ g_free(ban);
+ }
+
g_free(bs);
}
--
2.31.1
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH 07/14] block: implemet bdrv_try_unref()
2022-02-07 16:37 [PATCH 00/14] block: blockdev-del force=false Vladimir Sementsov-Ogievskiy
` (5 preceding siblings ...)
2022-02-07 16:37 ` [PATCH 06/14] block: bdrv_delete(): drop unnecessary zeroing Vladimir Sementsov-Ogievskiy
@ 2022-02-07 16:37 ` Vladimir Sementsov-Ogievskiy
2022-02-07 16:37 ` [PATCH 08/14] qapi/block-core: add 'force' argument to blockdev-del Vladimir Sementsov-Ogievskiy
` (6 subsequent siblings)
13 siblings, 0 replies; 15+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2022-02-07 16:37 UTC (permalink / raw)
To: qemu-block
Cc: qemu-devel, armbru, jsnow, vsementsov, eblake, hreitz, kwolf, den,
ktkhai, igor
Make a version of bdrv_unref() that honestly report any failure.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
include/block/block.h | 1 +
include/block/block_int.h | 2 +
block.c | 247 +++++++++++++++++++++++++++++++-------
3 files changed, 208 insertions(+), 42 deletions(-)
diff --git a/include/block/block.h b/include/block/block.h
index 768273b2db..42d78a7a31 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -672,6 +672,7 @@ void bdrv_disable_copy_on_read(BlockDriverState *bs);
void bdrv_ref(BlockDriverState *bs);
void bdrv_unref(BlockDriverState *bs);
void bdrv_unref_child(BlockDriverState *parent, BdrvChild *child);
+int bdrv_try_unref(BlockDriverState *bs, Error **errp);
BdrvChild *bdrv_attach_child(BlockDriverState *parent_bs,
BlockDriverState *child_bs,
const char *child_name,
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 767825aec4..3126868633 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -188,6 +188,8 @@ struct BlockDriver {
int (*bdrv_file_open)(BlockDriverState *bs, QDict *options, int flags,
Error **errp);
void (*bdrv_close)(BlockDriverState *bs);
+ int (*bdrv_close_safe)(BlockDriverState *bs, Transaction *tran,
+ Error **errp);
int coroutine_fn (*bdrv_co_create)(BlockdevCreateOptions *opts,
diff --git a/block.c b/block.c
index 231d1fc3ea..187732c6f8 100644
--- a/block.c
+++ b/block.c
@@ -101,6 +101,9 @@ static void bdrv_reopen_abort(BDRVReopenState *reopen_state);
static bool bdrv_backing_overridden(BlockDriverState *bs);
+static int bdrv_unref_safe(BlockDriverState *bs, Transaction *tran,
+ Error **errp);
+
/* If non-zero, use only whitelisted block drivers */
static int use_bdrv_whitelist;
@@ -3084,30 +3087,60 @@ out:
return ret < 0 ? NULL : child;
}
-/* Callers must ensure that child->frozen is false. */
-void bdrv_root_unref_child(BdrvChild *child)
+/*
+ * When @tran is NULL, function never fail and returns 0. Still, some states
+ * may not be saved correctly.
+ *
+ * When @tran is not NULL, first failure is returned and the action may be
+ * rolled back.
+ */
+static int bdrv_root_unref_child_safe(BdrvChild *child, Transaction *tran,
+ Error **errp)
{
+ int ret;
BlockDriverState *child_bs = child->bs;
- bdrv_replace_child_noperm(child, NULL);
- bdrv_child_free(child);
+ if (tran) {
+ bdrv_remove_child(child, tran);
+ } else {
+ bdrv_replace_child_noperm(child, NULL);
+ bdrv_child_free(child);
+ }
if (child_bs) {
/*
* Update permissions for old node. We're just taking a parent away, so
* we're loosening restrictions. Errors of permission update are not
- * fatal in this case, ignore them.
+ * fatal in this case, ignore them when tran is NULL.
*/
- bdrv_refresh_perms(child_bs, NULL, NULL);
+ ret = bdrv_refresh_perms(child_bs, tran, tran ? errp : NULL);
+ if (tran && ret < 0) {
+ return ret;
+ }
/*
* When the parent requiring a non-default AioContext is removed, the
* node moves back to the main AioContext
*/
- bdrv_try_set_aio_context(child_bs, qemu_get_aio_context(), NULL);
+ if (tran) {
+ ret = bdrv_try_set_aio_context_tran(child_bs,
+ qemu_get_aio_context(),
+ tran, errp);
+ if (ret < 0) {
+ return ret;
+ }
+ } else {
+ bdrv_try_set_aio_context(child_bs, qemu_get_aio_context(), NULL);
+ }
}
- bdrv_unref(child_bs);
+ return bdrv_unref_safe(child_bs, tran, errp);
+}
+
+/* Callers must ensure that child->frozen is false. */
+void bdrv_root_unref_child(BdrvChild *child)
+{
+ bdrv_root_unref_child_safe(child, NULL, &error_abort);
}
typedef struct BdrvSetInheritsFrom {
@@ -3176,15 +3209,28 @@ static void bdrv_unset_inherits_from(BlockDriverState *root, BdrvChild *child,
}
}
+/*
+ * When @tran is NULL, function never fail and returns 0. Still, some states
+ * may not be saved correctly.
+ *
+ * When @tran is not NULL, first failure is returned and the action may be
+ * rolled back.
+ */
+static int bdrv_unref_child_safe(BlockDriverState *parent, BdrvChild *child,
+ Transaction *tran, Error **errp)
+{
+ if (child == NULL) {
+ return 0;
+ }
+
+ bdrv_unset_inherits_from(parent, child, tran);
+ return bdrv_root_unref_child_safe(child, tran, errp);
+}
+
/* Callers must ensure that child->frozen is false. */
void bdrv_unref_child(BlockDriverState *parent, BdrvChild *child)
{
- if (child == NULL) {
- return;
- }
-
- bdrv_unset_inherits_from(parent, child, NULL);
- bdrv_root_unref_child(child);
+ bdrv_unref_child_safe(parent, child, NULL, &error_abort);
}
@@ -4784,14 +4830,17 @@ static void bdrv_reopen_abort(BDRVReopenState *reopen_state)
}
}
+static void bdrv_delete_abort(void *opaque)
+{
+ BlockDriverState *bs = opaque;
-static void bdrv_delete(BlockDriverState *bs)
+ bdrv_drained_end(bs);
+}
+
+static void bdrv_delete_commit(void *opaque)
{
BdrvAioNotifier *ban, *ban_next;
- BdrvChild *child, *next;
-
- assert(!bs->refcnt);
- assert(bdrv_op_blocker_is_empty(bs));
+ BlockDriverState *bs = opaque;
/* remove from list, if necessary */
if (bs->node_name[0] != '\0') {
@@ -4799,22 +4848,6 @@ static void bdrv_delete(BlockDriverState *bs)
}
QTAILQ_REMOVE(&all_bdrv_states, bs, bs_list);
- bdrv_drained_begin(bs); /* complete I/O */
- bdrv_flush(bs);
- bdrv_drain(bs); /* in case flush left pending I/O */
-
- if (bs->drv) {
- if (bs->drv->bdrv_close) {
- /* Must unfreeze all children, so bdrv_unref_child() works */
- bs->drv->bdrv_close(bs);
- }
- bs->drv = NULL;
- }
-
- QLIST_FOREACH_SAFE(child, &bs->children, next, next) {
- bdrv_unref_child(bs, child);
- }
-
bdrv_drained_end(bs);
/*
@@ -4843,6 +4876,88 @@ static void bdrv_delete(BlockDriverState *bs)
g_free(bs);
}
+static TransactionActionDrv bdrv_delete_drv = {
+ .commit = bdrv_delete_commit,
+ .abort = bdrv_delete_abort,
+};
+
+/*
+ * When @tran is NULL, function never fail and returns 0. Still, some states
+ * may not be saved correctly.
+ *
+ * When @tran is not NULL, first failure is returned and the action may be
+ * rolled back.
+ */
+static int bdrv_delete(BlockDriverState *bs, Transaction *tran, Error **errp)
+{
+ int ret;
+ BdrvChild *child, *next;
+
+ assert(!bs->refcnt);
+ assert(bdrv_op_blocker_is_empty(bs));
+
+ assert(!(bs->drv && bs->drv->bdrv_close_safe && bs->drv->bdrv_close));
+
+ if (tran && bs->drv && bs->drv->bdrv_close) {
+ /* .bdrv_close() is unsafe handler */
+ error_setg(errp, "Node '%s'(%s) doesn't support safe removing",
+ bdrv_get_node_name(bs), bdrv_get_format_name(bs));
+ return -EINVAL;
+ }
+
+ if (tran && !bs->drv) {
+ /* Node without driver is a sign of something wrong */
+ error_setg(errp, "Node '%s' is broken", bdrv_get_node_name(bs));
+ return -EINVAL;
+ }
+
+ /* complete I/O */
+ bdrv_drained_begin(bs);
+ if (tran) {
+ /* Add it now, as we want bdrv_drained_end() on abort */
+ tran_add(tran, &bdrv_delete_drv, bs);
+ }
+
+ ret = bdrv_flush(bs);
+ if (ret < 0 && tran) {
+ error_setg(errp, "Failed to flush node '%s'", bdrv_get_node_name(bs));
+ return ret;
+ }
+
+ bdrv_drain(bs); /* in case flush left pending I/O */
+
+ /*
+ * .bdrv_close[_safe] Must unfreeze all children, so bdrv_unref_child()
+ * works.
+ */
+ if (bs->drv) {
+ if (bs->drv->bdrv_close) {
+ assert(!tran);
+ bs->drv->bdrv_close(bs);
+ } else if (bs->drv->bdrv_close_safe) {
+ ret = bs->drv->bdrv_close_safe(bs, tran, errp);
+ if (ret < 0) {
+ assert(tran);
+ return ret;
+ }
+ }
+ }
+
+ QLIST_FOREACH_SAFE(child, &bs->children, next, next) {
+ ret = bdrv_unref_child_safe(bs, child, tran, errp);
+ if (ret < 0) {
+ assert(tran);
+ return ret;
+ }
+ }
+
+ if (!tran) {
+ bdrv_delete_commit(bs);
+ }
+
+ return 0;
+}
+
void bdrv_close_all(void)
{
assert(job_next(NULL) == NULL);
@@ -6571,18 +6686,66 @@ void bdrv_ref(BlockDriverState *bs)
bs->refcnt++;
}
+static void bdrv_unref_safe_abort(void *opaque)
+{
+ bdrv_ref(opaque);
+}
+
+static TransactionActionDrv bdrv_unref_safe_drv = {
+ .abort = bdrv_unref_safe_abort,
+};
+
+/*
+ * When @tran is NULL, function never fail and returns 0. Still, some states
+ * may not be saved correctly.
+ *
+ * When @tran is not NULL, first failure is returned and the action may be
+ * rolled back.
+ */
+static int bdrv_unref_safe(BlockDriverState *bs, Transaction *tran,
+ Error **errp)
+{
+ if (!bs) {
+ return 0;
+ }
+
+ assert(bs->refcnt > 0);
+
+ if (tran) {
+ tran_add(tran, &bdrv_unref_safe_drv, bs);
+ }
+
+ if (--bs->refcnt == 0) {
+ return bdrv_delete(bs, tran, errp);
+ }
+
+ return 0;
+}
+
/* Release a previously grabbed reference to bs.
* If after releasing, reference count is zero, the BlockDriverState is
* deleted. */
void bdrv_unref(BlockDriverState *bs)
{
- if (!bs) {
- return;
- }
- assert(bs->refcnt > 0);
- if (--bs->refcnt == 0) {
- bdrv_delete(bs);
- }
+ bdrv_unref_safe(bs, NULL, &error_abort);
+}
+
+/*
+ * Like bdrv_unref(), but don't ignore errors:
+ * On success, if node (nodes) were removed, it's guaranteed that all states
+ * are stored correctly (for example, metadata caches, persistent dirty
+ * bitmaps).
+ * On failure every change is rolled back, node is not unref'ed.
+ */
+int bdrv_try_unref(BlockDriverState *bs, Error **errp)
+{
+ int ret;
+ Transaction *tran = tran_new();
+
+ ret = bdrv_unref_safe(bs, tran, errp);
+ tran_finalize(tran, ret);
+
+ return ret;
}
struct BdrvOpBlocker {
--
2.31.1
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH 08/14] qapi/block-core: add 'force' argument to blockdev-del
2022-02-07 16:37 [PATCH 00/14] block: blockdev-del force=false Vladimir Sementsov-Ogievskiy
` (6 preceding siblings ...)
2022-02-07 16:37 ` [PATCH 07/14] block: implemet bdrv_try_unref() Vladimir Sementsov-Ogievskiy
@ 2022-02-07 16:37 ` Vladimir Sementsov-Ogievskiy
2022-02-07 16:37 ` [PATCH 09/14] qcow2: qcow2_inactivate(): use qcow2_flush_caches() Vladimir Sementsov-Ogievskiy
` (5 subsequent siblings)
13 siblings, 0 replies; 15+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2022-02-07 16:37 UTC (permalink / raw)
To: qemu-block
Cc: qemu-devel, armbru, jsnow, vsementsov, eblake, hreitz, kwolf, den,
ktkhai, igor
Default behavior is force=true and it's unchanged. New behavior is
force=false, which makes it possible to be sure that node removal is
done successfully with no error and all metadata is stored and flushed
successfully.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
qapi/block-core.json | 6 +++++-
block/monitor/block-hmp-cmds.c | 2 +-
blockdev.c | 17 +++++++++++++++--
hw/block/xen-block.c | 2 +-
4 files changed, 22 insertions(+), 5 deletions(-)
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 9a5a3641d0..b37d195772 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -4403,6 +4403,9 @@
#
# @node-name: Name of the graph node to delete.
#
+# @force: Ignore failures when closing block-nodes, like failed IO
+# when try to store metadata. Default true. (Since 7.0)
+#
# Since: 2.9
#
# Example:
@@ -4425,7 +4428,8 @@
# <- { "return": {} }
#
##
-{ 'command': 'blockdev-del', 'data': { 'node-name': 'str' } }
+{ 'command': 'blockdev-del',
+ 'data': { 'node-name': 'str', '*force': 'bool' } }
##
# @BlockdevCreateOptionsFile:
diff --git a/block/monitor/block-hmp-cmds.c b/block/monitor/block-hmp-cmds.c
index bfb3c043a0..1c35aa2d6f 100644
--- a/block/monitor/block-hmp-cmds.c
+++ b/block/monitor/block-hmp-cmds.c
@@ -145,7 +145,7 @@ void hmp_drive_del(Monitor *mon, const QDict *qdict)
bs = bdrv_find_node(id);
if (bs) {
- qmp_blockdev_del(id, &local_err);
+ qmp_blockdev_del(id, true, true, &local_err);
if (local_err) {
error_report_err(local_err);
}
diff --git a/blockdev.c b/blockdev.c
index 8197165bb5..34a195b592 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -3582,11 +3582,17 @@ fail:
g_slist_free_full(drained, (GDestroyNotify) bdrv_subtree_drained_end);
}
-void qmp_blockdev_del(const char *node_name, Error **errp)
+void qmp_blockdev_del(const char *node_name, bool has_force,
+ bool force, Error **errp)
{
AioContext *aio_context;
BlockDriverState *bs;
+ if (!has_force) {
+ /* Historical default is force remove */
+ force = true;
+ }
+
bs = bdrv_find_node(node_name);
if (!bs) {
error_setg(errp, "Failed to find node with node-name='%s'", node_name);
@@ -3616,7 +3622,14 @@ void qmp_blockdev_del(const char *node_name, Error **errp)
}
QTAILQ_REMOVE(&monitor_bdrv_states, bs, monitor_list);
- bdrv_unref(bs);
+ if (force) {
+ bdrv_unref(bs);
+ } else {
+ int ret = bdrv_try_unref(bs, errp);
+ if (ret < 0) {
+ bdrv_set_monitor_owned(bs);
+ }
+ }
out:
aio_context_release(aio_context);
diff --git a/hw/block/xen-block.c b/hw/block/xen-block.c
index 674953f1ad..0ac9b599c0 100644
--- a/hw/block/xen-block.c
+++ b/hw/block/xen-block.c
@@ -649,7 +649,7 @@ static void xen_block_blockdev_del(const char *node_name, Error **errp)
{
trace_xen_block_blockdev_del(node_name);
- qmp_blockdev_del(node_name, errp);
+ qmp_blockdev_del(node_name, true, true, errp);
}
static char *xen_block_blockdev_add(const char *id, QDict *qdict,
--
2.31.1
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH 09/14] qcow2: qcow2_inactivate(): use qcow2_flush_caches()
2022-02-07 16:37 [PATCH 00/14] block: blockdev-del force=false Vladimir Sementsov-Ogievskiy
` (7 preceding siblings ...)
2022-02-07 16:37 ` [PATCH 08/14] qapi/block-core: add 'force' argument to blockdev-del Vladimir Sementsov-Ogievskiy
@ 2022-02-07 16:37 ` Vladimir Sementsov-Ogievskiy
2022-02-07 16:37 ` [PATCH 10/14] qcow2: qcow2_inactivate(): don't call qcow2_mark_clean() when RO Vladimir Sementsov-Ogievskiy
` (4 subsequent siblings)
13 siblings, 0 replies; 15+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2022-02-07 16:37 UTC (permalink / raw)
To: qemu-block
Cc: qemu-devel, armbru, jsnow, vsementsov, eblake, hreitz, kwolf, den,
ktkhai, igor
Maintaining similar logic of flushing caches in both
qcow2_inactivate() and in qcow2_flush_caches() is not good.
Let's refactor things to use qcow2_flush_caches() directly from
qcow2_inactivate(). For this we need two things: textual error messages
(add Error **) and possibility to unconditionally flush refcounts (add
force_refcounts argument).
Iotests output updated correspondingly.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
block/qcow2.h | 6 +-
block/qcow2-bitmap.c | 2 +-
block/qcow2-refcount.c | 22 +++++--
block/qcow2.c | 25 +++-----
tests/qemu-iotests/026.out | 126 +++++++++++++------------------------
tests/qemu-iotests/071.out | 3 +-
tests/qemu-iotests/080.out | 4 +-
7 files changed, 76 insertions(+), 112 deletions(-)
diff --git a/block/qcow2.h b/block/qcow2.h
index fd48a89d45..a83183d533 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -874,8 +874,10 @@ void qcow2_free_any_cluster(BlockDriverState *bs, uint64_t l2_entry,
int qcow2_update_snapshot_refcount(BlockDriverState *bs,
int64_t l1_table_offset, int l1_size, int addend);
-int coroutine_fn qcow2_flush_caches(BlockDriverState *bs);
-int coroutine_fn qcow2_write_caches(BlockDriverState *bs);
+int coroutine_fn qcow2_flush_caches(BlockDriverState *bs, bool force_refcounts,
+ Error **errp);
+int coroutine_fn qcow2_write_caches(BlockDriverState *bs, bool force_refcounts,
+ Error **errp);
int qcow2_check_refcounts(BlockDriverState *bs, BdrvCheckResult *res,
BdrvCheckMode fix);
diff --git a/block/qcow2-bitmap.c b/block/qcow2-bitmap.c
index 8fb4731551..0f463ced4e 100644
--- a/block/qcow2-bitmap.c
+++ b/block/qcow2-bitmap.c
@@ -899,7 +899,7 @@ static int update_ext_header_and_dir(BlockDriverState *bs,
return ret;
}
- ret = qcow2_flush_caches(bs);
+ ret = qcow2_flush_caches(bs, false, NULL);
if (ret < 0) {
goto fail;
}
diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
index 4614572252..c8251412ce 100644
--- a/block/qcow2-refcount.c
+++ b/block/qcow2-refcount.c
@@ -1203,19 +1203,23 @@ void qcow2_free_any_cluster(BlockDriverState *bs, uint64_t l2_entry,
}
}
-int coroutine_fn qcow2_write_caches(BlockDriverState *bs)
+int coroutine_fn qcow2_write_caches(BlockDriverState *bs, bool force_refcounts,
+ Error **errp)
{
BDRVQcow2State *s = bs->opaque;
int ret;
ret = qcow2_cache_write(bs, s->l2_table_cache);
if (ret < 0) {
+ error_setg_errno(errp, -ret, "Failed to write the L2 table cache");
return ret;
}
- if (qcow2_need_accurate_refcounts(s)) {
+ if (force_refcounts || qcow2_need_accurate_refcounts(s)) {
ret = qcow2_cache_write(bs, s->refcount_block_cache);
if (ret < 0) {
+ error_setg_errno(errp, -ret,
+ "Failed to write the refcount block cache");
return ret;
}
}
@@ -1223,14 +1227,22 @@ int coroutine_fn qcow2_write_caches(BlockDriverState *bs)
return 0;
}
-int coroutine_fn qcow2_flush_caches(BlockDriverState *bs)
+int coroutine_fn qcow2_flush_caches(BlockDriverState *bs, bool force_refcounts,
+ Error **errp)
{
- int ret = qcow2_write_caches(bs);
+ int ret = qcow2_write_caches(bs, force_refcounts, errp);
if (ret < 0) {
return ret;
}
- return bdrv_flush(bs->file->bs);
+ ret = bdrv_flush(bs->file->bs);
+ if (ret < 0) {
+ error_setg_errno(errp, -ret,
+ "Failed to flush after writing caches");
+ return ret;
+ }
+
+ return 0;
}
/*********************************************************/
diff --git a/block/qcow2.c b/block/qcow2.c
index 614df0307f..04994df240 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -544,7 +544,7 @@ static int qcow2_mark_clean(BlockDriverState *bs)
s->incompatible_features &= ~QCOW2_INCOMPAT_DIRTY;
- ret = qcow2_flush_caches(bs);
+ ret = qcow2_flush_caches(bs, false, NULL);
if (ret < 0) {
return ret;
}
@@ -574,7 +574,7 @@ int qcow2_mark_consistent(BlockDriverState *bs)
BDRVQcow2State *s = bs->opaque;
if (s->incompatible_features & QCOW2_INCOMPAT_CORRUPT) {
- int ret = qcow2_flush_caches(bs);
+ int ret = qcow2_flush_caches(bs, false, NULL);
if (ret < 0) {
return ret;
}
@@ -2680,7 +2680,6 @@ fail_nometa:
static int qcow2_inactivate(BlockDriverState *bs)
{
- BDRVQcow2State *s = bs->opaque;
int ret, result = 0;
Error *local_err = NULL;
@@ -2690,20 +2689,14 @@ static int qcow2_inactivate(BlockDriverState *bs)
error_reportf_err(local_err, "Lost persistent bitmaps during "
"inactivation of node '%s': ",
bdrv_get_device_or_node_name(bs));
+ local_err = NULL;
}
- ret = qcow2_cache_flush(bs, s->l2_table_cache);
- if (ret) {
+ ret = qcow2_flush_caches(bs, true, &local_err);
+ if (ret < 0) {
result = ret;
- error_report("Failed to flush the L2 table cache: %s",
- strerror(-ret));
- }
-
- ret = qcow2_cache_flush(bs, s->refcount_block_cache);
- if (ret) {
- result = ret;
- error_report("Failed to flush the refcount block cache: %s",
- strerror(-ret));
+ error_report_err(local_err);
+ local_err = NULL;
}
if (result == 0) {
@@ -4517,7 +4510,7 @@ static int coroutine_fn qcow2_co_truncate(BlockDriverState *bs, int64_t offset,
if (prealloc != PREALLOC_MODE_OFF) {
/* Flush metadata before actually changing the image size */
- ret = qcow2_write_caches(bs);
+ ret = qcow2_write_caches(bs, false, NULL);
if (ret < 0) {
error_setg_errno(errp, -ret,
"Failed to flush the preallocated area to disk");
@@ -4936,7 +4929,7 @@ static coroutine_fn int qcow2_co_flush_to_os(BlockDriverState *bs)
int ret;
qemu_co_mutex_lock(&s->lock);
- ret = qcow2_write_caches(bs);
+ ret = qcow2_write_caches(bs, false, NULL);
qemu_co_mutex_unlock(&s->lock);
return ret;
diff --git a/tests/qemu-iotests/026.out b/tests/qemu-iotests/026.out
index 83989996ff..cdc1ed2748 100644
--- a/tests/qemu-iotests/026.out
+++ b/tests/qemu-iotests/026.out
@@ -14,15 +14,13 @@ No errors were found on the image.
Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824
Event: l1_update; errno: 5; imm: off; once: off; write
-qemu-io: Failed to flush the L2 table cache: Input/output error
-qemu-io: Failed to flush the refcount block cache: Input/output error
+qemu-io: Failed to write the refcount block cache: Input/output error
write failed: Input/output error
No errors were found on the image.
Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824
Event: l1_update; errno: 5; imm: off; once: off; write -b
-qemu-io: Failed to flush the L2 table cache: Input/output error
-qemu-io: Failed to flush the refcount block cache: Input/output error
+qemu-io: Failed to write the refcount block cache: Input/output error
write failed: Input/output error
No errors were found on the image.
Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824
@@ -38,15 +36,13 @@ No errors were found on the image.
Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824
Event: l1_update; errno: 28; imm: off; once: off; write
-qemu-io: Failed to flush the L2 table cache: No space left on device
-qemu-io: Failed to flush the refcount block cache: No space left on device
+qemu-io: Failed to write the refcount block cache: No space left on device
write failed: No space left on device
No errors were found on the image.
Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824
Event: l1_update; errno: 28; imm: off; once: off; write -b
-qemu-io: Failed to flush the L2 table cache: No space left on device
-qemu-io: Failed to flush the refcount block cache: No space left on device
+qemu-io: Failed to write the refcount block cache: No space left on device
write failed: No space left on device
No errors were found on the image.
Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824
@@ -126,15 +122,13 @@ No errors were found on the image.
Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824
Event: l2_update; errno: 5; imm: off; once: off; write
-qemu-io: Failed to flush the L2 table cache: Input/output error
-qemu-io: Failed to flush the refcount block cache: Input/output error
+qemu-io: Failed to write the L2 table cache: Input/output error
write failed: Input/output error
No errors were found on the image.
Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824
Event: l2_update; errno: 5; imm: off; once: off; write -b
-qemu-io: Failed to flush the L2 table cache: Input/output error
-qemu-io: Failed to flush the refcount block cache: Input/output error
+qemu-io: Failed to write the L2 table cache: Input/output error
write failed: Input/output error
No errors were found on the image.
Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824
@@ -150,15 +144,13 @@ No errors were found on the image.
Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824
Event: l2_update; errno: 28; imm: off; once: off; write
-qemu-io: Failed to flush the L2 table cache: No space left on device
-qemu-io: Failed to flush the refcount block cache: No space left on device
+qemu-io: Failed to write the L2 table cache: No space left on device
write failed: No space left on device
No errors were found on the image.
Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824
Event: l2_update; errno: 28; imm: off; once: off; write -b
-qemu-io: Failed to flush the L2 table cache: No space left on device
-qemu-io: Failed to flush the refcount block cache: No space left on device
+qemu-io: Failed to write the L2 table cache: No space left on device
write failed: No space left on device
No errors were found on the image.
Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824
@@ -174,15 +166,13 @@ No errors were found on the image.
Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824
Event: l2_alloc_write; errno: 5; imm: off; once: off; write
-qemu-io: Failed to flush the L2 table cache: Input/output error
-qemu-io: Failed to flush the refcount block cache: Input/output error
+qemu-io: Failed to write the refcount block cache: Input/output error
write failed: Input/output error
No errors were found on the image.
Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824
Event: l2_alloc_write; errno: 5; imm: off; once: off; write -b
-qemu-io: Failed to flush the L2 table cache: Input/output error
-qemu-io: Failed to flush the refcount block cache: Input/output error
+qemu-io: Failed to write the refcount block cache: Input/output error
write failed: Input/output error
No errors were found on the image.
Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824
@@ -198,15 +188,13 @@ No errors were found on the image.
Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824
Event: l2_alloc_write; errno: 28; imm: off; once: off; write
-qemu-io: Failed to flush the L2 table cache: No space left on device
-qemu-io: Failed to flush the refcount block cache: No space left on device
+qemu-io: Failed to write the refcount block cache: No space left on device
write failed: No space left on device
No errors were found on the image.
Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824
Event: l2_alloc_write; errno: 28; imm: off; once: off; write -b
-qemu-io: Failed to flush the L2 table cache: No space left on device
-qemu-io: Failed to flush the refcount block cache: No space left on device
+qemu-io: Failed to write the refcount block cache: No space left on device
write failed: No space left on device
No errors were found on the image.
Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824
@@ -222,15 +210,13 @@ No errors were found on the image.
Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824
Event: write_aio; errno: 5; imm: off; once: off; write
-qemu-io: Failed to flush the L2 table cache: Input/output error
-qemu-io: Failed to flush the refcount block cache: Input/output error
+qemu-io: Failed to write the refcount block cache: Input/output error
write failed: Input/output error
No errors were found on the image.
Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824
Event: write_aio; errno: 5; imm: off; once: off; write -b
-qemu-io: Failed to flush the L2 table cache: Input/output error
-qemu-io: Failed to flush the refcount block cache: Input/output error
+qemu-io: Failed to write the refcount block cache: Input/output error
write failed: Input/output error
No errors were found on the image.
Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824
@@ -246,15 +232,13 @@ No errors were found on the image.
Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824
Event: write_aio; errno: 28; imm: off; once: off; write
-qemu-io: Failed to flush the L2 table cache: No space left on device
-qemu-io: Failed to flush the refcount block cache: No space left on device
+qemu-io: Failed to write the refcount block cache: No space left on device
write failed: No space left on device
No errors were found on the image.
Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824
Event: write_aio; errno: 28; imm: off; once: off; write -b
-qemu-io: Failed to flush the L2 table cache: No space left on device
-qemu-io: Failed to flush the refcount block cache: No space left on device
+qemu-io: Failed to write the refcount block cache: No space left on device
write failed: No space left on device
No errors were found on the image.
Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824
@@ -270,15 +254,13 @@ No errors were found on the image.
Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824
Event: refblock_load; errno: 5; imm: off; once: off; write
-qemu-io: Failed to flush the L2 table cache: Input/output error
-qemu-io: Failed to flush the refcount block cache: Input/output error
+qemu-io: Failed to write the refcount block cache: Input/output error
write failed: Input/output error
No errors were found on the image.
Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824
Event: refblock_load; errno: 5; imm: off; once: off; write -b
-qemu-io: Failed to flush the L2 table cache: Input/output error
-qemu-io: Failed to flush the refcount block cache: Input/output error
+qemu-io: Failed to write the refcount block cache: Input/output error
write failed: Input/output error
No errors were found on the image.
Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824
@@ -294,15 +276,13 @@ No errors were found on the image.
Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824
Event: refblock_load; errno: 28; imm: off; once: off; write
-qemu-io: Failed to flush the L2 table cache: No space left on device
-qemu-io: Failed to flush the refcount block cache: No space left on device
+qemu-io: Failed to write the refcount block cache: No space left on device
write failed: No space left on device
No errors were found on the image.
Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824
Event: refblock_load; errno: 28; imm: off; once: off; write -b
-qemu-io: Failed to flush the L2 table cache: No space left on device
-qemu-io: Failed to flush the refcount block cache: No space left on device
+qemu-io: Failed to write the refcount block cache: No space left on device
write failed: No space left on device
No errors were found on the image.
Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824
@@ -318,15 +298,13 @@ No errors were found on the image.
Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824
Event: refblock_update_part; errno: 5; imm: off; once: off; write
-qemu-io: Failed to flush the L2 table cache: Input/output error
-qemu-io: Failed to flush the refcount block cache: Input/output error
+qemu-io: Failed to write the refcount block cache: Input/output error
write failed: Input/output error
No errors were found on the image.
Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824
Event: refblock_update_part; errno: 5; imm: off; once: off; write -b
-qemu-io: Failed to flush the L2 table cache: Input/output error
-qemu-io: Failed to flush the refcount block cache: Input/output error
+qemu-io: Failed to write the refcount block cache: Input/output error
write failed: Input/output error
No errors were found on the image.
Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824
@@ -342,15 +320,13 @@ No errors were found on the image.
Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824
Event: refblock_update_part; errno: 28; imm: off; once: off; write
-qemu-io: Failed to flush the L2 table cache: No space left on device
-qemu-io: Failed to flush the refcount block cache: No space left on device
+qemu-io: Failed to write the refcount block cache: No space left on device
write failed: No space left on device
No errors were found on the image.
Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824
Event: refblock_update_part; errno: 28; imm: off; once: off; write -b
-qemu-io: Failed to flush the L2 table cache: No space left on device
-qemu-io: Failed to flush the refcount block cache: No space left on device
+qemu-io: Failed to write the refcount block cache: No space left on device
write failed: No space left on device
No errors were found on the image.
Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824
@@ -366,15 +342,13 @@ No errors were found on the image.
Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824
Event: refblock_alloc; errno: 5; imm: off; once: off; write
-qemu-io: Failed to flush the L2 table cache: Input/output error
-qemu-io: Failed to flush the refcount block cache: Input/output error
+qemu-io: Failed to write the refcount block cache: Input/output error
write failed: Input/output error
No errors were found on the image.
Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824
Event: refblock_alloc; errno: 5; imm: off; once: off; write -b
-qemu-io: Failed to flush the L2 table cache: Input/output error
-qemu-io: Failed to flush the refcount block cache: Input/output error
+qemu-io: Failed to write the refcount block cache: Input/output error
write failed: Input/output error
No errors were found on the image.
Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824
@@ -390,15 +364,13 @@ No errors were found on the image.
Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824
Event: refblock_alloc; errno: 28; imm: off; once: off; write
-qemu-io: Failed to flush the L2 table cache: No space left on device
-qemu-io: Failed to flush the refcount block cache: No space left on device
+qemu-io: Failed to write the refcount block cache: No space left on device
write failed: No space left on device
No errors were found on the image.
Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824
Event: refblock_alloc; errno: 28; imm: off; once: off; write -b
-qemu-io: Failed to flush the L2 table cache: No space left on device
-qemu-io: Failed to flush the refcount block cache: No space left on device
+qemu-io: Failed to write the refcount block cache: No space left on device
write failed: No space left on device
No errors were found on the image.
Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824
@@ -457,15 +429,13 @@ No errors were found on the image.
Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824
Event: refblock_alloc_hookup; errno: 28; imm: off; once: off; write
-qemu-io: Failed to flush the L2 table cache: No space left on device
-qemu-io: Failed to flush the refcount block cache: No space left on device
+qemu-io: Failed to write the L2 table cache: No space left on device
write failed: No space left on device
No errors were found on the image.
Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824
Event: refblock_alloc_hookup; errno: 28; imm: off; once: off; write -b
-qemu-io: Failed to flush the L2 table cache: No space left on device
-qemu-io: Failed to flush the refcount block cache: No space left on device
+qemu-io: Failed to write the refcount block cache: No space left on device
write failed: No space left on device
No errors were found on the image.
Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824
@@ -481,15 +451,13 @@ No errors were found on the image.
Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824
Event: refblock_alloc_write; errno: 28; imm: off; once: off; write
-qemu-io: Failed to flush the L2 table cache: No space left on device
-qemu-io: Failed to flush the refcount block cache: No space left on device
+qemu-io: Failed to write the L2 table cache: No space left on device
write failed: No space left on device
No errors were found on the image.
Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824
Event: refblock_alloc_write; errno: 28; imm: off; once: off; write -b
-qemu-io: Failed to flush the L2 table cache: No space left on device
-qemu-io: Failed to flush the refcount block cache: No space left on device
+qemu-io: Failed to write the refcount block cache: No space left on device
write failed: No space left on device
No errors were found on the image.
Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824
@@ -505,15 +473,13 @@ No errors were found on the image.
Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824
Event: refblock_alloc_write_blocks; errno: 28; imm: off; once: off; write
-qemu-io: Failed to flush the L2 table cache: No space left on device
-qemu-io: Failed to flush the refcount block cache: No space left on device
+qemu-io: Failed to write the L2 table cache: No space left on device
write failed: No space left on device
No errors were found on the image.
Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824
Event: refblock_alloc_write_blocks; errno: 28; imm: off; once: off; write -b
-qemu-io: Failed to flush the L2 table cache: No space left on device
-qemu-io: Failed to flush the refcount block cache: No space left on device
+qemu-io: Failed to write the L2 table cache: No space left on device
write failed: No space left on device
No errors were found on the image.
Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824
@@ -529,15 +495,13 @@ No errors were found on the image.
Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824
Event: refblock_alloc_write_table; errno: 28; imm: off; once: off; write
-qemu-io: Failed to flush the L2 table cache: No space left on device
-qemu-io: Failed to flush the refcount block cache: No space left on device
+qemu-io: Failed to write the L2 table cache: No space left on device
write failed: No space left on device
No errors were found on the image.
Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824
Event: refblock_alloc_write_table; errno: 28; imm: off; once: off; write -b
-qemu-io: Failed to flush the L2 table cache: No space left on device
-qemu-io: Failed to flush the refcount block cache: No space left on device
+qemu-io: Failed to write the L2 table cache: No space left on device
write failed: No space left on device
No errors were found on the image.
Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824
@@ -553,15 +517,13 @@ No errors were found on the image.
Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824
Event: refblock_alloc_switch_table; errno: 28; imm: off; once: off; write
-qemu-io: Failed to flush the L2 table cache: No space left on device
-qemu-io: Failed to flush the refcount block cache: No space left on device
+qemu-io: Failed to write the L2 table cache: No space left on device
write failed: No space left on device
No errors were found on the image.
Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824
Event: refblock_alloc_switch_table; errno: 28; imm: off; once: off; write -b
-qemu-io: Failed to flush the L2 table cache: No space left on device
-qemu-io: Failed to flush the refcount block cache: No space left on device
+qemu-io: Failed to write the L2 table cache: No space left on device
write failed: No space left on device
No errors were found on the image.
@@ -595,8 +557,7 @@ No errors were found on the image.
Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824
Event: l1_grow_write_table; errno: 5; imm: off; once: off
-qemu-io: Failed to flush the L2 table cache: Input/output error
-qemu-io: Failed to flush the refcount block cache: Input/output error
+qemu-io: Failed to write the refcount block cache: Input/output error
write failed: Input/output error
No errors were found on the image.
Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824
@@ -607,8 +568,7 @@ No errors were found on the image.
Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824
Event: l1_grow_write_table; errno: 28; imm: off; once: off
-qemu-io: Failed to flush the L2 table cache: No space left on device
-qemu-io: Failed to flush the refcount block cache: No space left on device
+qemu-io: Failed to write the refcount block cache: No space left on device
write failed: No space left on device
No errors were found on the image.
Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824
@@ -619,8 +579,7 @@ No errors were found on the image.
Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824
Event: l1_grow_activate_table; errno: 5; imm: off; once: off
-qemu-io: Failed to flush the L2 table cache: Input/output error
-qemu-io: Failed to flush the refcount block cache: Input/output error
+qemu-io: Failed to write the refcount block cache: Input/output error
write failed: Input/output error
No errors were found on the image.
Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824
@@ -631,8 +590,7 @@ No errors were found on the image.
Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824
Event: l1_grow_activate_table; errno: 28; imm: off; once: off
-qemu-io: Failed to flush the L2 table cache: No space left on device
-qemu-io: Failed to flush the refcount block cache: No space left on device
+qemu-io: Failed to write the refcount block cache: No space left on device
write failed: No space left on device
No errors were found on the image.
diff --git a/tests/qemu-iotests/071.out b/tests/qemu-iotests/071.out
index bca0c02f5c..de1c59154d 100644
--- a/tests/qemu-iotests/071.out
+++ b/tests/qemu-iotests/071.out
@@ -86,7 +86,6 @@ read failed: Input/output error
{"return": ""}
{"return": {}}
{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "SHUTDOWN", "data": {"guest": false, "reason": "host-qmp-quit"}}
-QEMU_PROG: Failed to flush the L2 table cache: Input/output error
-QEMU_PROG: Failed to flush the refcount block cache: Input/output error
+QEMU_PROG: Failed to flush after writing caches: Input/output error
*** done
diff --git a/tests/qemu-iotests/080.out b/tests/qemu-iotests/080.out
index 45ab01db8e..860228f93b 100644
--- a/tests/qemu-iotests/080.out
+++ b/tests/qemu-iotests/080.out
@@ -66,7 +66,7 @@ wrote 512/512 bytes at offset 0
qemu-img: Failed to load snapshot: Snapshot L1 table offset invalid
qemu-img: Snapshot L1 table offset invalid
qemu-img: Failed to turn zero into data clusters: Invalid argument
-qemu-io: Failed to flush the refcount block cache: Invalid argument
+qemu-io: Failed to write the refcount block cache: Invalid argument
write failed: Invalid argument
qemu-img: Snapshot L1 table offset invalid
qemu-img: Could not apply snapshot 'test': Failed to load snapshot: Invalid argument
@@ -89,7 +89,7 @@ wrote 512/512 bytes at offset 0
qemu-img: Failed to load snapshot: Snapshot L1 table too large
qemu-img: Snapshot L1 table too large
qemu-img: Failed to turn zero into data clusters: File too large
-qemu-io: Failed to flush the refcount block cache: File too large
+qemu-io: Failed to write the refcount block cache: File too large
write failed: File too large
qemu-img: Snapshot L1 table too large
qemu-img: Could not apply snapshot 'test': Failed to load snapshot: File too large
--
2.31.1
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH 10/14] qcow2: qcow2_inactivate(): don't call qcow2_mark_clean() when RO
2022-02-07 16:37 [PATCH 00/14] block: blockdev-del force=false Vladimir Sementsov-Ogievskiy
` (8 preceding siblings ...)
2022-02-07 16:37 ` [PATCH 09/14] qcow2: qcow2_inactivate(): use qcow2_flush_caches() Vladimir Sementsov-Ogievskiy
@ 2022-02-07 16:37 ` Vladimir Sementsov-Ogievskiy
2022-02-07 16:37 ` [PATCH 11/14] qcow2: refactor qcow2_inactivate Vladimir Sementsov-Ogievskiy
` (3 subsequent siblings)
13 siblings, 0 replies; 15+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2022-02-07 16:37 UTC (permalink / raw)
To: qemu-block
Cc: qemu-devel, armbru, jsnow, vsementsov, eblake, hreitz, kwolf, den,
ktkhai, igor
qcow2_inactivate() prints errors on different failures, let's not
exclude qcow2_mark_clean() call.
Still, if image is read-only, no reason to report failed write and no
reason even to try write. Write failure is possible when we open dirty
image for check in read-only mode. Let's not do it.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
block/qcow2.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/block/qcow2.c b/block/qcow2.c
index 04994df240..ccfcd0db05 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -2699,8 +2699,12 @@ static int qcow2_inactivate(BlockDriverState *bs)
local_err = NULL;
}
- if (result == 0) {
- qcow2_mark_clean(bs);
+ if (result == 0 && bdrv_is_writable(bs)) {
+ ret = qcow2_mark_clean(bs);
+ if (ret < 0) {
+ error_report("Failed to mark qcow2 node '%s' clean",
+ bdrv_get_device_or_node_name(bs));
+ }
}
return result;
--
2.31.1
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH 11/14] qcow2: refactor qcow2_inactivate
2022-02-07 16:37 [PATCH 00/14] block: blockdev-del force=false Vladimir Sementsov-Ogievskiy
` (9 preceding siblings ...)
2022-02-07 16:37 ` [PATCH 10/14] qcow2: qcow2_inactivate(): don't call qcow2_mark_clean() when RO Vladimir Sementsov-Ogievskiy
@ 2022-02-07 16:37 ` Vladimir Sementsov-Ogievskiy
2022-02-07 16:37 ` [PATCH 12/14] qcow2: implement .bdrv_close_safe Vladimir Sementsov-Ogievskiy
` (2 subsequent siblings)
13 siblings, 0 replies; 15+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2022-02-07 16:37 UTC (permalink / raw)
To: qemu-block
Cc: qemu-devel, armbru, jsnow, vsementsov, eblake, hreitz, kwolf, den,
ktkhai, igor
Add a possibility for safe behavior: stop on first error end report in
in errp.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
block/qcow2.c | 42 ++++++++++++++++++++++++++++++++++++------
1 file changed, 36 insertions(+), 6 deletions(-)
diff --git a/block/qcow2.c b/block/qcow2.c
index ccfcd0db05..8ad555feb7 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -2678,13 +2678,29 @@ fail_nometa:
return ret;
}
-static int qcow2_inactivate(BlockDriverState *bs)
+/*
+ * With force=true, ignore failures and just print them. Don't set errp, but
+ * still return a value < 0 in case when something failed.
+ *
+ * With force=false, return error < 0 and set errp on first failure. Nothing is
+ * printed.
+ */
+static int qcow2_do_inactivate(BlockDriverState *bs, bool force, Error **errp)
{
int ret, result = 0;
+ bool ok;
Error *local_err = NULL;
- qcow2_store_persistent_dirty_bitmaps(bs, true, &local_err);
- if (local_err != NULL) {
+ if (force) {
+ errp = &local_err;
+ }
+
+ ok = qcow2_store_persistent_dirty_bitmaps(bs, true, errp);
+ if (!ok) {
+ if (!force) {
+ return -EINVAL;
+ }
+ assert(local_err);
result = -EINVAL;
error_reportf_err(local_err, "Lost persistent bitmaps during "
"inactivation of node '%s': ",
@@ -2692,8 +2708,11 @@ static int qcow2_inactivate(BlockDriverState *bs)
local_err = NULL;
}
- ret = qcow2_flush_caches(bs, true, &local_err);
+ ret = qcow2_flush_caches(bs, true, errp);
if (ret < 0) {
+ if (!force) {
+ return ret;
+ }
result = ret;
error_report_err(local_err);
local_err = NULL;
@@ -2702,14 +2721,25 @@ static int qcow2_inactivate(BlockDriverState *bs)
if (result == 0 && bdrv_is_writable(bs)) {
ret = qcow2_mark_clean(bs);
if (ret < 0) {
- error_report("Failed to mark qcow2 node '%s' clean",
- bdrv_get_device_or_node_name(bs));
+ error_setg(errp, "Failed to mark qcow2 node '%s' clean",
+ bdrv_get_device_or_node_name(bs));
+ if (!force) {
+ return ret;
+ }
+ result = ret;
+ error_report_err(local_err);
+ local_err = NULL;
}
}
return result;
}
+static int qcow2_inactivate(BlockDriverState *bs)
+{
+ return qcow2_do_inactivate(bs, true, &error_abort);
+}
+
static void qcow2_close(BlockDriverState *bs)
{
BDRVQcow2State *s = bs->opaque;
--
2.31.1
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH 12/14] qcow2: implement .bdrv_close_safe
2022-02-07 16:37 [PATCH 00/14] block: blockdev-del force=false Vladimir Sementsov-Ogievskiy
` (10 preceding siblings ...)
2022-02-07 16:37 ` [PATCH 11/14] qcow2: refactor qcow2_inactivate Vladimir Sementsov-Ogievskiy
@ 2022-02-07 16:37 ` Vladimir Sementsov-Ogievskiy
2022-02-07 16:37 ` [PATCH 13/14] block/file-posix: " Vladimir Sementsov-Ogievskiy
2022-02-07 16:37 ` [PATCH 14/14] iotests: add test for blockdev-del(force=false) Vladimir Sementsov-Ogievskiy
13 siblings, 0 replies; 15+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2022-02-07 16:37 UTC (permalink / raw)
To: qemu-block
Cc: qemu-devel, armbru, jsnow, vsementsov, eblake, hreitz, kwolf, den,
ktkhai, igor
Implement new API, so that qcow2 supports blockdev-del with
force=false.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
include/block/block.h | 2 +
block.c | 4 +-
block/qcow2.c | 85 ++++++++++++++++++++++++++++++++++---------
3 files changed, 72 insertions(+), 19 deletions(-)
diff --git a/include/block/block.h b/include/block/block.h
index 42d78a7a31..fbb5f3ff6d 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -672,6 +672,8 @@ void bdrv_disable_copy_on_read(BlockDriverState *bs);
void bdrv_ref(BlockDriverState *bs);
void bdrv_unref(BlockDriverState *bs);
void bdrv_unref_child(BlockDriverState *parent, BdrvChild *child);
+int bdrv_unref_child_safe(BlockDriverState *parent, BdrvChild *child,
+ Transaction *tran, Error **errp);
int bdrv_try_unref(BlockDriverState *bs, Error **errp);
BdrvChild *bdrv_attach_child(BlockDriverState *parent_bs,
BlockDriverState *child_bs,
diff --git a/block.c b/block.c
index 187732c6f8..dd2ddedc86 100644
--- a/block.c
+++ b/block.c
@@ -3216,8 +3216,8 @@ static void bdrv_unset_inherits_from(BlockDriverState *root, BdrvChild *child,
* When @tran is not NULL, first failure is returned and the action may be
* rolled back.
*/
-static int bdrv_unref_child_safe(BlockDriverState *parent, BdrvChild *child,
- Transaction *tran, Error **errp)
+int bdrv_unref_child_safe(BlockDriverState *parent, BdrvChild *child,
+ Transaction *tran, Error **errp)
{
if (child == NULL) {
return 0;
diff --git a/block/qcow2.c b/block/qcow2.c
index 8ad555feb7..a8f891ee34 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -2740,23 +2740,24 @@ static int qcow2_inactivate(BlockDriverState *bs)
return qcow2_do_inactivate(bs, true, &error_abort);
}
-static void qcow2_close(BlockDriverState *bs)
-{
- BDRVQcow2State *s = bs->opaque;
- qemu_vfree(s->l1_table);
- /* else pre-write overlap checks in cache_destroy may crash */
- s->l1_table = NULL;
+typedef struct Qcow2CloseState {
+ BlockDriverState *bs;
+ void *old_l1_table;
+} Qcow2CloseState;
- if (!(s->flags & BDRV_O_INACTIVE)) {
- qcow2_inactivate(bs);
- }
+static void qcow2_close_commit(void *opaque)
+{
+ Qcow2CloseState *cs = opaque;
+ BlockDriverState *bs = cs->bs;
+ BDRVQcow2State *s = bs->opaque;
+
+ qemu_vfree(cs->old_l1_table);
cache_clean_timer_del(bs);
qcow2_cache_destroy(s->l2_table_cache);
qcow2_cache_destroy(s->refcount_block_cache);
qcrypto_block_free(s->crypto);
- s->crypto = NULL;
qapi_free_QCryptoBlockOpenOptions(s->crypto_opts);
g_free(s->unknown_header_fields);
@@ -2766,15 +2767,65 @@ static void qcow2_close(BlockDriverState *bs)
g_free(s->image_backing_file);
g_free(s->image_backing_format);
- if (has_data_file(bs)) {
- bdrv_unref_child(bs, s->data_file);
- s->data_file = NULL;
- }
-
qcow2_refcount_close(bs);
qcow2_free_snapshots(bs);
}
+static void qcow2_close_abort(void *opaque)
+{
+ Qcow2CloseState *cs = opaque;
+ BlockDriverState *bs = cs->bs;
+ BDRVQcow2State *s = bs->opaque;
+
+ s->l1_table = cs->old_l1_table;
+}
+
+static TransactionActionDrv qcow2_close_drv = {
+ .commit = qcow2_close_commit,
+ .abort = qcow2_close_abort,
+ .clean = g_free,
+};
+
+static int qcow2_close_safe(BlockDriverState *bs, Transaction *tran,
+ Error **errp)
+{
+ BDRVQcow2State *s = bs->opaque;
+ int ret;
+ Qcow2CloseState *cs = g_new(Qcow2CloseState, 1);
+
+ *cs = (Qcow2CloseState) {
+ .bs = bs,
+ .old_l1_table = s->l1_table,
+ };
+
+ /* else pre-write overlap checks in cache_destroy may crash */
+ s->l1_table = NULL;
+ if (tran) {
+ tran_add(tran, &qcow2_close_drv, cs);
+ }
+
+ if (!(s->flags & BDRV_O_INACTIVE)) {
+ ret = qcow2_do_inactivate(bs, !tran, errp);
+ if (ret < 0 && tran) {
+ return ret;
+ }
+ }
+
+ if (has_data_file(bs)) {
+ ret = bdrv_unref_child_safe(bs, s->data_file, tran, errp);
+ if (ret < 0 && tran) {
+ return ret;
+ }
+ }
+
+ if (!tran) {
+ qcow2_close_commit(cs);
+ g_free(cs);
+ }
+
+ return 0;
+}
+
static void coroutine_fn qcow2_co_invalidate_cache(BlockDriverState *bs,
Error **errp)
{
@@ -2793,7 +2844,7 @@ static void coroutine_fn qcow2_co_invalidate_cache(BlockDriverState *bs,
crypto = s->crypto;
s->crypto = NULL;
- qcow2_close(bs);
+ qcow2_close_safe(bs, NULL, &error_abort);
memset(s, 0, sizeof(BDRVQcow2State));
options = qdict_clone_shallow(bs->options);
@@ -6043,7 +6094,7 @@ BlockDriver bdrv_qcow2 = {
.instance_size = sizeof(BDRVQcow2State),
.bdrv_probe = qcow2_probe,
.bdrv_open = qcow2_open,
- .bdrv_close = qcow2_close,
+ .bdrv_close_safe = qcow2_close_safe,
.bdrv_reopen_prepare = qcow2_reopen_prepare,
.bdrv_reopen_commit = qcow2_reopen_commit,
.bdrv_reopen_commit_post = qcow2_reopen_commit_post,
--
2.31.1
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH 13/14] block/file-posix: implement .bdrv_close_safe
2022-02-07 16:37 [PATCH 00/14] block: blockdev-del force=false Vladimir Sementsov-Ogievskiy
` (11 preceding siblings ...)
2022-02-07 16:37 ` [PATCH 12/14] qcow2: implement .bdrv_close_safe Vladimir Sementsov-Ogievskiy
@ 2022-02-07 16:37 ` Vladimir Sementsov-Ogievskiy
2022-02-07 16:37 ` [PATCH 14/14] iotests: add test for blockdev-del(force=false) Vladimir Sementsov-Ogievskiy
13 siblings, 0 replies; 15+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2022-02-07 16:37 UTC (permalink / raw)
To: qemu-block
Cc: qemu-devel, armbru, jsnow, vsementsov, eblake, hreitz, kwolf, den,
ktkhai, igor
Implement new close API for 'file' driver, so that we now have the
minimal set working: qcow2 + file.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
block/file-posix.c | 38 +++++++++++++++++++++++++++++++-------
1 file changed, 31 insertions(+), 7 deletions(-)
diff --git a/block/file-posix.c b/block/file-posix.c
index 1f1756e192..90642d8185 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -2195,14 +2195,38 @@ static void raw_aio_attach_aio_context(BlockDriverState *bs,
#endif
}
-static void raw_close(BlockDriverState *bs)
+static void raw_close_commit(void *opaque)
+{
+ BDRVRawState *s = opaque;
+
+ if (s->fd >= 0) {
+ /*
+ * Closing fd is unrecoverable action, that's why it is in .commit.
+ * So, yes, it may fail, but we ignore the failure.
+ */
+ qemu_close(s->fd);
+ s->fd = -1;
+ }
+}
+
+TransactionActionDrv raw_close_drv = {
+ .commit = raw_close_commit,
+};
+
+static int raw_close_safe(BlockDriverState *bs, Transaction *tran,
+ Error **errp)
{
BDRVRawState *s = bs->opaque;
if (s->fd >= 0) {
- qemu_close(s->fd);
- s->fd = -1;
+ if (tran) {
+ tran_add(tran, &raw_close_drv, s);
+ } else {
+ raw_close_commit(s);
+ }
}
+
+ return 0;
}
/**
@@ -3278,7 +3302,7 @@ BlockDriver bdrv_file = {
.bdrv_reopen_prepare = raw_reopen_prepare,
.bdrv_reopen_commit = raw_reopen_commit,
.bdrv_reopen_abort = raw_reopen_abort,
- .bdrv_close = raw_close,
+ .bdrv_close_safe = raw_close_safe,
.bdrv_co_create = raw_co_create,
.bdrv_co_create_opts = raw_co_create_opts,
.bdrv_has_zero_init = bdrv_has_zero_init_1,
@@ -3643,7 +3667,7 @@ static BlockDriver bdrv_host_device = {
.bdrv_probe_device = hdev_probe_device,
.bdrv_parse_filename = hdev_parse_filename,
.bdrv_file_open = hdev_open,
- .bdrv_close = raw_close,
+ .bdrv_close_safe = raw_close_safe,
.bdrv_reopen_prepare = raw_reopen_prepare,
.bdrv_reopen_commit = raw_reopen_commit,
.bdrv_reopen_abort = raw_reopen_abort,
@@ -3771,7 +3795,7 @@ static BlockDriver bdrv_host_cdrom = {
.bdrv_probe_device = cdrom_probe_device,
.bdrv_parse_filename = cdrom_parse_filename,
.bdrv_file_open = cdrom_open,
- .bdrv_close = raw_close,
+ .bdrv_close_safe = raw_close_safe,
.bdrv_reopen_prepare = raw_reopen_prepare,
.bdrv_reopen_commit = raw_reopen_commit,
.bdrv_reopen_abort = raw_reopen_abort,
@@ -3902,7 +3926,7 @@ static BlockDriver bdrv_host_cdrom = {
.bdrv_probe_device = cdrom_probe_device,
.bdrv_parse_filename = cdrom_parse_filename,
.bdrv_file_open = cdrom_open,
- .bdrv_close = raw_close,
+ .bdrv_close_safe = raw_close_safe,
.bdrv_reopen_prepare = raw_reopen_prepare,
.bdrv_reopen_commit = raw_reopen_commit,
.bdrv_reopen_abort = raw_reopen_abort,
--
2.31.1
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH 14/14] iotests: add test for blockdev-del(force=false)
2022-02-07 16:37 [PATCH 00/14] block: blockdev-del force=false Vladimir Sementsov-Ogievskiy
` (12 preceding siblings ...)
2022-02-07 16:37 ` [PATCH 13/14] block/file-posix: " Vladimir Sementsov-Ogievskiy
@ 2022-02-07 16:37 ` Vladimir Sementsov-Ogievskiy
13 siblings, 0 replies; 15+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2022-02-07 16:37 UTC (permalink / raw)
To: qemu-block
Cc: qemu-devel, armbru, jsnow, vsementsov, eblake, hreitz, kwolf, den,
ktkhai, igor
Test for new option: use NBD server killing to simulate failure on file
close.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
.../tests/blockdev-del-close-failure | 54 +++++++++++++++++++
.../tests/blockdev-del-close-failure.out | 4 ++
2 files changed, 58 insertions(+)
create mode 100755 tests/qemu-iotests/tests/blockdev-del-close-failure
create mode 100644 tests/qemu-iotests/tests/blockdev-del-close-failure.out
diff --git a/tests/qemu-iotests/tests/blockdev-del-close-failure b/tests/qemu-iotests/tests/blockdev-del-close-failure
new file mode 100755
index 0000000000..12b442f43f
--- /dev/null
+++ b/tests/qemu-iotests/tests/blockdev-del-close-failure
@@ -0,0 +1,54 @@
+#!/usr/bin/env python3
+#
+# Test blockdev-add force=false
+#
+# Copyright (c) 2022 Virtuozzo International GmbH.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program. If not, see <http://www.gnu.org/licenses/>.
+#
+
+import iotests
+from iotests import qemu_img_create, qemu_img, qemu_nbd_popen, qemu_img_pipe
+
+iotests.script_initialize(supported_fmts=['qcow2'],
+ unsupported_imgopts=['compat'])
+
+disk, nbd_sock = iotests.file_path('disk', 'nbd-sock')
+size = '1M'
+
+assert qemu_img_create('-f', iotests.imgfmt, disk, size) == 0
+assert qemu_img('bitmap', '--add', disk, 'bitmap0') == 0
+assert 'bitmaps' in qemu_img_pipe('info', disk)
+
+vm = iotests.VM()
+vm.launch()
+
+with qemu_nbd_popen('-k', nbd_sock, '-f', 'raw', disk):
+ result = vm.qmp('blockdev-add', {
+ 'node-name': 'disk0',
+ 'driver': 'qcow2',
+ 'file': {
+ 'driver': 'nbd',
+ 'server': {
+ 'type': 'unix',
+ 'path': nbd_sock
+ }
+ }
+ })
+ assert result == {'return': {}}
+
+# Now bitmap is loaded and marked IN_USE in the image, but connection is lost
+# Bitmap can't be saved and blockdev-del(force=false) should fail
+vm.qmp_log('blockdev-del', node_name='disk0', force=False)
+vm.shutdown()
diff --git a/tests/qemu-iotests/tests/blockdev-del-close-failure.out b/tests/qemu-iotests/tests/blockdev-del-close-failure.out
new file mode 100644
index 0000000000..4a4776cc98
--- /dev/null
+++ b/tests/qemu-iotests/tests/blockdev-del-close-failure.out
@@ -0,0 +1,4 @@
+Start NBD server
+Kill NBD server
+{"execute": "blockdev-del", "arguments": {"force": false, "node-name": "disk0"}}
+{"error": {"class": "GenericError", "desc": "Failed to flush node 'disk0'"}}
--
2.31.1
^ permalink raw reply related [flat|nested] 15+ messages in thread