[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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

From: Paolo Bonzini
Subject: Re: [PULL 31/31] qemu-option: warn for short-form boolean options
Date: Tue, 16 Feb 2021 11:43:50 +0100
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.6.0

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

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.


reply via email to

[Prev in Thread] Current Thread [Next in Thread]