Re: [Qemu-devel] [PATCH v3 4/6] qemu-option: support +foo/-foo command line agruments

[Igor Mammedov <imammedo@redhat.com>]

On Tue, 12 Nov 2013 10:49:58 +1100
Alexey Kardashevskiy <aik@ozlabs.ru> wrote:

> On 11/12/2013 01:25 AM, Igor Mammedov wrote:
> > On Mon, 11 Nov 2013 13:41:05 +0100
> > Andreas Färber <afaerber@suse.de> wrote:
> > 
> >> Am 11.11.2013 08:44, schrieb Alexey Kardashevskiy:
> >>> This converts +foo/-foo to "foo=on"/"foo=off" respectively when
> >>> QEMU parser is used for the command line options.
> >>>
> >>> "-cpu" parsers in x86 and other architectures should be unaffected
> >>> by this change.
> >>>
> >>> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> >>> ---
> >>>  util/qemu-option.c | 6 ++++++
> >>>  1 file changed, 6 insertions(+)
> >>>
> >>> diff --git a/util/qemu-option.c b/util/qemu-option.c
> >>> index efcb5dc..6c8667c 100644
> >>> --- a/util/qemu-option.c
> >>> +++ b/util/qemu-option.c
> >>> @@ -890,6 +890,12 @@ static int opts_do_parse(QemuOpts *opts, const char *params,
> >>>                  if (strncmp(option, "no", 2) == 0) {
> >>>                      memmove(option, option+2, strlen(option+2)+1);
> >>>                      pstrcpy(value, sizeof(value), "off");
> >>> +                } else if (strncmp(option, "-", 1) == 0) {
> >>> +                    memmove(option, option+1, strlen(option+1)+1);
> >>> +                    pstrcpy(value, sizeof(value), "off");
> >>> +                } else if (strncmp(option, "+", 1) == 0) {
> >>> +                    memmove(option, option+1, strlen(option+1)+1);
> >>> +                    pstrcpy(value, sizeof(value), "on");
> >>>                  } else {
> >>>                      pstrcpy(value, sizeof(value), "on");
> >>>                  }
> >>
> >> This looks like an interesting idea! However this is much too big a
> >> change to just CC ppc folks on...
> >>
> >> Jan, I wonder if this might break slirp's hostfwd option?
> >>
> >> Not sure what other options potentially starting with '-' might be
> >> affected. Test cases would be a helpful way of demonstrating that this
> >> change does not have undesired side effects.
> > on x86 there is several value fixups for compatibility reason and a manual
> > value parsing in cpu_x86_parse_featurestr(), so above won't just work there.
> 
> 
> What particular x86 CPU option cannot be handled the way as PPC's "VSX" is
> handled two patches below? As I see, even static properties will work there
> fine.
There is legacy code that is kept for CLI compatibility reasons.
Please, look at following features in cpu_x86_parse_featurestr():
  xlevel, tsc-freq hv-spinlocks
the rest feature flags on x86 should be handled just fine by your patch,
once x86properties series is applied. 

that's why we are talking about parser hook that could be overridden
by target if necessary.

PS:
extending QemuOpts to parsing +/-opts format, seems like good workaround
above problem. But I was under impression that general movement was to convert
custom formats to canonical format "prop=value".

> 
> 
> -- 
> Alexey
> 


-- 
Regards,
  Igor