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

[DERUMIER, Alexandre]

> 
> 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!)

ok, I see 

>>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?

yes, I think that it could be manually cleaned, no problem.
I'll revert the delete starting with current.
> 
> 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.

ok, no problem, I'll do it like this.

Do you want the a snapshot sub directory like ?
/var/lib/vz/images/<vmid>/snapshots/ 


or a global snapshots like

/var/lib/vz/snapshots/<vmid>/

?