Open Stack

On 10/28/2014 08:31 AM, Daniel P. Berrange wrote:
> On Tue, Oct 28, 2014 at 08:07:14AM -0400, Sean Dague wrote:
>> On 10/28/2014 07:44 AM, Daniel P. Berrange wrote:
>>> On Tue, Oct 28, 2014 at 07:34:11AM -0400, Sean Dague wrote:
>>>> We're dealing with some issues on devstack pass through with really
>>>> complicated config option types, the fixes are breaking other things.
>>>>
>>>> The issue at hand is the fact that the pci pass through device listing
>>>> is an olso MultiStrOpt in which each option value is fully valid json
>>>> document, which must parse as such. That leads to things like:
>>>>
>>>> pci_passthrough_whitelist = {"address":"*:0a:00.*",
>>>> "physical_network":"physnet1"}
>>>> pci_passthrough_whitelist = {"vendor_id":"1137","product_id":"0071"}
>>>>
>>>> Which, honestly, seems a little weird for configs.
>>>>
>>>> We're talking about a small number of fixed fields here, so the use of a
>>>> full json doc seems weird. I'd like to reopen why this was the value
>>>> format, and if we could have a more simple one.
>>>
>>> Do you have ant suggestion for an alternative config syntax for specifying
>>> a list of dicts which would be suitable ?
>>>
>>> One option would be a more  CSV like syntax eg
>>>
>>>    pci_passthrough_whitelist = address=*0a:00.*,physical_network=physnet1
>>>    pci_passthrough_whitelist = vendor_id=1137,product_id=0071
>>>
>>> But this gets confusing if we want to specifying multiple sets of data
>>> so might need to use semi-colons as first separator, and comma for list
>>> element separators
>>>
>>>    pci_passthrough_whitelist = vendor_id=8085;product_id=4fc2, vendor_id=1137;product_id=0071
>>>
>>> Overall it isn't clear that inventing a special case language for this PCI
>>> config value is a good idea.
>>>
>>> I think it illustrates a gap in oslo.config, which ought to be able to
>>> support a config option type which was a "list of dicts of strings"
>>> so anywhere which needs such a beast will use the same syntax.
>>
>> Mostly, why do we need name= at all. This seems like it would be fine as
>> an fstab like format (with 'x' as an ignore value).
>>
>> # vendor_id product_id address
>> pci_passthrough_whitelist = 8085
>> pci_passthrough_whitelist = 1137 4fc2
>> pci_passthrough_whitelist = x 0071
>> pci_passthrough_whitelist = x x *0a:00.*
>>
>> Basically going to a full name = value seems incredibly overkill for
>> something with < 6 fields.
> 
> I don't think that is really very extensible for the future to drop the
> key name. We've already extended the info we record here at least once,
> and I expect we'd want to add more fields later. It is also makes it
> less clear to the user - it is very easy to get confused about vendor
> vs product IDs if we leave out the name.

If we really need that level of arbitrary complexity and future name
values we should then just:

pci_passthrough_cfg = /etc/nova/pci_pass.yaml

And build fully nested structures over there.

Doing multi level nesting inside of .ini format files is just kind of gross.

	-Sean

-- 
Sean Dague
http://dague.net