Re: [PATCH] tpm: Return QMP error when TPM is disabled in build

[Philippe Mathieu-Daudé]

On 6/9/21 7:36 PM, Daniel P. Berrangé wrote:
> On Wed, Jun 09, 2021 at 07:34:32PM +0200, Philippe Mathieu-Daudé wrote:
>> On 6/9/21 7:27 PM, Philippe Mathieu-Daudé wrote:
>>> On 6/9/21 6:01 PM, Marc-André Lureau wrote:
>>>> Hi
>>>>
>>>> On Wed, Jun 9, 2021 at 7:33 PM Philippe Mathieu-Daudé <philmd@redhat.com
>>>> <mailto:philmd@redhat.com>> wrote:
>>>>
>>>>     When the management layer queries a binary built using --disable-tpm
>>>>     for TPM devices, it gets confused by getting empty responses:
>>>>
>>>>       { "execute": "query-tpm" }
>>>>       {
>>>>           "return": [
>>>>           ]
>>>>       }
>>>>       { "execute": "query-tpm-types" }
>>>>       {
>>>>           "return": [
>>>>           ]
>>>>       }
>>>>       { "execute": "query-tpm-models" }
>>>>       {
>>>>           "return": [
>>>>           ]
>>>>       }
>>>>
>>>>     Make it clearer by returning an error, mentioning the feature is
>>>>     disabled:
>>>>
>>>>       { "execute": "query-tpm" }
>>>>       {
>>>>           "error": {
>>>>               "class": "GenericError",
>>>>               "desc": "this feature or command is not currently supported"
>>>>           }
>>>>       }
>>>>
>>>>     Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com
>>>>     <mailto:philmd@redhat.com>>
>>>>
>>>>
>>>> Why not make the qapi schema conditional?
>>
>> Using your suggestion (and ignoring QAPI marshaling error) I'm getting:
>>
>> { "execute": "query-tpm" }
>> {
>>     "error": {
>>         "class": "CommandNotFound",
>>         "desc": "The command query-tpm has not been found"
>>     }
>> }
>>
>> Is that OK from a management perspective?
> 
> That's fairly typical of what we'd expect to see from a feature
> which is either removed at compile time, or never existed in the first
> place. mgmt apps don't really need to distinguish those two scenarios,
> so this is fine.

Thank you!