<meta http-equiv="Content-Type" content="text/html; charset=utf-8"><div dir="ltr"><div dir="ltr"><div class="gmail_default" style="font-size:large"><br></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">ср, 8 сент. 2021 г. в 12:27, Peter Krempa <<a href="mailto:pkrempa@redhat.com">pkrempa@redhat.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">On Tue, Aug 31, 2021 at 16:46:25 +0300, Nikolay Shirokovskiy wrote:<br>
> Hi, all.<br>
<br>
Hi, sorry for the late reply I was on PTO.<br>
<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<br>
> 2018.<br>
> Looks like it was not clear how the API should look like.<br>
<br>
One additional thing is that me and phrdina started discussing this (in<br>
person so I can't point you to a discussion) 2 weeks ago.<br>
<br>
I'll summarize the points we agreed upon.<br>
<br>
> For example we have disk.qcow2 <- disk.snap1 chain <- disk.snap2 chain<br>
> (after<br>
> having snap1 and snap2 snapshots). Now we want to revert to snap1 snapshot.<br>
<br>
There's one implementation snag we currently have which complicates<br>
stuff. Let's expand your above scenario with the snapshot states:<br>
<br>
<br>
                           s                      s<br>
                           n                      n<br>
                           a                      a<br>
                           p                      p<br>
                           1                      2<br>
                                                                          p<br>
o                          │                      │                       r<br>
r    base.qcow2            │    snap1.qcow2       │    snap2.qcow2        e<br>
i ───────────────────────► │ ────────────────────►│ ────────────────────► s<br>
g                          │                      │                       e<br>
i                          │                      │                       n<br>
n                                                                         t<br>
<br>
A rather big set of problems which we will necessarily encounter when<br>
implementing this comes from the following:<br>
<br>
1) users can modify the VM betwen snapshots or between the last snapshot<br>
and the abandoned state and this modification (e.g. changing a disk<br>
image) must be considered to prevent data loss when manipulating the<br>
images.<br></blockquote><div><br></div><div class="gmail_default" style="font-size:large">Hi, Peter!</div><div><br></div><div><div class="gmail_default" style="font-size:large">Can you please explain in more detail what the issue is? For example if</div><div class="gmail_default" style="font-size:large">after snap1 I changed disk image to some base2.qcow2 then snap1.qcow2 is </div><div class="gmail_default" style="font-size:large">expected to be managed by mgmt (expected to be deleted I guess). Then I made</div><div class="gmail_default" style="font-size:large">a snap2. Now I have base.qcow2 (keeping snap1 state) and  base2.qcow2 (keeping snap2 state) <-snap2.qcow2</div><div class="gmail_default" style="font-size:large">chain. Thanks to metadata I can revert to both snap1 and snap2 as I know the domain</div><div class="gmail_default" style="font-size:large">xml in those states and these xmls references right images (snap1 references base.qcow2</div><div class="gmail_default" style="font-size:large">and snap2 references base2.qcow2).</div></div><span class="gmail-im" style="color:rgb(80,0,80)"><div><span style="font-size:large;color:rgb(34,34,34)"></span><br></div></span><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
2) libvirt doesn't record the XML used to create the snapshot (e.g.<br></blockquote><div> </div><div><div class="gmail_default" style="font-size:large">It is not quite so. According to <a href="https://libvirt.org/formatsnapshot.html#example" target="_blank">https://libvirt.org/formatsnapshot.html#example</a></div><div class="gmail_default" style="font-size:large">xml given to create API saved in snapshot metadata. Or I miss something.</div><br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
snap1.xm) thus we don't actually know what the (historical) snapshot was<br>
actually recording. Coupled with the fact that the user could have<br>
changed the VM definition between 'snap1' and 'snap2' we can't even<br>
infer what the state was.<br></blockquote><div><br></div><div><div class="gmail_default" style="font-size:large">Here too I cannot follow what the issue is. Can you please give an example?</div></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
<br>
> 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>
Exactly. Reversion of external snapshots will necessarily require a new<br>
API, which will take a new "snapshot" XML describing the new overlays as<br>
you describe below.<br>
<br>
In the simple case such as with local files we can use the same<br>
algorithm for creating overlay filenames as we do when creating<br>
snapshots but generally we need to give the MGMT the ability to specify<br>
the overlay name.<br>
<br>
<br>
> The [1] has some discussion on adding extra options to reverting<br>
> functionality.<br>
> Like for example if we revert back to snap2 then having the ability to run<br>
> from<br>
> state in disk.snap2 rather than disk.snap1. My opinion is there is no need<br>
> to<br>
> as if one wants to revert to the state of disk2.snap2 it can take a<br>
> snapshot (some<br>
> snap3).<br>
<br>
It's possible to avoid doing a combined "take snapshot&revert" operation<br>
as long as we have the possibility to take a snapshot and destroy the<br>
VM as running it after the point you want to return to is undesirable<br>
and pointless.<br>
<br>
One thing that you need to consider here is that when you are reverting<br>
to an arbitrary snapshot, the overlay files after the last snapshot<br>
(e.g. snap2.qcow2 (in my diagram), which is the state between snap2 and<br>
present) are abandoned and will never be used again.<br>
<br>
At this point we disucssed that we should be removing those as<br>
semantically it's the only point where we can do that as we know the<br>
state of the VM which will be abandoned.<br>
<br>
The problem lies in the fact that between 'snap2' and present the user<br>
could have exchanged disks and then we don't have enough metadata to<br>
create the overlays.<br>
<br>
One big additional caveat is that if the user exchanged disk images we<br>
_must not_ delete the changed image, so the metadata wrangling might be<br>
non-trivial in this case.<br>
<br>
> 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>
<br>
That is an acceptable caveat for the user. As noted above as long as you<br>
can take a snapshot & atomicaly destroy the VM it's acceptable.<br>
<br>
For libvirt it's harder a bit as described above especially if we don't<br>
want to keep litering the disk with unused and invalid images.<br>
<br>
> The [2] takes an approach of reusing current top image as overlay on revert<br>
> 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>
No in our discussion with Pavel  we've ruled out all special cases as<br>
too much hassle to implement with little benefit.<br>
<br>
If a user wants to rever&abandon some state, they can do 2 operations of<br>
reverting and then deleting the unwanted snapshots.<br>
<br>
> > The [3] suggests a different scheme for naming images. For example after<br>
> 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>
<br>
IMO we can't really change the naming any more, users baked scripts on<br>
top of this. Using a flag is also mostly pointless as users will keep<br>
creating snapshots without it. Thus I don't think this is a viable<br>
approach. The only advantage it would have that it would be simpler to<br>
vizualize what happened, but ideally users would not need to care about<br>
the steps and filenames at all.<br>
<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>
Yup, too much hassle, questionable outcome. Not worth it.<br>
<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 flag<br>
<br>
I was briefly considering that the new snapshot api should also have<br>
provisions for specifying a snapshot XML to take the snapshot of the<br>
abandoned state, but as discussed it's not really necessary if you can<br>
do it in 2 steps.<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>
I don't see any other option. In case you have e.g. networked storage<br>
where we can't figure out stuff by ourselves you basically don't have<br>
other option.<br>
<br>
> If overlay name is omitted the generated name will be like disk.snap1-1 in<br>
> the<br>
> above example to be in alignment with creating a snapshot case. So that<br>
> disk.snap1*<br>
> images hold states derived from snap1 state. We can also support reverting<br>
> to<br>
> external snapshot thru existing virDomainRevertToSnapshot for those who<br>
> rely on<br>
> generated names using this naming scheme.<br>
<br>
Yes, this would be possible, but quite fragile. But with regular<br>
snapshots without any magic it should work.<br>
<br>
<br>
One thing you've missed though is that deletion of snapshots now becomes<br>
quite a challenge.<br></blockquote><div><br></div><div><div class="gmail_default" style="font-size:large">Yeah I did not consider implementing deleting at that moment. However to make</div><div class="gmail_default" style="font-size:large">external snapshots usable it should be implemented as well.</div><div class="gmail_default" style="font-size:large"><br></div><div class="gmail_default" style="font-size:large">Anyway is anybody in Red Hat working on this (considering you and Pavel discussed</div><div class="gmail_default" style="font-size:large">the topic recently)? If not I would like to proceed with implementation.</div><div class="gmail_default" style="font-size:large"><br></div><div class="gmail_default" style="font-size:large">Nikolay</div><br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
<br>
                     s              s              s<br>
                     n              n              n<br>
                     a              a              a<br>
                     p              p              p<br>
                     1              2              3<br>
