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 C69FB1FF16B
	for <inbox@lore.proxmox.com>; Mon, 16 Sep 2024 18:39:18 +0200 (CEST)
Received: from firstgate.proxmox.com (localhost [127.0.0.1])
	by firstgate.proxmox.com (Proxmox) with ESMTP id 71F89B9B7;
	Mon, 16 Sep 2024 18:39:15 +0200 (CEST)
From: Daniel Kral <d.kral@proxmox.com>
To: pve-devel@lists.proxmox.com
Date: Mon, 16 Sep 2024 18:38:32 +0200
Message-Id: <20240916163839.236908-3-d.kral@proxmox.com>
X-Mailer: git-send-email 2.39.5
In-Reply-To: <20240916163839.236908-1-d.kral@proxmox.com>
References: <20240916163839.236908-1-d.kral@proxmox.com>
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 2/9] cfg2cmd: improve error message
 for invalid volume storage content type
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>

Improve the error message when a VM start fails due to a volume storage
not supporting the volume's required content type by prefixing it with
the disk handle (e.g. scsi0).

This moves and splits the subroutine for checking the volume storage's
content type to increase its reusability.

Signed-off-by: Daniel Kral <d.kral@proxmox.com>
---
This introduces the check_storage_content_type and
check_volume_content_type subroutines, which are later used throughout
the refactoring of the other locations with similar checks.

 PVE/QemuServer.pm                             | 16 +------
 PVE/QemuServer/Helpers.pm                     | 45 +++++++++++++++++++
 .../unsupported-storage-content-type.conf     |  2 +-
 3 files changed, 48 insertions(+), 15 deletions(-)

diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index b26da505..e24c741c 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -3961,7 +3961,8 @@ sub config_to_command {
 	my ($ds, $drive) = @_;
 
 	if (PVE::Storage::parse_volume_id($drive->{file}, 1)) {
-	    check_volume_storage_type($storecfg, $drive->{file});
+	    eval { PVE::QemuServer::Helpers::check_volume_content_type($storecfg, $drive->{file}) };
+	    die "$ds: $@" if $@;
 	    push @$vollist, $drive->{file};
 	}
 
@@ -8794,19 +8795,6 @@ sub vm_is_paused {
     );
 }
 
-sub check_volume_storage_type {
-    my ($storecfg, $vol) = @_;
-
-    my ($storeid, $volname) = PVE::Storage::parse_volume_id($vol);
-    my $scfg = PVE::Storage::storage_config($storecfg, $storeid);
-    my ($vtype) = PVE::Storage::parse_volname($storecfg, $vol);
-
-    die "storage '$storeid' does not support content-type '$vtype'\n"
-	if !$scfg->{content}->{$vtype};
-
-    return 1;
-}
-
 sub add_nets_bridge_fdb {
     my ($conf, $vmid) = @_;
 
diff --git a/PVE/QemuServer/Helpers.pm b/PVE/QemuServer/Helpers.pm
index 0afb6317..9d0f24aa 100644
--- a/PVE/QemuServer/Helpers.pm
+++ b/PVE/QemuServer/Helpers.pm
@@ -106,6 +106,51 @@ sub vm_running_locally {
     return;
 }
 
+=head3 check_storage_content_type($storecfg, $storeid [, $content_type])
+
+Checks whether the storage with the identifier C<$storeid>, that is defined in C<$storecfg>,
+supports the content type C<$content_type>. If the C<$content_type> is undefined, it will default
+to the value C<"images">.
+
+If the check fails, the subroutine will C<die> with an error message containing the storage and
+content type that is not supported.
+
+Returns C<1> if the check is successful.
+
+=cut
+
+sub check_storage_content_type : prototype($$;$) {
+    my ($storecfg, $storeid, $content_type) = @_;
+
+    $content_type = "images" if !defined($content_type);
+    my $scfg = PVE::Storage::storage_config($storecfg, $storeid);
+    die "storage '$storeid' does not support content type '$content_type'\n"
+	if !$scfg->{content}->{$content_type};
+
+    return 1;
+}
+
+=head3 check_volume_content_type($storecfg, $volid)
+
+Checks whether the volume with the identifier C<$volid>, that is defined in C<$storecfg>, supports
+the content type, which is determined by L<PVE::Storage::parse_volname>.
+
+If the check fails, the subroutine will C<die> with an error message containing the storage and
+content type that is not supported.
+
+Returns C<1> if the check is successful.
+
+=cut
+
+sub check_volume_content_type : prototype($$) {
+    my ($storecfg, $volid) = @_;
+
+    my ($storeid) = PVE::Storage::parse_volume_id($volid);
+    my ($vtype) = PVE::Storage::parse_volname($storecfg, $volid);
+
+    return check_storage_content_type($storecfg, $storeid, $vtype);
+}
+
 sub min_version {
     my ($verstr, $major, $minor, $pve) = @_;
 
diff --git a/test/cfg2cmd/unsupported-storage-content-type.conf b/test/cfg2cmd/unsupported-storage-content-type.conf
index 35e789b2..7e7952aa 100644
--- a/test/cfg2cmd/unsupported-storage-content-type.conf
+++ b/test/cfg2cmd/unsupported-storage-content-type.conf
@@ -1,3 +1,3 @@
 # TEST: Unsupported storage content type in a volume disk
-# EXPECT_ERROR: storage 'no-images' does not support content-type 'images'
+# EXPECT_ERROR: scsi0: storage 'no-images' does not support content type 'images'
 scsi0: no-images:8006/vm-8006-disk-0.raw,iothread=1,size=32G
-- 
2.39.5



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel