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?).
>>>> +{ '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.
