@@ -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,
@@ -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,
@@ -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 {
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(-)