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

[Thomas Lamprecht]

On 6/18/19 9:04 AM, Alexandre DERUMIER wrote:
> 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 ?

Ok, the big was probably a bit wrong, as after re-thinking it it does not really
matter that much, the file sizes in /etc/pve are never really big.

The "how often one polls this" matters much more, as each IPCC call has some
overhead (less than FUSE, if the connections is already etablished, but still).

So yes, in your case it may really make sense to make this an 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 :) 
> 
>