From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <f.ebner@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 B56EC90670
 for <pve-devel@lists.proxmox.com>; Fri,  3 Feb 2023 14:44:37 +0100 (CET)
Received: from firstgate.proxmox.com (localhost [127.0.0.1])
 by firstgate.proxmox.com (Proxmox) with ESMTP id 9D4D251DE
 for <pve-devel@lists.proxmox.com>; Fri,  3 Feb 2023 14:44:37 +0100 (CET)
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))
 (No client certificate requested)
 by firstgate.proxmox.com (Proxmox) with ESMTPS
 for <pve-devel@lists.proxmox.com>; Fri,  3 Feb 2023 14:44:37 +0100 (CET)
Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1])
 by proxmox-new.maurer-it.com (Proxmox) with ESMTP id CA272452CD;
 Fri,  3 Feb 2023 14:44:36 +0100 (CET)
Message-ID: <974597df-ae43-29ee-dfeb-b5441ab2f1c0@proxmox.com>
Date: Fri, 3 Feb 2023 14:44:35 +0100
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.6.0
From: Fiona Ebner <f.ebner@proxmox.com>
To: pve-devel@lists.proxmox.com, "aderumier@odiso.com" <aderumier@odiso.com>
References: <20230202110344.840195-1-aderumier@odiso.com>
 <20230202110344.840195-7-aderumier@odiso.com>
Content-Language: en-US
In-Reply-To: <20230202110344.840195-7-aderumier@odiso.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
X-SPAM-LEVEL: Spam detection results:  0
 AWL 0.043 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.09 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 06/13] config: memory: add
 'max' option
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: Fri, 03 Feb 2023 13:44:37 -0000

Am 02.02.23 um 12:03 schrieb Alexandre Derumier:
> diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
> index 9db1d8e..1f73605 100644
> --- a/PVE/QemuServer.pm
> +++ b/PVE/QemuServer.pm
> @@ -5042,7 +5042,7 @@ sub vmconfig_hotplug_pending {
>  		vm_deviceunplug($vmid, $conf, $opt);
>  		vmconfig_delete_or_detach_drive($vmid, $storecfg, $conf, $opt, $force);
>  	    } elsif ($opt =~ m/^memory$/) {
> -		die "skip\n" if !$hotplug_features->{memory};
> +		die "skip\n" if !PVE::QemuServer::Memory::can_hotplug($hotplug_features->{memory}, $conf);
>  		PVE::QemuServer::Memory::qemu_memory_hotplug($vmid, $conf);
>  	    } elsif ($opt eq 'cpuunits') {
>  		$cgroup->change_cpu_shares(undef);

(...)

> @@ -24,9 +26,23 @@ our $memory_fmt = {
>  	default_key => 1,
>  	minimum => 16,
>  	default => 512,
> +    },
> +    max => {
> +	type => 'integer',
> +	optional => 1,
> +	minimum => 65536,
> +	maximum => 4194304,
> +	format => 'pve-qm-memory-max',
>      }
>  };
>  
> +PVE::JSONSchema::register_format('pve-qm-memory-max', \&verify_qm_memory_max);
> +sub verify_qm_memory_max {
> +    my ($max, $noerr) = @_;
> +

If $noerr is set and the check fails, you need to return undef and not die.

> +    die "max memory need to be a multiple of 64GiB\n" if $max && $max % 65536 != 0;

return $max;

> +}
> +
>  sub print_memory {
>      my $memory = shift;
>  
> @@ -311,6 +327,19 @@ sub qemu_memory_hotplug {
>      return $conf->{memory};
>  }
>  
> +sub can_hotplug {
> +    my ($hotplug, $conf, $value) = @_;
> +
> +    return if !$hotplug || !$value;

We can hotplug if there is no value. Calling qemu_memory_hotplug()
without value is legal. That's the call in vmconfig_hotplug_pending()
when the 'memory' is deleted from the config. While it currently errors
out later (because the default of 512 MiB is less than the static memory
of 1024 MiB), that can change if the default value changes. So we should
still return 1 in this case.

> +
> +    my $oldmem = parse_memory($conf->{memory});
> +    my $newmem = parse_memory($value);
> +
> +    return if safe_num_ne($newmem->{max}, $oldmem->{max});
> +
> +    return 1;
> +}
> +
>  sub qemu_dimm_list {
>      my ($vmid) = @_;
>