Re: [pve-devel] [PATCH qemu-server 02/10] add memory parser

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

Am 02.01.23 um 12:23 schrieb DERUMIER, Alexandre:
> Le vendredi 16 décembre 2022 à 14:38 +0100, Fiona Ebner a écrit :
>> From a modularity standpoint, it would be nice to move the format
>> description, parsing+printing to PVE/QemuServer/Memory.pm, similar to
>> how it is done in PVE/QemuServer/PCI.pm
>>
>> Since $confdesc->{memory}->{default} is now gone, load_defaults()
>> will
>> not return a default for 'memory' anymore. And $conf->{memory} needs
>> to
>> be parsed everywhere. These two things will break getting static
>> usage
>> in HA manager, which uses load_defaults() and relies on $conf-
>>> {memory}
>> to be a single value right now. We can switch there to use
>> get_current_memory() too of course, but it'd require a versioned
>> Breaks+Depends.
>>
>> Alternatively, we could add a function in qemu-server for calculating
>> the static stats and call that from HA. Requires a versioned
>> Breaks+Depends too, but then we'd be safe in the future and also
>> catch
>> such changes more easily. OTOH, it'd make the coupling go in the
>> other
>> direction: if HA manager wants to change what it uses for static
>> consideration, then qemu-server would need to adapt. So not sure.
> 
> I was think about this,
> When dynamic scheduler will be implemented, you'll need to use values
> streamed from pvestatd.
> So why can't we do the same for static values ?  (maxmem && maxcpus are
> already send by pvestatd).
> 
> This should avoid the need to parse vm config,
> and maybe avoid to use load_config ?
> (from your initial commit,
> https://git.proxmox.com/?p=pve-ha-manager.git;a=commit;h=561e7f4bfb235fcdca5b0bbb8422ce742a5da75f,
> it seem to be slow)
> 
> 

The information is already readily available on the cluster file system,
so sending it around via pvestatd additionally isn't ideal IMHO. maxmem
and maxcpus are only one per node and were not available before.

The load_config() call is not really problematic, because the result
from cfs_read_file() is cached. The real issue is that
recompute_online_node_usage() and thus getting the static info is called
very often currently. There was an RFC [0] to get the information using
PVE::Cluster::get_guest_config_properties(), but it's only twice as
fast. Optimizing how often we call recompute_online_node_usage() can
give us much more.

In any case, HA manager needs to be adapted before the memory setting
can be turned into a property string.

[0]: https://lists.proxmox.com/pipermail/pve-devel/2022-November/054956.html