From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <t.lamprecht@proxmox.com>
Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68])
 (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)
 key-exchange X25519 server-signature RSA-PSS (2048 bits))
 (No client certificate requested)
 by lists.proxmox.com (Postfix) with ESMTPS id 4892874326
 for <pve-devel@lists.proxmox.com>; Mon, 21 Jun 2021 11:12:22 +0200 (CEST)
Received: from firstgate.proxmox.com (localhost [127.0.0.1])
 by firstgate.proxmox.com (Proxmox) with ESMTP id 39F061A277
 for <pve-devel@lists.proxmox.com>; Mon, 21 Jun 2021 11:11:52 +0200 (CEST)
Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com
 [94.136.29.106])
 (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)
 key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256)
 (No client certificate requested)
 by firstgate.proxmox.com (Proxmox) with ESMTPS id B55A51A268
 for <pve-devel@lists.proxmox.com>; Mon, 21 Jun 2021 11:11:51 +0200 (CEST)
Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1])
 by proxmox-new.maurer-it.com (Proxmox) with ESMTP id 7DD2340BC5
 for <pve-devel@lists.proxmox.com>; Mon, 21 Jun 2021 11:11:51 +0200 (CEST)
Message-ID: <158f11d6-d8a7-c355-2d4f-3645a66f7ce3@proxmox.com>
Date: Mon, 21 Jun 2021 11:11:42 +0200
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:90.0) Gecko/20100101
 Thunderbird/90.0
Content-Language: en-US
To: Proxmox VE development discussion <pve-devel@lists.proxmox.com>,
 Lorenz Stechauner <l.stechauner@proxmox.com>
References: <20210527122331.86302-1-l.stechauner@proxmox.com>
 <20210527122331.86302-3-l.stechauner@proxmox.com>
From: Thomas Lamprecht <t.lamprecht@proxmox.com>
In-Reply-To: <20210527122331.86302-3-l.stechauner@proxmox.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
X-SPAM-LEVEL: Spam detection results:  0
 AWL 0.729 Adjusted score from AWL reputation of From: address
 BAYES_00                 -1.9 Bayes spam probability is 0 to 1%
 KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment
 NICE_REPLY_A           -0.001 Looks like a legit reply (A)
 SPF_HELO_NONE           0.001 SPF: HELO does not publish an SPF Record
 SPF_PASS               -0.001 SPF: sender matches SPF record
Subject: Re: [pve-devel] [PATCH v3 qemu-server 1/1] vm_start: check if
 storages of volumes support content images
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>
X-List-Received-Date: Mon, 21 Jun 2021 09:12:22 -0000

On 27.05.21 14:23, Lorenz Stechauner wrote:
> it is now necessary for storages to support the 'images' content in
> order to start vms on them. all native storage plugins already
> report the images content correctly.
> 
> Signed-off-by: Lorenz Stechauner <l.stechauner@proxmox.com>
> ---
>  PVE/QemuServer.pm | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
> index 0bfbca4..3b656a1 100644
> --- a/PVE/QemuServer.pm
> +++ b/PVE/QemuServer.pm
> @@ -5124,6 +5124,13 @@ sub vm_start_nolock {
>      my ($cmd, $vollist, $spice_port) = config_to_command($storecfg, $vmid,
>  	$conf, $defaults, $forcemachine, $forcecpu, $params->{'pbs-backing'});
>  
> +    for my $vol (@$vollist) {
> +	if (my ($storeid, $volname) = PVE::Storage::parse_volume_id($vol, 1)) {
> +	    my $scfg = PVE::Storage::storage_config($storecfg, $storeid);
> +	    die "storage '$storeid' does not support vm disks\n" if !$scfg->{content}->{images};
> +	}
> +    }

we'd already loop overall volumes in cfg2cmd, so why not check there (with a small
helper method to avoid adding to much bloat there)?

Also, are you aware that efidisk and VM state images are included by this check 
ensured that it's consistent regarding the handling of those special "files" elsewhere?