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

[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