Re: [PATCH v4 2/4] qdev-properties: Accept bool for OnOffAuto

[Markus Armbruster <armbru@redhat.com>]

Daniel P. Berrangé <berrange@redhat.com> writes:

> On Wed, Jan 08, 2025 at 03:17:51PM +0900, Akihiko Odaki wrote:
>> Accept bool literals for OnOffAuto properties for consistency with bool
>> properties. This enables users to set the "on" or "off" value in a
>> uniform syntax without knowing whether the "auto" value is accepted.
>> This behavior is especially useful when converting an existing bool
>> property to OnOffAuto or vice versa.
>
> Again, to repeat my previous feedback, OnOffAuto is a well defined
> QAPI type - making it secretly accept other values/types behind
> the scenes which are not visible in QAPI scheme is not acceptable.
>
> Effectively this is a backdoor impl of a QAPI alternate
>
>   { 'alternate': 'OnOffAutoOrBool',
>     'data': {
>       'o': 'OnOffAuto',
>       'b': 'bool'
>     }
>   }
>
> except this isn't permitted as the QAPI generator explicitly blocks
> use of alternate when the two branches are 'bool' and 'enum'.
>
> I'm assuming this is because in the QemuOpts scenario, it cannot
> guess upfront whether the input is a bool or enum. This is unfortunate
> though, because at the JSON visitor level it is unambiguous. 
>
> I wonder if the QAPI generator could be relaxed in any viable way ?

Discussed in review of a related prior patch:
https://lore.kernel.org/qemu-devel/87h6c4fqz6.fsf@pond.sub.org/

Here's the relevant part for your convenience:

    >> parse the value as enum, and if that fails, as uint32_t.  QAPI already
    >> provides a way to express "either this type or that type": alternate.
    >> Like this:
    >> 
    >>      { 'alternate': 'OnOffAutoUint32',
    >>        'data': { 'sym': 'OnOffAuto',
    >>                  'uint': 'uint32' } }
    >> 
    >> Unfortunately, such alternates don't work on the command line due to
    >> keyval visitor restrictions.  These cannot be lifted entirely, but we
    >> might be able to lift them sufficiently to make this alternate work.
    >
    > The keyval visitor cannot implement alternates because the command line 
    > input does not have type information. For example, you cannot 
    > distinguish string "0" and integer 0.

    Correct.

    For alternate types, an input visitor picks the branch based on the
    QType.

    With JSON, we have scalar types null, number, string, and bool.

    With keyval, we only have string.  Alternates with more than one scalar
    branch don't work.

    They could be made to work to some degree, though.  Observe:

    * Any value can be a string.

    * "frob" can be nothing else.

    * "on" and "off" can also be bool.

    * "123" and "1e3" can also be number or enum.

    Instead of picking the branch based on the QType, we could pick based on
    QType and value, where the value part is delegated to a visitor method.