Re: [pve-devel] [PATCH pve-storage 08/10] qcow2: add external snapshot support

["Fabian Grünbichler" <f.gruenbichler@proxmox.com>]

> DERUMIER, Alexandre <alexandre.derumier@groupe-cyllene.com> hat am 04.07.2025 15:22 CEST geschrieben:

> > +        #delete external snapshots
> > +        if ($scfg->{snapext}) {
> > +            my $snapshots = $class->volume_snapshot_info($scfg,
> > $storeid, $volname);
> > +            for my $snapid (
> > +                sort { $snapshots->{$b}->{order} <=> $snapshots-
> > >{$a}->{order} }
> > +                keys %$snapshots
> > +            ) {
> > +                my $snap = $snapshots->{$snapid};
> > +                next if $snapid eq 'current';
> > +                next if !$snap->{ext};
> > +                free_snap_image($class, $storeid, $scfg, $volname,
> > $snapid);
> > +            }
> > +        }
> > +
> 
> >>this is a bit tricky.. once we've deleted the first snapshot, we've
> >>basically invalidated
> >>the whole image..
> Well, we want to delete it anyway, we don't care about invalidate it ?
> 
> >> should we try to continue freeing as much as possible? and maybe
> even
> >>start with the "current" image, so that a partial removal doesn't
> >>look like valid image
> >>anymore?
> 
> currently the volume_snapshot_info is reading the snapshot chain from
> the current image (to do only 1 qemu-img call), that why I'm removing
> snapshots in reverse order.
> If something hang with partial delete, you can still try again later.
> If we want to delete from the current to last snap, I'll need something
> like calling qemu-info info on each snap file to find each parent.
> 
> or maybe use something else than volume_snapshot_info here, simply glob
> all the vm disk && snap files and delete them in random order, as we
> want to delete it anyway.

yes, this is exactly what I meant with tricky ;)

if we start deleting snapshots from the "first"/root one, then it's
easier to retry deletion after a partial deletion - but the image still
"looks" like a valid one at first glance (although it is of course
unusable as soon as any snapshot is missing!)

if we start deleting with the image file, then it is immediately clear
that there is no more image to be used - but retrying a partial deletion
is impossible via the PVE API, you need to do it manually.

I just tried, and this would need more work if we want to support the
"retry partial deletion" approach - because:

root.qcow2 <- snap.qcow2 <- current.qcow2

delete root.qcow2

$ qemu-img info --output json --backing-chain -f qcow2 current.qcow2
qemu-img: Could not open 'root.qcow2': Could not open 'root.qcow2': No such file or directory

so we'd need to actually query image by image and detect when the chain
is broken, if we want to support that which might not be worth it, if
the snapshot deletion log contains the information about which snapshot
volumes are leftover and need to be manually cleaned?

> >>the naming scheme here still clashes with regular volids
> unfortunately:
> 
> >>$ pvesm alloc ext4 12344321 snap-foobar-12344321-disk-foofoobar.qcow2
> >>1G
> >>Formatting '/mnt/pve/ext4/images/12344321/snap-foobar-12344321-disk-
> >>foofoobar.qcow2', fmt=qcow2 cluster_size=65536 extended_l2=off
> >>preallocation=off compression_type=zlib size=1073741824
> >>lazy_refcounts=off refcount_bits=16
> >>successfully created 'ext4:12344321/snap-foobar-12344321-disk-
> >>foofoobar.qcow2'
> >>$ pvesm list ext4 -content images -vmid 12344321 | grep foobar
> >>ext4:12344321/snap-foobar-12344321-disk-foofoobar.qcow2 qcow2  
> >>images    1073741824 12344321
> >>$ qm set 12344321 --scsi0 ext4:12344321/snap-foobar-12344321-disk-
> >>foofoobar.qcow2
> 
> >>should we maybe move snapshot files into a subdir, since `/` is not
> >>allowed in volnames?
> 
> what about lvm ? (I think it should be great to have same scheme for
> both, but lvm have also reserved characters like @)

the naming scheme is already different:

<VMID>/<anything except slash and spaces>.<fmt> (dir)

vs

vm-<VMID>-<anything except spaces>[.<qcow2>] (LVM)

the LVM one is in practicate limited further by what is allowed in LV
names, like you said. but for LVM it's fine as it is because of the
`vm-` prefix, we can add a second namespace besides it with `snap-`
without the risk of collisions. or we could switch to make it uniform
with LVM thin, which uses

snap_<volname>_<snap>

as snapshot LV name.. that would make it (for example)

snap_vm-100-disk-1.qcow2_foobar

for a snapshot nameed "foobar" of the volume "vm-100-disk-1.qcow2"


for the DirPlugin we need to add another level for the snapshots
on-disk, else we cannot differentiate between weirdly-named image
files and snapshot files..

so we could add a directory "snapshots" and put all the snapshot
files in there. since snapshots are never references a volume ID,
this just affects the on disk structure, not the volume ID format.


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel