Re: [pve-devel] [PATCH v3 qemu-server 06/13] config: memory: add 'max' option

[Fiona Ebner <f.ebner@proxmox.com>]

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) = @_;
>