Hi! What do you think of this approach to implementing reverting? вт, 31 авг. 2021 г. в 16:46, Nikolay Shirokovskiy < nshirokovs...@virtuozzo.com>:
> 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 >