Re: [pve-devel] [PATCH common 1/1] add PVE/HardwareMap

[Dominik Csapak <d.csapak@proxmox.com>]

On 8/1/22 14:58, Fabian Grünbichler wrote:
> On July 19, 2022 1:46 pm, Dominik Csapak wrote:
>> this adds functionality for the hardwaremap config (as json)
>> the format of the config is like this:
>>
>> {
>>      usb => {
>> 	name => {
>> 	    nodename1 => { /* mapping object */ },
>> 	    nodename2 => { /* mapping object */ }
>> 	}
>>      },
>>      pci => {
>> 	/* same as above */
>>      },
>>      digest => "<DIGEST-STRING>"
>> }
> 
> kind of missing an argument for why
> - this is a json file instead of a regular section config (the two
>    stored types share most of their structure?)

i gave the explanation in the cover letter only (sry)

basically, having some properties with different formats (e.g. path)
makes it really cumbersome (e.g.  i'd have to rename them 'usbpath' and 'pcipath'
everywhere only to merge them again in the ui somewhere...

also having a 'nodename' <-> 'properties' mapping is not really doable in a
section config. we could do an array, but that makes handling of duplicates
etc. also not really nice. a 'non section config' config makes this a lot easier,
and one other format we already use is json (e.g. tfa.json; also json handling
in generally is easy in perl)

> - this is a cluster-wide file containing a map per node instead of
>    being one config file per node?

the basic idea is to have the mappings together. the most use we get out of this
is the ui where we show the cluster-wide mapping config. there we would have
to load/parse all node configs...

i am also not opposed to make it per node, but with my notes above,
i'd rather not use a section config, and what are then the advantages of having
a config per node?

> 
> from what I can piece together from the rest of the series ;)
> - cannot be a cluster-wide section config since values differ
>    (potentially) on each node (so a simple `nodes` filter is not enough)

exactly (otherwise we wouldn't really need a mapping in the first place)

> - we don't want ID foo to be type USB on one node and type PCI on
>    another
> - we want to check all nodes for migration precondition checks
> 
> the first one is a given obviously. the type mismatch wouldn't actually
> cause any problems although it could be confusing possibly. the
> migration check could just check all (relevant) nodes.
> 
> not saying I'm opposed to the "one json file" approach per se, but it
> would be nice to weigh the pros and cons before deviating from our usual
> approach to config files.
> 

so to summarize:

single section config: not impossible, but hard/ugly in many ways
multiple section configs: easier, but still ugly in parts
single json: easy to use, but barely any code reuse with our existing configs
  (though the 'duplicate' code is not that much either)
multiple jsons: little code reuse, weird to use, don't really see the point of this
other config format: ???

code-wise i much prefer the 'single json' approach, but if you or anybody
else really don't want/like it, ofc i'd redo that. it's not really important
to me which way we go, but i'd like to decide at some point.