Looks mostly good to me, mostly nits and suggestions for some
improvements. The only real issues I found are the < 9 instead of < 10
for checking the API version, and the check for running containers not
being inside lock_config anymore (maybe that is not even a big deal,
didn't check in detail).
The suggested changes for the find_free_diskname interface would be nice
to have IMHO, now that it's not set in stone yet, but it's not a must
either.
Am 19.07.21 um 16:52 schrieb Aaron Lauterer:
This is the continuation of 'disk-reassign' but instead of a separate
API endpoint we now follow the approach to make it part of the
'move-disk' and 'move-volume' endpoints for VMs and containers.
The main idea is to make it easy to move a disk/volume to another guest.
Currently this is a manual and error prone process that requires
knowledge of how PVE handles disks/volumes and the mapping which guest
they belong to.
With this, the 'qm move-disk' and 'pct move-volume' are changed in the
way that the storage parameter is optional as well as the new
target-vmid and target-{disk,mp}. This will keep old calls to move the
disk/volume to another storage working. To move to another guest, the
storage needs to be omitted.
Major changes since the last iteration as dedicated API endpoint [0] are
that the storage layer only implements the renaming itself. The layer
above (qemu-server and pve-container) define the name of the new
volume/disk. Therefore it was necessary to expose the
'find_free_diskname' function. The rename function on the storage layer
handles possible template referneces and the creation of the new volid
as that is highly dependent on the actual storage.
The following storage types are implemented at the moment:
* dir based ones
* ZFS
* (thin) LVM
* Ceph RBD
Most parts of the disk-reassign code has been taken and moved into the
'move_disk' and 'move_volume' endpoints with conditional checking if the
reassign code or the move to other storage code is meant to run
depending on the given parameters.
Changes since the RFC [1]:
* added check if target guest is replicated and fail if storage does not
support replication
* only pass minimum of needed parameters to the storage layer and infer
other needed information from that
* lock storage and check if the volume aready exists (handling a
possible race condition between calling find_free_disk and the actual
renaming)
* use a helper method to determine if the plugin needs the fmt suffix
in the volume name
* getting format of the source and pass it to find_free_disk
* style fixes (long lines, multiline post-if, ...)
[0] https://lists.proxmox.com/pipermail/pve-devel/2021-April/047481.html
[1] https://lists.proxmox.com/pipermail/pve-devel/2021-June/048400.html
storage: Aaron Lauterer (2):
storage: expose find_free_diskname
add disk rename feature
PVE/Storage.pm | 31 +++++++++++++++--
PVE/Storage/LVMPlugin.pm | 32 ++++++++++++++++++
PVE/Storage/LvmThinPlugin.pm | 1 +
PVE/Storage/Plugin.pm | 65 ++++++++++++++++++++++++++++++++++++
PVE/Storage/RBDPlugin.pm | 34 +++++++++++++++++++
PVE/Storage/ZFSPoolPlugin.pm | 29 ++++++++++++++++
6 files changed, 190 insertions(+), 2 deletions(-)
qemu-server: Aaron Lauterer (4):
cli: qm: change move_disk to move-disk
Drive: add valid_drive_names_with_unused
api: move-disk: add move to other VM
api: move-disk: cleanup very long lines
PVE/API2/Qemu.pm | 254 ++++++++++++++++++++++++++++++++++++++--
PVE/CLI/qm.pm | 3 +-
PVE/QemuServer/Drive.pm | 4 +
3 files changed, 250 insertions(+), 11 deletions(-)
container: Aaron Lauterer (3):
cli: pct: change move_volume to move-volume
api: move-volume: add move to another container
api: move-volume: cleanup very long lines
src/PVE/API2/LXC.pm | 303 ++++++++++++++++++++++++++++++++++++++++----
src/PVE/CLI/pct.pm | 3 +-
2 files changed, 278 insertions(+), 28 deletions(-)
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
