[pve-devel] [RFC qemu-server 0/9] consistent checks for storage content types on volume disk allocation

[Daniel Kral <d.kral@proxmox.com>]

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