[pve-devel] [PATCH v2 qemu-server 4/9] config: memory: add 'max' option

[Fiona Ebner]

Am 27.01.23 um 16:03 schrieb DERUMIER, Alexandre:
>>
>>> +    # skip non hotpluggable value
>>> +    if (safe_num_ne($newmem->{max}, $oldmem->{max})) {
>>> +       die "skip\n";
>>> +    }
>>
>> Please move this to the call sites. The "die "skip""-logic should not
>> cross function boundaries.
>>
>>
> Just a note: This is exactly how it's done on nic && disk hotplug.
> 
> for example:
> vmconfig_update_disk {
>  ...
>     # skip non hotpluggable value
>    if (safe_string_ne($drive->{discard}, $old_drive->{discard}) ||
>         safe_string_ne($drive->{iothread}, $old_drive->{iothread}) ||
>         safe_string_ne($drive->{queues}, $old_drive->{queues}) ||
>         safe_string_ne($drive->{cache}, $old_drive->{cache}) ||
>         safe_string_ne($drive->{ssd}, $old_drive->{ssd}) ||
>         safe_string_ne($drive->{ro}, $old_drive->{ro})) {
>              die "skip\n";
>    }
> 
> 

Well, I'm not a fan of that either. At least that is in the same module.

To keep the logic inside the Memory.pm module, we could add a
can_hotplug function and then in QemuServer.pm do

die "skip\n" if !PVE::Memory::can_hotplug($old, $new);

Like that, it's cleanly separated.