Hello Eric,
thank you for the long description of <active>
Am Mittwoch 10 August 2011 14:18:22 schrieb Eric Blake:
Right now, qemu's _only_ use of <active> (which is set to
non-zero by
qemuDomainSnapshotSetCurrentActive) is to tell qemuBuildCommandLine to
use -loadvm to revert to that point, rather than to do a fresh boot.
Your approach of keeping the call means that qemuBuildCommandLine has to
special-case to not use -loadvm for inactive snapshots; whereas my
approach of no longer marking the snapshot active for inactive snapshots
means that the existing qemuBuildCommandLine no longer sees an active
snapshot in the first place; both approaches end up omitting the -loadvm
call on the next domain boot, but mine does it with less code and
without using <active>.
Since <active> is persisted in the snapshot/ directory, I didn't want to
change that, so I didn't remove that call.
Once that is done, qemu is no longer using active for its own
purposes,
so conf/domain_conf can then take over <active> to uniformly identify
which snapshot is current as part of the snapshot xml (that is, all but
one snapshot will be <active>0</active>, and the current snapshot will
be <active>1</active>). Every time the current snapshot changes, ...
I find the name "current" highly confusing, because at least for Qemus
internal shapshot it is something more like "last snapshot restored": As soon
as the VM begins to run, it diverges from the snapshot state and is no longer
equal to it, so it is not "current" any more. Only when restoring offline or
paused states would I have the chance to realistically see the VM being in
the same state as the snapshot.
t branch.
So if <active> is only used to track the last restored snapshot and to fill in
the <parent> data in the <domsinsnapshot> data, your patch looks fine.
Sincerely
Philipp Hahn
--
Philipp Hahn Open Source Software Engineer hahn(a)univention.de
Univention GmbH Linux for Your Business fon: +49 421 22 232- 0
Mary-Somerville-Str.1 D-28359 Bremen fax: +49 421 22 232-99
http://www.univention.de/