[pve-devel] [PATCH v3 pve-storage 1/3] qcow2: add external snapshot support

[Fabian Grünbichler]

> DERUMIER, Alexandre <alexandre.derumier at groupe-cyllene.com> hat am 10.01.2025 12:51 CET geschrieben:
> > > + } else {
> > > +     print"commit $snappath\n";
> > > +     $cmd = ['/usr/bin/qemu-img', 'commit', $snappath];
> > 
> > > > leftover from previous version? not used/overwritten below ;)
> > 
> > no, this is really to commit the the snapshot to parent
> 
> >>but it is not executed..
> 
> Ah, ok ! sorrry ! I think I have dropped some code during rebase before
> sending patches, because I had tested it a lot of time !
> 
> 
> 
> > this is for this usecase :
> > 
> > A<----B<----C.
> > 
> > you commit B to A,  then you need to change the backing file of C to
> > A
> > (instead B)
> > 
> > A<----C
> 
> >>but this is the wrong semantics.. the writes/delta in B need to go to
> >>C (they happened after A), not to A!
> 
> I think they can go to A (commit) or C (stream)
> 
> here an example:
> 
> current (1TB)
>  - take snap A
> 
> (A (1TB)<------new current 500MB (backing file A))
> 
> - take snap B
> 
> (A (1TB)<------B 500MB (backingfile A)<------new current 10MB
> (backingfile B))
> 
> 
> Then, you want to delete B.
> 
> 
> so, you stream it to current. (so copy 500MB to current in this
> example)
> 
> Then, you want to delete snapshot A
> you don't want stream A to current, because A is the big initial image.
> So, instead, you need to commit the current to A (with the extra 500MB)
> 
> 
> So, if you have a lot of snapshot to delete, you are going do a copy
> same datas each time to the upper snapshot for nothing, because at the
> end we are going to commit to the initial "first" snapshot/image.

but you don't know up front that you want to collapse all the snapshots. for each single removal, you have to merge the delta towards the overlay, not the base, else the base contents is no longer matching its name.

think about it this way:

you take a snapshot B at time X. this snapshot must never contain a modification that happened after X. that means you cannot ever commit a newer snapshot into B, unless you are removing and renaming B.

if you start with a chain A -> B -> C -> D (with A being the first snapshot/base, and D being the current active overlay. if you want to remove B, you can either
- stream B into C, remove B
- commit C into B, remove C, rename B to C

in both cases you will end up with a chain A -> C' -> D where C' is the combination of the old B and C.

the downside of the streaming variant is that if B's delta is bigger than C's, you have more I/O. the upside is that there is no inbetween state where the backing chain is broken and error handling can go very wrong.

what you are doing right now is:

chain A->B->C->D as before. remove B by commiting B into A and then rebasing C on top of A. that means you end up with:
A'->C->D where A' is A+B. but now this snapshot A contains writes that happened after the original A was taken. this is wrong.