Am 08.11.23 um 17:52 schrieb Markus Ebner: > When the move_disk endpoint is used to reassign a disk image from one > vm to another, the target-filename of the image is typically chosen > automatically with the known naming schema. > > This patch adds the optional parameter target-filename, allowing > to manually specify a filename for the disk image when doing a vm > to vm reassignment. It's not currently implemented for storage to > storage moving. >
I'm not fully convinced we want this at the guest level. For example, when allocating a volume via the special "qm set 123 --scsi0 storeid:size-in-GiB" syntax, we also don't allow specifying a name. Exposing the rename functionality as part of the storage API instead might fit better with the status quo? But I do have another suggestion too: Should we rather automatically preserve the current volume name (just replacing the VM ID) if there is no other volume with that name and choose a new name if there is? For offline storage migration, we also do it like that (sans replacing the VM ID). Then we could get away without needing a new option and users specifying it. Would that be enough to cover your use case? If we choose a guest-level approach, it should also be implemented for containers for consistency. Regarding your approach: It'd make sense to support using the same VM ID if target-filename is specified. root@pve8a1 ~ # qm disk move 120 scsi1 --target-vmid 120 --target-filename foobar This currently fails with 400 Parameter verification failed. target-vmid: must be different than source VMID to reassign disk Also, there is an issue when the file name doesn't match the usual naming convention: https://pve.proxmox.com/pve-docs/chapter-pvesm.html#_file_naming_conventions For example, with root@pve8a1 ~ # qm disk move 120 scsi1 --target-vmid 125 --target-filename foobar moving disk 'scsi1' from VM '120' to '125' removing disk 'scsi1' from VM '120' config unable to parse volume filename 'foobar' you end up with a situation where the volume is in neither config and rescan won't find it, but it still got renamed: root@pve8a1 ~ # ls /mnt/pve/nfs/images/125 foobar This issue is pre-existing, because it is the storage plugin's job to check for that, but your patch exposes it. The rename_volume() implementations should verify that the target filename is valid (i.e. can be parsed) before actually doing the rename. Still, it'd be best to also do an early check in the API function here. Best Regards, Fiona _______________________________________________ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
