From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: <pve-devel-bounces@lists.proxmox.com> Received: from firstgate.proxmox.com (firstgate.proxmox.com [IPv6:2a01:7e0:0:424::9]) by lore.proxmox.com (Postfix) with ESMTPS id 60CE71FF16B for <inbox@lore.proxmox.com>; Mon, 16 Sep 2024 18:38:43 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 6E324B75E; Mon, 16 Sep 2024 18:38:46 +0200 (CEST) From: Daniel Kral <d.kral@proxmox.com> To: pve-devel@lists.proxmox.com Date: Mon, 16 Sep 2024 18:38:30 +0200 Message-Id: <20240916163839.236908-1-d.kral@proxmox.com> X-Mailer: git-send-email 2.39.5 MIME-Version: 1.0 X-SPAM-LEVEL: Spam detection results: 0 AWL -0.005 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment RCVD_IN_VALIDITY_CERTIFIED_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. RCVD_IN_VALIDITY_RPBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. RCVD_IN_VALIDITY_SAFE_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Subject: [pve-devel] [RFC qemu-server 0/9] consistent checks for storage content types on volume disk allocation X-BeenThere: pve-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox VE development discussion <pve-devel.lists.proxmox.com> List-Unsubscribe: <https://lists.proxmox.com/cgi-bin/mailman/options/pve-devel>, <mailto:pve-devel-request@lists.proxmox.com?subject=unsubscribe> List-Archive: <http://lists.proxmox.com/pipermail/pve-devel/> List-Post: <mailto:pve-devel@lists.proxmox.com> List-Help: <mailto:pve-devel-request@lists.proxmox.com?subject=help> List-Subscribe: <https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel>, <mailto:pve-devel-request@lists.proxmox.com?subject=subscribe> Reply-To: Proxmox VE development discussion <pve-devel@lists.proxmox.com> Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: pve-devel-bounces@lists.proxmox.com Sender: "pve-devel" <pve-devel-bounces@lists.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