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

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

[Corey Bryant]

Signed-off-by: Corey Bryant <coreyb@linux.vnet.ibm.com>
---
 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
+@code{-device} 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
+
     {
         .name       = "query-tpm-models",
         .args_type  = "",
         .mhandler.cmd_new = qmp_marshal_input_query_tpm_models,
     },
 
+SQMP
+query-tpm-models
+----------------
+
+Return a list of supported TPM models.
+
+Arguments: None
+
+Example:
+
+-> { "execute": "query-tpm-models" }
+<- { "return": [ "tpm-tis" ] }
+
+EQMP
+
     {
         .name       = "query-tpm-types",
         .args_type  = "",
         .mhandler.cmd_new = qmp_marshal_input_query_tpm_types,
     },
 
+SQMP
+query-tpm-types
+---------------
+
+Return a list of supported TPM types.
+
+Arguments: None
+
+Example:
+
+-> { "execute": "query-tpm-types" }
+<- { "return": [ "passthrough" ] }
+
+EQMP
+
     {
         .name       = "chardev-add",
         .args_type  = "id:s,backend:q",
-- 
1.8.1.4

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

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 434 bytes --]

On 03/15/2013 11:17 AM, Corey Bryant wrote:
> Signed-off-by: Corey Bryant <coreyb@linux.vnet.ibm.com>
> ---
>  qemu-options.hx |  3 ++-
>  qmp-commands.hx | 59 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 61 insertions(+), 1 deletion(-)

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 621 bytes --]

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

[Markus Armbruster]

Corey Bryant <coreyb@linux.vnet.ibm.com> writes:

> Signed-off-by: Corey Bryant <coreyb@linux.vnet.ibm.com>
> ---
>  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
> +@code{-device} 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")?

[...]

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

[Stefan Berger]

On 03/18/2013 12:16 PM, Markus Armbruster wrote:
> Corey Bryant <coreyb@linux.vnet.ibm.com> writes:
>
>> Signed-off-by: Corey Bryant <coreyb@linux.vnet.ibm.com>
>> ---
>>   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
>> +@code{-device} 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.

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

[Corey Bryant]

On 03/18/2013 01:46 PM, Stefan Berger wrote:
> On 03/18/2013 12:16 PM, Markus Armbruster wrote:
>> Corey Bryant <coreyb@linux.vnet.ibm.com> writes:
>>
>>> Signed-off-by: Corey Bryant <coreyb@linux.vnet.ibm.com>
>>> ---
>>>   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
>>> +@code{-device} 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.
>

The sample above could be the result when the following command line 
options are in effect:

qemu-system-x86_64
   -tpmdev 
passthrough,id=tpm0,path=/dev/tpm0,cancel-path=/sys/class/misc/tpm0/device/cancel
   -device tpm-tis,tpmdev=tpm0

-- 
Regards,
Corey Bryant

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

[Markus Armbruster]

[Note cc: Anthony for QAPI schema expertise]

Stefan Berger <stefanb@linux.vnet.ibm.com> writes:

> On 03/18/2013 12:16 PM, Markus Armbruster wrote:
>> Corey Bryant <coreyb@linux.vnet.ibm.com> writes:
>>
>>> Signed-off-by: Corey Bryant <coreyb@linux.vnet.ibm.com>
>>> ---
>>>   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
>>> +@code{-device} 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?

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

[Corey Bryant]

On 03/19/2013 03:26 AM, Markus Armbruster wrote:
> [Note cc: Anthony for QAPI schema expertise]
>
> Stefan Berger <stefanb@linux.vnet.ibm.com> writes:
>
>> On 03/18/2013 12:16 PM, Markus Armbruster wrote:
>>> Corey Bryant <coreyb@linux.vnet.ibm.com> writes:
>>>
>>>> Signed-off-by: Corey Bryant <coreyb@linux.vnet.ibm.com>
>>>> ---
>>>>    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
>>>> +@code{-device} 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' } }
>

I can send a patch for this update if you'd like.

> 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?
>

Are you saying it still gets duplicated with this fix?  I'm not sure 
what you mean.

-- 
Regards,
Corey Bryant

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

[Markus Armbruster]

Corey Bryant <coreyb@linux.vnet.ibm.com> writes:

> On 03/19/2013 03:26 AM, Markus Armbruster wrote:
>> [Note cc: Anthony for QAPI schema expertise]
>>
>> Stefan Berger <stefanb@linux.vnet.ibm.com> writes:
>>
>>> On 03/18/2013 12:16 PM, Markus Armbruster wrote:
>>>> Corey Bryant <coreyb@linux.vnet.ibm.com> writes:
>>>>
>>>>> Signed-off-by: Corey Bryant <coreyb@linux.vnet.ibm.com>
>>>>> ---
>>>>>    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
>>>>> +@code{-device} 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' } }
>>
>
> I can send a patch for this update if you'd like.

Yes, please!

>> 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?
>>
>
> Are you saying it still gets duplicated with this fix?  I'm not sure
> what you mean.

A union in the schema implicitely defines an C enumeration type to be
used for its discriminator.  For instance, union TpmTypeOptions
implicitely defines this one:

    typedef enum TpmTypeOptionsKind
    {
        TPM_TYPE_OPTIONS_KIND_TPM_PASSTHROUGH_OPTIONS = 0,
        TPM_TYPE_OPTIONS_KIND_MAX = 1,
    } TpmTypeOptionsKind;

