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