On 2025/02/06 22:23, BALATON Zoltan wrote:
On Thu, 6 Feb 2025, Akihiko Odaki wrote:
On 2025/02/06 18:48, Markus Armbruster wrote:
This problem can be solved
using an existing mechanism, OnOffAuto, which differentiates the "auto"
state and explicit the "on" state.
I guess you're proposing something like this:
* auto: on if possible, else off
* on: on if possible, else error
* off: off (always possible)
Which one is the default?
I converted on to auto and off to false in a following patch.
However, converting bool to OnOffAuto surfaces another problem: they
disagree how "on" and "off" should be written. Please note that the
disagreement already exists and so it is nice to solve anyway.
Yes, converting bool to OnOffAuto is an incompatible change.
Not just about conversion, but this inconsistency require users to
know whether a property is bool or OnOffAuto and change how the values
are written in JSON accordingly. This somewhat hurts usability.
Worse than that, the help text is also confusing.
Excerpt from -device virtio-gpu,help
blob=<bool> - on/off (default: false)
busnr=<busnr>
disable-legacy=<OnOffAuto> - on/off/auto (default: "auto")
disable-modern=<bool> - (default: false)
edid=<bool> - on/off (default: true)
event_idx=<bool> - on/off (default: true)
For bools it says on/off yet expects true/false. Wasn't originally true/
on/1 and false/off/0 accepted for bools? Where that got lost? I think
getting back that behaviour would be easiest for users. Users don't care
if OnOffAuto is an enum internally and can't (or don't want to) remember
if bools are written on/off or true/false so accepting these as synonyms
would help users.
The help shows another problem: it mixes two different syntaxes. I sent
a patch to fix the inconsistency in the help. Please review it for more
detailed explanation and actual fix:
https://lore.kernel.org/qemu-devel/20250207-bool-v1-1-5749d5d6d...@daynix.com
Let me go back to the discussion of the bool/OnOffAuto problem below:
The values the command line syntax accepts are on/yes/true/y and
off/no/false/n.
For the command line syntax, you can always use on/off whether the type
is bool or OnOffAuto. In my opinion, it is still not good to reject
yes/true/y and no/false/n for OnOffAuto; why do we suddenly reject them
when the property gets the "auto" value? As you pointed out, the usage
of enum is our internal concern and should not bother users.
The situation is worse for JSON as there is no common literals that are
compatible with both of bool and OnOffAuto, which forces users to
remember the type.
So I think this patch makes sense in terms of usability. Accepting
multiple representations for one value is ugly, but it is better than
exposing the ugliness to users. We should deprecate the representations
except one if we really hate the ugliness.
Regards,
Akihiko Odaki
