qemu-devel
[Top][All Lists]
Advanced

[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:20:12 +0800
User-agent: Mozilla/5.0 (Windows NT 5.1; rv:17.0) Gecko/20130509 Thunderbird/17.0.6

于 2013-5-27 23:18, 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
needed.

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>

I am not totally convinced by this approach, especially when it's nowhere
documented that the order of BDSes in the list is important. However, I
think the current code is suboptimal as well, so I'll apply this for
now.

What we really want is this: savevm lets you choose which image to save
the VM state to, and if you don't specify one, it automatically picks
one like today. loadvm checks all images and loads the VM state from the
image that has the VM state for this snapshot. If loadvm finds that it's
not exactly one image that has a VM state, this is an error condition.

Is anyone interested in implementing this?

Kevin

  I think that can came up after Pavel's savevm transaction, which
add parameter telling which image to save vmstate. This patch simply
keep what it is now not touching that part.



--
Best Regards

Wenchao Xia




reply via email to

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