QAPI's discriminated union becomes a C struct containing the
discriminator and the union of the members:

    struct TpmTypeOptions
    {
        TpmTypeOptionsKind kind;
        union {
            void *data;
            TPMPassthroughOptions * tpm_passthrough_options;
        };
    };

Enum TpmType and type TpmInfo become:

    typedef enum TpmType
    {
        TPM_TYPE_PASSTHROUGH = 0,
        TPM_TYPE_MAX = 1,
    } TpmType;

    struct TPMInfo
    {
        char * id;
        TpmModel model;
        TpmType type;
        TpmTypeOptions * tpm_options;
    };

With the change I propose, TPM_TYPE_OPTIONS_KIND_TPM_PASSTHROUGH_OPTIONS
becomes TPM_TYPE_OPTIONS_KIND_TPM_PASSTHROUGH, and TPMInfo member type
goes away.

Because TpmType is still used elsewhere, it doesn't go away, thus still
duplicates TpmTypeOptionsKind.  My question to Anthony was about ways to
avoid that remaining bit of duplication.  I don't expect you do answer
it :)

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

[Corey Bryant]

On 03/20/2013 08:32 AM, Markus Armbruster wrote:
> Corey Bryant <coreyb@linux.vnet.ibm.com> writes:
>
>> On 03/19/2013 03:26 AM, Markus Armbruster wrote:
>>> [Note cc: Anthony for QAPI schema expertise]
>>>
>>> Stefan Berger <stefanb@linux.vnet.ibm.com> writes:
>>>
>>>> On 03/18/2013 12:16 PM, Markus Armbruster wrote:
>>>>> Corey Bryant <coreyb@linux.vnet.ibm.com> writes:
>>>>>
>>>>>> Signed-off-by: Corey Bryant <coreyb@linux.vnet.ibm.com>
>>>>>> ---
>>>>>>     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
>>>>>> +@code{-device} 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' } }
>>>
>>
>> I can send a patch for this update if you'd like.
>
> Yes, please!
>

Will do.

>>> 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?
>>>
>>
>> Are you saying it still gets duplicated with this fix?  I'm not sure
>> what you mean.
>
> A union in the schema implicitely defines an C enumeration type to be
> used for its discriminator.  For instance, union TpmTypeOptions
> implicitely defines this one:
>
>      typedef enum TpmTypeOptionsKind
>      {
>          TPM_TYPE_OPTIONS_KIND_TPM_PASSTHROUGH_OPTIONS = 0,
>          TPM_TYPE_OPTIONS_KIND_MAX = 1,
>      } TpmTypeOptionsKind;
>
> QAPI's discriminated union becomes a C struct containing the
> discriminator and the union of the members:
>
>      struct TpmTypeOptions
>      {
>          TpmTypeOptionsKind kind;
>          union {
>              void *data;
>              TPMPassthroughOptions * tpm_passthrough_options;
>          };
>      };
>
> Enum TpmType and type TpmInfo become:
>
>      typedef enum TpmType
>      {
>          TPM_TYPE_PASSTHROUGH = 0,
>          TPM_TYPE_MAX = 1,
>      } TpmType;
>
>      struct TPMInfo
>      {
>          char * id;
>          TpmModel model;
>          TpmType type;
>          TpmTypeOptions * tpm_options;
>      };
>
> With the change I propose, TPM_TYPE_OPTIONS_KIND_TPM_PASSTHROUGH_OPTIONS
> becomes TPM_TYPE_OPTIONS_KIND_TPM_PASSTHROUGH, and TPMInfo member type
> goes away.
>
> Because TpmType is still used elsewhere, it doesn't go away, thus still
> duplicates TpmTypeOptionsKind.  My question to Anthony was about ways to
> avoid that remaining bit of duplication.  I don't expect you do answer
> it :)
>

Ok well thanks for the explanation anyway!

-- 
Regards,
Corey Bryant

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

[Corey Bryant]

I just resubmitted this patch as part of another series, so please 
ignore this one.

-- 
Regards,
Corey Bryant

On 03/15/2013 01:17 PM, Corey Bryant wrote:
> Signed-off-by: Corey Bryant <coreyb@linux.vnet.ibm.com>
> ---
>   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
> +@code{-device} 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
> +
>       {
>           .name       = "query-tpm-models",
>           .args_type  = "",
>           .mhandler.cmd_new = qmp_marshal_input_query_tpm_models,
>       },
>
> +SQMP
> +query-tpm-models
> +----------------
> +
> +Return a list of supported TPM models.
> +
> +Arguments: None
> +
> +Example:
> +
> +-> { "execute": "query-tpm-models" }
> +<- { "return": [ "tpm-tis" ] }
> +
> +EQMP
> +
>       {
>           .name       = "query-tpm-types",
>           .args_type  = "",
>           .mhandler.cmd_new = qmp_marshal_input_query_tpm_types,
>       },
>
> +SQMP
> +query-tpm-types
> +---------------
> +
> +Return a list of supported TPM types.
> +
> +Arguments: None
> +
> +Example:
> +
> +-> { "execute": "query-tpm-types" }
> +<- { "return": [ "passthrough" ] }
> +
> +EQMP
> +
>       {
>           .name       = "chardev-add",
>           .args_type  = "id:s,backend:q",
>