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 [212.224.123.68])
by lore.proxmox.com (Postfix) with ESMTPS id E0FCA1FF164
for <inbox@lore.proxmox.com>; Fri, 31 Jan 2025 10:36:28 +0100 (CET)
Received: from firstgate.proxmox.com (localhost [127.0.0.1])
by firstgate.proxmox.com (Proxmox) with ESMTP id 9BB6228AE7;
Fri, 31 Jan 2025 10:36:25 +0100 (CET)
Message-ID: <b37b38d8-7a26-41c7-951b-8461f567a423@proxmox.com>
Date: Fri, 31 Jan 2025 10:36:22 +0100
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
From: Fiona Ebner <f.ebner@proxmox.com>
To: Proxmox VE development discussion <pve-devel@lists.proxmox.com>,
Daniel Herzig <d.herzig@proxmox.com>
References: <20250130113121.157273-1-d.herzig@proxmox.com>
<20250130113121.157273-2-d.herzig@proxmox.com>
Content-Language: en-US
In-Reply-To: <20250130113121.157273-2-d.herzig@proxmox.com>
X-SPAM-LEVEL: Spam detection results: 0
AWL -0.048 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
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 qemu-server v3 1/6] fix #4225: qemuserver:
drive: add parameter to mark drive required
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>
The 'qemuserver' prefix in the commit title doesn't add any information
and should not be there. Commit title prefixes are not for file names.
This also doesn't fix the issue yet, so I'd also drop that prefix too.
Am 30.01.25 um 12:31 schrieb Daniel Herzig:
> This commit add the parameter `essential` to mark a drive as required
> for booting the VM.
>
> Signed-off-by: Daniel Herzig <d.herzig@proxmox.com>
> ---
> PVE/QemuServer/Drive.pm | 9 ++++++++-
> 1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/PVE/QemuServer/Drive.pm b/PVE/QemuServer/Drive.pm
> index 1041c1dd..38136787 100644
> --- a/PVE/QemuServer/Drive.pm
> +++ b/PVE/QemuServer/Drive.pm
> @@ -266,7 +266,14 @@ my %drivedesc_base = (
> verbose_description => "Mark this locally-managed volume as available on all nodes.\n\nWARNING: This option does not share the volume automatically, it assumes it is shared already!",
> optional => 1,
> default => 0,
> - }
> + },
> + essential => {
> + type => 'boolean',
> + description => 'Mark this iso volume as required for booting the VM.',
Nit: Since the 'media=cdrom' option is used to decide this, I'd state
"CD-ROM" here to be more precise. I'd also say "for starting the VM"
rather than "for booting the VM". The device isn't necessarily involved
into booting, but can still be considered essential to allow starting
the VM.
> + verbose_description => 'If unset or set to 1, and the iso file is unavailable, the VM will not start.\nThis parameter is considered for cdrom iso drives only.',
> + optional => 1,
> + default => 1,
> + },
> );
>
> my %iothread_fmt = ( iothread => {
There should be some checking/handling in the API endpoints and/or
parse_drive() for making sure this can only be set in combination with
'media=cdrom'. There are already such checks in parse_drive() which will
return undef in those cases, but please also issue a warning for a new
such check so that it will be clear what went wrong ;)
Nit: Usually, it's nicer to have booleans be 0 if not present. So can we
invert this e.g. "ignore-if-missing" (or "detach-if-missing" or
"eject-if-missing")? Otherwise, maybe "essential-for-start" to be more
descriptive?
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel