qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [RFC V3 6/7] block: Create authorizations mechanism for


From: Fam Zheng
Subject: Re: [Qemu-devel] [RFC V3 6/7] block: Create authorizations mechanism for external snapshots.
Date: Wed, 04 Dec 2013 11:47:22 +0800
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.1.1

On 2013年12月03日 21:26, Benoît Canet wrote:
---
  block.c                   | 64 +++++++++++++++++++++++++++++++++++++++++------
  block/blkverify.c         |  2 +-
  include/block/block.h     | 16 +++++++++---
  include/block/block_int.h |  9 ++++---
  4 files changed, 75 insertions(+), 16 deletions(-)

diff --git a/block.c b/block.c
index 8016ff2..0569cb2 100644
--- a/block.c
+++ b/block.c
@@ -4945,21 +4945,69 @@ int bdrv_amend_options(BlockDriverState *bs, 
QEMUOptionParameter *options)
      return bs->drv->bdrv_amend_options(bs, options);
  }

-ExtSnapshotPerm bdrv_check_ext_snapshot(BlockDriverState *bs)
+/* will be used to recurse on single child block filter until first format
+ * (single child block filter will store their child in bs->file)
+ */
+ExtSnapshotPerm bdrv_generic_check_ext_snapshot(BlockDriverState *bs,
+                                                BlockDriverState *candidate)
  {
-    if (bs->drv->bdrv_check_ext_snapshot) {
-        return bs->drv->bdrv_check_ext_snapshot(bs);
+    if (!bs->drv) {
+        return EXT_SNAPSHOT_FORBIDDEN;
      }

-    if (bs->file && bs->file->drv && bs->file->drv->bdrv_check_ext_snapshot) {
-        return bs->file->drv->bdrv_check_ext_snapshot(bs);
+    if (!bs->drv->authorizations[BS_CANT_SNAPSHOT]) {

This double negative feels hard to read for me.

+        if (bs == candidate) {
+             return EXT_SNAPSHOT_ALLOWED;
+        } else {
+             return EXT_SNAPSHOT_FORBIDDEN;
+        }
      }

-    /* external snapshots are allowed by default */
-    return EXT_SNAPSHOT_ALLOWED;
+    if (!bs->drv->authorizations[BS_FILTER_PASS_DOWN]) {
+        return EXT_SNAPSHOT_FORBIDDEN;
+    }
+
+    if (!bs->file) {
+        return EXT_SNAPSHOT_FORBIDDEN;
+    }
+
+    return bdrv_recurse_check_ext_snapshot(bs->file, candidate);
  }

-ExtSnapshotPerm bdrv_check_ext_snapshot_forbidden(BlockDriverState *bs)
+ExtSnapshotPerm bdrv_recurse_check_ext_snapshot(BlockDriverState *bs,
+                                                BlockDriverState *candidate)
  {
+    if (bs->drv && bs->drv->bdrv_check_ext_snapshot) {
+        return bs->drv->bdrv_check_ext_snapshot(bs, candidate);
+    }

Maybe I'm missing something, but if a driver always returns positive permit, despite of what candidate is (or even it's relevant to bs), then doesn't it also affect other devices? because...

+
+    return bdrv_generic_check_ext_snapshot(bs, candidate);
+}
+
+/* This function check if the candidate bs has snapshots authorized by going
+ * down the forest of bs, skipping filters and stopping on the the first bses
+ * authorizing snapshots
+ */
+ExtSnapshotPerm bdrv_check_ext_snapshot(BlockDriverState *candidate)
+{
+    BlockDriverState *bs;
+
+    /* walk down the bs forest recursively */
+    QTAILQ_FOREACH(bs, &bdrv_states, device_list) {

this iterates through all the known graph trees (device_list), instead of limiting to only the device that candidate belongs to.

Why not just check candidate's permission bitmap and go down from it? If an ancestor need to disable its descendants, it could simply set permission bits of its children and recurse down.

Fam



reply via email to

[Prev in Thread] Current Thread [Next in Thread]