From: Fiona Ebner <f.ebner@proxmox.com>
To: Markus Ebner <info@ebner-markus.de>, pve-devel@lists.proxmox.com
Subject: Re: [pve-devel] [PATCH qemu-server 1/1] fix #5033: api: Add target-filename option to move_disk reassign
Date: Fri, 10 Nov 2023 10:45:01 +0100 [thread overview]
Message-ID: <aeacd878-8bba-4b96-a3d3-412498ede6cb@proxmox.com> (raw)
In-Reply-To: <20231108165239.22145-2-info@ebner-markus.de>
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
next parent reply other threads:[~2023-11-10 9:45 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <20231108165239.22145-1-info@ebner-markus.de>
[not found] ` <20231108165239.22145-2-info@ebner-markus.de>
2023-11-10 9:45 ` Fiona Ebner [this message]
[not found] ` <1877948.CQOukoFCf9@pc.markus.lan>
2023-11-10 13:07 ` Fiona Ebner
2023-11-13 9:38 ` Fabian Grünbichler
2023-11-13 10:14 ` Thomas Lamprecht
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=aeacd878-8bba-4b96-a3d3-412498ede6cb@proxmox.com \
--to=f.ebner@proxmox.com \
--cc=info@ebner-markus.de \
--cc=pve-devel@lists.proxmox.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox