qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
To: qemu-block@nongnu.org
Cc: qemu-devel@nongnu.org, armbru@redhat.com, fam@euphon.net,
	stefanha@redhat.com, vsementsov@virtuozzo.com, jsnow@redhat.com,
	mreitz@redhat.com, kwolf@redhat.com
Subject: [PATCH v3 36/36] block: refactor bdrv_node_check_perm()
Date: Wed, 17 Mar 2021 17:35:29 +0300	[thread overview]
Message-ID: <20210317143529.615584-37-vsementsov@virtuozzo.com> (raw)
In-Reply-To: <20210317143529.615584-1-vsementsov@virtuozzo.com>

Now, bdrv_node_check_perm() is called only with fresh cumulative
permissions, so its actually "refresh_perm".

Move permission calculation to the function. Also, drop unreachable
error message and rewrite the remaining one to be more generic (as now
we don't know which node is added and which was already here).

Add also Virtuozzo copyright, as big work is done at this point.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 block.c                | 38 +++++++++++---------------------------
 tests/qemu-iotests/245 |  2 +-
 2 files changed, 12 insertions(+), 28 deletions(-)

diff --git a/block.c b/block.c
index b61ac9ff19..95d8246d92 100644
--- a/block.c
+++ b/block.c
@@ -2,6 +2,7 @@
  * QEMU System Emulator block driver
  *
  * Copyright (c) 2003 Fabrice Bellard
+ * Copyright (c) 2020 Virtuozzo International GmbH.
  *
  * Permission is hereby granted, free of charge, to any person obtaining a copy
  * of this software and associated documentation files (the "Software"), to deal
@@ -2236,22 +2237,18 @@ static void bdrv_replace_child(BdrvChild *child, BlockDriverState *new_bs,
 }
 
 /*
- * Check whether permissions on this node can be changed in a way that
- * @cumulative_perms and @cumulative_shared_perms are the new cumulative
- * permissions of all its parents. This involves checking whether all necessary
- * permission changes to child nodes can be performed.
- *
- * A call to this function must always be followed by a call to bdrv_set_perm()
- * or bdrv_abort_perm_update().
+ * Refresh permissions in @bs subtree. The function is intended to be called
+ * after some graph modification that was done without permission update.
  */
-static int bdrv_node_check_perm(BlockDriverState *bs, BlockReopenQueue *q,
-                                uint64_t cumulative_perms,
-                                uint64_t cumulative_shared_perms,
-                                Transaction *tran, Error **errp)
+static int bdrv_node_refresh_perm(BlockDriverState *bs, BlockReopenQueue *q,
+                                  Transaction *tran, Error **errp)
 {
     BlockDriver *drv = bs->drv;
     BdrvChild *c;
     int ret;
+    uint64_t cumulative_perms, cumulative_shared_perms;
+
+    bdrv_get_cumulative_perm(bs, &cumulative_perms, &cumulative_shared_perms);
 
     /* Write permissions never work with read-only images */
     if ((cumulative_perms & (BLK_PERM_WRITE | BLK_PERM_WRITE_UNCHANGED)) &&
@@ -2260,15 +2257,8 @@ static int bdrv_node_check_perm(BlockDriverState *bs, BlockReopenQueue *q,
         if (!bdrv_is_writable_after_reopen(bs, NULL)) {
             error_setg(errp, "Block node is read-only");
         } else {
-            uint64_t current_perms, current_shared;
-            bdrv_get_cumulative_perm(bs, &current_perms, &current_shared);
-            if (current_perms & (BLK_PERM_WRITE | BLK_PERM_WRITE_UNCHANGED)) {
-                error_setg(errp, "Cannot make block node read-only, there is "
-                           "a writer on it");
-            } else {
-                error_setg(errp, "Cannot make block node read-only and create "
-                           "a writer on it");
-            }
+            error_setg(errp, "Read-only block node '%s' cannot support "
+                       "read-write users", bdrv_get_node_name(bs));
         }
 
         return -EPERM;
@@ -2324,7 +2314,6 @@ static int bdrv_list_refresh_perms(GSList *list, BlockReopenQueue *q,
                                    Transaction *tran, Error **errp)
 {
     int ret;
-    uint64_t cumulative_perms, cumulative_shared_perms;
     BlockDriverState *bs;
 
     for ( ; list; list = list->next) {
@@ -2334,12 +2323,7 @@ static int bdrv_list_refresh_perms(GSList *list, BlockReopenQueue *q,
             return -EINVAL;
         }
 
-        bdrv_get_cumulative_perm(bs, &cumulative_perms,
-                                 &cumulative_shared_perms);
-
-        ret = bdrv_node_check_perm(bs, q, cumulative_perms,
-                                   cumulative_shared_perms,
-                                   tran, errp);
+        ret = bdrv_node_refresh_perm(bs, q, tran, errp);
         if (ret < 0) {
             return ret;
         }
diff --git a/tests/qemu-iotests/245 b/tests/qemu-iotests/245
index 11104b9208..fc5297e268 100755
--- a/tests/qemu-iotests/245
+++ b/tests/qemu-iotests/245
@@ -905,7 +905,7 @@ class TestBlockdevReopen(iotests.QMPTestCase):
         # We can't reopen hd1 to read-only, as block-stream requires it to be
         # read-write
         self.reopen(opts['backing'], {'read-only': True},
-                    "Cannot make block node read-only, there is a writer on it")
+                    "Read-only block node 'hd1' cannot support read-write users")
 
         # We can't remove hd2 while the stream job is ongoing
         opts['backing']['backing'] = None
-- 
2.29.2



  parent reply	other threads:[~2021-03-17 15:15 UTC|newest]

Thread overview: 62+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-03-17 14:34 [PATCH v3 00/36] block: update graph permissions update Vladimir Sementsov-Ogievskiy
2021-03-17 14:34 ` [PATCH v3 01/36] tests/test-bdrv-graph-mod: add test_parallel_exclusive_write Vladimir Sementsov-Ogievskiy
2021-04-23 12:25   ` Kevin Wolf
2021-04-23 12:46     ` Vladimir Sementsov-Ogievskiy
2021-04-23 12:59       ` Kevin Wolf
2021-04-23 13:03         ` Vladimir Sementsov-Ogievskiy
2021-03-17 14:34 ` [PATCH v3 02/36] tests/test-bdrv-graph-mod: add test_parallel_perm_update Vladimir Sementsov-Ogievskiy
2021-03-17 14:34 ` [PATCH v3 03/36] tests/test-bdrv-graph-mod: add test_append_greedy_filter Vladimir Sementsov-Ogievskiy
2021-03-17 14:34 ` [PATCH v3 04/36] block: bdrv_append(): don't consume reference Vladimir Sementsov-Ogievskiy
2021-04-07 17:46   ` Alberto Garcia
2021-04-23 14:11   ` Kevin Wolf
2021-03-17 14:34 ` [PATCH v3 05/36] block: BdrvChildClass: add .get_parent_aio_context handler Vladimir Sementsov-Ogievskiy
2021-04-12 15:06   ` Alberto Garcia
2021-03-17 14:34 ` [PATCH v3 06/36] block: drop ctx argument from bdrv_root_attach_child Vladimir Sementsov-Ogievskiy
2021-04-12 14:23   ` Alberto Garcia
2021-03-17 14:35 ` [PATCH v3 07/36] block: make bdrv_reopen_{prepare, commit, abort} private Vladimir Sementsov-Ogievskiy via
2021-03-17 14:35 ` [PATCH v3 08/36] util: add transactions.c Vladimir Sementsov-Ogievskiy
2021-04-23 14:43   ` Kevin Wolf
2021-03-17 14:35 ` [PATCH v3 09/36] block: bdrv_refresh_perms: check for parents permissions conflict Vladimir Sementsov-Ogievskiy
2021-04-12 15:57   ` Alberto Garcia
2021-03-17 14:35 ` [PATCH v3 10/36] block: refactor bdrv_child* permission functions Vladimir Sementsov-Ogievskiy
2021-03-17 14:35 ` [PATCH v3 11/36] block: rewrite bdrv_child_try_set_perm() using bdrv_refresh_perms() Vladimir Sementsov-Ogievskiy
2021-03-17 14:35 ` [PATCH v3 12/36] block: inline bdrv_child_*() permission functions calls Vladimir Sementsov-Ogievskiy
2021-03-17 14:35 ` [PATCH v3 13/36] block: use topological sort for permission update Vladimir Sementsov-Ogievskiy
2021-03-17 16:50   ` Alberto Garcia
2021-03-17 14:35 ` [PATCH v3 14/36] block: add bdrv_drv_set_perm transaction action Vladimir Sementsov-Ogievskiy
2021-03-17 14:35 ` [PATCH v3 15/36] block: add bdrv_list_* permission update functions Vladimir Sementsov-Ogievskiy
2021-03-17 14:35 ` [PATCH v3 16/36] block: add bdrv_replace_child_safe() transaction action Vladimir Sementsov-Ogievskiy
2021-03-17 14:35 ` [PATCH v3 17/36] block: fix bdrv_replace_node_common Vladimir Sementsov-Ogievskiy
2021-03-17 14:35 ` [PATCH v3 18/36] block: add bdrv_attach_child_common() transaction action Vladimir Sementsov-Ogievskiy
2021-04-26 16:14   ` Kevin Wolf
2021-04-26 16:56     ` Vladimir Sementsov-Ogievskiy
2021-03-17 14:35 ` [PATCH v3 19/36] block: add bdrv_attach_child_noperm() " Vladimir Sementsov-Ogievskiy
2021-03-17 14:35 ` [PATCH v3 20/36] block: split out bdrv_replace_node_noperm() Vladimir Sementsov-Ogievskiy
2021-03-17 14:35 ` [PATCH v3 21/36] block: adapt bdrv_append() for inserting filters Vladimir Sementsov-Ogievskiy
2021-03-17 14:35 ` [PATCH v3 22/36] block: add bdrv_remove_filter_or_cow transaction action Vladimir Sementsov-Ogievskiy
2021-04-26 16:26   ` Kevin Wolf
2021-04-26 17:18     ` Vladimir Sementsov-Ogievskiy
2021-04-27 11:09       ` Kevin Wolf
2021-04-27 11:41         ` Vladimir Sementsov-Ogievskiy
2021-03-17 14:35 ` [PATCH v3 23/36] block: introduce bdrv_drop_filter() Vladimir Sementsov-Ogievskiy
2021-03-17 14:35 ` [PATCH v3 24/36] block/backup-top: drop .active Vladimir Sementsov-Ogievskiy
2021-03-17 14:35 ` [PATCH v3 25/36] block: drop ignore_children for permission update functions Vladimir Sementsov-Ogievskiy
2021-03-17 14:35 ` [PATCH v3 26/36] block: make bdrv_unset_inherits_from to be a transaction action Vladimir Sementsov-Ogievskiy
2021-03-17 14:35 ` [PATCH v3 27/36] block: make bdrv_refresh_limits() " Vladimir Sementsov-Ogievskiy
2021-03-17 14:35 ` [PATCH v3 28/36] block: add bdrv_set_backing_noperm() " Vladimir Sementsov-Ogievskiy
2021-03-17 14:35 ` [PATCH v3 29/36] block: bdrv_reopen_multiple(): move bdrv_flush to separate pre-prepare Vladimir Sementsov-Ogievskiy
2021-03-17 14:35 ` [PATCH v3 30/36] block: bdrv_reopen_multiple: refresh permissions on updated graph Vladimir Sementsov-Ogievskiy
2021-03-17 14:35 ` [PATCH v3 31/36] block: drop unused permission update functions Vladimir Sementsov-Ogievskiy
2021-03-17 14:35 ` [PATCH v3 32/36] block: inline bdrv_check_perm_common() Vladimir Sementsov-Ogievskiy
2021-03-17 14:35 ` [PATCH v3 33/36] block: inline bdrv_replace_child() Vladimir Sementsov-Ogievskiy
2021-03-17 14:35 ` [PATCH v3 34/36] block: refactor bdrv_child_set_perm_safe() transaction action Vladimir Sementsov-Ogievskiy
2021-03-17 14:35 ` [PATCH v3 35/36] block: rename bdrv_replace_child_safe() to bdrv_replace_child() Vladimir Sementsov-Ogievskiy
2021-03-17 14:35 ` Vladimir Sementsov-Ogievskiy [this message]
2021-03-17 15:21 ` [PATCH v3 00/36] block: update graph permissions update no-reply
2021-03-17 15:38   ` Vladimir Sementsov-Ogievskiy
2021-03-17 17:33     ` Eric Blake
2021-03-17 17:42       ` Eric Blake
2021-03-18  8:04       ` Vladimir Sementsov-Ogievskiy
2021-04-23  8:34         ` Kevin Wolf
2021-04-23 16:16 ` Kevin Wolf
2021-04-28 11:28 ` Kevin Wolf

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210317143529.615584-37-vsementsov@virtuozzo.com \
    --to=vsementsov@virtuozzo.com \
    --cc=armbru@redhat.com \
    --cc=fam@euphon.net \
    --cc=jsnow@redhat.com \
    --cc=kwolf@redhat.com \
    --cc=mreitz@redhat.com \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    --cc=stefanha@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).