Re: [Qemu-devel] [PATCH] Replace '-machine accel=xyz' with '-accel xyz'

[Markus Armbruster]

Paolo Bonzini <address@hidden> writes:

> On 04/09/19 07:27, Thomas Huth wrote:
>> We've got a separate option to configure the accelerator nowadays, which
>> is shorter to type and the preferred way of specifying an accelerator.
>> Use it in the source and examples to show that it is the favored option.
>> (However, do not touch the places yet which also specify other machine
>> options or multiple accelerators - these are currently still better
>> handled with one single "-machine" statement instead)
>> 
>> Signed-off-by: Thomas Huth <address@hidden>
>> ---
>>  python/qemu/qtest.py                | 2 +-
>>  qemu-deprecated.texi                | 3 +--
>>  qemu-options.hx                     | 2 +-
>>  tests/libqtest.c                    | 2 +-
>>  tests/migration/guestperf/engine.py | 2 +-
>>  tests/qemu-iotests/172              | 2 +-
>>  6 files changed, 6 insertions(+), 7 deletions(-)
>> 
>> diff --git a/python/qemu/qtest.py b/python/qemu/qtest.py
>> index eebcc233ed..3f1d2cb325 100644
>> --- a/python/qemu/qtest.py
>> +++ b/python/qemu/qtest.py
>> @@ -96,7 +96,7 @@ class QEMUQtestMachine(QEMUMachine):
>>      def _base_args(self):
>>          args = super(QEMUQtestMachine, self)._base_args()
>>          args.extend(['-qtest', 'unix:path=' + self._qtest_path,
>> -                     '-machine', 'accel=qtest'])
>> +                     '-accel', 'qtest'])
>>          return args
>>  
>>      def _pre_launch(self):
>> diff --git a/qemu-deprecated.texi b/qemu-deprecated.texi
>> index 00a4b6f350..0982e41698 100644
>> --- a/qemu-deprecated.texi
>> +++ b/qemu-deprecated.texi
>> @@ -26,8 +26,7 @@ The @option{enforce-config-section} parameter is replaced 
>> by the
>>  
>>  @subsection -no-kvm (since 1.3.0)
>>  
>> -The ``-no-kvm'' argument is now a synonym for setting
>> -``-machine accel=tcg''.
>> +The ``-no-kvm'' argument is now a synonym for setting ``-accel tcg''.
>>  
>>  @subsection -usbdevice (since 2.10.0)
>>  
>> diff --git a/qemu-options.hx b/qemu-options.hx
>> index 09e6439646..e0bba2abd1 100644
>> --- a/qemu-options.hx
>> +++ b/qemu-options.hx
>> @@ -4156,7 +4156,7 @@ STEXI
>>  Enable FIPS 140-2 compliance mode.
>>  ETEXI
>>  
>> -HXCOMM Deprecated by -machine accel=tcg property
>> +HXCOMM Deprecated by -accel tcg
>>  DEF("no-kvm", 0, QEMU_OPTION_no_kvm, "", QEMU_ARCH_I386)
>>  
>>  DEF("msg", HAS_ARG, QEMU_OPTION_msg,
>> diff --git a/tests/libqtest.c b/tests/libqtest.c
>> index 2713b86cf7..67e39c59e7 100644
>> --- a/tests/libqtest.c
>> +++ b/tests/libqtest.c
>> @@ -238,7 +238,7 @@ QTestState *qtest_init_without_qmp_handshake(const char 
>> *extra_args)
>>                                "-qtest-log %s "
>>                                "-chardev socket,path=%s,id=char0 "
>>                                "-mon chardev=char0,mode=control "
>> -                              "-machine accel=qtest "
>> +                              "-accel qtest "
>>                                "-display none "
>>                                "%s", qemu_binary, socket_path,
>>                                getenv("QTEST_LOG") ? "/dev/fd/2" : 
>> "/dev/null",
>> diff --git a/tests/migration/guestperf/engine.py 
>> b/tests/migration/guestperf/engine.py
>> index f13dbea800..1dd04ce33b 100644
>> --- a/tests/migration/guestperf/engine.py
>> +++ b/tests/migration/guestperf/engine.py
>> @@ -287,7 +287,7 @@ class Engine(object):
>>              cmdline = "'" + cmdline + "'"
>>  
>>          argv = [
>> -            "-machine", "accel=kvm",
>> +            "-accel", "kvm",
>>              "-cpu", "host",
>>              "-kernel", self._kernel,
>>              "-initrd", self._initrd,
>> diff --git a/tests/qemu-iotests/172 b/tests/qemu-iotests/172
>> index ba7dad9057..d67997e5f6 100755
>> --- a/tests/qemu-iotests/172
>> +++ b/tests/qemu-iotests/172
>> @@ -55,7 +55,7 @@ do_run_qemu()
>>              done
>>          fi
>>          echo quit
>> -    ) | $QEMU -machine accel=qtest -nographic -monitor stdio -serial none 
>> "$@"
>> +    ) | $QEMU -accel qtest -nographic -monitor stdio -serial none "$@"
>>      echo
>>  }
>>  
>> 
>
> Acked-by: Paolo Bonzini <address@hidden>
>
> Thanks.  While "-accel kvm:tcg" is not going to be supported, the above
> replacement are all good.

-accel is yet another convenience option.  We have so many of them.  I
dislike the complexity they add to the CLI.  Here's how this one got in:

commit 8d4e9146b3568022ea5730d92841345d41275d66
Author: KONRAD Frederic <address@hidden>
Date:   Thu Feb 23 18:29:08 2017 +0000

    tcg: add options for enabling MTTCG
    
    We know there will be cases where MTTCG won't work until additional work
    is done in the front/back ends to support. It will however be useful to
    be able to turn it on.
    
    As a result MTTCG will default to off unless the combination is
    supported. However the user can turn it on for the sake of testing.
    
    Signed-off-by: KONRAD Frederic <address@hidden>
    [AJB: move to -accel tcg,thread=multi|single, defaults]
    Signed-off-by: Alex Bennée <address@hidden>
    Reviewed-by: Richard Henderson <address@hidden>

Not a peep on why the existing options are so insufficient we must have
another one.

Our CLI will remain the steaming mess it has become until we reform the
habits that got us there.

No objection to your patch.