[Top][All Lists]

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

Re: [Qemu-devel] [PATCH V3 1/4] block: drop bs_snapshots global variable

From: Wenchao Xia
Subject: Re: [Qemu-devel] [PATCH V3 1/4] block: drop bs_snapshots global variable
Date: Tue, 28 May 2013 10:24:26 +0800
User-agent: Mozilla/5.0 (Windows NT 5.1; rv:17.0) Gecko/20130509 Thunderbird/17.0.6

于 2013-5-27 23:25, Kevin Wolf 写道:
Am 25.05.2013 um 05:09 hat Wenchao Xia geschrieben:
From: Stefan Hajnoczi <address@hidden>

The bs_snapshots global variable points to the BlockDriverState which
will be used to save vmstate.  This is really a savevm.c concept but was
moved into block.c:bdrv_snapshots() when it became clear that hotplug
could result in a dangling pointer.

While auditing the block layer's global state I came upon bs_snapshots
and realized that a variable is not necessary here.  Simply find the
first BlockDriverState capable of internal snapshots each time this is

The behavior of bdrv_snapshots() is preserved across hotplug because new
drives are always appended to the bdrv_states list.  This means that
calling the new find_vmstate_bs() function is idempotent - it returns
the same BlockDriverState unless it was hot-unplugged.

Signed-off-by: Stefan Hajnoczi <address@hidden>
Reviewed-by: Eric Blake <address@hidden>
Reviewed-by: Wenchao Xia <address@hidden>
Signed-off-by: Wenchao Xia <address@hidden>
  block.c               |   28 ----------------------------
  include/block/block.h |    1 -
  savevm.c              |   19 +++++++++++++++----
  3 files changed, 15 insertions(+), 33 deletions(-)

diff --git a/block.c b/block.c
index 3f87489..478a3b2 100644
--- a/block.c
+++ b/block.c
@@ -99,9 +99,6 @@ static QTAILQ_HEAD(, BlockDriverState) bdrv_states =
  static QLIST_HEAD(, BlockDriver) bdrv_drivers =

-/* The device to use for VM snapshots */
-static BlockDriverState *bs_snapshots;
  /* If non-zero, use only whitelisted block drivers */
  static int use_bdrv_whitelist;

@@ -1357,9 +1354,6 @@ void bdrv_close(BlockDriverState *bs)
      notifier_list_notify(&bs->close_notifiers, bs);

      if (bs->drv) {
-        if (bs == bs_snapshots) {
-            bs_snapshots = NULL;
-        }
          if (bs->backing_hd) {
              bs->backing_hd = NULL;
@@ -1591,7 +1585,6 @@ void bdrv_delete(BlockDriverState *bs)


-    assert(bs != bs_snapshots);

@@ -1635,9 +1628,6 @@ void bdrv_set_dev_ops(BlockDriverState *bs, const 
BlockDevOps *ops,
      bs->dev_ops = ops;
      bs->dev_opaque = opaque;
-    if (bdrv_dev_has_removable_media(bs) && bs == bs_snapshots) {
-        bs_snapshots = NULL;
-    }

This hunk isn't replaced by any other code. If I understand correctly
what it's doing, it prevented you from saving the VM state to a
removable device, which would be allowed after this patch.

Is this a change we want to make? Why isn't it described in the commit
  How about adding it back in find_vmstate_bs()?


Best Regards

Wenchao Xia

reply via email to

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