Re: [Qemu-devel] [PATCH] QMP: TPM QMP and man page documentation updates

[Markus Armbruster]

[Note cc: Anthony for QAPI schema expertise]

Stefan Berger <address@hidden> writes:

> On 03/18/2013 12:16 PM, Markus Armbruster wrote:
>> Corey Bryant <address@hidden> writes:
>>
>>> Signed-off-by: Corey Bryant <address@hidden>
>>> ---
>>>   qemu-options.hx |  3 ++-
>>>   qmp-commands.hx | 59 
>>> +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>>   2 files changed, 61 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/qemu-options.hx b/qemu-options.hx
>>> index 30fb85d..3b3cd0f 100644
>>> --- a/qemu-options.hx
>>> +++ b/qemu-options.hx
>>> @@ -2237,7 +2237,8 @@ Backend type must be:
>>>   @option{passthrough}.
>>>     The specific backend type will determine the applicable
>>> options.
>>> -The @code{-tpmdev} option requires a @code{-device} option.
>>> +The @code{-tpmdev} option creates the TPM backend and requires a
>>> address@hidden option that specifies the TPM frontend interface model.
>>>     Options to each backend are described below.
>>>   diff --git a/qmp-commands.hx b/qmp-commands.hx
>>> index b370060..4eda5ea 100644
>>> --- a/qmp-commands.hx
>>> +++ b/qmp-commands.hx
>>> @@ -2721,18 +2721,77 @@ EQMP
>>>           .mhandler.cmd_new = qmp_marshal_input_query_tpm,
>>>       },
>>>   +SQMP
>>> +query-tpm
>>> +---------
>>> +
>>> +Return information about the TPM device.
>>> +
>>> +Arguments: None
>>> +
>>> +Example:
>>> +
>>> +-> { "execute": "query-tpm" }
>>> +<- { "return":
>>> +     [
>>> +       { "model": "tpm-tis",
>>> +         "tpm-options":
>>> +           { "type": "tpm-passthrough-options",
>>> +             "data":
>>> +               { "cancel-path": "/sys/class/misc/tpm0/device/cancel",
>>> +                 "path": "/dev/tpm0"
>>> +               }
>>> +           },
>>> +         "type": "passthrough",
>>> +         "id": "tpm0"
>>> +       }
>>> +     ]
>>> +   }
>>> +
>>> +EQMP
>>> +
>> "tpm-options" is a discriminated union.  How is its discriminator "type"
>> (here: "tpm-passthrough-options") related to the outer "type" (here:
>> "passthrough")?
>
> It gives you similar information twice. So there is a direct
> relationship between the two types.

Awkward and undocumented.

Relevant parts of qapi-schema.json:

    { 'enum': 'TpmType', 'data': [ 'passthrough' ] }

    { 'union': 'TpmTypeOptions',
       'data': { 'tpm-passthrough-options' : 'TPMPassthroughOptions' } }

    { 'type': 'TPMInfo',
      'data': {'id': 'str',
               'model': 'TpmModel',
               'type': 'TpmType',
               'tpm-options': 'TpmTypeOptions' } }

Type Netdev solves the same problem more elegantly:

    { 'union': 'NetClientOptions',
      'data': {
        'none':     'NetdevNoneOptions',
        'nic':      'NetLegacyNicOptions',
        'user':     'NetdevUserOptions',
        'tap':      'NetdevTapOptions',
        'socket':   'NetdevSocketOptions',
        'vde':      'NetdevVdeOptions',
        'dump':     'NetdevDumpOptions',
        'bridge':   'NetdevBridgeOptions',
        'hubport':  'NetdevHubPortOptions' } }

    { 'type': 'Netdev',
      'data': {
        'id':   'str',
        'opts': 'NetClientOptions' } }

Uses the union's discriminator.  Straightforward.

Following Netdev precedence, we get:

    { 'union': 'TpmTypeOptions',
      'data': { 'passthrough' : 'TPMPassthroughOptions' } }

    { 'type': 'TPMInfo',
      'data': {'id': 'str',
               'model': 'TpmModel',
               'opts': 'TpmTypeOptions' } }

Duplication of type is gone.  No need for documentation.

Since enum TpmType is used elsewhere, it still gets duplicated in the
union's discriminator.  Anthony, is there a way to name the implicit
discriminator enum type for reference elsewhere in the schema?