<br>
o                    │              │              │<br>
r    base.qcow2      │  snap1.qcow2 │  snap2.qcow2 │<br>
i ─────────────────► │ ────────────►│ ────────────►│<br>
g                    │              │              │<br>
i                    │              │              │<br>
n                    │<br>
                     │                                    p<br>
                     │                                    r<br>
                     │  alternatehistory.qcow2            e<br>
                     │ ────────────────────────────────►  s<br>
                     │                                    e<br>
                     │                                    n<br>
                                                          t<br>
<br>
Specifically the premise is that we don't want to keep unnecessary<br>
images (e.g. the trivial solution for deleting 'snap1' would be to<br>
remove the metadata and keep base.qcow2 intact) as without the metadata<br>
it will become very hard to determine which images are actually still<br>
used.<br>
<br>
So in the above scenario:<br>
<br>
- User wants to delete snap3:<br>
  'snap2.qcow2' overlay becomes unreachable and should be deleted.<br>
<br>
- User wants to delete snap2:<br>
  There are 2 options in this case:<br>
    1) block-pull snap1 into snap2<br>
    2) block-commit snap2 into snap1<br>
<br>
  The optimal solution is the one that transfers less data, but in the<br>
  end either of them is equally good in terms of saved storage.<br>
<br>
- User wants to delete snap1:<br>
  This is a bit harder:<br>
   1) commit into base.qcow2 would invalidate 'alternatehistory.qcow2'<br>
   2) pull of base into both snap1.qcow2 and alternatehistory.qcow2 is<br>
      possible, but may yield big images<br>
   3) the last option is to forbid deletion of snapshots that has<br>
      alternate histories<br>
<br>
The first thing we should do though is to forbid creation of mixed trees<br>
of internal and external snapshots because the matrix of stuff to handle<br>
would explode way beyond this already rather complex situation.<br>
<br>
</blockquote></div></div>