RFC: revert to external snapshot API

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