On 03/08/19 19:07, Philippe Mathieu-Daudé wrote:
> On 3/8/19 6:31 PM, Eric Blake wrote:
>> On 3/8/19 5:08 AM, Philippe Mathieu-Daudé wrote:
>>
>>>>> {
>>>>> "return": [
>>>>> {
>>>>> "architecture_specific": false,
>>>>> "key": 0,
>>>>> "writeable": false,
>>>>> "size": 4,
>>>>> "keyname": "signature"
>>>>
>>>> You could return a base64-encoded representation of the field (we do
>>>> that in other interfaces where raw binary can't be passed reliably over
>>>> JSON). For 4 bytes, that makes sense,
>>>>
>>>>
>>>>> {
>>>>> "architecture_specific": true,
>>>>> "key": 3,
>>>>> "writeable": false,
>>>>> "size": 324,
>>>>> "keyname": "e820_tables"
>>>>
>>>> for 324 bytes, it gets a bit long. Still, may be worth it for the added
>>>> debug visibility.
>>>
>>> Until you see values in the next patch...:
>>>
>>> $ (echo info fw_cfg; echo q) | qemu-system-x86_64 -S -monitor stdio
>>> Selector Well-Known Key Pathname ArchSpec Perm Size Order Hex Data
>>> [...]
>>> 0x0019 file_dir RO 2052 0000000b..
>>> 0x0022 file: etc/acpi/tables RO 131072 130 46414353..
>>
>> Yeah, that's a no-go.
>>>
>>> What about using a 512B limit on this QMP answer?
>>
>> Seems reasonable. Either omit the field when its size exceeds the limit,
>> or use the field to give a truncated version (where the size field still
>> shows the full length, either way).
>
> OK.
>
>>>>> +# @path: If the key is a 'file', the named file path.
>>>>> +# @order: If the key is a 'file', the named file order.
>>>>> +#
>>>>> +# Since 4.0
>>>>> +##
>>>>> +{ 'struct': 'FirmwareConfigurationItem',
>>>>> + 'data': { 'key': 'uint16',
>>>>> + '*keyname': 'str',
>>>>> + 'architecture_specific': 'bool',
>>>>> + 'writeable': 'bool',
>>>>> + '*data': 'str',
>>>>> + 'size': 'int',
>>>>> + '*path': 'str',
>>>>> + '*order': 'int' } }
>>>>
>>>> Is it worth making 'keyname' an enum type, and turning this into a flat
>>>> union where 'path' and 'order' appear on the branch associated with
>>>> 'file', and where all other well-known keynames have smaller branches?
>>>
>>> I have no idea about that, I will have a look at QMP flat unions.
>>
>> Markus can help if you get stuck on what it might look like. But by now,
>> there are several examples in .qapi files to copy from (look for
>> 'discriminator').
>
> OK.
>
>>>
>>>>> +
>>>>> +
>>>>> +##
>>>>> +# @query-fw_cfg-items:
>>>>
>>>> That looks weird to mix - and _. Any reason we can't just go with
>>>> query-firmware-config?
>>>
>>> Way better! I'll use query-firmware-config-items.
>>
>> Do we need the -items suffix? Also, is there a chance that we might ever
>> want to extend the command to return more information that is global to
>> firmware-config rather than per-item?
>
> Laszlo suggested it could be useful to ask for a specific item (with the
> full data encoded?).
It's possible I referred to that a long time ago, but most recently, I
think it has been raised by Dave.
Thanks
Laszlo
>
>>>>> +{ 'command': 'query-fw_cfg-items', 'returns':
>>>>> ['FirmwareConfigurationItem']}
>>
>> As written, this results in:
>>
>> { "return": [ { ... }, { ... } ] }
>>
>> but if you add a layer of nesting, you could have easier extensions:
>>
>> { "return": { "global": {}, "items": [ { ... }, { ... } ] } }
>>
>
> Oh I guess I got it, I'll try it.
>
> Thanks!
>
> Phil.
>
