There currently is some inconsistency in how storages are checked when they are used to allocate new VM disk images on them at different API endpoints and internal subroutines. The usual checks that are done in these contexts are:
- check whether the storage is enabled, - check whether the content type 'images' (or the volume-specific content type) is supported on the storage, and - check whether the authenticated user has the necessary permissions to allocate space on the storage; The origin of this patch series is bug report #5284, where a PVE user indicated that when moving VM disks from one storage to another, this process also works for storages that do not support VM images, but causes the VM to fail when started afterwards. This also happens in the case when a VM is cloned to a storage without VM images support, which will deliver the same result. As there are many similar checks throughout the qemu-server package, this patch series refactors the common denominator of these checks into universal helper functions to have predictable error messages throughout the repository for the same error and improve parameter context consistency (`raise_param_exc`) for error messages on API endpoints to make it easier for users to find the error more quickly. As I'm still new to the codebase I am unsure about some changes in the codebase, as the indent was to find places where the checks are missing and adding consistency for storing VM images only on storages that support them. I have tested the changes to my current best ability, but as the package is functionally quite fundamental and large, I'm still putting this out as a RFC and added notes throughout the patches. I'm particular unsure about the changes made to fleecing images, snapshot statefiles and the cloudinit allocation, which I've pointed out in the patches #6 and #9 respectively. Also, my goal was in making it harder for the user to get into 'invalid' states, but not to move out of them. So it's still possible to move disks from non-supporting to supporting storages and update the VM config with unsupported storages. Are there any cases that I'm missing? Daniel Kral (9): test: cfg2cmd: expect error for invalid volume's storage content type cfg2cmd: improve error message for invalid volume storage content type fix #5284: move_vm: add check if target storage supports vm images api: clone_vm: add check if storage supports vm images api: create_vm: improve checks if storages for disks support vm images cloudinit: add check if storage for cloudinit disk supports vm images api: migrate_vm: improve check if target storages support vm images api: importdisk: improve check if storage supports vm images restore_vm: improve checks if storage supports vm images PVE/API2/Qemu.pm | 70 +++++------ PVE/CLI/qm.pm | 12 +- PVE/QemuConfig.pm | 4 +- PVE/QemuMigrate.pm | 13 +- PVE/QemuServer.pm | 55 +++------ PVE/QemuServer/Cloudinit.pm | 4 +- PVE/QemuServer/Helpers.pm | 111 ++++++++++++++++++ PVE/QemuServer/ImportDisk.pm | 4 +- PVE/VZDump/QemuServer.pm | 4 +- .../unsupported-storage-content-type.conf | 3 + test/run_config2command_tests.pl | 6 +- 11 files changed, 188 insertions(+), 98 deletions(-) create mode 100644 test/cfg2cmd/unsupported-storage-content-type.conf -- 2.39.5 _______________________________________________ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
