| Message ID | 20210123143128.1167797-32-pbonzini@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [PULL,01/31] runstate: cleanup reboot and panic actions | expand |
On Sat, 23 Jan 2021 at 14:50, Paolo Bonzini <pbonzini@redhat.com> wrote: > Options such as "server" or "nowait", that are commonly found in -chardev, > are sugar for "server=on" and "wait=off". This is quite surprising and > also does not have any notion of typing attached. It is even possible to > do "-device e1000,noid" and get a device with "id=off". > > Deprecate it and print a warning when it is encountered. In general, > this short form for boolean options only seems to be in wide use for > -chardev and -spice. > > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> Hi -- if you want to deprecate these ways of writing these options, could you also go through the documentation and correct the various examples that still use it, please? "git grep ',server' docs" finds some of them; I bet there are more. Alternatively, maybe we could not deprecate this really common syntax pattern that's going to be in lots of peoples' examples, tutorials, shell scripts and command lines ? thanks -- PMM
Il lun 15 feb 2021, 20:56 Peter Maydell <peter.maydell@linaro.org> ha scritto: > Alternatively, maybe we could not deprecate this really common syntax > pattern that's going to be in lots of peoples' examples, tutorials, > shell scripts and command lines ? > 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. I don't plan to remove the syntax altogether from QemuOpts, but I want to keep open the possibility of switching some options (especially -machine and -object) to a parser which doesn't support it. Paolo > thanks > -- PMM > >
On Mon, 15 Feb 2021 at 23:15, Paolo Bonzini <pbonzini@redhat.com> wrote: > > Il lun 15 feb 2021, 20:56 Peter Maydell <peter.maydell@linaro.org> ha scritto: >> >> Alternatively, maybe we could not deprecate this really common syntax >> pattern that's going to be in lots of peoples' examples, tutorials, >> shell scripts and command lines ? > > > 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. -- PMM
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
On Tue, 16 Feb 2021 at 10:43, Paolo Bonzini <pbonzini@redhat.com> wrote: > 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". The point is not that it's saving five keystrokes. The point is that "server,nowait" is how we've documented that users should select that functionality since forever, and indeed is how we're still documenting it today, and therefore changing it is breaking the existing working setups of a very large group of users. Historical warts that nobody much was really using can be safely excised; historical warts with many many users much less so. thanks -- PMM
On 16/02/21 12:04, Peter Maydell wrote: > On Tue, 16 Feb 2021 at 10:43, Paolo Bonzini <pbonzini@redhat.com> wrote: >> 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". > > The point is not that it's saving five keystrokes. The point > is that "server,nowait" is how we've documented that users > should select that functionality since forever, and indeed > is how we're still documenting it today, and therefore changing > it is breaking the existing working setups of a very large > group of users. > > Historical warts that nobody much was really using can be safely > excised; historical warts with many many users much less so. I agree, and that's why I have no plans to move -chardev off QemuOpts; warning is a different step than excising and sometimes years pass from one to the other. However, that doesn't prevent introducing a warning so that users slowly move away from the problematic functionality. Also, "-chardev" is not the way that most users will configure sockets. The more common "-serial tcp:localhost:12345,server,nowait" will not trigger the warning; that was not clear at all from the the commit message. It may even make sense to deprecate the *long* form in that case (which I am not planning to do, to be clear). I'll fix the documentation for those uses that are affected, though; thanks for reporting that. Paolo
On Tue, 16 Feb 2021 at 11:23, Paolo Bonzini <pbonzini@redhat.com> wrote: > I agree, and that's why I have no plans to move -chardev off QemuOpts; > warning is a different step than excising and sometimes years pass from > one to the other. However, that doesn't prevent introducing a warning > so that users slowly move away from the problematic functionality. If we want to continue to support the functionality then complaining about it doesn't serve much purpose IMHO. > Also, "-chardev" is not the way that most users will configure sockets. > The more common "-serial tcp:localhost:12345,server,nowait" will not > trigger the warning; that was not clear at all from the the commit > message. It may even make sense to deprecate the *long* form in that > case (which I am not planning to do, to be clear). I'll fix the > documentation for those uses that are affected, though; thanks for > reporting that. A bunch of my local scripts for running QEMU got hit by this new deprecation warning; "git grep 'chardev.*nowait'" gets hits in our documentation; if you google for 'chardev "server,nowait"' there are plenty of in-the-wild uses. Is there any other serious use of the 'no' prefix other than 'nowait' ? Perhaps we could get most of the benefit here by banning the 'no' prefix and adding a compatibility 'nowait' bool which had the inverse sense to the existing 'wait' bool ? (TBH I had assumed that 'nowait' was already implemented that way; 'nodelay' is.) -- PMM
On 16/02/21 12:58, Peter Maydell wrote: > On Tue, 16 Feb 2021 at 11:23, Paolo Bonzini <pbonzini@redhat.com> wrote: >> I agree, and that's why I have no plans to move -chardev off QemuOpts; >> warning is a different step than excising and sometimes years pass from >> one to the other. However, that doesn't prevent introducing a warning >> so that users slowly move away from the problematic functionality. > > If we want to continue to support the functionality then complaining > about it doesn't serve much purpose IMHO. It depends. I don't want to support it forever for all options; -machine, -accel and -object are those for which I do intend to remove support for short-form options after the two release deprecation period. My first submission of this patch even special cased "-chardev" to hide the warning, but this was dropped in response to reviews. (https://patchew.org/QEMU/20201103151452.416784-1-pbonzini@redhat.com/20201103151452.416784-5-pbonzini@redhat.com/). I can add that back if you prefer, since it's very simple. Paolo > A bunch of my local scripts for running QEMU got hit by this new > deprecation warning; "git grep 'chardev.*nowait'" gets hits in > our documentation; if you google for 'chardev "server,nowait"' there > are plenty of in-the-wild uses. > > Is there any other serious use of the 'no' prefix other than > 'nowait' ? Perhaps we could get most of the benefit here by > banning the 'no' prefix and adding a compatibility 'nowait' > bool which had the inverse sense to the existing 'wait' bool ? > (TBH I had assumed that 'nowait' was already implemented that > way; 'nodelay' is.)
On Tue, 16 Feb 2021 at 13:30, Paolo Bonzini <pbonzini@redhat.com> wrote: > On 16/02/21 12:58, Peter Maydell wrote: > > On Tue, 16 Feb 2021 at 11:23, Paolo Bonzini <pbonzini@redhat.com> wrote: > >> I agree, and that's why I have no plans to move -chardev off QemuOpts; > >> warning is a different step than excising and sometimes years pass from > >> one to the other. However, that doesn't prevent introducing a warning > >> so that users slowly move away from the problematic functionality. > > > > If we want to continue to support the functionality then complaining > > about it doesn't serve much purpose IMHO. > > It depends. I don't want to support it forever for all options; > -machine, -accel and -object are those for which I do intend to remove > support for short-form options after the two release deprecation period. > > My first submission of this patch even special cased "-chardev" to hide > the warning, but this was dropped in response to reviews. > (https://patchew.org/QEMU/20201103151452.416784-1-pbonzini@redhat.com/20201103151452.416784-5-pbonzini@redhat.com/). > I can add that back if you prefer, since it's very simple. I agree with Daniel that it would be better to be consistent about whether we like these short options or not, but disagree that the answer is to deprecate everywhere :-) Broadly, I think that being able to say 'foo' when foo is a boolean option being set to true is obvious and nice-to-use syntax, and I don't really want it to go away. 'nofoo' for 'foo=false' is much less obvious and I'm happy if we only support it as a special-case for 'nowait'. -- PMM
On 16/02/21 14:36, Peter Maydell wrote: >> My first submission of this patch even special cased "-chardev" to hide >> the warning, but this was dropped in response to reviews. >> (https://patchew.org/QEMU/20201103151452.416784-1-pbonzini@redhat.com/20201103151452.416784-5-pbonzini@redhat.com/). >> I can add that back if you prefer, since it's very simple. > I agree with Daniel that it would be better to be consistent about > whether we like these short options or not, but disagree that > the answer is to deprecate everywhere:-) > > Broadly, I think that being able to say 'foo' when foo is a > boolean option being set to true is obvious and nice-to-use > syntax, and I don't really want it to go away. 'nofoo' for > 'foo=false' is much less obvious and I'm happy if we only > support it as a special-case for 'nowait'. It really depends on what the default "-M pc,nographics" arguably makes sense too (more so than "-M pc,graphics" since true is the default). Likewise for "usb", where the default even depends on the machine type. How do you propose to resolve the issues and ambiguities in the grammar? 1) due to QemuOpts not understanding types, you can specify "serial" and get "serial=on" instead 2) with a parser that understands other types than strings, you would not be able to specify "-M kernel-irqchip" because it would be converted to the boolean "true" and not the enum "'on'" 3) one is not be able to specify "-M pc" -M usb" because the second kernel-irqchip would be interpreted as a machine type? Thanks, Paolo
On Tue, Feb 16, 2021 at 01:36:46PM +0000, Peter Maydell wrote: > On Tue, 16 Feb 2021 at 13:30, Paolo Bonzini <pbonzini@redhat.com> wrote: > > On 16/02/21 12:58, Peter Maydell wrote: > > > On Tue, 16 Feb 2021 at 11:23, Paolo Bonzini <pbonzini@redhat.com> wrote: > > >> I agree, and that's why I have no plans to move -chardev off QemuOpts; > > >> warning is a different step than excising and sometimes years pass from > > >> one to the other. However, that doesn't prevent introducing a warning > > >> so that users slowly move away from the problematic functionality. > > > > > > If we want to continue to support the functionality then complaining > > > about it doesn't serve much purpose IMHO. > > > > It depends. I don't want to support it forever for all options; > > -machine, -accel and -object are those for which I do intend to remove > > support for short-form options after the two release deprecation period. > > > > My first submission of this patch even special cased "-chardev" to hide > > the warning, but this was dropped in response to reviews. > > (https://patchew.org/QEMU/20201103151452.416784-1-pbonzini@redhat.com/20201103151452.416784-5-pbonzini@redhat.com/). > > I can add that back if you prefer, since it's very simple. > > I agree with Daniel that it would be better to be consistent about > whether we like these short options or not, but disagree that > the answer is to deprecate everywhere :-) > > Broadly, I think that being able to say 'foo' when foo is a > boolean option being set to true is obvious and nice-to-use > syntax, and I don't really want it to go away. 'nofoo' for > 'foo=false' is much less obvious and I'm happy if we only > support it as a special-case for 'nowait'. There's an inherant tension in our goals here. It is widely thought that QEMU configuration is complex and painful to understand. From my POV a big part of that believe comes from the fact that we have so many inconsistencies in our parsing code, and many ways of doing the same thing. Every time we have special cases like "foo" as a short hand for "foo=on" or "nofoo" as a short hand for "foo=off", we increase the complexity of QEMU and that impacts how our users view QEMU. IMHO we'd be better off eliminating the boolean short forms entirely in all QEMU options, so that we get consistency and a clearer right way of doing things. The short bool format was created with good intentions, but on balance a bare "foo" isn't a big enough win over "foo=on" to justify its existance long term. I do agree though, that we should not be deprecating something if our documentation is still showing people the deprecated syntax, as that makes us look even worse. Regards, Daniel
On Tue, 16 Feb 2021 at 13:44, Paolo Bonzini <pbonzini@redhat.com> wrote: > > On 16/02/21 14:36, Peter Maydell wrote: > > Broadly, I think that being able to say 'foo' when foo is a > > boolean option being set to true is obvious and nice-to-use > > syntax, and I don't really want it to go away. 'nofoo' for > > 'foo=false' is much less obvious and I'm happy if we only > > support it as a special-case for 'nowait'. > > It really depends on what the default "-M pc,nographics" arguably makes > sense too (more so than "-M pc,graphics" since true is the default). Is anybody using 'pc,nographics' ? google didn't find any examples. > Likewise for "usb", where the default even depends on the machine type. > > How do you propose to resolve the issues and ambiguities in the grammar? > > 1) due to QemuOpts not understanding types, you can specify "serial" and > get "serial=on" instead We should fix this by plumbing through the type information, so that we only allow 'foo' to mean 'foo=on' if foo is really a boolean (or other type that specifies that it has similar behaviour). > 2) with a parser that understands other types than strings, you would > not be able to specify "-M kernel-irqchip" because it would be converted > to the boolean "true" and not the enum "'on'" We should decide whether 'kernel-irqchip' has a type that allows 'no parameter specified' => 'use this default value' or not, and if we go for the latter use whatever default value the backend expects. (And probably "boolean-and-an-extra-value" types should allow the boolean bit to be specified in all the same ways that a plain-old-boolean is.) > 3) one is not be able to specify "-M pc" -M usb" because the second > kernel-irqchip would be interpreted as a machine type? I don't understand this one, I'm afraid. -- PMM
Il mar 16 feb 2021, 15:11 Peter Maydell <peter.maydell@linaro.org> ha scritto: > On Tue, 16 Feb 2021 at 13:44, Paolo Bonzini <pbonzini@redhat.com> wrote: > > > > On 16/02/21 14:36, Peter Maydell wrote: > > > Broadly, I think that being able to say 'foo' when foo is a > > > boolean option being set to true is obvious and nice-to-use > > > syntax, and I don't really want it to go away. 'nofoo' for > > > 'foo=false' is much less obvious and I'm happy if we only > > > support it as a special-case for 'nowait'. > > > > It really depends on what the default "-M pc,nographics" arguably makes > > sense too (more so than "-M pc,graphics" since true is the default). > > Is anybody using 'pc,nographics' ? google didn't find any examples. > It's just an example that the prevalence of "nowait" over "wait" is simply because the default of "server" is false while the default of "wait" is true. Any boolean option whose default is true could benefit from a "no"-prefixed short form. But I am pretty sure that there are users in the wild for noipv4 or noipv6. > How do you propose to resolve the issues and ambiguities in the grammar? > > > > 1) due to QemuOpts not understanding types, you can specify "serial" and > > get "serial=on" instead > > We should fix this by plumbing through the type information, > so that we only allow 'foo' to mean 'foo=on' if foo is really > a boolean (or other type that specifies that it has similar behaviour). > There's already type information for non-freeform options (those where the QemuOptsList includes the list of valid suboptions, such as -smp or -m). Adding it for freeform options (-M, -device) is basically impossible since the type information comes from a mix of QAPI schema, QOM class declarations and C code. One would basically have to do an incremental visit of the schema (assuming there is a schema, and it's not just C code) during the parsing. This is understandably not something I plan to spend time on. I could change QemuOpts to allow short forms for non-freeform option groups, and turn -chardev into a non-freeform option. That would solve the immediate issue with chardev. But I agree that consistent behavior is better, so I don't think this is a good idea either. > 2) with a parser that understands other types than strings, you would > > not be able to specify "-M kernel-irqchip" because it would be converted > > to the boolean "true" and not the enum "'on'" > > We should decide whether 'kernel-irqchip' has a type that > allows 'no parameter specified' => 'use this default value' > or not, and if we go for the latter use whatever default value > the backend expects. I don't understand, the point of short-form options is to use a *non-default* value. (In other words, the affirmative short form would typically be used to specify true when the default is false). (And probably "boolean-and-an-extra-value" > types should allow the boolean bit to be specified in all the > same ways that a plain-old-boolean is.) > > 3) one is not be able to specify "-M pc" -M usb" because the second > > kernel-irqchip would be interpreted as a machine type? > > I don't understand this one, I'm afraid. > I mean that "-M pc -M usb" is parsed as "-machine type=pc -machine type=usb" and then merged into "-machine type=usb". The user would expect "-machine type=c -machine usb=on" since "-M pc -M usb=on" works. So "usb=on" cannot always be shortened to "usb", which is surprising. I think this one only affects -M though. Paolo > -- PMM > >
On Tue, 16 Feb 2021 at 14:45, Paolo Bonzini <pbonzini@redhat.com> wrote: > > Il mar 16 feb 2021, 15:11 Peter Maydell <peter.maydell@linaro.org> ha scritto: >> >> On Tue, 16 Feb 2021 at 13:44, Paolo Bonzini <pbonzini@redhat.com> wrote: >> > >> > On 16/02/21 14:36, Peter Maydell wrote: >> > > Broadly, I think that being able to say 'foo' when foo is a >> > > boolean option being set to true is obvious and nice-to-use >> > > syntax, and I don't really want it to go away. 'nofoo' for >> > > 'foo=false' is much less obvious and I'm happy if we only >> > > support it as a special-case for 'nowait'. >> > >> > It really depends on what the default "-M pc,nographics" arguably makes >> > sense too (more so than "-M pc,graphics" since true is the default). >> >> Is anybody using 'pc,nographics' ? google didn't find any examples. > > > It's just an example that the prevalence of "nowait" over "wait" is simply because the default of "server" is false while the default of "wait" is true. Any boolean option whose default is true could benefit from a "no"-prefixed short form. But I am pretty sure that there are users in the wild for noipv4 or noipv6. I think 'nowait' is special only because for so long our documentation has recommended 'server,nowait' (and possibly also because inetd uses 'nowait'?). I don't think it's inherently much better than "wait=off" or whatever. I just think that if we have a situation where exactly 1 boolean option has very widespread use of 'nofoo' then it's worth special casing it. If we had 50 boolean options which all had about 10% use of 'nofoo' vs 90% 'foo=off' that would be different. -- PMM
Il mar 16 feb 2021, 15:52 Peter Maydell <peter.maydell@linaro.org> ha scritto: > > It's just an example that the prevalence of "nowait" over "wait" is > simply because the default of "server" is false while the default of "wait" > is true. Any boolean option whose default is true could benefit from a > "no"-prefixed short form. But I am pretty sure that there are users in the > wild for noipv4 or noipv6. > > I think 'nowait' is special only because for so long our documentation > has recommended 'server,nowait' (and possibly also because inetd > uses 'nowait'?). I don't think it's inherently much better than > "wait=off" or whatever. I just think that if we have a situation > where exactly 1 boolean option has very widespread use of 'nofoo' then > it's worth special casing it. > If the point is just the one option that needs to be special cased because it's in the documentation, that also applies at a higher level to -chardev: let's just not warn for that one, and that's the end of the discussion. Paolo > -- PMM > >
diff --git a/docs/system/deprecated.rst b/docs/system/deprecated.rst index 651182b2df..9de663526a 100644 --- a/docs/system/deprecated.rst +++ b/docs/system/deprecated.rst @@ -127,6 +127,12 @@ Drives with interface types other than ``if=none`` are for onboard devices. It is possible to use drives the board doesn't pick up with -device. This usage is now deprecated. Use ``if=none`` instead. +Short-form boolean options (since 6.0) +'''''''''''''''''''''''''''''''''''''' + +Boolean options such as ``share=on``/``share=off`` could be written +in short form as ``share`` and ``noshare``. This is now deprecated +and will cause a warning. QEMU Machine Protocol (QMP) commands ------------------------------------ diff --git a/tests/test-qemu-opts.c b/tests/test-qemu-opts.c index 2aab831d10..8bbb17b1c7 100644 --- a/tests/test-qemu-opts.c +++ b/tests/test-qemu-opts.c @@ -515,7 +515,7 @@ static void test_opts_parse(void) error_free_or_abort(&err); g_assert(!opts); - /* Implied value */ + /* Implied value (qemu_opts_parse warns but accepts it) */ opts = qemu_opts_parse(&opts_list_03, "an,noaus,noaus=", false, &error_abort); g_assert_cmpuint(opts_count(opts), ==, 3); diff --git a/util/qemu-option.c b/util/qemu-option.c index 5f27d4369d..40564a12eb 100644 --- a/util/qemu-option.c +++ b/util/qemu-option.c @@ -756,10 +756,12 @@ void qemu_opts_print(QemuOpts *opts, const char *separator) static const char *get_opt_name_value(const char *params, const char *firstname, + bool warn_on_flag, bool *help_wanted, char **name, char **value) { const char *p; + const char *prefix = ""; size_t len; bool is_help = false; @@ -776,10 +778,15 @@ static const char *get_opt_name_value(const char *params, if (strncmp(*name, "no", 2) == 0) { memmove(*name, *name + 2, strlen(*name + 2) + 1); *value = g_strdup("off"); + prefix = "no"; } else { *value = g_strdup("on"); is_help = is_help_option(*name); } + if (!is_help && warn_on_flag) { + warn_report("short-form boolean option '%s%s' deprecated", prefix, *name); + error_printf("Please use %s=%s instead\n", *name, *value); + } } } else { /* found "foo=bar,more" */ @@ -801,14 +808,14 @@ static const char *get_opt_name_value(const char *params, static bool opts_do_parse(QemuOpts *opts, const char *params, const char *firstname, bool prepend, - bool *help_wanted, Error **errp) + bool warn_on_flag, bool *help_wanted, Error **errp) { char *option, *value; const char *p; QemuOpt *opt; for (p = params; *p;) { - p = get_opt_name_value(p, firstname, help_wanted, &option, &value); + p = get_opt_name_value(p, firstname, warn_on_flag, help_wanted, &option, &value); if (help_wanted && *help_wanted) { g_free(option); g_free(value); @@ -839,7 +846,7 @@ static char *opts_parse_id(const char *params) char *name, *value; for (p = params; *p;) { - p = get_opt_name_value(p, NULL, NULL, &name, &value); + p = get_opt_name_value(p, NULL, false, NULL, &name, &value); if (!strcmp(name, "id")) { g_free(name); return value; @@ -858,7 +865,7 @@ bool has_help_option(const char *params) bool ret = false; for (p = params; *p;) { - p = get_opt_name_value(p, NULL, &ret, &name, &value); + p = get_opt_name_value(p, NULL, false, &ret, &name, &value); g_free(name); g_free(value); if (ret) { @@ -878,12 +885,12 @@ bool has_help_option(const char *params) bool qemu_opts_do_parse(QemuOpts *opts, const char *params, const char *firstname, Error **errp) { - return opts_do_parse(opts, params, firstname, false, NULL, errp); + return opts_do_parse(opts, params, firstname, false, false, NULL, errp); } static QemuOpts *opts_parse(QemuOptsList *list, const char *params, bool permit_abbrev, bool defaults, - bool *help_wanted, Error **errp) + bool warn_on_flag, bool *help_wanted, Error **errp) { const char *firstname; char *id = opts_parse_id(params); @@ -906,8 +913,8 @@ static QemuOpts *opts_parse(QemuOptsList *list, const char *params, return NULL; } - if (!opts_do_parse(opts, params, firstname, defaults, help_wanted, - errp)) { + if (!opts_do_parse(opts, params, firstname, defaults, + warn_on_flag, help_wanted, errp)) { qemu_opts_del(opts); return NULL; } @@ -925,7 +932,7 @@ static QemuOpts *opts_parse(QemuOptsList *list, const char *params, QemuOpts *qemu_opts_parse(QemuOptsList *list, const char *params, bool permit_abbrev, Error **errp) { - return opts_parse(list, params, permit_abbrev, false, NULL, errp); + return opts_parse(list, params, permit_abbrev, false, false, NULL, errp); } /** @@ -943,7 +950,7 @@ QemuOpts *qemu_opts_parse_noisily(QemuOptsList *list, const char *params, QemuOpts *opts; bool help_wanted = false; - opts = opts_parse(list, params, permit_abbrev, false, + opts = opts_parse(list, params, permit_abbrev, false, true, opts_accepts_any(list) ? NULL : &help_wanted, &err); if (!opts) { @@ -962,7 +969,7 @@ void qemu_opts_set_defaults(QemuOptsList *list, const char *params, { QemuOpts *opts; - opts = opts_parse(list, params, permit_abbrev, true, NULL, NULL); + opts = opts_parse(list, params, permit_abbrev, true, false, NULL, NULL); assert(opts); }
Options such as "server" or "nowait", that are commonly found in -chardev, are sugar for "server=on" and "wait=off". This is quite surprising and also does not have any notion of typing attached. It is even possible to do "-device e1000,noid" and get a device with "id=off". Deprecate it and print a warning when it is encountered. In general, this short form for boolean options only seems to be in wide use for -chardev and -spice. Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> --- docs/system/deprecated.rst | 6 ++++++ tests/test-qemu-opts.c | 2 +- util/qemu-option.c | 29 ++++++++++++++++++----------- 3 files changed, 25 insertions(+), 12 deletions(-)