On Wed, Feb 24, 2021 at 06:26:51PM -0300, Wainer dos Santos Moschetta wrote: > Currently the acceptance tests tagged with "machine" have the "-M TYPE" > automatically added to the list of arguments of the QEMUMachine object. > In other words, that option is passed to the launched QEMU. On this > series it is implemented the same feature but instead for tests marked > with "cpu". > Good! > There is a caveat, however, in case the test needs additional arguments to > the CPU type they cannot be passed via tag, because the tags parser split > values by comma. For example, in tests/acceptance/x86_cpu_model_versions.py, > there are cases where: > > * -cpu is set to "Cascadelake-Server,x-force-features=on,check=off,enforce=off" > * if it was tagged like "cpu:Cascadelake-Server,x-force-features=on,check=off,enforce=off" > then the parser would break it into 4 tags ("cpu:Cascadelake-Server", > "x-force-features=on", "check=off", "enforce=off") > * resulting on "-cpu Cascadelake-Server" and the remaining arguments are ignored. > > For the example above, one should tag it (or not at all) as "cpu:Cascadelake-Server" > AND self.vm.add_args('-cpu', "Cascadelake-Server,x-force-features=on,check=off,enforce=off"), > and that results on something like: > > "qemu-system-x86_64 (...) -cpu Cascadelake-Server -cpu Cascadelake-Server,x-force-features=on,check=off,enforce=off". > There are clearly two problems here: 1) the tag is meant to be succinct, so that it can be used by users selecting which tests to run. At the same time, it's a waste to throw away the other information or keep it duplicate or incosistent. 2) QEMUMachine doesn't keep track of command line arguments (add_args() makes it pretty clear what's doing). But, on this type of use case, a "set_args()" is desirable, in which case it would overwrite the existing arguments for a given command line option. > QEMU is going to ignore the first -cpu argument. See the patch 0003 for a reference. > But this would still be creating a QEMU command line with multiple '-cpu' arguments, right? I understand this could be useful for testing the behavior of the parameter parsing (if that's intended) but it's bad practice to be generating incorrect command line in tests. Maybe just by tackling issue #2 this could be avoided. Cheers, - Cleber.