Hi! What do you think of this approach to implementing reverting?
вт, 31 авг. 2021 г. в 16:46, Nikolay Shirokovskiy <nshirokovskiy@xxxxxxxxxxxxx>:
Hi, all.
I want to implement reverting to external snapshot functionality.
I've checked the mailing list history and found some previous attempts
(not sure if this is a complete list of them).
[1] was done in 2012 by the Redhat team itself. [2] and [3] was done in 2018.
Looks like it was not clear how the API should look like.
For example we have disk.qcow2 <- disk.snap1 chain <- disk.snap2 chain (after
having snap1 and snap2 snapshots). Now we want to revert to snap1 snapshot. The
snapshot state is held by disk.qcow2 image. We can run reverted domain on
disk.qcow2 itself but then we lose snap1 (held by disk.qcow2) and snap2
(held by disk.snap1). So we definitely should run on some overlay over
disk.qcow2. But then what name should be given to overlay? We should have
an option for mgmt to specify this name like in case of snapshots itself.
The [1] has some discussion on adding extra options to reverting functionality.
Like for example if we revert back to snap2 then having the ability to run from
state in disk.snap2 rather than disk.snap1. My opinion is there is no need to
as if one wants to revert to the state of disk2.snap2 it can take a snapshot (some
snap3). At the same time one needs to be aware that revert operation loses
current state and later one can revert only to the states of snapshots.
This is the way internal snapshots work and the way one expects external
snapshots to work too.The [2] takes an approach of reusing current top image as overlay on revert so
that in the above example disk.snap2 will be overlay over disk.qcow2 on
reverting to snap1 snapshot. IMHO this is a confusing naming scheme.
The [3] suggests a different scheme for naming images. For example after taking
snapshot snap1 the chain should be disk.snap1 <- disk.qcow2 which looks very
appealing to me. With this naming using the [2] approach is quite natural.
Implementing this does not look hard even for a running domain but this is
a big change to API and all mgmt need to be aware of (well it could be done
safely using a new flag).
Anyway we can go on with current image names. In order to specify overlay
image name let's introduce new API:
int virDomainRevertToSnapshotXML(virDomainSnapshotPtr snapshot,
char *xmlDesc,
unsigned int flags)
with XML like:
<domainsnapshotrevert>
<disks>
<disk name='vda'>
<source file='/path/to/revert/overlay'/>
</disk>
</disks>
</domainsnapshotrevert>
Having an XML looks like a bit overkill right now but I could not
find a better solution.
If overlay name is omitted the generated name will be like disk.snap1-1 in the
above example to be in alignment with creating a snapshot case. So that disk.snap1*
images hold states derived from snap1 state. We can also support reverting to
external snapshot thru existing virDomainRevertToSnapshot for those who rely on
generated names using this naming scheme.[1] Re: [PATCHv4 4/4] snapshot: qemu: Implement reverting of external snapshots
https://listman.redhat.com/archives/libvir-list/2012-November/msg00932.html
[2] Re: [PATCH 1/5] snapshot: Implement reverting for external disk snapshots
https://listman.redhat.com/archives/libvir-list/2018-November/msg00338.html
[3] External disk snapshot paths and the revert operation
https://listman.redhat.com/archives/libvir-list/2018-October/msg01110.html
