Re: [PULL 31/31] qemu-option: warn for short-form boolean options

[Paolo Bonzini]

On 16/02/21 10:58, Peter Maydell wrote:
> > Unfortunately there is no way to change the code to distinguish okay
> > uses from broken ones. The fundamental issue is that QemuOpts is
> > sometimes typed and sometimes not, so it lacks the information to say
> > that "-chardev socket,server" is fine but "-device virtio-blk-pci,noserial"
> > ("set serial number to the string 'no'") is not.
>
> That is definitely a nonsensical example. But it's not clear to me
> that it's an improvement to start forbidding previously sensible and
> working command lines in order to be able to diagnose nonsensical
> command lines which it seems unlikely that anybody was ever actually
> using.

The problem with QemuOpts is twofold, in general and specifically for 
short-form boolean options.

On one hand it's the parser itself that is too permissive, because it 
applies a concept that is valid for boolean (short-form options) to all 
types.  This is the above "noserial" case.

On the other hand, the typing of QemuOpts itself is not something that 
is used a lot, especially as more and more options become a sort of 
discriminated union and therefore cannot really have a schema that is 
described in QemuOptsList and this means that it's not really possible 
to fix the other problem within QemuOpts.

The question is whether it's fixable in general.

For this to work, one would need to have a typed string->QAPI parser, 
i.e. one that takes the schema as input rather than doing a generic 
parsing to QDict and then using the schema via the visitor.  That would 
IMHO be overengineered for the purpose of saving five keystrokes on 
"server,nowait".  But even if that were possible, there are two issues:

1) the short-form "-machine kernel-irqchip" is applied not to a boolean 
but rather to an on/off/split enum.  So we would have to either 
introduce extra complication in the schema visitor to distinguish 
"extended boolean" enums from the others (and then, once we have defined 
the concept of "extended boolean enums", would we also accept yes/no in 
addition to on/off?).

2) the lexing is ambiguous.  For example virtio devices define a 
"notify_on_empty" property.  Attempting to set this property with a 
short form would fail, as it would be interpreted as 
"tify_on_empty=off".  Even worse is hw/isa/lpc_ich9.c's "noreboot" 
property (though the device is not user-creatable) for which it is 
possible to write "nonoreboot" but not "noreboot" (interpreted as 
reboot=yes).

I understand that none of these problems make it *impossible* to keep 
the short-form boolean options or even to reimplement them.  However, 
they do make me question whether they are a good idea and not just a 
historical wart.

Again, I do not plan to remove the short forms from QemuOpts.  However, 
I would like not to lock QEMU into the QemuOpts parser and its many 
issues, which are preventing a cleaner evolution of the QEMU command 
line.  (In particular I would like many command line options such as 
-smp, -m or -icount to be syntactic sugar for record properties -machine 
smp.xxx, -machine mem.xxx or -accel tcg,icount.xxx).  Even though I have 
not yet posted patches for this due to the deprecation period of 
short-form boolean options, I _did_ propose the deprecation only after 
writing a bunch of other code around command-line parsing cleanups.

Paolo