[pve-devel] [PATCH pve-storage 04/10] rename_volume: add source && target snap

[Thomas Lamprecht]

Am 04.07.25 um 13:52 schrieb Fabian Grünbichler:
>> Alexandre Derumier via pve-devel <pve-devel at lists.proxmox.com> hat am 04.07.2025 08:45 CEST geschrieben:
>> allow to rename from|to external snapshot volname
> 
> we could consider adding a new API method `rename_snapshot` instead:
> 
> my ($class, $scfg, $storeid, $volname, $source_snap, $target_snap) = @_;
> 
> for the two plugins here it could easily share most of the implementation
> with rename_volume, without blowing up the interface for a fairly limited
> use case?
> 
> rename_volume right now is used for re-assigning a volume from one
> owner/vmid to another only, AFAICT, with $target_volname never being
> actually provided by callers. the new call would then never provide
> $target_vmid and never provide $target_volname, while existing ones
> never provide the snapshot parameters. OTOH, just like the existing
> rename_volume, such a rename_snapshot method would only have a
> single use case/call site, unless we plan to also add generic
> snapshot renaming as a feature down the line..

I'm currently not too closely into this code, but IMO it might be
indeed a bit nicer to have this as separate and specialized method.
The amount of call sites is not so important to me, having a more
clear and less "multiplexed" API can provide its benefits on it's
own, like avoiding some dangerous edge cases you pointed out below.