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.
Additionally we need to add aio_context_acquire_tran() transaction
action, to keep aio context acquired including final bdrv_delete() in
commit of bdrv_unref_tran().
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsement...@openvz.org>
---
blockdev.c | 87 +++++++++++++++++++++++++++++++++++++------
qapi/block-core.json | 11 ++++--
qapi/transaction.json | 12 ++++++
3 files changed, 95 insertions(+), 15 deletions(-)
diff --git a/blockdev.c b/blockdev.c
index a7287bf64f..1cd95f4f02 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -63,6 +63,9 @@
#include "qemu/main-loop.h"
#include "qemu/throttle-options.h"
+static int blockdev_del(const char *node_name, GSList **detached,
+ Transaction *tran, Error **errp);
+
/* Protected by BQL */
QTAILQ_HEAD(, BlockDriverState) monitor_bdrv_states =
QTAILQ_HEAD_INITIALIZER(monitor_bdrv_states);
@@ -2163,6 +2166,7 @@ static void abort_commit(void *opaque)
}
static void transaction_action(TransactionAction *act, JobTxn *block_job_txn,
+ GSList **refresh_list,
Transaction *tran, Error **errp)
{
switch (act->type) {
@@ -2209,6 +2213,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,
@@ -2234,6 +2242,7 @@ void qmp_transaction(TransactionActionList *actions,
struct TransactionProperties *properties,
Error **errp)
{
+ int ret;
TransactionActionList *act;
JobTxn *block_job_txn = NULL;
Error *local_err = NULL;
@@ -2241,6 +2250,7 @@ void qmp_transaction(TransactionActionList *actions,
ActionCompletionMode comp_mode =
has_properties ? properties->completion_mode :
ACTION_COMPLETION_MODE_INDIVIDUAL;
+ g_autoptr(GSList) refresh_list = NULL;
GLOBAL_STATE_CODE();
@@ -2271,13 +2281,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 */
@@ -3520,9 +3549,25 @@ fail:
g_slist_free(drained);
}
-void qmp_blockdev_del(const char *node_name, Error **errp)
+static void aio_context_acquire_clean(void *opaque)
+{
+ aio_context_release(opaque);
+}
+
+TransactionActionDrv aio_context_acquire_drv = {
+ .clean = aio_context_acquire_clean,
+};
+
+static void aio_context_acquire_tran(AioContext *ctx, Transaction *tran)
+{
+ aio_context_acquire(ctx);
+ tran_add(tran, &aio_context_acquire_drv, ctx);
+}
+
+/* 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;
GLOBAL_STATE_CODE();
@@ -3530,36 +3575,54 @@ void qmp_blockdev_del(const char *node_name, Error
**errp)
bs = bdrv_find_node(node_name);
if (!bs) {
error_setg(errp, "Failed to find node with node-name='%s'", node_name);
- return;
+ return -EINVAL;
}
if (bdrv_has_blk(bs)) {
error_setg(errp, "Node %s is in use", node_name);
- return;
+ return -EINVAL;
}
- aio_context = bdrv_get_aio_context(bs);
- aio_context_acquire(aio_context);
+ aio_context_acquire_tran(bdrv_get_aio_context(bs), tran);
if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_DRIVE_DEL, errp)) {
- goto out;
+ return -EINVAL;
}
if (!QTAILQ_IN_USE(bs, monitor_list)) {
error_setg(errp, "Node %s is not owned by the monitor",
bs->node_name);
- goto out;
+ return -EINVAL;
}
if (bs->refcnt > 1) {
error_setg(errp, "Block device %s is in use",
bdrv_get_device_or_node_name(bs));
- goto out;
+ return -EINVAL;
}
QTAILQ_REMOVE(&monitor_bdrv_states, bs, monitor_list);
- bdrv_unref(bs);
+ bdrv_unref_tran(bs, refresh_list, tran);
+
+ return 0;
+}
+
+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:
- aio_context_release(aio_context);
+ 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 e89f2dfb5b..d915cddde9 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -4407,6 +4407,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:
#
@@ -4414,8 +4421,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:
@@ -4438,7 +4443,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
