Re: Priority of -accel (was: [PATCH] tests/qemu-iotests: Update tests to recent desugarized -accel option)

[Christophe de Dinechin]

> On 7 Jan 2020, at 11:14, Paolo Bonzini <address@hidden> wrote:
> 
> On 07/01/20 11:03, Thomas Huth wrote:
>>> 
>>> vm = QEMUMachine(iotests.qemu_prog)
>>> -vm.add_args('-machine', 'accel=kvm:tcg')
>>> +vm.add_args('-accel', 'kvm', '-accel', 'tcg')
>> Looking at this, I wonder whether we really want the "-accel" option to
>> prioritize the accelerators in the order of appearance? A lot of other
>> CLI tools give the highest priority to the last parameter instead, e.g.
>> "gcc -O3 -O1" compiles with -O1, and not with -O3.
>> 
>> Also I think it might be quite common that there are shell scripts which
>> call "qemu-system-xxx -accel xyz $*" ... and if we don't invert the
>> priorities of -accel, it will be impossible to override -accel in that
>> case...
> 
> Hmm, it does match "-machine accel=kvm:tcg" and in general I think it's
> more self-explanatory.  However, it is indeed less friendly to scripts.
> On one hand those could be changed to place "-accel xyz" after $* (or
> better "$@"), on the other hand we could also add a priority option to
> "-accel".  What do you think?

I tend to agree with Thomas’ assessment that in general, later options
tend to override earlier ones, if only because that’s typically how getopt
works with the typical usage of setting global variables.

I also agree that when it’s a single list given to an option, then it
generally makes more sense to interpret the later ones as a “fallback”
if you could not get the previous ones. Which I believe gives me a
hint of what the intent of specifying multiple options would be…

So what about ranking the accelerators, so that all combinaisons
-accel=kvm:tcg, -accel=tcg:kvm, -accel kvm -accel tcg, etc would
all pickup kvm if available, and tcg as a fallback? Implementation-wise,
it would simply mean ranking the accelerators and updating the accelerator
only if it’s available and better.

To allow full override, we might add something like -accel reset
to reset to the default, i.e. as if no option had been given.

I personally don’t like “priority=n” being explicit on the command-line
because that IMO pushes the problem of ranking the accelerators
to the user, without guidance on how to do that. For example, if I
have a script that reliably overrode before, then it might stop
overriding because the inner script has set priority=2, so I put
priority=3, not knowing that the maintainer thinks my override
broke something, so the inner script maintainers escalates to
priority=4 to “fix” my priority=3. In the end, the whole universe
crumbles or something equally unpredictable.


Cheers,
Christophe

> 
> Paolo
> 
>