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

[Akihiko Odaki]

On 2025/01/10 21:16, Daniel P. Berrangé wrote:
> On Fri, Jan 10, 2025 at 08:31:57PM +0900, Akihiko Odaki wrote:
> > On 2025/01/10 20:09, Daniel P. Berrangé wrote:
> > > 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'.
> >
> > The QAPI generator specifically blocks the case where the enum contains 'on'
> > or 'off'.
> >
> > > 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.
> >
> > It's probably for the command line and possibly HMP.
> >
> > > I wonder if the QAPI generator could be relaxed in any viable way ?
> > It will make the interpretation of 'on' and 'off' on the command line
> > ambigious; it can be either of OnOffAuto or bool.
>
> The ambiguity would not neccessarily matter from a functional POV
> though.
>
> The consumer of an "OnOffAutoOrBool" field, would likely want to apply
> logic to collapse it into just "OnOffAuto". As such, whether "on" is
> parsed as a enum value or a bool value would have no functional
> difference in the end. The OnOffAutoOrBool is essentially there to
> just make sure we clearly express our input schema.

I think it is worth creating a special type that requires the logic to 
collapse into OnOffAuto.

If we don't have a special type, the QAPI generator will write C struct 
like as follows:

struct OnOffAutoOrBool {
    QType type;
    union { /* union tag is @type */
        OnOffAuto o;
        bool b;
    } u;
};

It's easier if we have a special type that directly maps into a 
tristate. The story shouldn't be different for other possible consumers; 
a special type will make it easy to map QAPI types into 
consumer-specific type systems.

Regards,
Akihiko Odaki