[pve-devel] [PATCH qemu-server 1/1] fix #5033: api: Add target-filename option to move_disk reassign
f.ebner at proxmox.com
Fri Nov 10 10:45:01 CET 2023
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
root at pve8a1 ~ # qm disk move 120 scsi1 --target-vmid 120
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
For example, with
root at pve8a1 ~ # qm disk move 120 scsi1 --target-vmid 125
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 at pve8a1 ~ # ls /mnt/pve/nfs/images/125
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.
More information about the pve-devel