[PATCH v4 28/45] qapi: block: add blockdev-del transaction action

[Vladimir Sementsov-Ogievskiy]

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-og@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