> Fabian Grünbichler <f.gruenbich...@proxmox.com> hat am 23.10.2024 12:12 CEST > geschrieben: > > > some high level comments: > > I am not sure how much we gain here with the raw support? it's a bit > confusing to have a volid ending with raw, with the current volume and all > but the first snapshot actually being stored in qcow2 files, with the raw > file being the "oldest" snapshot in the chain.. > > if possible, I'd be much happier with the snapshot name in the snapshot file > being a 1:1 match, see comments inline > - makes it a lot easier to understand (admin wants to manually remove > snapshot "foo", if "foo" was the last snapshot then right now the volume > called "foo" is actually the current contents!) > - means we don't have to do lookups via the full snapshot list all the time > (e.g., if I want to do a full clone from a snapshot "foo", I can just pass > the snap-foo volume to qemu-img) > > the naming scheme for snapshots needs to be adapted to not clash with regular > volumes: > > $ pvesm alloc extsnap 131314 vm-131314-disk-foobar.qcow2 2G > Formatting '/mnt/pve/ext4/extsnap/images/131314/vm-131314-disk-foobar.qcow2', > fmt=qcow2 cluster_size=65536 extended_l2=off preallocation=off > compression_type=zlib size=2147483648 lazy_refcounts=off refcount_bits=16 > successfully created 'extsnap:131314/vm-131314-disk-foobar.qcow2' > $ qm rescan --vmid 131314 > rescan volumes... > can't parse snapname from path at /usr/share/perl5/PVE/Storage/Plugin.pm line > 1934. > > storage_migrate needs to handle external snapshots, or at least error out. I > haven't tested that part or linked clones or a lot of other advanced related > actions at all ;)
I'll add some more high-level comments (the threading seems to be broken for some reason, so I'll use this as "entrypoint"): - snapext should probably be fixed for dir-type storages as well - the volume ID should be static for both plugins, snapshots should be encoded on the storage layer in a fashion that doesn't "break through" to the API layers and makes it impossible to confuse the "main" volname with snapshots: -- alloc_image shouldn't be able to allocate a volume that is then interpreted as snapshot -- free_image shouldn't be able to free a snapshot volume directly -- listing images should never return snapshots -- .. - for the LVM part, snapshots still require allocation a full-sized volume+some overhead for the qcow2 each. should we attempt to shrink them once they become read-only? in practice, only the LV backing the top image needs to be full-sized.. how do we ensure the underlying storage doesn't waste all that "empty" space? _______________________________________________ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
