[Qemu-devel] [PATCH 6/7] block: Add *loosen_restrictions to *check*_perm()

[Max Reitz]

This patch makes three functions report whether the necessary permission
change purely loosens restrictions or not.  These functions are:
- bdrv_check_perm()
- bdrv_check_update_perm()
- bdrv_child_check_perm()

Callers can use this result to decide whether a failure is fatal or not
(see the next patch).

Signed-off-by: Max Reitz <address@hidden>
---
 block.c | 81 +++++++++++++++++++++++++++++++++++++++++++++------------
 1 file changed, 65 insertions(+), 16 deletions(-)

diff --git a/block.c b/block.c
index 21e4514426..105866d15a 100644
--- a/block.c
+++ b/block.c
@@ -1686,9 +1686,12 @@ static int bdrv_fill_options(QDict **options, const char 
*filename,
 
 static int bdrv_child_check_perm(BdrvChild *c, BlockReopenQueue *q,
                                  uint64_t perm, uint64_t shared,
-                                 GSList *ignore_children, Error **errp);
+                                 GSList *ignore_children,
+                                 bool *loosen_restrictions, Error **errp);
 static void bdrv_child_abort_perm_update(BdrvChild *c);
 static void bdrv_child_set_perm(BdrvChild *c, uint64_t perm, uint64_t shared);
