Support blockdev-del in a transaction.
The tricky thing is how we update permissions: not after every
blockdev-del operation, but after group of such operations. Soon we'll
support blockdev-add and new blockdev-replace in the same manner, and
we'll be able to do a wide range of block-graph modifying operation in
a bunch, so that permissions are updated only after the whole group, to
avoid intermediate permission conflicts.
Signed-off-by: Vladimir Sementsov-Ogievskiy <v.sementsov...@mail.ru>
---
blockdev.c | 66 +++++++++++++++++++++++++++++++++++++++----
qapi/block-core.json | 11 ++++++--
qapi/transaction.json | 12 ++++++++
3 files changed, 81 insertions(+), 8 deletions(-)
diff --git a/blockdev.c b/blockdev.c
index 30a5b27b28..517be48399 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -2141,7 +2141,12 @@ static void abort_commit(void *opaque)
g_assert_not_reached(); /* this action never succeeds */
}
+
+static int blockdev_del(const char *node_name, GSList **detached,
+ Transaction *tran, Error **errp);
+
static void transaction_action(TransactionAction *act, JobTxn *block_job_txn,
+ GSList **refresh_list,
Transaction *tran, Error **errp)
{
switch (act->type) {
@@ -2188,6 +2193,10 @@ static void transaction_action(TransactionAction *act,
JobTxn *block_job_txn,
block_dirty_bitmap_remove_action(act->u.block_dirty_bitmap_remove.data,
tran, errp);
return;
+ case TRANSACTION_ACTION_KIND_BLOCKDEV_DEL:
+ blockdev_del(act->u.blockdev_del.data->node_name,
+ refresh_list, tran, errp);
+ return;
/*
* Where are transactions for MIRROR, COMMIT and STREAM?
* Although these blockjobs use transaction callbacks like the backup job,
@@ -2211,6 +2220,7 @@ void qmp_transaction(TransactionActionList *actions,
struct TransactionProperties *properties,
Error **errp)
{
+ int ret;
TransactionActionList *act;
JobTxn *block_job_txn = NULL;
Error *local_err = NULL;
@@ -2218,6 +2228,7 @@ void qmp_transaction(TransactionActionList *actions,
ActionCompletionMode comp_mode =
has_properties ? properties->completion_mode :
ACTION_COMPLETION_MODE_INDIVIDUAL;
+ g_autoptr(GSList) refresh_list = NULL;
/* Does this transaction get canceled as a group on failure?
* If not, we don't really need to make a JobTxn.
@@ -2246,13 +2257,32 @@ void qmp_transaction(TransactionActionList *actions,
/* We don't do anything in this loop that commits us to the operations */
for (act = actions; act; act = act->next) {
- transaction_action(act->value, block_job_txn, tran, &local_err);
+ TransactionActionKind type = act->value->type;
+
+ if (refresh_list &&
+ type != TRANSACTION_ACTION_KIND_BLOCKDEV_DEL)
+ {
+ ret = bdrv_list_refresh_perms(refresh_list, NULL, tran, errp);
+ if (ret < 0) {
+ goto delete_and_fail;
+ }
+ g_slist_free(refresh_list);
+ refresh_list = NULL;
+ }
+
+ transaction_action(act->value, block_job_txn, &refresh_list, tran,
+ &local_err);
if (local_err) {
error_propagate(errp, local_err);
goto delete_and_fail;
}
}
+ ret = bdrv_list_refresh_perms(refresh_list, NULL, tran, errp);
+ if (ret < 0) {
+ goto delete_and_fail;
+ }
+
tran_commit(tran);
/* success */
@@ -3486,19 +3516,22 @@ fail:
g_slist_free_full(drained, (GDestroyNotify) bdrv_subtree_drained_end);
}
-void qmp_blockdev_del(const char *node_name, Error **errp)
+/* Function doesn't update permissions, it's a responsibility of caller. */
+static int blockdev_del(const char *node_name, GSList **refresh_list,
+ Transaction *tran, Error **errp)
{
AioContext *aio_context;
BlockDriverState *bs;
+ int ret = -EINVAL;
bs = bdrv_find_node(node_name);
if (!bs) {
error_setg(errp, "Failed to find node with node-name='%s'", node_name);
- return;
+ return ret;
}
if (bdrv_has_blk(bs)) {
error_setg(errp, "Node %s is in use", node_name);
- return;
+ return ret;
}
aio_context = bdrv_get_aio_context(bs);
aio_context_acquire(aio_context);
@@ -3520,10 +3553,33 @@ void qmp_blockdev_del(const char *node_name, Error
**errp)
}
QTAILQ_REMOVE(&monitor_bdrv_states, bs, monitor_list);
- bdrv_unref(bs);
+ bdrv_unref_tran(bs, refresh_list, tran);
+
+ ret = 0;
out:
aio_context_release(aio_context);
+ return ret;
+}
+
+void qmp_blockdev_del(const char *node_name, Error **errp)
+{
+ int ret;
+ Transaction *tran = tran_new();
+ g_autoptr(GSList) refresh_list = NULL;
+
+ ret = blockdev_del(node_name, &refresh_list, tran, errp);
+ if (ret < 0) {
+ goto out;
+ }
+
+ ret = bdrv_list_refresh_perms(refresh_list, NULL, tran, errp);
+ if (ret < 0) {
+ goto out;
+ }
+
+out:
+ tran_finalize(tran, ret);
}
static BdrvChild *bdrv_find_child(BlockDriverState *parent_bs,
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 9a5a3641d0..c0d56b7128 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -4394,6 +4394,13 @@
{ 'command': 'blockdev-reopen',
'data': { 'options': ['BlockdevOptions'] } }
+##
+# @BlockdevDel:
+#
+# @node-name: Name of the graph node to delete.
+##
+{ 'struct': 'BlockdevDel', 'data': { 'node-name': 'str' } }
+
##
# @blockdev-del:
#
@@ -4401,8 +4408,6 @@
# The command will fail if the node is attached to a device or is
# otherwise being used.
#
-# @node-name: Name of the graph node to delete.
-#
# Since: 2.9
#
# Example:
@@ -4425,7 +4430,7 @@
# <- { "return": {} }
#
##
-{ 'command': 'blockdev-del', 'data': { 'node-name': 'str' } }
+{ 'command': 'blockdev-del', 'data': 'BlockdevDel' }
##
# @BlockdevCreateOptionsFile:
diff --git a/qapi/transaction.json b/qapi/transaction.json
index 381a2df782..ea20df770c 100644
--- a/qapi/transaction.json
+++ b/qapi/transaction.json
@@ -53,6 +53,7 @@
# @blockdev-snapshot-internal-sync: Since 1.7
# @blockdev-snapshot-sync: since 1.1
# @drive-backup: Since 1.6
+# @blockdev-del: since 7.1
#
# Features:
# @deprecated: Member @drive-backup is deprecated. Use member
@@ -66,6 +67,7 @@
'block-dirty-bitmap-disable', 'block-dirty-bitmap-merge',
'blockdev-backup', 'blockdev-snapshot',
'blockdev-snapshot-internal-sync', 'blockdev-snapshot-sync',
+ 'blockdev-del',
{ 'name': 'drive-backup', 'features': [ 'deprecated' ] } ] }
##
@@ -140,6 +142,15 @@
{ 'struct': 'DriveBackupWrapper',
'data': { 'data': 'DriveBackup' } }
+##
+# @BlockdevDelWrapper:
+#
+# Since: 7.1
+##
+{ 'struct': 'BlockdevDelWrapper',
+ 'data': { 'data': 'BlockdevDel' } }
+
+
##
# @TransactionAction:
#
@@ -163,6 +174,7 @@
'blockdev-snapshot': 'BlockdevSnapshotWrapper',
'blockdev-snapshot-internal-sync': 'BlockdevSnapshotInternalWrapper',
'blockdev-snapshot-sync': 'BlockdevSnapshotSyncWrapper',
+ 'blockdev-del': 'BlockdevDelWrapper',
'drive-backup': 'DriveBackupWrapper'
} }
--
2.35.1
