[pve-devel] [PATCH pve-cluster] add 'sdn.cfg' && 'sdn.cfg.new' to observed files

[Alexandre DERUMIER]

Thanks Thomas to have take time to explain it!


>>IMO, you either could: 
>>* add a own small config parser / writer methods in SDN and omit 
>>cfs_register_file for this. In the reader I'd still use get_config 
>>as it has the benefit that it can handle pmxcfs restarts better (it blocks 
>>and retries to reconnect over IPCC transparetnly for ~10seconds) 

>>* if this is often used and can get big it can be worth to have as "real" 
>>observed file. In this case please also add it to status.c so that the 
>>caching logic can be leveraged. 

can you define big ?
because, technically, it could be possible to define 4000 vnets, so maybe 50kb ?
But I'll use it for network status monitoring (in pve-statd), so file will be call
each x seconds for reading vnets.
So maybe it make sense to use observed file ?



----- Mail original -----
De: "Thomas Lamprecht" <t.lamprecht at proxmox.com>
À: "pve-devel" <pve-devel at pve.proxmox.com>, "aderumier" <aderumier at odiso.com>
Envoyé: Mardi 18 Juin 2019 07:22:30
Objet: Re: [pve-devel] [PATCH pve-cluster] add 'sdn.cfg' && 'sdn.cfg.new' to observed files

On 6/17/19 12:02 PM, Alexandre DERUMIER wrote: 
>>> FYI: if this really needs to be an observed file one also needs to add 
>>> it to the respective hash in data/src/status.c (memdb_change_array) 
> 
> didn't known that. As almost all datacenter config files where in observed file, 
> I simply put also sdn.cfg too. 
> 
> I'm not sure if we need it here ? (Don't know exactly how is work in background) 

So, this is normally done to use caching one file read. 

The files registered in status.c are observed, and their status can 
be requested with the "CFS_IPC_GET_FS_VERSION" IPCC call. 
Each registered file has a version (simple integer) which gets 
increased on changes to the file. 

In the reader logic we check if there's a version and if it's at least 
as new as the ones we got from the last cfs_update call (which is normally 
done every API call entry or CLI execution) if the version is not available 
yet, or older, we request the full file content through the get_config IPCC 
call and save the result in the $ccache (config-cache) so the next time a 
cfs_read_file is done on this file we can omit the IPCC call and directly 
return the cached version (much faster) at least if the version of the file 
is still the same. 

Now, for you it here "works" because you never have a version available so 
the sdn.cfg just get read _always_ the caching mechanism is never hit. 

cfs_write_file works too as it just uses file_set_content on the path, so 
no IPCC/pmxcfs specific logic at all there. 

IMO, you either could: 
* add a own small config parser / writer methods in SDN and omit 
cfs_register_file for this. In the reader I'd still use get_config 
as it has the benefit that it can handle pmxcfs restarts better (it blocks 
and retries to reconnect over IPCC transparetnly for ~10seconds) 
* if this is often used and can get big it can be worth to have as "real" 
observed file. In this case please also add it to status.c so that the 
caching logic can be leveraged. 

hope that helps, else just ask :)