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

[Daniel P . Berrangé]

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.

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|