+static void bdrv_get_cumulative_perm(BlockDriverState *bs, uint64_t *perm,
+                                     uint64_t *shared_perm);
 
 typedef struct BlockReopenQueueEntry {
      bool prepared;
@@ -1759,18 +1762,37 @@ static void bdrv_child_perm(BlockDriverState *bs, 
BlockDriverState *child_bs,
  * permissions of all its parents. This involves checking whether all necessary
  * permission changes to child nodes can be performed.
  *
+ * Will set *loosen_restrictions to true if and only if no new permissions have
+ * to be taken and no existing shared permissions are to be unshared.  In this
+ * case, errors are not fatal, as long as the caller accepts that the
+ * restrictions remain tighter than they need to be.  The caller still has to
+ * abort the transaction.
+ *
  * A call to this function must always be followed by a call to bdrv_set_perm()
  * or bdrv_abort_perm_update().
  */
 static int bdrv_check_perm(BlockDriverState *bs, BlockReopenQueue *q,
                            uint64_t cumulative_perms,
                            uint64_t cumulative_shared_perms,
-                           GSList *ignore_children, Error **errp)
+                           GSList *ignore_children,
+                           bool *loosen_restrictions, Error **errp)
 {
     BlockDriver *drv = bs->drv;
     BdrvChild *c;
     int ret;
 
+    if (loosen_restrictions) {
+        uint64_t current_perms, current_shared;
+        uint64_t added_perms, removed_shared_perms;
+
+        bdrv_get_cumulative_perm(bs, &current_perms, &current_shared);
+
+        added_perms = cumulative_perms & ~current_perms;
+        removed_shared_perms = current_shared & ~cumulative_shared_perms;
+
+        *loosen_restrictions = !added_perms && !removed_shared_perms;
+    }
+
     /* Write permissions never work with read-only images */
     if ((cumulative_perms & (BLK_PERM_WRITE | BLK_PERM_WRITE_UNCHANGED)) &&
         !bdrv_is_writable_after_reopen(bs, q))
@@ -1798,11 +1820,16 @@ static int bdrv_check_perm(BlockDriverState *bs, 
BlockReopenQueue *q,
     /* Check all children */
     QLIST_FOREACH(c, &bs->children, next) {
         uint64_t cur_perm, cur_shared;
+        bool child_loosen_restr;
+
         bdrv_child_perm(bs, c->bs, c, c->role, q,
                         cumulative_perms, cumulative_shared_perms,
                         &cur_perm, &cur_shared);
         ret = bdrv_child_check_perm(c, q, cur_perm, cur_shared,
-                                    ignore_children, errp);
+                                    ignore_children, &child_loosen_restr, 
errp);
+        if (loosen_restrictions) {
+            *loosen_restrictions &= child_loosen_restr;
+        }
         if (ret < 0) {
             return ret;
         }
@@ -1926,12 +1953,20 @@ char *bdrv_perm_names(uint64_t perm)
  * set, the BdrvChild objects in this list are ignored in the calculations;
  * this allows checking permission updates for an existing reference.
  *
+ * Will set *loosen_restrictions to true if and only if no new permissions have
+ * to be taken and no existing shared permissions are to be unshared.  In this
+ * case, errors are not fatal, as long as the caller accepts that the
+ * restrictions remain tighter than they need to be.  The caller still has to
+ * abort the transaction.
+ *
  * Needs to be followed by a call to either bdrv_set_perm() or
  * bdrv_abort_perm_update(). */
 static int bdrv_check_update_perm(BlockDriverState *bs, BlockReopenQueue *q,
                                   uint64_t new_used_perm,
                                   uint64_t new_shared_perm,
-                                  GSList *ignore_children, Error **errp)
+                                  GSList *ignore_children,
+                                  bool *loosen_restrictions,
+                                  Error **errp)
 {
     BdrvChild *c;
     uint64_t cumulative_perms = new_used_perm;
@@ -1948,6 +1983,11 @@ static int bdrv_check_update_perm(BlockDriverState *bs, 
BlockReopenQueue *q,
         if ((new_used_perm & c->shared_perm) != new_used_perm) {
             char *user = bdrv_child_user_desc(c);
             char *perm_names = bdrv_perm_names(new_used_perm & 
~c->shared_perm);
+
+            if (loosen_restrictions) {
+                *loosen_restrictions = false;
+            }
+
             error_setg(errp, "Conflicts with use by %s as '%s', which does not 
"
                              "allow '%s' on %s",
                        user, c->name, perm_names, bdrv_get_node_name(c->bs));
@@ -1959,6 +1999,11 @@ static int bdrv_check_update_perm(BlockDriverState *bs, 
BlockReopenQueue *q,
         if ((c->perm & new_shared_perm) != c->perm) {
             char *user = bdrv_child_user_desc(c);
             char *perm_names = bdrv_perm_names(c->perm & ~new_shared_perm);
+
+            if (loosen_restrictions) {
+                *loosen_restrictions = false;
+            }
+
             error_setg(errp, "Conflicts with use by %s as '%s', which uses "
                              "'%s' on %s",
                        user, c->name, perm_names, bdrv_get_node_name(c->bs));
@@ -1972,19 +2017,21 @@ static int bdrv_check_update_perm(BlockDriverState *bs, 
BlockReopenQueue *q,
     }
 
     return bdrv_check_perm(bs, q, cumulative_perms, cumulative_shared_perms,
-                           ignore_children, errp);
+                           ignore_children, loosen_restrictions, errp);
 }
 
 /* Needs to be followed by a call to either bdrv_child_set_perm() or
  * bdrv_child_abort_perm_update(). */
 static int bdrv_child_check_perm(BdrvChild *c, BlockReopenQueue *q,
                                  uint64_t perm, uint64_t shared,
-                                 GSList *ignore_children, Error **errp)
+                                 GSList *ignore_children,
+                                 bool *loosen_restrictions, Error **errp)
 {
     int ret;
 
     ignore_children = g_slist_prepend(g_slist_copy(ignore_children), c);
-    ret = bdrv_check_update_perm(c->bs, q, perm, shared, ignore_children, 
errp);
+    ret = bdrv_check_update_perm(c->bs, q, perm, shared, ignore_children,
+                                 loosen_restrictions, errp);
     g_slist_free(ignore_children);
 
     if (ret < 0) {
@@ -2037,7 +2084,7 @@ int bdrv_child_try_set_perm(BdrvChild *c, uint64_t perm, 
uint64_t shared,
 {
     int ret;
 
-    ret = bdrv_child_check_perm(c, NULL, perm, shared, NULL, errp);
+    ret = bdrv_child_check_perm(c, NULL, perm, shared, NULL, NULL, errp);
     if (ret < 0) {
         bdrv_child_abort_perm_update(c);
         return ret;
@@ -2223,7 +2270,8 @@ static void bdrv_replace_child(BdrvChild *child, 
BlockDriverState *new_bs)
          * because we're just taking a parent away, so we're loosening
          * restrictions. */
         bdrv_get_cumulative_perm(old_bs, &perm, &shared_perm);
-        bdrv_check_perm(old_bs, NULL, perm, shared_perm, NULL, &error_abort);
+        bdrv_check_perm(old_bs, NULL, perm, shared_perm, NULL,
+                        NULL, &error_abort);
         bdrv_set_perm(old_bs, perm, shared_perm);
     }
 }
@@ -2237,7 +2285,8 @@ BdrvChild *bdrv_root_attach_child(BlockDriverState 
*child_bs,
     BdrvChild *child;
     int ret;
 
-    ret = bdrv_check_update_perm(child_bs, NULL, perm, shared_perm, NULL, 
errp);
+    ret = bdrv_check_update_perm(child_bs, NULL, perm, shared_perm, NULL, NULL,
+                                 errp);
     if (ret < 0) {
         bdrv_abort_perm_update(child_bs);
         return NULL;
@@ -3292,7 +3341,7 @@ int bdrv_reopen_multiple(BlockReopenQueue *bs_queue, 
Error **errp)
     QSIMPLEQ_FOREACH(bs_entry, bs_queue, entry) {
         BDRVReopenState *state = &bs_entry->state;
         ret = bdrv_check_perm(state->bs, bs_queue, state->perm,
-                              state->shared_perm, NULL, errp);
+                              state->shared_perm, NULL, NULL, errp);
         if (ret < 0) {
             goto cleanup_perm;
         }
@@ -3304,7 +3353,7 @@ int bdrv_reopen_multiple(BlockReopenQueue *bs_queue, 
Error **errp)
                             state->perm, state->shared_perm,
                             &nperm, &nshared);
             ret = bdrv_check_update_perm(state->new_backing_bs, NULL,
-                                         nperm, nshared, NULL, errp);
+                                         nperm, nshared, NULL, NULL, errp);
             if (ret < 0) {
                 goto cleanup_perm;
             }
@@ -4031,7 +4080,7 @@ void bdrv_replace_node(BlockDriverState *from, 
BlockDriverState *to,
 
     /* Check whether the required permissions can be granted on @to, ignoring
      * all BdrvChild in @list so that they can't block themselves. */
-    ret = bdrv_check_update_perm(to, NULL, perm, shared, list, errp);
+    ret = bdrv_check_update_perm(to, NULL, perm, shared, list, NULL, errp);
     if (ret < 0) {
         bdrv_abort_perm_update(to);
         goto out;
@@ -4378,7 +4427,7 @@ int bdrv_drop_intermediate(BlockDriverState *top, 
BlockDriverState *base,
         /* Check whether we are allowed to switch c from top to base */
         GSList *ignore_children = g_slist_prepend(NULL, c);
         ret = bdrv_check_update_perm(base, NULL, c->perm, c->shared_perm,
-                                     ignore_children, &local_err);
+                                     ignore_children, NULL, &local_err);
         g_slist_free(ignore_children);
         if (ret < 0) {
             error_report_err(local_err);
@@ -5153,7 +5202,7 @@ static void coroutine_fn 
bdrv_co_invalidate_cache(BlockDriverState *bs,
      */
     bs->open_flags &= ~BDRV_O_INACTIVE;
     bdrv_get_cumulative_perm(bs, &perm, &shared_perm);
-    ret = bdrv_check_perm(bs, NULL, perm, shared_perm, NULL, &local_err);
+    ret = bdrv_check_perm(bs, NULL, perm, shared_perm, NULL, NULL, &local_err);
     if (ret < 0) {
         bs->open_flags |= BDRV_O_INACTIVE;
         error_propagate(errp, local_err);
@@ -5303,7 +5352,7 @@ static int bdrv_inactivate_recurse(BlockDriverState *bs)
 
     /* Update permissions, they may differ for inactive nodes */
     bdrv_get_cumulative_perm(bs, &perm, &shared_perm);
-    bdrv_check_perm(bs, NULL, perm, shared_perm, NULL, &error_abort);
+    bdrv_check_perm(bs, NULL, perm, shared_perm, NULL, NULL, &error_abort);
     bdrv_set_perm(bs, perm, shared_perm);
 
 
-- 
2.20.1