qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH 0/3] tests/acceptance: Handle tests with "cpu" tag


From: Wainer dos Santos Moschetta
Subject: Re: [PATCH 0/3] tests/acceptance: Handle tests with "cpu" tag
Date: Wed, 17 Mar 2021 16:16:06 -0300
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.7.0

Added John and Eduardo,

On 3/9/21 3:52 PM, Cleber Rosa wrote:
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.

I like the idea of a "set_args()" to QEMUMachine as you describe above but it needs further discussion because I can see at least one corner case; for example, one can set the machine type as either -machine or -M, then what key it should be searched-and-replaced (if any) on the list of args?

Unlike your suggestion, I thought on implement the method to deal with a single argument at time, as:

    def set_arg(self, arg: Union[str, list], value: str) -> None:
        """
        Set the value of an argument from the list of extra arguments to be
        given to the QEMU binary. If the argument does not exist then it is
        added to the list.

        If the ``arg`` parameter is a list then it will search and replace all
        occurencies (if any). Otherwise a new argument is added and it is
        used the first value of the ``arg`` list.
        """
        pass

Does it sound good to you?

Thanks!

Wainer

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.




reply via email to

[Prev in Thread] Current Thread [Next in Thread]