Re: [Qemu-devel] Combining "loadvm" and "-snapshot"

[Kevin Wolf]

Stephen McCamant schrieb:
>>>>>> "KW" == Kevin Wolf <address@hidden> writes:
> 
> KW> Stephen McCamant schrieb:
> 
> SMcC> I've tried implementing an alternate approach that makes
> SMcC> "loadvm" work under "-snapshot" by finding and loading the VM
> SMcC> state from the backing image, if a snapshot with that name isn't
> SMcC> found in the scratch image.  In some preliminary testing, this
> SMcC> seems to do what I want; you can also make new snapshots with
> SMcC> savevm that live only in the scratch image (and so go away at
> SMcC> the end of the session).
> 
> KW> I think, this is the point where your approach will break. A
> KW> snapshot in the scratch image is still based on a snapshot in the
> KW> backing image. So on loading the scratch snapshot you would also
> KW> need to load the snapshot for the backing file. However, you don't
> KW> know which one because qcow2 can't save a snapshot name for the
> KW> backing file.
> 
> Ah yes, I think I understand the problem you're referring to. For
> instance, if you have two states snap1 and snap2 in the backing image,
> and then you save a snapshot "snap1a" in the scratch image based on
> snap1, then load snap2, then try to load snap1a, the disk will have an
> inconsistent state that's a mixture of snap2 and the differences
> between snap1 and snap1a. By playing around with file and directory
> contents in snapshots (and unmounting and remounting to bypass the
> buffer cache), I can make this show up as corrupted file contents.

Right, this is the scenario I meant.

> I think you could work around this problem by loading the appropriate
> backing disk snapshot first, but the need for that isn't intuitive, so
> I can see it easily tripping users up. (I presume you can get into
> similar trouble if you modify a backing image that a persistent
> scratch image is pointing to, though in that case it's clearer what
> you're doing wrong.)

We really should avoid to even make it possible to get in such states.
It means that at least the scratch image will be horribly corrupted
(which is even worse if you don't limit this scenario to -snapshot). I
really don't want to imagine what happens when you commit to the backing
file then...

> As you suggest, the right fix would seem to be to have the scratch
> disk snapshots remember what backing disk state they're based on,
> though there's no field for that in the current QCOW2 format. It looks
> like there is some space left for expansion at the end of the snapshot
> structure, though that would be a bigger change. Or if you limit
> yourself to the "-snapshot" case, and not persistent backed disk
> images, it would be enough to keep the information in memory.

Yes, that should work in any case. Only being able to use it with
-snapshot is a major limitation, though.

But you're right that there seems to be this extra_data_size field in
the snapshot structure that I completely missed until now. I don't know
if it's respected everywhere, but I think this could be worth a try. If
it works this would be a really nice feature.

Kevin