[Qemu-devel] [PATCH v2 0/7] Acceptance Tests: basic architecture support

[Qemu-devel] [PATCH v2 0/7] Acceptance Tests: basic architecture support

[Cleber Rosa]

The current version of the Acceptance Tests have been basically tested
on x86_64.  Most of them should be valid tests on many different
architectures.

This introduces another standard test parameter, 'arch', and a public
test attribute with the same name.  Then, because of the different
behavior in different QEMU targets, it adds a more explicit
configuration of the QEMUMachine machine type used on the tests (the
self.vm attribute).

Finally, for tests that are known to be architecture specific, it
changes the approach, from using tags to canceling the test.  The
difference is that this reuses the same 'arch' parameter (so no need
to pass tags for the same reason), and instead of completely excluding
the test from the job, it just won't be executed on architectures that
are not supported.  More details about this on the last commit.

Changes from v1:
================

 * Fixed typo in docstring (s/param/type) (Murilo)

 * Pretty print the "arch.json" variants file, in order to make
   editing easier and avoid hitting email line length limits (Eric,
   Philippe)

 * Distinguish between host and target arch when canceling tests
   (Philippe)

Cleber Rosa (7):
  Acceptance Tests: improve docstring on pick_default_qemu_bin()
  Acceptance Tests: introduce arch parameter and attribute
  scripts/qemu.py: add method and private attribute for arch
  scripts/qemu.py: set predefined machine type based on arch
  Acceptance Tests: set machine type
  Acceptance Tests: add variants definition for architectures
  Acceptance Tests: change the handling of tests for specific archs

 docs/devel/testing.rst                    | 18 +++++
 scripts/qemu.py                           | 29 ++++++-
 tests/acceptance/avocado_qemu/__init__.py | 17 ++++-
 tests/acceptance/boot_linux_console.py    |  6 +-
 tests/acceptance/variants/arch.json       | 92 +++++++++++++++++++++++
 tests/acceptance/version.py               |  2 +
 tests/acceptance/vnc.py                   |  5 ++
 7 files changed, 163 insertions(+), 6 deletions(-)
 create mode 100644 tests/acceptance/variants/arch.json

-- 
2.17.1

[Qemu-devel] [PATCH v2 1/7] Acceptance Tests: improve docstring on pick_default_qemu_bin()

[Cleber Rosa]

Making it clear what is returned by this utility function.

Signed-off-by: Cleber Rosa <crosa@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 tests/acceptance/avocado_qemu/__init__.py | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/tests/acceptance/avocado_qemu/__init__.py b/tests/acceptance/avocado_qemu/__init__.py
index 1e54fd5932..d8d5b48dac 100644
--- a/tests/acceptance/avocado_qemu/__init__.py
+++ b/tests/acceptance/avocado_qemu/__init__.py
@@ -27,6 +27,10 @@ def pick_default_qemu_bin():
     """
     Picks the path of a QEMU binary, starting either in the current working
     directory or in the source tree root directory.
+
+    :returns: the path to the default QEMU binary or None if one could not
+              be found
+    :rtype: str or None
     """
     arch = os.uname()[4]
     qemu_bin_relative_path = os.path.join("%s-softmmu" % arch,
-- 
2.17.1

[Qemu-devel] [PATCH v2 2/7] Acceptance Tests: introduce arch parameter and attribute

[Cleber Rosa]

On a number of different scenarios, such as when choosing a QEMU
binary to be used on tests (or a image to use to boot a test VM), it's
useful to define the architecture that should be used.

This introduces both a test parameter and a test instance attribute,
that will contain such a value.

The selection of the default QEMU binary, if one is not given
explicitly, has also been changed to look at the arch attribute.

Signed-off-by: Cleber Rosa <crosa@redhat.com>
---
 docs/devel/testing.rst                    | 18 ++++++++++++++++++
 tests/acceptance/avocado_qemu/__init__.py | 13 ++++++++++---
 2 files changed, 28 insertions(+), 3 deletions(-)

diff --git a/docs/devel/testing.rst b/docs/devel/testing.rst
index 727c4019b5..4178ae5022 100644
--- a/docs/devel/testing.rst
+++ b/docs/devel/testing.rst
@@ -659,6 +659,17 @@ vm
 A QEMUMachine instance, initially configured according to the given
 ``qemu_bin`` parameter.
 
+arch
+~~~~
+
+The architecture that will be used on a number of different
+scenarios.  For instance, when a QEMU binary is not explicitly given,
+the one selected will depend on this attribute.
+
+The ``arch`` attribute will be set to the test parameter of the same
+name, and if one is not given explicitly, it will default to the
+system architecture (as given by ``uname``).
+
 qemu_bin
 ~~~~~~~~
 
@@ -681,6 +692,13 @@ like the following:
 
   PARAMS (key=qemu_bin, path=*, default=x86_64-softmmu/qemu-system-x86_64) => 'x86_64-softmmu/qemu-system-x86_64
 
+arch
+~~~~
+
+The architecture that will be used on a number of different scenarios.
+This parameter has a direct relation with the ``arch`` attribute.  If
+not given, it will default to None.
+
 qemu_bin
 ~~~~~~~~
 
diff --git a/tests/acceptance/avocado_qemu/__init__.py b/tests/acceptance/avocado_qemu/__init__.py
index d8d5b48dac..b8d934382d 100644
--- a/tests/acceptance/avocado_qemu/__init__.py
+++ b/tests/acceptance/avocado_qemu/__init__.py
@@ -23,16 +23,22 @@ def is_readable_executable_file(path):
     return os.path.isfile(path) and os.access(path, os.R_OK | os.X_OK)
 
 
-def pick_default_qemu_bin():
+def pick_default_qemu_bin(arch=None):
     """
     Picks the path of a QEMU binary, starting either in the current working
     directory or in the source tree root directory.
 
+    :param arch: the arch to use when looking for a QEMU binary (the target
+                 will match the arch given).  If None (the default) arch
+                 will be the current host system arch (as given by
+                 :func:`os.uname`).
+    :type arch: str
     :returns: the path to the default QEMU binary or None if one could not
               be found
     :rtype: str or None
     """
-    arch = os.uname()[4]
+    if arch is None:
+        arch = os.uname()[4]
     qemu_bin_relative_path = os.path.join("%s-softmmu" % arch,
                                           "qemu-system-%s" % arch)
     if is_readable_executable_file(qemu_bin_relative_path):
@@ -47,8 +53,9 @@ def pick_default_qemu_bin():
 class Test(avocado.Test):
     def setUp(self):
         self.vm = None
+        self.arch = self.params.get('arch', default=os.uname()[4])
         self.qemu_bin = self.params.get('qemu_bin',
-                                        default=pick_default_qemu_bin())
+                                        default=pick_default_qemu_bin(self.arch))
         if self.qemu_bin is None:
             self.cancel("No QEMU binary defined or found in the source tree")
         self.vm = QEMUMachine(self.qemu_bin)
-- 
2.17.1

[Qemu-devel] [PATCH v2 3/7] scripts/qemu.py: add method and private attribute for arch

[Cleber Rosa]

Because some sane defaults may require the knowledge of the arch,
let's give the QEMUMachine the opportunity to hold that information.

Signed-off-by: Cleber Rosa <crosa@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 scripts/qemu.py | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/scripts/qemu.py b/scripts/qemu.py
index f099ce7278..d9e24a0c1a 100644
--- a/scripts/qemu.py
+++ b/scripts/qemu.py
@@ -113,6 +113,7 @@ class QEMUMachine(object):
         self._test_dir = test_dir
         self._temp_dir = None
         self._launched = False
+        self._arch = None
         self._machine = None
         self._console_device_type = None
         self._console_address = None
@@ -406,6 +407,12 @@ class QEMUMachine(object):
         '''
         self._args.extend(args)
 
+    def set_arch(self, arch):
+        """
+        Sets the architecture of this machine
+        """
+        self._arch = arch
+
     def set_machine(self, machine_type):
         '''
         Sets the machine type
-- 
2.17.1

[Qemu-devel] [PATCH v2 4/7] scripts/qemu.py: set predefined machine type based on arch

[Cleber Rosa]

Some targets require a machine type to be set, as there's no default
(aarch64 is one example).  To give a consistent interface to users of
this API, this changes set_machine() so that a predefined default can
be used, if one is not given.  The approach used is exactly the same
with the console device type.

Also, even when there's a default machine type, for some purposes,
testing included, it's better if outside code is explicit about the
machine type, instead of relying on whatever is set internally.

Signed-off-by: Cleber Rosa <crosa@redhat.com>
---
 scripts/qemu.py | 22 +++++++++++++++++++++-
 1 file changed, 21 insertions(+), 1 deletion(-)

diff --git a/scripts/qemu.py b/scripts/qemu.py
index d9e24a0c1a..fca9b76990 100644
--- a/scripts/qemu.py
+++ b/scripts/qemu.py
@@ -36,6 +36,15 @@ CONSOLE_DEV_TYPES = {
     r'^s390-ccw-virtio.*': 'sclpconsole',
     }
 
+#: Maps archictures to the preferred machine type
+MACHINE_TYPES = {
+    r'^aarch64$': 'virt',
+    r'^ppc$': 'g3beige',
+    r'^ppc64$': 'pseries',
+    r'^s390x$': 's390-ccw-virtio',
+    r'^x86_64$': 'q35',
+    }
+
 
 class QEMUMachineError(Exception):
     """
@@ -413,13 +422,24 @@ class QEMUMachine(object):
         """
         self._arch = arch
 
-    def set_machine(self, machine_type):
+    def set_machine(self, machine_type=None):
         '''
         Sets the machine type
 
         If set, the machine type will be added to the base arguments
         of the resulting QEMU command line.
         '''
+        if machine_type is None:
+            if self._arch is None:
+                raise QEMUMachineError("Can not set a default machine type: "
+                                       "QEMU instance without a defined arch")
+            for regex, machine in MACHINE_TYPES.items():
+                if re.match(regex, self._arch):
+                    machine_type = machine
+                    break
+            if machine_type is None:
+                raise QEMUMachineError("Can not set a machine type: no "
+                                       "matching machine type definition")
         self._machine = machine_type
 
     def set_console(self, device_type=None):
-- 
2.17.1

[Qemu-devel] [PATCH v2 5/7] Acceptance Tests: set machine type

[Cleber Rosa]

By setting the machine type, even if it's the one that will be picked
based on the arch, it's possible to run the same tests with targets
that require a machine type (in addition to those that don't).

Given that only boot_linux_console.py contains code specific to x86_64
(an explicit reference to the kernel image that will be used) the
other tests can be used to test different targets:

  $ avocado run -p arch=aarch64 --filter-by-tags='-x86_64' -- tests/acceptance/

Eventually, to reduce boiler plate code, the idea is to concentrate
the basic configuration (arch, machine, console) in either another
utility method, or make that happen by default.  This is of course the
subject of a future discussion.

Signed-off-by: Cleber Rosa <crosa@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 tests/acceptance/boot_linux_console.py | 3 ++-
 tests/acceptance/version.py            | 2 ++
 tests/acceptance/vnc.py                | 5 +++++
 3 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/tests/acceptance/boot_linux_console.py b/tests/acceptance/boot_linux_console.py
index 98324f7591..58032f971c 100644
--- a/tests/acceptance/boot_linux_console.py
+++ b/tests/acceptance/boot_linux_console.py
@@ -30,7 +30,8 @@ class BootLinuxConsole(Test):
         kernel_hash = '238e083e114c48200f80d889f7e32eeb2793e02a'
         kernel_path = self.fetch_asset(kernel_url, asset_hash=kernel_hash)
 
-        self.vm.set_machine('pc')
+        self.vm.set_arch(self.arch)
+        self.vm.set_machine()
         self.vm.set_console()
         kernel_command_line = 'console=ttyS0'
         self.vm.add_args('-kernel', kernel_path,
diff --git a/tests/acceptance/version.py b/tests/acceptance/version.py
index 13b0a7440d..7a3a20945f 100644
--- a/tests/acceptance/version.py
+++ b/tests/acceptance/version.py
@@ -18,6 +18,8 @@ class Version(Test):
     :avocado: tags=quick
     """
     def test_qmp_human_info_version(self):
+        self.vm.set_arch(self.arch)
+        self.vm.set_machine()
         self.vm.launch()
         res = self.vm.command('human-monitor-command',
                               command_line='info version')
diff --git a/tests/acceptance/vnc.py b/tests/acceptance/vnc.py
index b1ef9d71b1..4a8a83025f 100644
--- a/tests/acceptance/vnc.py
+++ b/tests/acceptance/vnc.py
@@ -16,6 +16,11 @@ class Vnc(Test):
     :avocado: enable
     :avocado: tags=vnc,quick
     """
+    def setUp(self):
+        super(Vnc, self).setUp()
+        self.vm.set_arch(self.arch)
+        self.vm.set_machine()
+
     def test_no_vnc(self):
         self.vm.add_args('-nodefaults', '-S')
         self.vm.launch()
-- 
2.17.1

[Qemu-devel] [PATCH v2 6/7] Acceptance Tests: add variants definition for architectures

[Cleber Rosa]

One of the Avocado features relevant to virtualization testing is the
ability to reuse tests in different scenarios, known as variants.
This adds a JSON based variants file, that can be used to run most
tests in a number of different architectures.  It can be run with:

   $ avocado run \
     --json-variants-load=tests/acceptance/variants/arch.json \
     --filter-by-tags='-x86_64' -- tests/acceptance/

Currently this covers 5 architectures, resulting in the execution
of 25 different combinations.

Signed-off-by: Cleber Rosa <crosa@redhat.com>
---
 tests/acceptance/variants/arch.json | 92 +++++++++++++++++++++++++++++
 1 file changed, 92 insertions(+)
 create mode 100644 tests/acceptance/variants/arch.json

diff --git a/tests/acceptance/variants/arch.json b/tests/acceptance/variants/arch.json
new file mode 100644
index 0000000000..64ff6ad278
--- /dev/null
+++ b/tests/acceptance/variants/arch.json
@@ -0,0 +1,92 @@
+[
+    {
+        "paths": [
+            "/run/*"
+        ],
+        "variant": [
+            [
+                "/run/aarch64",
+                [
+                    [
+                        "/run/aarch64",
+                        "arch",
+                        "aarch64"
+                    ]
+                ]
+            ]
+        ],
+        "variant_id": "aarch64"
+    },
+    {
+        "paths": [
+            "/run/*"
+        ],
+        "variant": [
+            [
+                "/run/ppc",
+                [
+                    [
+                        "/run/ppc",
+                        "arch",
+                        "ppc"
+                    ]
+                ]
+            ]
+        ],
+        "variant_id": "ppc"
+    },
+    {
+        "paths": [
+            "/run/*"
+        ],
+        "variant": [
+            [
+                "/run/ppc64",
+                [
+                    [
+                        "/run/ppc64",
+                        "arch",
+                        "ppc64"
+                    ]
+                ]
+            ]
+        ],
+        "variant_id": "ppc64"
+    },
+    {
+        "paths": [
+            "/run/*"
+        ],
+        "variant": [
+            [
+                "/run/s390x",
+                [
+                    [
+                        "/run/s390x",
+                        "arch",
+                        "s390x"
+                    ]
+                ]
+            ]
+        ],
+        "variant_id": "s390x"
+    },
+    {
+        "paths": [
+            "/run/*"
+        ],
+        "variant": [
+            [
+                "/run/x86_64",
+                [
+                    [
+                        "/run/x86_64",
+                        "arch",
+                        "x86_64"
+                    ]
+                ]
+            ]
+        ],
+        "variant_id": "x86_64"
+    }
+]
-- 
2.17.1

[Qemu-devel] [PATCH v2 7/7] Acceptance Tests: change the handling of tests for specific archs

[Cleber Rosa]

With the introduction of a variants file that can run the same
tests on various architectures, it makes sense to make most tests
to be reusable on those environments.  The exception should be
when a test is really testing a specific architecture feature.

With the change proposed here, on a command line such as:

  $ avocado run \
     --json-variants-load=tests/acceptance/variants/arch.json \
     -- tests/acceptance/

The boot_linux_console.py tests will appear as "CANCELED: Currently
specific to the x86_64 arch", which is as a good thing when compared
to being ignored by tags because:

 * The architecture specific parts can be addressed
 * It will be run on the matching architecture (as opposed to always
   being filtered out by the tags mechanism)
 * CANCELED tests do no influence negatively the overall job results,
   they're not considered an error or failure

Signed-off-by: Cleber Rosa <crosa@redhat.com>
---
 tests/acceptance/boot_linux_console.py | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tests/acceptance/boot_linux_console.py b/tests/acceptance/boot_linux_console.py
index 58032f971c..ba3ac036da 100644
--- a/tests/acceptance/boot_linux_console.py
+++ b/tests/acceptance/boot_linux_console.py
@@ -19,12 +19,13 @@ class BootLinuxConsole(Test):
     and the kernel command line is properly passed from QEMU to the kernel
 
     :avocado: enable
-    :avocado: tags=x86_64
     """
 
     timeout = 60
 
     def test(self):
+        if self.arch != 'x86_64':
+            self.cancel('Currently specific to the x86_64 target arch')
         kernel_url = ('https://mirrors.kernel.org/fedora/releases/28/'
                       'Everything/x86_64/os/images/pxeboot/vmlinuz')
         kernel_hash = '238e083e114c48200f80d889f7e32eeb2793e02a'
-- 
2.17.1

Re: [Qemu-devel] [PATCH v2 7/7] Acceptance Tests: change the handling of tests for specific archs

[Philippe Mathieu-Daudé]

On 10/10/2018 01:26, Cleber Rosa wrote:
> With the introduction of a variants file that can run the same
> tests on various architectures, it makes sense to make most tests
> to be reusable on those environments.  The exception should be
> when a test is really testing a specific architecture feature.
> 
> With the change proposed here, on a command line such as:
> 
>   $ avocado run \
>      --json-variants-load=tests/acceptance/variants/arch.json \
>      -- tests/acceptance/
> 
> The boot_linux_console.py tests will appear as "CANCELED: Currently
> specific to the x86_64 arch", which is as a good thing when compared
> to being ignored by tags because:
> 
>  * The architecture specific parts can be addressed
>  * It will be run on the matching architecture (as opposed to always
>    being filtered out by the tags mechanism)
>  * CANCELED tests do no influence negatively the overall job results,
>    they're not considered an error or failure
> 
> Signed-off-by: Cleber Rosa <crosa@redhat.com>
> ---
>  tests/acceptance/boot_linux_console.py | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/tests/acceptance/boot_linux_console.py b/tests/acceptance/boot_linux_console.py
> index 58032f971c..ba3ac036da 100644
> --- a/tests/acceptance/boot_linux_console.py
> +++ b/tests/acceptance/boot_linux_console.py
> @@ -19,12 +19,13 @@ class BootLinuxConsole(Test):
>      and the kernel command line is properly passed from QEMU to the kernel
>  
>      :avocado: enable
> -    :avocado: tags=x86_64
>      """
>  
>      timeout = 60
>  
>      def test(self):
> +        if self.arch != 'x86_64':
> +            self.cancel('Currently specific to the x86_64 target arch')
>          kernel_url = ('https://mirrors.kernel.org/fedora/releases/28/'
>                        'Everything/x86_64/os/images/pxeboot/vmlinuz')
>          kernel_hash = '238e083e114c48200f80d889f7e32eeb2793e02a'
> 

For some reason this test run quicker on a aarch64 host than my x86 laptop:

ThunderX 88XX (aarch64 Little Endian):

 (05/30)
tests/acceptance/boot_linux_console.py:BootLinuxConsole.test;x86_64:
PASS (12.88 s)

Intel(R) Core(TM) i7-8650U CPU @ 1.90GHz:

 (05/30)
tests/acceptance/boot_linux_console.py:BootLinuxConsole.test;x86_64:
PASS (31.13 s)

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Re: [Qemu-devel] [PATCH v2 6/7] Acceptance Tests: add variants definition for architectures

[Philippe Mathieu-Daudé]

Hi Cleber,

On 10/10/2018 01:26, Cleber Rosa wrote:
> One of the Avocado features relevant to virtualization testing is the
> ability to reuse tests in different scenarios, known as variants.
> This adds a JSON based variants file, that can be used to run most
> tests in a number of different architectures.  It can be run with:
> 
>    $ avocado run \
>      --json-variants-load=tests/acceptance/variants/arch.json \
>      --filter-by-tags='-x86_64' -- tests/acceptance/
> 
> Currently this covers 5 architectures, resulting in the execution
> of 25 different combinations.

Can you provide here the command you used to generate this JSON?

Thanks,

Phil.

> 
> Signed-off-by: Cleber Rosa <crosa@redhat.com>
> ---
>  tests/acceptance/variants/arch.json | 92 +++++++++++++++++++++++++++++
>  1 file changed, 92 insertions(+)
>  create mode 100644 tests/acceptance/variants/arch.json
> 
> diff --git a/tests/acceptance/variants/arch.json b/tests/acceptance/variants/arch.json
> new file mode 100644
> index 0000000000..64ff6ad278
> --- /dev/null
> +++ b/tests/acceptance/variants/arch.json
> @@ -0,0 +1,92 @@
> +[
> +    {
> +        "paths": [
> +            "/run/*"
> +        ],
> +        "variant": [
> +            [
> +                "/run/aarch64",
> +                [
> +                    [
> +                        "/run/aarch64",
> +                        "arch",
> +                        "aarch64"
> +                    ]
> +                ]
> +            ]
> +        ],
> +        "variant_id": "aarch64"
> +    },
> +    {
> +        "paths": [
> +            "/run/*"
> +        ],
> +        "variant": [
> +            [
> +                "/run/ppc",
> +                [
> +                    [
> +                        "/run/ppc",
> +                        "arch",
> +                        "ppc"
> +                    ]
> +                ]
> +            ]
> +        ],
> +        "variant_id": "ppc"
> +    },
> +    {
> +        "paths": [
> +            "/run/*"
> +        ],
> +        "variant": [
> +            [
> +                "/run/ppc64",
> +                [
> +                    [
> +                        "/run/ppc64",
> +                        "arch",
> +                        "ppc64"
> +                    ]
> +                ]
> +            ]
> +        ],
> +        "variant_id": "ppc64"
> +    },
> +    {
> +        "paths": [
> +            "/run/*"
> +        ],
> +        "variant": [
> +            [
> +                "/run/s390x",
> +                [
> +                    [
> +                        "/run/s390x",
> +                        "arch",
> +                        "s390x"
> +                    ]
> +                ]
> +            ]
> +        ],
> +        "variant_id": "s390x"
> +    },
> +    {
> +        "paths": [
> +            "/run/*"
> +        ],
> +        "variant": [
> +            [
> +                "/run/x86_64",
> +                [
> +                    [
> +                        "/run/x86_64",
> +                        "arch",
> +                        "x86_64"
> +                    ]
> +                ]
> +            ]
> +        ],
> +        "variant_id": "x86_64"
> +    }
> +]
> 

Re: [Qemu-devel] [PATCH v2 3/7] scripts/qemu.py: add method and private attribute for arch

[Philippe Mathieu-Daudé]

On 10/10/2018 01:26, Cleber Rosa wrote:
> Because some sane defaults may require the knowledge of the arch,
> let's give the QEMUMachine the opportunity to hold that information.
> 
> Signed-off-by: Cleber Rosa <crosa@redhat.com>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com>

> ---
>  scripts/qemu.py | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/scripts/qemu.py b/scripts/qemu.py
> index f099ce7278..d9e24a0c1a 100644
> --- a/scripts/qemu.py
> +++ b/scripts/qemu.py
> @@ -113,6 +113,7 @@ class QEMUMachine(object):
>          self._test_dir = test_dir
>          self._temp_dir = None
>          self._launched = False
> +        self._arch = None
>          self._machine = None
>          self._console_device_type = None
>          self._console_address = None
> @@ -406,6 +407,12 @@ class QEMUMachine(object):
>          '''
>          self._args.extend(args)
>  
> +    def set_arch(self, arch):
> +        """
> +        Sets the architecture of this machine
> +        """
> +        self._arch = arch
> +
>      def set_machine(self, machine_type):
>          '''
>          Sets the machine type
> 

Re: [Qemu-devel] [PATCH v2 6/7] Acceptance Tests: add variants definition for architectures

[Philippe Mathieu-Daudé]

On 10/10/2018 01:26, Cleber Rosa wrote:
> One of the Avocado features relevant to virtualization testing is the
> ability to reuse tests in different scenarios, known as variants.
> This adds a JSON based variants file, that can be used to run most
> tests in a number of different architectures.  It can be run with:
> 
>    $ avocado run \
>      --json-variants-load=tests/acceptance/variants/arch.json \
>      --filter-by-tags='-x86_64' -- tests/acceptance/
> 
> Currently this covers 5 architectures, resulting in the execution
> of 25 different combinations.

IMHO we should add this file with all supported archs, and CANCEL a job
if there is no preferred machine in Qemu::MACHINE_TYPES.

> 
> Signed-off-by: Cleber Rosa <crosa@redhat.com>
> ---
>  tests/acceptance/variants/arch.json | 92 +++++++++++++++++++++++++++++
>  1 file changed, 92 insertions(+)
>  create mode 100644 tests/acceptance/variants/arch.json

Re: [Qemu-devel] [PATCH v2 4/7] scripts/qemu.py: set predefined machine type based on arch

[Philippe Mathieu-Daudé]

On 10/10/2018 01:26, Cleber Rosa wrote:
> Some targets require a machine type to be set, as there's no default
> (aarch64 is one example).  To give a consistent interface to users of
> this API, this changes set_machine() so that a predefined default can
> be used, if one is not given.  The approach used is exactly the same
> with the console device type.
> 
> Also, even when there's a default machine type, for some purposes,
> testing included, it's better if outside code is explicit about the
> machine type, instead of relying on whatever is set internally.
> 
> Signed-off-by: Cleber Rosa <crosa@redhat.com>
> ---
>  scripts/qemu.py | 22 +++++++++++++++++++++-
>  1 file changed, 21 insertions(+), 1 deletion(-)
> 
> diff --git a/scripts/qemu.py b/scripts/qemu.py
> index d9e24a0c1a..fca9b76990 100644
> --- a/scripts/qemu.py
> +++ b/scripts/qemu.py
> @@ -36,6 +36,15 @@ CONSOLE_DEV_TYPES = {
>      r'^s390-ccw-virtio.*': 'sclpconsole',
>      }
>  
> +#: Maps archictures to the preferred machine type
> +MACHINE_TYPES = {
> +    r'^aarch64$': 'virt',
> +    r'^ppc$': 'g3beige',
> +    r'^ppc64$': 'pseries',
> +    r'^s390x$': 's390-ccw-virtio',
> +    r'^x86_64$': 'q35',

Why choose Q35 rather than PC (the default)?

I was wondering about how to generate variants/machines.json but this is
definitively something we want to do via a QMP query.

Eduardo what do you think?

> +    }
> +
>  
>  class QEMUMachineError(Exception):
>      """
> @@ -413,13 +422,24 @@ class QEMUMachine(object):
>          """
>          self._arch = arch
>  
> -    def set_machine(self, machine_type):
> +    def set_machine(self, machine_type=None):
>          '''
>          Sets the machine type
>  
>          If set, the machine type will be added to the base arguments
>          of the resulting QEMU command line.
>          '''
> +        if machine_type is None:
> +            if self._arch is None:
> +                raise QEMUMachineError("Can not set a default machine type: "
> +                                       "QEMU instance without a defined arch")
> +            for regex, machine in MACHINE_TYPES.items():
> +                if re.match(regex, self._arch):
> +                    machine_type = machine
> +                    break
> +            if machine_type is None:
> +                raise QEMUMachineError("Can not set a machine type: no "
> +                                       "matching machine type definition")
>          self._machine = machine_type
>  
>      def set_console(self, device_type=None):
> 

Re: [Qemu-devel] [PATCH v2 2/7] Acceptance Tests: introduce arch parameter and attribute

[Philippe Mathieu-Daudé]

On 10/10/2018 01:26, Cleber Rosa wrote:
> On a number of different scenarios, such as when choosing a QEMU
> binary to be used on tests (or a image to use to boot a test VM), it's
> useful to define the architecture that should be used.
> 
> This introduces both a test parameter and a test instance attribute,
> that will contain such a value.
> 
> The selection of the default QEMU binary, if one is not given
> explicitly, has also been changed to look at the arch attribute.
> 
> Signed-off-by: Cleber Rosa <crosa@redhat.com>

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com>

> ---
>  docs/devel/testing.rst                    | 18 ++++++++++++++++++
>  tests/acceptance/avocado_qemu/__init__.py | 13 ++++++++++---
>  2 files changed, 28 insertions(+), 3 deletions(-)
> 
> diff --git a/docs/devel/testing.rst b/docs/devel/testing.rst
> index 727c4019b5..4178ae5022 100644
> --- a/docs/devel/testing.rst
> +++ b/docs/devel/testing.rst
> @@ -659,6 +659,17 @@ vm
>  A QEMUMachine instance, initially configured according to the given
>  ``qemu_bin`` parameter.
>  
> +arch
> +~~~~
> +
> +The architecture that will be used on a number of different
> +scenarios.  For instance, when a QEMU binary is not explicitly given,
> +the one selected will depend on this attribute.
> +
> +The ``arch`` attribute will be set to the test parameter of the same
> +name, and if one is not given explicitly, it will default to the
> +system architecture (as given by ``uname``).
> +
>  qemu_bin
>  ~~~~~~~~
>  
> @@ -681,6 +692,13 @@ like the following:
>  
>    PARAMS (key=qemu_bin, path=*, default=x86_64-softmmu/qemu-system-x86_64) => 'x86_64-softmmu/qemu-system-x86_64
>  
> +arch
> +~~~~
> +
> +The architecture that will be used on a number of different scenarios.
> +This parameter has a direct relation with the ``arch`` attribute.  If
> +not given, it will default to None.
> +
>  qemu_bin
>  ~~~~~~~~
>  
> diff --git a/tests/acceptance/avocado_qemu/__init__.py b/tests/acceptance/avocado_qemu/__init__.py
> index d8d5b48dac..b8d934382d 100644
> --- a/tests/acceptance/avocado_qemu/__init__.py
> +++ b/tests/acceptance/avocado_qemu/__init__.py
> @@ -23,16 +23,22 @@ def is_readable_executable_file(path):
>      return os.path.isfile(path) and os.access(path, os.R_OK | os.X_OK)
>  
>  
> -def pick_default_qemu_bin():
> +def pick_default_qemu_bin(arch=None):
>      """
>      Picks the path of a QEMU binary, starting either in the current working
>      directory or in the source tree root directory.
>  
> +    :param arch: the arch to use when looking for a QEMU binary (the target
> +                 will match the arch given).  If None (the default) arch
> +                 will be the current host system arch (as given by
> +                 :func:`os.uname`).
> +    :type arch: str
>      :returns: the path to the default QEMU binary or None if one could not
>                be found
>      :rtype: str or None
>      """
> -    arch = os.uname()[4]
> +    if arch is None:
> +        arch = os.uname()[4]
>      qemu_bin_relative_path = os.path.join("%s-softmmu" % arch,
>                                            "qemu-system-%s" % arch)
>      if is_readable_executable_file(qemu_bin_relative_path):
> @@ -47,8 +53,9 @@ def pick_default_qemu_bin():
>  class Test(avocado.Test):
>      def setUp(self):
>          self.vm = None
> +        self.arch = self.params.get('arch', default=os.uname()[4])
>          self.qemu_bin = self.params.get('qemu_bin',
> -                                        default=pick_default_qemu_bin())
> +                                        default=pick_default_qemu_bin(self.arch))
>          if self.qemu_bin is None:
>              self.cancel("No QEMU binary defined or found in the source tree")
>          self.vm = QEMUMachine(self.qemu_bin)
> 

Re: [Qemu-devel] [PATCH v2 4/7] scripts/qemu.py: set predefined machine type based on arch

[Cleber Rosa]

On 10/10/18 7:00 AM, Philippe Mathieu-Daudé wrote:
> On 10/10/2018 01:26, Cleber Rosa wrote:
>> Some targets require a machine type to be set, as there's no default
>> (aarch64 is one example).  To give a consistent interface to users of
>> this API, this changes set_machine() so that a predefined default can
>> be used, if one is not given.  The approach used is exactly the same
>> with the console device type.
>>
>> Also, even when there's a default machine type, for some purposes,
>> testing included, it's better if outside code is explicit about the
>> machine type, instead of relying on whatever is set internally.
>>
>> Signed-off-by: Cleber Rosa <crosa@redhat.com>
>> ---
>>  scripts/qemu.py | 22 +++++++++++++++++++++-
>>  1 file changed, 21 insertions(+), 1 deletion(-)
>>
>> diff --git a/scripts/qemu.py b/scripts/qemu.py
>> index d9e24a0c1a..fca9b76990 100644
>> --- a/scripts/qemu.py
>> +++ b/scripts/qemu.py
>> @@ -36,6 +36,15 @@ CONSOLE_DEV_TYPES = {
>>      r'^s390-ccw-virtio.*': 'sclpconsole',
>>      }
>>  
>> +#: Maps archictures to the preferred machine type
>> +MACHINE_TYPES = {
>> +    r'^aarch64$': 'virt',
>> +    r'^ppc$': 'g3beige',
>> +    r'^ppc64$': 'pseries',
>> +    r'^s390x$': 's390-ccw-virtio',
>> +    r'^x86_64$': 'q35',
> 
> Why choose Q35 rather than PC (the default)?
> 
> I was wondering about how to generate variants/machines.json but this is
> definitively something we want to do via a QMP query.
> 
> Eduardo what do you think?
> 

It was motivated by Eduardo's initiative to make q35 the default "across
the board".  He can confirm and give more details.

- Cleber.

>> +    }
>> +
>>  
>>  class QEMUMachineError(Exception):
>>      """
>> @@ -413,13 +422,24 @@ class QEMUMachine(object):
>>          """
>>          self._arch = arch
>>  
>> -    def set_machine(self, machine_type):
>> +    def set_machine(self, machine_type=None):
>>          '''
>>          Sets the machine type
>>  
>>          If set, the machine type will be added to the base arguments
>>          of the resulting QEMU command line.
>>          '''
>> +        if machine_type is None:
>> +            if self._arch is None:
>> +                raise QEMUMachineError("Can not set a default machine type: "
>> +                                       "QEMU instance without a defined arch")
>> +            for regex, machine in MACHINE_TYPES.items():
>> +                if re.match(regex, self._arch):
>> +                    machine_type = machine
>> +                    break
>> +            if machine_type is None:
>> +                raise QEMUMachineError("Can not set a machine type: no "
>> +                                       "matching machine type definition")
>>          self._machine = machine_type
>>  
>>      def set_console(self, device_type=None):
>>

Re: [Qemu-devel] [PATCH v2 6/7] Acceptance Tests: add variants definition for architectures

[Cleber Rosa]

On 10/10/18 6:59 AM, Philippe Mathieu-Daudé wrote:
> On 10/10/2018 01:26, Cleber Rosa wrote:
>> One of the Avocado features relevant to virtualization testing is the
>> ability to reuse tests in different scenarios, known as variants.
>> This adds a JSON based variants file, that can be used to run most
>> tests in a number of different architectures.  It can be run with:
>>
>>    $ avocado run \
>>      --json-variants-load=tests/acceptance/variants/arch.json \
>>      --filter-by-tags='-x86_64' -- tests/acceptance/
>>
>> Currently this covers 5 architectures, resulting in the execution
>> of 25 different combinations.
> 
> IMHO we should add this file with all supported archs, and CANCEL a job
> if there is no preferred machine in Qemu::MACHINE_TYPES.
> 

While your proposal works, I see two issues:

 1) It defines a policy, which will affect all tests, existing and new -
even for tests that don't launch a VM and don't care about the arch.

 2) CANCEL is mostly silent, and the amount of noise in test results
would be quite large.

So, the real problem is that there are other issues when making basic
use of QEMUMachine with other archs.  The idea is to incrementally test
other archs, make the modifications necessary to scripts/qemu.py, and
end up with all architectures covered.

What do you think?

- Cleber.

>>
>> Signed-off-by: Cleber Rosa <crosa@redhat.com>
>> ---
>>  tests/acceptance/variants/arch.json | 92 +++++++++++++++++++++++++++++
>>  1 file changed, 92 insertions(+)
>>  create mode 100644 tests/acceptance/variants/arch.json

Re: [Qemu-devel] [PATCH v2 6/7] Acceptance Tests: add variants definition for architectures

[Cleber Rosa]

On 10/10/18 6:51 AM, Philippe Mathieu-Daudé wrote:
> Hi Cleber,
> 
> On 10/10/2018 01:26, Cleber Rosa wrote:
>> One of the Avocado features relevant to virtualization testing is the
>> ability to reuse tests in different scenarios, known as variants.
>> This adds a JSON based variants file, that can be used to run most
>> tests in a number of different architectures.  It can be run with:
>>
>>    $ avocado run \
>>      --json-variants-load=tests/acceptance/variants/arch.json \
>>      --filter-by-tags='-x86_64' -- tests/acceptance/
>>
>> Currently this covers 5 architectures, resulting in the execution
>> of 25 different combinations.
> 
> Can you provide here the command you used to generate this JSON?
> 

Yep, there are many ways to do it, one is to have the
varianter-yaml-to-mux plugin installed, and create a YAML file such as:

---- archs.yaml START -----
!mux
x86_64:
  arch: x86_64
aarch64:
  arch: aarch64
ppc64:
  arch: ppc64
ppc:
  arch: ppc
s390x:
  arch: s390x
---- archs.yaml END -----

And run:

 $ avocado run -m archs.yaml -- /bin/true
 $ python -m json.tool ~/avocado/job-results/latest/jobdata/variants.json

I did tweak the "variant_id" value of the resulting JSON file, because
the hashes are somewhat specific to yaml-to-mux, and don't add anything
to the meaning of the variants.

- Cleber.

> Thanks,
> 
> Phil.
> 
>>
>> Signed-off-by: Cleber Rosa <crosa@redhat.com>
>> ---
>>  tests/acceptance/variants/arch.json | 92 +++++++++++++++++++++++++++++
>>  1 file changed, 92 insertions(+)
>>  create mode 100644 tests/acceptance/variants/arch.json
>>
>> diff --git a/tests/acceptance/variants/arch.json b/tests/acceptance/variants/arch.json
>> new file mode 100644
>> index 0000000000..64ff6ad278
>> --- /dev/null
>> +++ b/tests/acceptance/variants/arch.json
>> @@ -0,0 +1,92 @@
>> +[
>> +    {
>> +        "paths": [
>> +            "/run/*"
>> +        ],
>> +        "variant": [
>> +            [
>> +                "/run/aarch64",
>> +                [
>> +                    [
>> +                        "/run/aarch64",
>> +                        "arch",
>> +                        "aarch64"
>> +                    ]
>> +                ]
>> +            ]
>> +        ],
>> +        "variant_id": "aarch64"
>> +    },
>> +    {
>> +        "paths": [
>> +            "/run/*"
>> +        ],
>> +        "variant": [
>> +            [
>> +                "/run/ppc",
>> +                [
>> +                    [
>> +                        "/run/ppc",
>> +                        "arch",
>> +                        "ppc"
>> +                    ]
>> +                ]
>> +            ]
>> +        ],
>> +        "variant_id": "ppc"
>> +    },
>> +    {
>> +        "paths": [
>> +            "/run/*"
>> +        ],
>> +        "variant": [
>> +            [
>> +                "/run/ppc64",
>> +                [
>> +                    [
>> +                        "/run/ppc64",
>> +                        "arch",
>> +                        "ppc64"
>> +                    ]
>> +                ]
>> +            ]
>> +        ],
>> +        "variant_id": "ppc64"
>> +    },
>> +    {
>> +        "paths": [
>> +            "/run/*"
>> +        ],
>> +        "variant": [
>> +            [
>> +                "/run/s390x",
>> +                [
>> +                    [
>> +                        "/run/s390x",
>> +                        "arch",
>> +                        "s390x"
>> +                    ]
>> +                ]
>> +            ]
>> +        ],
>> +        "variant_id": "s390x"
>> +    },
>> +    {
>> +        "paths": [
>> +            "/run/*"
>> +        ],
>> +        "variant": [
>> +            [
>> +                "/run/x86_64",
>> +                [
>> +                    [
>> +                        "/run/x86_64",
>> +                        "arch",
>> +                        "x86_64"
>> +                    ]
>> +                ]
>> +            ]
>> +        ],
>> +        "variant_id": "x86_64"
>> +    }
>> +]
>>

-- 
Cleber Rosa
[ Sr Software Engineer - Virtualization Team - Red Hat ]
[ Avocado Test Framework - avocado-framework.github.io ]
[  7ABB 96EB 8B46 B94D 5E0F  E9BB 657E 8D33 A5F2 09F3  ]

Re: [Qemu-devel] [PATCH v2 7/7] Acceptance Tests: change the handling of tests for specific archs

[Cleber Rosa]

On 10/10/18 6:50 AM, Philippe Mathieu-Daudé wrote:
> On 10/10/2018 01:26, Cleber Rosa wrote:
>> With the introduction of a variants file that can run the same
>> tests on various architectures, it makes sense to make most tests
>> to be reusable on those environments.  The exception should be
>> when a test is really testing a specific architecture feature.
>>
>> With the change proposed here, on a command line such as:
>>
>>   $ avocado run \
>>      --json-variants-load=tests/acceptance/variants/arch.json \
>>      -- tests/acceptance/
>>
>> The boot_linux_console.py tests will appear as "CANCELED: Currently
>> specific to the x86_64 arch", which is as a good thing when compared
>> to being ignored by tags because:
>>
>>  * The architecture specific parts can be addressed
>>  * It will be run on the matching architecture (as opposed to always
>>    being filtered out by the tags mechanism)
>>  * CANCELED tests do no influence negatively the overall job results,
>>    they're not considered an error or failure
>>
>> Signed-off-by: Cleber Rosa <crosa@redhat.com>
>> ---
>>  tests/acceptance/boot_linux_console.py | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/tests/acceptance/boot_linux_console.py b/tests/acceptance/boot_linux_console.py
>> index 58032f971c..ba3ac036da 100644
>> --- a/tests/acceptance/boot_linux_console.py
>> +++ b/tests/acceptance/boot_linux_console.py
>> @@ -19,12 +19,13 @@ class BootLinuxConsole(Test):
>>      and the kernel command line is properly passed from QEMU to the kernel
>>  
>>      :avocado: enable
>> -    :avocado: tags=x86_64
>>      """
>>  
>>      timeout = 60
>>  
>>      def test(self):
>> +        if self.arch != 'x86_64':
>> +            self.cancel('Currently specific to the x86_64 target arch')
>>          kernel_url = ('https://mirrors.kernel.org/fedora/releases/28/'
>>                        'Everything/x86_64/os/images/pxeboot/vmlinuz')
>>          kernel_hash = '238e083e114c48200f80d889f7e32eeb2793e02a'
>>
> 
> For some reason this test run quicker on a aarch64 host than my x86 laptop:
> 
> ThunderX 88XX (aarch64 Little Endian):
> 
>  (05/30)
> tests/acceptance/boot_linux_console.py:BootLinuxConsole.test;x86_64:
> PASS (12.88 s)
> 
> Intel(R) Core(TM) i7-8650U CPU @ 1.90GHz:
> 
>  (05/30)
> tests/acceptance/boot_linux_console.py:BootLinuxConsole.test;x86_64:
> PASS (31.13 s)
> 

The only reason I can think of is faster networking on the aarch64
machine.  Are those numbers more or less the same on subsequent runs?

Thanks for testing it!
- Cleber.

> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> 

Re: [Qemu-devel] [PATCH v2 4/7] scripts/qemu.py: set predefined machine type based on arch

[Eduardo Habkost]

On Wed, Oct 10, 2018 at 08:35:38AM -0400, Cleber Rosa wrote:
> 
> 
> On 10/10/18 7:00 AM, Philippe Mathieu-Daudé wrote:
> > On 10/10/2018 01:26, Cleber Rosa wrote:
> >> Some targets require a machine type to be set, as there's no default
> >> (aarch64 is one example).  To give a consistent interface to users of
> >> this API, this changes set_machine() so that a predefined default can
> >> be used, if one is not given.  The approach used is exactly the same
> >> with the console device type.
> >>
> >> Also, even when there's a default machine type, for some purposes,
> >> testing included, it's better if outside code is explicit about the
> >> machine type, instead of relying on whatever is set internally.
> >>
> >> Signed-off-by: Cleber Rosa <crosa@redhat.com>
> >> ---
> >>  scripts/qemu.py | 22 +++++++++++++++++++++-
> >>  1 file changed, 21 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/scripts/qemu.py b/scripts/qemu.py
> >> index d9e24a0c1a..fca9b76990 100644
> >> --- a/scripts/qemu.py
> >> +++ b/scripts/qemu.py
> >> @@ -36,6 +36,15 @@ CONSOLE_DEV_TYPES = {
> >>      r'^s390-ccw-virtio.*': 'sclpconsole',
> >>      }
> >>  
> >> +#: Maps archictures to the preferred machine type
> >> +MACHINE_TYPES = {
> >> +    r'^aarch64$': 'virt',
> >> +    r'^ppc$': 'g3beige',
> >> +    r'^ppc64$': 'pseries',
> >> +    r'^s390x$': 's390-ccw-virtio',
> >> +    r'^x86_64$': 'q35',
> > 
> > Why choose Q35 rather than PC (the default)?
> > 
> > I was wondering about how to generate variants/machines.json but this is
> > definitively something we want to do via a QMP query.
> > 
> > Eduardo what do you think?
> > 
> 
> It was motivated by Eduardo's initiative to make q35 the default "across
> the board".  He can confirm and give more details.

Making Q35 the default on applications using QEMU and libvirt is
something I'd like to happen.  But I think the simplest way to do
that is to change the QEMU default.  This way you won't need this
table on qemu.py: you can just use the default provided by QEMU.

-- 
Eduardo

Re: [Qemu-devel] [PATCH v2 4/7] scripts/qemu.py: set predefined machine type based on arch

[Cleber Rosa]

On 10/10/18 9:46 AM, Eduardo Habkost wrote:
> On Wed, Oct 10, 2018 at 08:35:38AM -0400, Cleber Rosa wrote:
>>
>>
>> On 10/10/18 7:00 AM, Philippe Mathieu-Daudé wrote:
>>> On 10/10/2018 01:26, Cleber Rosa wrote:
>>>> Some targets require a machine type to be set, as there's no default
>>>> (aarch64 is one example).  To give a consistent interface to users of
>>>> this API, this changes set_machine() so that a predefined default can
>>>> be used, if one is not given.  The approach used is exactly the same
>>>> with the console device type.
>>>>
>>>> Also, even when there's a default machine type, for some purposes,
>>>> testing included, it's better if outside code is explicit about the
>>>> machine type, instead of relying on whatever is set internally.
>>>>
>>>> Signed-off-by: Cleber Rosa <crosa@redhat.com>
>>>> ---
>>>>  scripts/qemu.py | 22 +++++++++++++++++++++-
>>>>  1 file changed, 21 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/scripts/qemu.py b/scripts/qemu.py
>>>> index d9e24a0c1a..fca9b76990 100644
>>>> --- a/scripts/qemu.py
>>>> +++ b/scripts/qemu.py
>>>> @@ -36,6 +36,15 @@ CONSOLE_DEV_TYPES = {
>>>>      r'^s390-ccw-virtio.*': 'sclpconsole',
>>>>      }
>>>>  
>>>> +#: Maps archictures to the preferred machine type
>>>> +MACHINE_TYPES = {
>>>> +    r'^aarch64$': 'virt',
>>>> +    r'^ppc$': 'g3beige',
>>>> +    r'^ppc64$': 'pseries',
>>>> +    r'^s390x$': 's390-ccw-virtio',
>>>> +    r'^x86_64$': 'q35',
>>>
>>> Why choose Q35 rather than PC (the default)?
>>>
>>> I was wondering about how to generate variants/machines.json but this is
>>> definitively something we want to do via a QMP query.
>>>
>>> Eduardo what do you think?
>>>
>>
>> It was motivated by Eduardo's initiative to make q35 the default "across
>> the board".  He can confirm and give more details.
> 
> Making Q35 the default on applications using QEMU and libvirt is
> something I'd like to happen.  But I think the simplest way to do
> that is to change the QEMU default.  This way you won't need this
> table on qemu.py: you can just use the default provided by QEMU.
> 

The idea is to bring consistency on how we're calling
"qemu-system-$(ARCH)", and at the same time apply the "explicit is
better than implicit" rule.

The most important fact is that some targets do not (currently) have
"the default provided by QEMU", aarch64 is one of them.

- Cleber.

Re: [Qemu-devel] [PATCH v2 4/7] scripts/qemu.py: set predefined machine type based on arch

[Cleber Rosa]

On 10/10/18 9:59 AM, Cleber Rosa wrote:
> 
> 
> On 10/10/18 9:46 AM, Eduardo Habkost wrote:
>> On Wed, Oct 10, 2018 at 08:35:38AM -0400, Cleber Rosa wrote:
>>>
>>>
>>> On 10/10/18 7:00 AM, Philippe Mathieu-Daudé wrote:
>>>> On 10/10/2018 01:26, Cleber Rosa wrote:
>>>>> Some targets require a machine type to be set, as there's no default
>>>>> (aarch64 is one example).  To give a consistent interface to users of
>>>>> this API, this changes set_machine() so that a predefined default can
>>>>> be used, if one is not given.  The approach used is exactly the same
>>>>> with the console device type.
>>>>>
>>>>> Also, even when there's a default machine type, for some purposes,
>>>>> testing included, it's better if outside code is explicit about the
>>>>> machine type, instead of relying on whatever is set internally.
>>>>>
>>>>> Signed-off-by: Cleber Rosa <crosa@redhat.com>
>>>>> ---
>>>>>  scripts/qemu.py | 22 +++++++++++++++++++++-
>>>>>  1 file changed, 21 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/scripts/qemu.py b/scripts/qemu.py
>>>>> index d9e24a0c1a..fca9b76990 100644
>>>>> --- a/scripts/qemu.py
>>>>> +++ b/scripts/qemu.py
>>>>> @@ -36,6 +36,15 @@ CONSOLE_DEV_TYPES = {
>>>>>      r'^s390-ccw-virtio.*': 'sclpconsole',
>>>>>      }
>>>>>  
>>>>> +#: Maps archictures to the preferred machine type
>>>>> +MACHINE_TYPES = {
>>>>> +    r'^aarch64$': 'virt',
>>>>> +    r'^ppc$': 'g3beige',
>>>>> +    r'^ppc64$': 'pseries',
>>>>> +    r'^s390x$': 's390-ccw-virtio',
>>>>> +    r'^x86_64$': 'q35',
>>>>
>>>> Why choose Q35 rather than PC (the default)?
>>>>
>>>> I was wondering about how to generate variants/machines.json but this is
>>>> definitively something we want to do via a QMP query.
>>>>
>>>> Eduardo what do you think?
>>>>
>>>
>>> It was motivated by Eduardo's initiative to make q35 the default "across
>>> the board".  He can confirm and give more details.
>>
>> Making Q35 the default on applications using QEMU and libvirt is
>> something I'd like to happen.  But I think the simplest way to do
>> that is to change the QEMU default.  This way you won't need this
>> table on qemu.py: you can just use the default provided by QEMU.
>>
> 
> The idea is to bring consistency on how we're calling
> "qemu-system-$(ARCH)", and at the same time apply the "explicit is
> better than implicit" rule.
> 
> The most important fact is that some targets do not (currently) have
> "the default provided by QEMU", aarch64 is one of them.
> 
> - Cleber.
> 

So I ended up not relaying the question properly: should we default
(even if explicitly adding "-machine") to "pc"?

Thanks!
- Cleber.

Re: [Qemu-devel] [PATCH v2 4/7] scripts/qemu.py: set predefined machine type based on arch

[Eduardo Habkost]

On Wed, Oct 10, 2018 at 10:15:15AM -0400, Cleber Rosa wrote:
> 
> 
> On 10/10/18 9:59 AM, Cleber Rosa wrote:
> > 
> > 
> > On 10/10/18 9:46 AM, Eduardo Habkost wrote:
> >> On Wed, Oct 10, 2018 at 08:35:38AM -0400, Cleber Rosa wrote:
> >>>
> >>>
> >>> On 10/10/18 7:00 AM, Philippe Mathieu-Daudé wrote:
> >>>> On 10/10/2018 01:26, Cleber Rosa wrote:
> >>>>> Some targets require a machine type to be set, as there's no default
> >>>>> (aarch64 is one example).  To give a consistent interface to users of
> >>>>> this API, this changes set_machine() so that a predefined default can
> >>>>> be used, if one is not given.  The approach used is exactly the same
> >>>>> with the console device type.
> >>>>>
> >>>>> Also, even when there's a default machine type, for some purposes,
> >>>>> testing included, it's better if outside code is explicit about the
> >>>>> machine type, instead of relying on whatever is set internally.
> >>>>>
> >>>>> Signed-off-by: Cleber Rosa <crosa@redhat.com>
> >>>>> ---
> >>>>>  scripts/qemu.py | 22 +++++++++++++++++++++-
> >>>>>  1 file changed, 21 insertions(+), 1 deletion(-)
> >>>>>
> >>>>> diff --git a/scripts/qemu.py b/scripts/qemu.py
> >>>>> index d9e24a0c1a..fca9b76990 100644
> >>>>> --- a/scripts/qemu.py
> >>>>> +++ b/scripts/qemu.py
> >>>>> @@ -36,6 +36,15 @@ CONSOLE_DEV_TYPES = {
> >>>>>      r'^s390-ccw-virtio.*': 'sclpconsole',
> >>>>>      }
> >>>>>  
> >>>>> +#: Maps archictures to the preferred machine type
> >>>>> +MACHINE_TYPES = {
> >>>>> +    r'^aarch64$': 'virt',
> >>>>> +    r'^ppc$': 'g3beige',
> >>>>> +    r'^ppc64$': 'pseries',
> >>>>> +    r'^s390x$': 's390-ccw-virtio',
> >>>>> +    r'^x86_64$': 'q35',
> >>>>
> >>>> Why choose Q35 rather than PC (the default)?
> >>>>
> >>>> I was wondering about how to generate variants/machines.json but this is
> >>>> definitively something we want to do via a QMP query.
> >>>>
> >>>> Eduardo what do you think?
> >>>>
> >>>
> >>> It was motivated by Eduardo's initiative to make q35 the default "across
> >>> the board".  He can confirm and give more details.
> >>
> >> Making Q35 the default on applications using QEMU and libvirt is
> >> something I'd like to happen.  But I think the simplest way to do
> >> that is to change the QEMU default.  This way you won't need this
> >> table on qemu.py: you can just use the default provided by QEMU.
> >>
> > 
> > The idea is to bring consistency on how we're calling
> > "qemu-system-$(ARCH)", and at the same time apply the "explicit is
> > better than implicit" rule.
> > 
> > The most important fact is that some targets do not (currently) have
> > "the default provided by QEMU", aarch64 is one of them.
> > 
> > - Cleber.
> > 
> 
> So I ended up not relaying the question properly: should we default
> (even if explicitly adding "-machine") to "pc"?

I think using the default machine-type (when QEMU has a default)
would be less surprising for users of the qemu.py API.

Implicitly adding -machine when there's no default is also
surprising, but then it's a nice surprise: instead of crashing
you get a running VM.

Now, there are two other questions related to this:

If using 'pc' as default, should we always add -machine, or just
omit the machine-type name?  I think we should omit it unless the
caller asked for a specific machine-type name (because it would
be less surprising for users of the API).

About our default testing configuration for acceptance tests:
should acceptance tests run against PC by default?  Should it
test Q35?  Should we test both PC and Q35?  I'm not sure what's
the answer, but I think these decisions shouldn't affect the
qemu.py API at all.

-- 
Eduardo

Re: [Qemu-devel] [PATCH v2 4/7] scripts/qemu.py: set predefined machine type based on arch

[Philippe Mathieu-Daudé]

On 10/10/2018 16:28, Eduardo Habkost wrote:
> On Wed, Oct 10, 2018 at 10:15:15AM -0400, Cleber Rosa wrote:
>>
>>
>> On 10/10/18 9:59 AM, Cleber Rosa wrote:
>>>
>>>
>>> On 10/10/18 9:46 AM, Eduardo Habkost wrote:
>>>> On Wed, Oct 10, 2018 at 08:35:38AM -0400, Cleber Rosa wrote:
>>>>>
>>>>>
>>>>> On 10/10/18 7:00 AM, Philippe Mathieu-Daudé wrote:
>>>>>> On 10/10/2018 01:26, Cleber Rosa wrote:
>>>>>>> Some targets require a machine type to be set, as there's no default
>>>>>>> (aarch64 is one example).  To give a consistent interface to users of
>>>>>>> this API, this changes set_machine() so that a predefined default can
>>>>>>> be used, if one is not given.  The approach used is exactly the same
>>>>>>> with the console device type.
>>>>>>>
>>>>>>> Also, even when there's a default machine type, for some purposes,
>>>>>>> testing included, it's better if outside code is explicit about the
>>>>>>> machine type, instead of relying on whatever is set internally.
>>>>>>>
>>>>>>> Signed-off-by: Cleber Rosa <crosa@redhat.com>
>>>>>>> ---
>>>>>>>  scripts/qemu.py | 22 +++++++++++++++++++++-
>>>>>>>  1 file changed, 21 insertions(+), 1 deletion(-)
>>>>>>>
>>>>>>> diff --git a/scripts/qemu.py b/scripts/qemu.py
>>>>>>> index d9e24a0c1a..fca9b76990 100644
>>>>>>> --- a/scripts/qemu.py
>>>>>>> +++ b/scripts/qemu.py
>>>>>>> @@ -36,6 +36,15 @@ CONSOLE_DEV_TYPES = {
>>>>>>>      r'^s390-ccw-virtio.*': 'sclpconsole',
>>>>>>>      }
>>>>>>>  
>>>>>>> +#: Maps archictures to the preferred machine type
>>>>>>> +MACHINE_TYPES = {
>>>>>>> +    r'^aarch64$': 'virt',
>>>>>>> +    r'^ppc$': 'g3beige',
>>>>>>> +    r'^ppc64$': 'pseries',
>>>>>>> +    r'^s390x$': 's390-ccw-virtio',
>>>>>>> +    r'^x86_64$': 'q35',
>>>>>>
>>>>>> Why choose Q35 rather than PC (the default)?
>>>>>>
>>>>>> I was wondering about how to generate variants/machines.json but this is
>>>>>> definitively something we want to do via a QMP query.
>>>>>>
>>>>>> Eduardo what do you think?
>>>>>>
>>>>>
>>>>> It was motivated by Eduardo's initiative to make q35 the default "across
>>>>> the board".  He can confirm and give more details.
>>>>
>>>> Making Q35 the default on applications using QEMU and libvirt is
>>>> something I'd like to happen.  But I think the simplest way to do
>>>> that is to change the QEMU default.  This way you won't need this
>>>> table on qemu.py: you can just use the default provided by QEMU.
>>>>
>>>
>>> The idea is to bring consistency on how we're calling
>>> "qemu-system-$(ARCH)", and at the same time apply the "explicit is
>>> better than implicit" rule.
>>>
>>> The most important fact is that some targets do not (currently) have
>>> "the default provided by QEMU", aarch64 is one of them.
>>>
>>> - Cleber.
>>>
>>
>> So I ended up not relaying the question properly: should we default
>> (even if explicitly adding "-machine") to "pc"?
> 
> I think using the default machine-type (when QEMU has a default)
> would be less surprising for users of the qemu.py API.
> 
> Implicitly adding -machine when there's no default is also
> surprising, but then it's a nice surprise: instead of crashing
> you get a running VM.
> 
> Now, there are two other questions related to this:
> 
> If using 'pc' as default, should we always add -machine, or just
> omit the machine-type name?  I think we should omit it unless the
> caller asked for a specific machine-type name (because it would
> be less surprising for users of the API).

I agree with that.

> 
> About our default testing configuration for acceptance tests:
> should acceptance tests run against PC by default?  Should it
> test Q35?  Should we test both PC and Q35?  I'm not sure what's
> the answer, but I think these decisions shouldn't affect the
> qemu.py API at all.

If I'm going to submit contributions to some subsystem, I'd like to run
all the tests that cover this subsystem, previous to annoy the maintainer.

For example if a series target the "X86 Machines" subsystem, then I'd
expect the JSON variant to test both PC and Q35.

Regards,

Phil.

Re: [Qemu-devel] [PATCH v2 4/7] scripts/qemu.py: set predefined machine type based on arch

[Daniel P. Berrangé]

On Wed, Oct 10, 2018 at 11:28:40AM -0300, Eduardo Habkost wrote:
> On Wed, Oct 10, 2018 at 10:15:15AM -0400, Cleber Rosa wrote:
> > 
> > 
> > On 10/10/18 9:59 AM, Cleber Rosa wrote:
> > > 
> > > 
> > > On 10/10/18 9:46 AM, Eduardo Habkost wrote:
> > >> On Wed, Oct 10, 2018 at 08:35:38AM -0400, Cleber Rosa wrote:
> > >>>
> > >>>
> > >>> On 10/10/18 7:00 AM, Philippe Mathieu-Daudé wrote:
> > >>>> On 10/10/2018 01:26, Cleber Rosa wrote:
> > >>>>> Some targets require a machine type to be set, as there's no default
> > >>>>> (aarch64 is one example).  To give a consistent interface to users of
> > >>>>> this API, this changes set_machine() so that a predefined default can
> > >>>>> be used, if one is not given.  The approach used is exactly the same
> > >>>>> with the console device type.
> > >>>>>
> > >>>>> Also, even when there's a default machine type, for some purposes,
> > >>>>> testing included, it's better if outside code is explicit about the
> > >>>>> machine type, instead of relying on whatever is set internally.
> > >>>>>
> > >>>>> Signed-off-by: Cleber Rosa <crosa@redhat.com>
> > >>>>> ---
> > >>>>>  scripts/qemu.py | 22 +++++++++++++++++++++-
> > >>>>>  1 file changed, 21 insertions(+), 1 deletion(-)
> > >>>>>
> > >>>>> diff --git a/scripts/qemu.py b/scripts/qemu.py
> > >>>>> index d9e24a0c1a..fca9b76990 100644
> > >>>>> --- a/scripts/qemu.py
> > >>>>> +++ b/scripts/qemu.py
> > >>>>> @@ -36,6 +36,15 @@ CONSOLE_DEV_TYPES = {
> > >>>>>      r'^s390-ccw-virtio.*': 'sclpconsole',
> > >>>>>      }
> > >>>>>  
> > >>>>> +#: Maps archictures to the preferred machine type
> > >>>>> +MACHINE_TYPES = {
> > >>>>> +    r'^aarch64$': 'virt',
> > >>>>> +    r'^ppc$': 'g3beige',
> > >>>>> +    r'^ppc64$': 'pseries',
> > >>>>> +    r'^s390x$': 's390-ccw-virtio',
> > >>>>> +    r'^x86_64$': 'q35',
> > >>>>
> > >>>> Why choose Q35 rather than PC (the default)?
> > >>>>
> > >>>> I was wondering about how to generate variants/machines.json but this is
> > >>>> definitively something we want to do via a QMP query.
> > >>>>
> > >>>> Eduardo what do you think?
> > >>>>
> > >>>
> > >>> It was motivated by Eduardo's initiative to make q35 the default "across
> > >>> the board".  He can confirm and give more details.
> > >>
> > >> Making Q35 the default on applications using QEMU and libvirt is
> > >> something I'd like to happen.  But I think the simplest way to do
> > >> that is to change the QEMU default.  This way you won't need this
> > >> table on qemu.py: you can just use the default provided by QEMU.
> > >>
> > > 
> > > The idea is to bring consistency on how we're calling
> > > "qemu-system-$(ARCH)", and at the same time apply the "explicit is
> > > better than implicit" rule.
> > > 
> > > The most important fact is that some targets do not (currently) have
> > > "the default provided by QEMU", aarch64 is one of them.
> > > 
> > > - Cleber.
> > > 
> > 
> > So I ended up not relaying the question properly: should we default
> > (even if explicitly adding "-machine") to "pc"?
> 
> I think using the default machine-type (when QEMU has a default)
> would be less surprising for users of the qemu.py API.
> 
> Implicitly adding -machine when there's no default is also
> surprising, but then it's a nice surprise: instead of crashing
> you get a running VM.
> 
> Now, there are two other questions related to this:
> 
> If using 'pc' as default, should we always add -machine, or just
> omit the machine-type name?  I think we should omit it unless the
> caller asked for a specific machine-type name (because it would
> be less surprising for users of the API).
> 
> About our default testing configuration for acceptance tests:
> should acceptance tests run against PC by default?  Should it
> test Q35?  Should we test both PC and Q35?  I'm not sure what's
> the answer, but I think these decisions shouldn't affect the
> qemu.py API at all.

As a general goal we should be aiming to test everything we provide
to users if, we expect them to actually use it and not have it
break later :-)

Given that 'pc' has been the default for entire existance of QEMU,
99% of guests are using 'pc' and thus we definitely want to be
testing it.

Equally we'd like to encourage use of 'q35' so we should also be
testing that, even though it has very little usage right now.

Thus, we really need to be running any tests against both machine
types.  If we do that, then the question of default doesn't really
matter as one of them is always not the default and needs testing
regardless.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [Qemu-devel] [PATCH v2 4/7] scripts/qemu.py: set predefined machine type based on arch

[Cleber Rosa]

On 10/10/18 10:28 AM, Eduardo Habkost wrote:
> On Wed, Oct 10, 2018 at 10:15:15AM -0400, Cleber Rosa wrote:
>>
>>
>> On 10/10/18 9:59 AM, Cleber Rosa wrote:
>>>
>>>
>>> On 10/10/18 9:46 AM, Eduardo Habkost wrote:
>>>> On Wed, Oct 10, 2018 at 08:35:38AM -0400, Cleber Rosa wrote:
>>>>>
>>>>>
>>>>> On 10/10/18 7:00 AM, Philippe Mathieu-Daudé wrote:
>>>>>> On 10/10/2018 01:26, Cleber Rosa wrote:
>>>>>>> Some targets require a machine type to be set, as there's no default
>>>>>>> (aarch64 is one example).  To give a consistent interface to users of
>>>>>>> this API, this changes set_machine() so that a predefined default can
>>>>>>> be used, if one is not given.  The approach used is exactly the same
>>>>>>> with the console device type.
>>>>>>>
>>>>>>> Also, even when there's a default machine type, for some purposes,
>>>>>>> testing included, it's better if outside code is explicit about the
>>>>>>> machine type, instead of relying on whatever is set internally.
>>>>>>>
>>>>>>> Signed-off-by: Cleber Rosa <crosa@redhat.com>
>>>>>>> ---
>>>>>>>  scripts/qemu.py | 22 +++++++++++++++++++++-
>>>>>>>  1 file changed, 21 insertions(+), 1 deletion(-)
>>>>>>>
>>>>>>> diff --git a/scripts/qemu.py b/scripts/qemu.py
>>>>>>> index d9e24a0c1a..fca9b76990 100644
>>>>>>> --- a/scripts/qemu.py
>>>>>>> +++ b/scripts/qemu.py
>>>>>>> @@ -36,6 +36,15 @@ CONSOLE_DEV_TYPES = {
>>>>>>>      r'^s390-ccw-virtio.*': 'sclpconsole',
>>>>>>>      }
>>>>>>>  
>>>>>>> +#: Maps archictures to the preferred machine type
>>>>>>> +MACHINE_TYPES = {
>>>>>>> +    r'^aarch64$': 'virt',
>>>>>>> +    r'^ppc$': 'g3beige',
>>>>>>> +    r'^ppc64$': 'pseries',
>>>>>>> +    r'^s390x$': 's390-ccw-virtio',
>>>>>>> +    r'^x86_64$': 'q35',
>>>>>>
>>>>>> Why choose Q35 rather than PC (the default)?
>>>>>>
>>>>>> I was wondering about how to generate variants/machines.json but this is
>>>>>> definitively something we want to do via a QMP query.
>>>>>>
>>>>>> Eduardo what do you think?
>>>>>>
>>>>>
>>>>> It was motivated by Eduardo's initiative to make q35 the default "across
>>>>> the board".  He can confirm and give more details.
>>>>
>>>> Making Q35 the default on applications using QEMU and libvirt is
>>>> something I'd like to happen.  But I think the simplest way to do
>>>> that is to change the QEMU default.  This way you won't need this
>>>> table on qemu.py: you can just use the default provided by QEMU.
>>>>
>>>
>>> The idea is to bring consistency on how we're calling
>>> "qemu-system-$(ARCH)", and at the same time apply the "explicit is
>>> better than implicit" rule.
>>>
>>> The most important fact is that some targets do not (currently) have
>>> "the default provided by QEMU", aarch64 is one of them.
>>>
>>> - Cleber.
>>>
>>
>> So I ended up not relaying the question properly: should we default
>> (even if explicitly adding "-machine") to "pc"?
> 
> I think using the default machine-type (when QEMU has a default)
> would be less surprising for users of the qemu.py API.
> 

OK, agreed.

> Implicitly adding -machine when there's no default is also
> surprising, but then it's a nice surprise: instead of crashing
> you get a running VM.
> 
> Now, there are two other questions related to this:
> 
> If using 'pc' as default, should we always add -machine, or just
> omit the machine-type name?  I think we should omit it unless the
> caller asked for a specific machine-type name (because it would
> be less surprising for users of the API).
> 

OK.

> About our default testing configuration for acceptance tests:
> should acceptance tests run against PC by default?  Should it
> test Q35?  Should we test both PC and Q35?  I'm not sure what's
> the answer, but I think these decisions shouldn't affect the
> qemu.py API at all.
> 

OK.

To make sure we're on the same page, we're still going to have default
machine types, based on the arch, for those targets that don't provide
one (aarch64 is one example).  Right?

- Cleber.

Re: [Qemu-devel] [PATCH v2 4/7] scripts/qemu.py: set predefined machine type based on arch

[Cleber Rosa]

On 10/10/18 11:26 AM, Philippe Mathieu-Daudé wrote:
> On 10/10/2018 16:28, Eduardo Habkost wrote:
>> On Wed, Oct 10, 2018 at 10:15:15AM -0400, Cleber Rosa wrote:
>>>
>>>
>>> On 10/10/18 9:59 AM, Cleber Rosa wrote:
>>>>
>>>>
>>>> On 10/10/18 9:46 AM, Eduardo Habkost wrote:
>>>>> On Wed, Oct 10, 2018 at 08:35:38AM -0400, Cleber Rosa wrote:
>>>>>>
>>>>>>
>>>>>> On 10/10/18 7:00 AM, Philippe Mathieu-Daudé wrote:
>>>>>>> On 10/10/2018 01:26, Cleber Rosa wrote:
>>>>>>>> Some targets require a machine type to be set, as there's no default
>>>>>>>> (aarch64 is one example).  To give a consistent interface to users of
>>>>>>>> this API, this changes set_machine() so that a predefined default can
>>>>>>>> be used, if one is not given.  The approach used is exactly the same
>>>>>>>> with the console device type.
>>>>>>>>
>>>>>>>> Also, even when there's a default machine type, for some purposes,
>>>>>>>> testing included, it's better if outside code is explicit about the
>>>>>>>> machine type, instead of relying on whatever is set internally.
>>>>>>>>
>>>>>>>> Signed-off-by: Cleber Rosa <crosa@redhat.com>
>>>>>>>> ---
>>>>>>>>  scripts/qemu.py | 22 +++++++++++++++++++++-
>>>>>>>>  1 file changed, 21 insertions(+), 1 deletion(-)
>>>>>>>>
>>>>>>>> diff --git a/scripts/qemu.py b/scripts/qemu.py
>>>>>>>> index d9e24a0c1a..fca9b76990 100644
>>>>>>>> --- a/scripts/qemu.py
>>>>>>>> +++ b/scripts/qemu.py
>>>>>>>> @@ -36,6 +36,15 @@ CONSOLE_DEV_TYPES = {
>>>>>>>>      r'^s390-ccw-virtio.*': 'sclpconsole',
>>>>>>>>      }
>>>>>>>>  
>>>>>>>> +#: Maps archictures to the preferred machine type
>>>>>>>> +MACHINE_TYPES = {
>>>>>>>> +    r'^aarch64$': 'virt',
>>>>>>>> +    r'^ppc$': 'g3beige',
>>>>>>>> +    r'^ppc64$': 'pseries',
>>>>>>>> +    r'^s390x$': 's390-ccw-virtio',
>>>>>>>> +    r'^x86_64$': 'q35',
>>>>>>>
>>>>>>> Why choose Q35 rather than PC (the default)?
>>>>>>>
>>>>>>> I was wondering about how to generate variants/machines.json but this is
>>>>>>> definitively something we want to do via a QMP query.
>>>>>>>
>>>>>>> Eduardo what do you think?
>>>>>>>
>>>>>>
>>>>>> It was motivated by Eduardo's initiative to make q35 the default "across
>>>>>> the board".  He can confirm and give more details.
>>>>>
>>>>> Making Q35 the default on applications using QEMU and libvirt is
>>>>> something I'd like to happen.  But I think the simplest way to do
>>>>> that is to change the QEMU default.  This way you won't need this
>>>>> table on qemu.py: you can just use the default provided by QEMU.
>>>>>
>>>>
>>>> The idea is to bring consistency on how we're calling
>>>> "qemu-system-$(ARCH)", and at the same time apply the "explicit is
>>>> better than implicit" rule.
>>>>
>>>> The most important fact is that some targets do not (currently) have
>>>> "the default provided by QEMU", aarch64 is one of them.
>>>>
>>>> - Cleber.
>>>>
>>>
>>> So I ended up not relaying the question properly: should we default
>>> (even if explicitly adding "-machine") to "pc"?
>>
>> I think using the default machine-type (when QEMU has a default)
>> would be less surprising for users of the qemu.py API.
>>
>> Implicitly adding -machine when there's no default is also
>> surprising, but then it's a nice surprise: instead of crashing
>> you get a running VM.
>>
>> Now, there are two other questions related to this:
>>
>> If using 'pc' as default, should we always add -machine, or just
>> omit the machine-type name?  I think we should omit it unless the
>> caller asked for a specific machine-type name (because it would
>> be less surprising for users of the API).
> 
> I agree with that.
> 

OK!

>>
>> About our default testing configuration for acceptance tests:
>> should acceptance tests run against PC by default?  Should it
>> test Q35?  Should we test both PC and Q35?  I'm not sure what's
>> the answer, but I think these decisions shouldn't affect the
>> qemu.py API at all.
> 
> If I'm going to submit contributions to some subsystem, I'd like to run
> all the tests that cover this subsystem, previous to annoy the maintainer.
> 
> For example if a series target the "X86 Machines" subsystem, then I'd
> expect the JSON variant to test both PC and Q35.
> 

I agree, and we'll get there, but I'd rather do it in small steps.

The reason is that we want every single FAIL/ERROR on the acceptance
tests to really flag a regression, so we need careful execution and
validation prior to increasing the "test matrix".

At the same time, we need to be careful to not grow the default
acceptance tests execution to a point that people won't run it. I've
just heard similar feedback regarding Avocado-VT, that has *too many*
tests that make it impractical for the individual developer to run.

The plans we have for that is to define some sane test sets (probably
based on tags and/or variants files), and at the same time, rely on
combinatorial independent testing to reduce the number of test
variations to make it practical for the regular developer to run on his
environment.

Regards!
- Cleber.

> Regards,
> 
> Phil.
> 

Re: [Qemu-devel] [PATCH v2 4/7] scripts/qemu.py: set predefined machine type based on arch

[Cleber Rosa]

On 10/10/18 11:31 AM, Daniel P. Berrangé wrote:
> On Wed, Oct 10, 2018 at 11:28:40AM -0300, Eduardo Habkost wrote:
>> On Wed, Oct 10, 2018 at 10:15:15AM -0400, Cleber Rosa wrote:
>>>
>>>
>>> On 10/10/18 9:59 AM, Cleber Rosa wrote:
>>>>
>>>>
>>>> On 10/10/18 9:46 AM, Eduardo Habkost wrote:
>>>>> On Wed, Oct 10, 2018 at 08:35:38AM -0400, Cleber Rosa wrote:
>>>>>>
>>>>>>
>>>>>> On 10/10/18 7:00 AM, Philippe Mathieu-Daudé wrote:
>>>>>>> On 10/10/2018 01:26, Cleber Rosa wrote:
>>>>>>>> Some targets require a machine type to be set, as there's no default
>>>>>>>> (aarch64 is one example).  To give a consistent interface to users of
>>>>>>>> this API, this changes set_machine() so that a predefined default can
>>>>>>>> be used, if one is not given.  The approach used is exactly the same
>>>>>>>> with the console device type.
>>>>>>>>
>>>>>>>> Also, even when there's a default machine type, for some purposes,
>>>>>>>> testing included, it's better if outside code is explicit about the
>>>>>>>> machine type, instead of relying on whatever is set internally.
>>>>>>>>
>>>>>>>> Signed-off-by: Cleber Rosa <crosa@redhat.com>
>>>>>>>> ---
>>>>>>>>  scripts/qemu.py | 22 +++++++++++++++++++++-
>>>>>>>>  1 file changed, 21 insertions(+), 1 deletion(-)
>>>>>>>>
>>>>>>>> diff --git a/scripts/qemu.py b/scripts/qemu.py
>>>>>>>> index d9e24a0c1a..fca9b76990 100644
>>>>>>>> --- a/scripts/qemu.py
>>>>>>>> +++ b/scripts/qemu.py
>>>>>>>> @@ -36,6 +36,15 @@ CONSOLE_DEV_TYPES = {
>>>>>>>>      r'^s390-ccw-virtio.*': 'sclpconsole',
>>>>>>>>      }
>>>>>>>>  
>>>>>>>> +#: Maps archictures to the preferred machine type
>>>>>>>> +MACHINE_TYPES = {
>>>>>>>> +    r'^aarch64$': 'virt',
>>>>>>>> +    r'^ppc$': 'g3beige',
>>>>>>>> +    r'^ppc64$': 'pseries',
>>>>>>>> +    r'^s390x$': 's390-ccw-virtio',
>>>>>>>> +    r'^x86_64$': 'q35',
>>>>>>>
>>>>>>> Why choose Q35 rather than PC (the default)?
>>>>>>>
>>>>>>> I was wondering about how to generate variants/machines.json but this is
>>>>>>> definitively something we want to do via a QMP query.
>>>>>>>
>>>>>>> Eduardo what do you think?
>>>>>>>
>>>>>>
>>>>>> It was motivated by Eduardo's initiative to make q35 the default "across
>>>>>> the board".  He can confirm and give more details.
>>>>>
>>>>> Making Q35 the default on applications using QEMU and libvirt is
>>>>> something I'd like to happen.  But I think the simplest way to do
>>>>> that is to change the QEMU default.  This way you won't need this
>>>>> table on qemu.py: you can just use the default provided by QEMU.
>>>>>
>>>>
>>>> The idea is to bring consistency on how we're calling
>>>> "qemu-system-$(ARCH)", and at the same time apply the "explicit is
>>>> better than implicit" rule.
>>>>
>>>> The most important fact is that some targets do not (currently) have
>>>> "the default provided by QEMU", aarch64 is one of them.
>>>>
>>>> - Cleber.
>>>>
>>>
>>> So I ended up not relaying the question properly: should we default
>>> (even if explicitly adding "-machine") to "pc"?
>>
>> I think using the default machine-type (when QEMU has a default)
>> would be less surprising for users of the qemu.py API.
>>
>> Implicitly adding -machine when there's no default is also
>> surprising, but then it's a nice surprise: instead of crashing
>> you get a running VM.
>>
>> Now, there are two other questions related to this:
>>
>> If using 'pc' as default, should we always add -machine, or just
>> omit the machine-type name?  I think we should omit it unless the
>> caller asked for a specific machine-type name (because it would
>> be less surprising for users of the API).
>>
>> About our default testing configuration for acceptance tests:
>> should acceptance tests run against PC by default?  Should it
>> test Q35?  Should we test both PC and Q35?  I'm not sure what's
>> the answer, but I think these decisions shouldn't affect the
>> qemu.py API at all.
> 
> As a general goal we should be aiming to test everything we provide
> to users if, we expect them to actually use it and not have it
> break later :-)
> 
> Given that 'pc' has been the default for entire existance of QEMU,
> 99% of guests are using 'pc' and thus we definitely want to be
> testing it.
> 
> Equally we'd like to encourage use of 'q35' so we should also be
> testing that, even though it has very little usage right now.
> 
> Thus, we really need to be running any tests against both machine
> types.  If we do that, then the question of default doesn't really
> matter as one of them is always not the default and needs testing
> regardless.
> 
> Regards,
> Daniel
> 

Hi Daniel,

I think we're in sync.  Please look at my previous reply (and accept my
apologies if this is not a better "netiquette" than replying the same
thing again).

Regards!
- Cleber.

Re: [Qemu-devel] [PATCH v2 4/7] scripts/qemu.py: set predefined machine type based on arch

[Philippe Mathieu-Daudé]

On 10/10/2018 17:58, Cleber Rosa wrote:
> 
> 
> On 10/10/18 11:26 AM, Philippe Mathieu-Daudé wrote:
>> On 10/10/2018 16:28, Eduardo Habkost wrote:
>>> On Wed, Oct 10, 2018 at 10:15:15AM -0400, Cleber Rosa wrote:
>>>>
>>>>
>>>> On 10/10/18 9:59 AM, Cleber Rosa wrote:
>>>>>
>>>>>
>>>>> On 10/10/18 9:46 AM, Eduardo Habkost wrote:
>>>>>> On Wed, Oct 10, 2018 at 08:35:38AM -0400, Cleber Rosa wrote:
>>>>>>>
>>>>>>>
>>>>>>> On 10/10/18 7:00 AM, Philippe Mathieu-Daudé wrote:
>>>>>>>> On 10/10/2018 01:26, Cleber Rosa wrote:
>>>>>>>>> Some targets require a machine type to be set, as there's no default
>>>>>>>>> (aarch64 is one example).  To give a consistent interface to users of
>>>>>>>>> this API, this changes set_machine() so that a predefined default can
>>>>>>>>> be used, if one is not given.  The approach used is exactly the same
>>>>>>>>> with the console device type.
>>>>>>>>>
>>>>>>>>> Also, even when there's a default machine type, for some purposes,
>>>>>>>>> testing included, it's better if outside code is explicit about the
>>>>>>>>> machine type, instead of relying on whatever is set internally.
>>>>>>>>>
>>>>>>>>> Signed-off-by: Cleber Rosa <crosa@redhat.com>
>>>>>>>>> ---
>>>>>>>>>  scripts/qemu.py | 22 +++++++++++++++++++++-
>>>>>>>>>  1 file changed, 21 insertions(+), 1 deletion(-)
>>>>>>>>>
>>>>>>>>> diff --git a/scripts/qemu.py b/scripts/qemu.py
>>>>>>>>> index d9e24a0c1a..fca9b76990 100644
>>>>>>>>> --- a/scripts/qemu.py
>>>>>>>>> +++ b/scripts/qemu.py
>>>>>>>>> @@ -36,6 +36,15 @@ CONSOLE_DEV_TYPES = {
>>>>>>>>>      r'^s390-ccw-virtio.*': 'sclpconsole',
>>>>>>>>>      }
>>>>>>>>>  
>>>>>>>>> +#: Maps archictures to the preferred machine type
>>>>>>>>> +MACHINE_TYPES = {
>>>>>>>>> +    r'^aarch64$': 'virt',
>>>>>>>>> +    r'^ppc$': 'g3beige',
>>>>>>>>> +    r'^ppc64$': 'pseries',
>>>>>>>>> +    r'^s390x$': 's390-ccw-virtio',
>>>>>>>>> +    r'^x86_64$': 'q35',
>>>>>>>>
>>>>>>>> Why choose Q35 rather than PC (the default)?
>>>>>>>>
>>>>>>>> I was wondering about how to generate variants/machines.json but this is
>>>>>>>> definitively something we want to do via a QMP query.
>>>>>>>>
>>>>>>>> Eduardo what do you think?
>>>>>>>>
>>>>>>>
>>>>>>> It was motivated by Eduardo's initiative to make q35 the default "across
>>>>>>> the board".  He can confirm and give more details.
>>>>>>
>>>>>> Making Q35 the default on applications using QEMU and libvirt is
>>>>>> something I'd like to happen.  But I think the simplest way to do
>>>>>> that is to change the QEMU default.  This way you won't need this
>>>>>> table on qemu.py: you can just use the default provided by QEMU.
>>>>>>
>>>>>
>>>>> The idea is to bring consistency on how we're calling
>>>>> "qemu-system-$(ARCH)", and at the same time apply the "explicit is
>>>>> better than implicit" rule.
>>>>>
>>>>> The most important fact is that some targets do not (currently) have
>>>>> "the default provided by QEMU", aarch64 is one of them.
>>>>>
>>>>> - Cleber.
>>>>>
>>>>
>>>> So I ended up not relaying the question properly: should we default
>>>> (even if explicitly adding "-machine") to "pc"?
>>>
>>> I think using the default machine-type (when QEMU has a default)
>>> would be less surprising for users of the qemu.py API.
>>>
>>> Implicitly adding -machine when there's no default is also
>>> surprising, but then it's a nice surprise: instead of crashing
>>> you get a running VM.
>>>
>>> Now, there are two other questions related to this:
>>>
>>> If using 'pc' as default, should we always add -machine, or just
>>> omit the machine-type name?  I think we should omit it unless the
>>> caller asked for a specific machine-type name (because it would
>>> be less surprising for users of the API).
>>
>> I agree with that.
>>
> 
> OK!
> 
>>>
>>> About our default testing configuration for acceptance tests:
>>> should acceptance tests run against PC by default?  Should it
>>> test Q35?  Should we test both PC and Q35?  I'm not sure what's
>>> the answer, but I think these decisions shouldn't affect the
>>> qemu.py API at all.
>>
>> If I'm going to submit contributions to some subsystem, I'd like to run
>> all the tests that cover this subsystem, previous to annoy the maintainer.
>>
>> For example if a series target the "X86 Machines" subsystem, then I'd
>> expect the JSON variant to test both PC and Q35.
>>
> 
> I agree, and we'll get there, but I'd rather do it in small steps.

Sure.

> 
> The reason is that we want every single FAIL/ERROR on the acceptance
> tests to really flag a regression, so we need careful execution and
> validation prior to increasing the "test matrix".
> 
> At the same time, we need to be careful to not grow the default
> acceptance tests execution to a point that people won't run it. I've
> just heard similar feedback regarding Avocado-VT, that has *too many*
> tests that make it impractical for the individual developer to run.

The problem here is not the number of tests but the set of tests selected.

A test should have a description of what it is testing, the covered
devices/modes/...
>From this description it should be easy to add filtering rules.

>From a developer point of view, I'll run the tests covering the areas I
modified.
A maintainer runs more extensive tests on his subsystems.

Now if you plan a release, you are not an individual developer :)

> 
> The plans we have for that is to define some sane test sets (probably
> based on tags and/or variants files), and at the same time, rely on
> combinatorial independent testing to reduce the number of test
> variations to make it practical for the regular developer to run on his
> environment.

OK, we'll get there! :)

(I don't want to reject people to contribute tests because "we already
have too many".)

Regards,

Phil.

Re: [Qemu-devel] [PATCH v2 4/7] scripts/qemu.py: set predefined machine type based on arch

[Peter Maydell]

On 10 October 2018 at 16:47, Cleber Rosa <crosa@redhat.com> wrote:
> To make sure we're on the same page, we're still going to have default
> machine types, based on the arch, for those targets that don't provide
> one (aarch64 is one example).  Right?

Does it make sense to define a default? The reason arm
doesn't specify a default machine type is because you
can't just run any old guest on any old machine type.
You need to know "this guest image will run on machine
type X", and run it on machine type X. This is like
knowing you need to run a test on x86 PC and not
on PPC spapr.

Would it make more sense for each test to specify
which machine types it can work on?

thanks
-- PMM

Re: [Qemu-devel] [PATCH v2 4/7] scripts/qemu.py: set predefined machine type based on arch

[Cleber Rosa]

On 10/10/18 12:23 PM, Peter Maydell wrote:
> On 10 October 2018 at 16:47, Cleber Rosa <crosa@redhat.com> wrote:
>> To make sure we're on the same page, we're still going to have default
>> machine types, based on the arch, for those targets that don't provide
>> one (aarch64 is one example).  Right?
> 
> Does it make sense to define a default? The reason arm
> doesn't specify a default machine type is because you
> can't just run any old guest on any old machine type.
> You need to know "this guest image will run on machine
> type X", and run it on machine type X. This is like
> knowing you need to run a test on x86 PC and not
> on PPC spapr.
> 

While requiring tests to specify every single aspect of the environment
that will be used may be OK for low level unit tests, it puts a lot of
burden on higher level tests (which is supposed to be the vast majority
under tests/acceptance).

>From a test writer perspective, working on these higher level tests, it
may want to make sure that feature "X", unrelated to the target arch,
machine type, etc, "just works".  You man want to look at the "vnc.py"
test for a real world example.

Eduardo has suggested that "make check-acceptance" runs all (possible)
tests on all target archs by default.

> Would it make more sense for each test to specify
> which machine types it can work on?
> 

I think it does, but I believe in the black list approach, instead of
the white list.

The reason for that is that I believe that majority of the tests under
"tests/acceptance" can be made to work on every target (which would be
the default).  So far, I've made sure tests behave correctly on the 5
arches included in the "archs.json" file in this series (x86_64, ppc64,
ppc, aarch64, s390x).

To give a full disclosure, "boot_linux.py" (boots a linux kernel) is
x86_64 specific, and CANCELS when asked to be run on other archs.  But,
on the work I've done top of these series, it already works with ppc64
and aarch64.  Also, "boot_linux.py" sent in another series, (which boots
a full linux guest) is also being adapted to work on most of the target
archs.

And, as an exception, you can still define/check the arch and machine
type *in the test*, if it's not supposed to work on most.

Does that make sense?
- Cleber.

> thanks
> -- PMM
> 

Re: [Qemu-devel] [PATCH v2 4/7] scripts/qemu.py: set predefined machine type based on arch

[Peter Maydell]

On 10 October 2018 at 18:52, Cleber Rosa <crosa@redhat.com> wrote:
>
>
> On 10/10/18 12:23 PM, Peter Maydell wrote:
>> On 10 October 2018 at 16:47, Cleber Rosa <crosa@redhat.com> wrote:
>>> To make sure we're on the same page, we're still going to have default
>>> machine types, based on the arch, for those targets that don't provide
>>> one (aarch64 is one example).  Right?
>>
>> Does it make sense to define a default? The reason arm
>> doesn't specify a default machine type is because you
>> can't just run any old guest on any old machine type.
>> You need to know "this guest image will run on machine
>> type X", and run it on machine type X. This is like
>> knowing you need to run a test on x86 PC and not
>> on PPC spapr.
>>
>
> While requiring tests to specify every single aspect of the environment
> that will be used may be OK for low level unit tests, it puts a lot of
> burden on higher level tests (which is supposed to be the vast majority
> under tests/acceptance).
>
> From a test writer perspective, working on these higher level tests, it
> may want to make sure that feature "X", unrelated to the target arch,
> machine type, etc, "just works".  You man want to look at the "vnc.py"
> test for a real world example.

OK, if it doesn't have a dependency on machine at all, it
should state that somehow.

> Eduardo has suggested that "make check-acceptance" runs all (possible)
> tests on all target archs by default.

Yeah; or we have some mechanism for trimming down the
matrix of what we run. But I think it's better coverage
if we have 3 tests ABC that don't depend on machine
and 3 machines XYZ to run AX BY CZ than AX BX CX by
specifying X as an arbitrary "default".

It looks like the 'vnc' test is just testing QEMU functionality,
not anything that involves interacting with the guest or
machine model? There's a good argument that that only really
needs to be run once, not once per architecture.

You might also want to consider the "none" machine, which exists
for bits of test infrastructure that aren't actually trying to
run guests.

>> Would it make more sense for each test to specify
>> which machine types it can work on?
>>
>
> I think it does, but I believe in the black list approach, instead of
> the white list.
>
> The reason for that is that I believe that majority of the tests under
> "tests/acceptance" can be made to work on every target (which would be
> the default).  So far, I've made sure tests behave correctly on the 5
> arches included in the "archs.json" file in this series (x86_64, ppc64,
> ppc, aarch64, s390x).
>
> To give a full disclosure, "boot_linux.py" (boots a linux kernel) is
> x86_64 specific, and CANCELS when asked to be run on other archs.  But,
> on the work I've done top of these series, it already works with ppc64
> and aarch64.  Also, "boot_linux.py" sent in another series, (which boots
> a full linux guest) is also being adapted to work on most of the target
> archs.

Right, "boot Linux" is machine specific. The kernel/disk
/etc that boots on aarch64 virt is probably not going to boot
on the 64-bit xilinx board; and on 32-bit arm you definitely
are going to want a different kernel in some places. This
is likely to be true of most tests that actually try to run
code in the guest.

We should aim to test the machines we care about (regardless
of what architectures they are), rather than thinking about it
in terms of "testing architectures X, Y, Z", I think.

I think you're going to need at least some whitelist functionality;
otherwise half the tests are going to break every time we add
a new machine (and "add every new machine to the blacklist for
half the tests" doesn't scale very well).

thanks
-- PMM

Re: [Qemu-devel] [PATCH v2 4/7] scripts/qemu.py: set predefined machine type based on arch

[Cleber Rosa]

On 10/10/18 12:08 PM, Philippe Mathieu-Daudé wrote:
> On 10/10/2018 17:58, Cleber Rosa wrote:
>>
>>
>> On 10/10/18 11:26 AM, Philippe Mathieu-Daudé wrote:
>>> On 10/10/2018 16:28, Eduardo Habkost wrote:
>>>> On Wed, Oct 10, 2018 at 10:15:15AM -0400, Cleber Rosa wrote:
>>>>>
>>>>>
>>>>> On 10/10/18 9:59 AM, Cleber Rosa wrote:
>>>>>>
>>>>>>
>>>>>> On 10/10/18 9:46 AM, Eduardo Habkost wrote:
>>>>>>> On Wed, Oct 10, 2018 at 08:35:38AM -0400, Cleber Rosa wrote:
>>>>>>>>
>>>>>>>>
>>>>>>>> On 10/10/18 7:00 AM, Philippe Mathieu-Daudé wrote:
>>>>>>>>> On 10/10/2018 01:26, Cleber Rosa wrote:
>>>>>>>>>> Some targets require a machine type to be set, as there's no default
>>>>>>>>>> (aarch64 is one example).  To give a consistent interface to users of
>>>>>>>>>> this API, this changes set_machine() so that a predefined default can
>>>>>>>>>> be used, if one is not given.  The approach used is exactly the same
>>>>>>>>>> with the console device type.
>>>>>>>>>>
>>>>>>>>>> Also, even when there's a default machine type, for some purposes,
>>>>>>>>>> testing included, it's better if outside code is explicit about the
>>>>>>>>>> machine type, instead of relying on whatever is set internally.
>>>>>>>>>>
>>>>>>>>>> Signed-off-by: Cleber Rosa <crosa@redhat.com>
>>>>>>>>>> ---
>>>>>>>>>>  scripts/qemu.py | 22 +++++++++++++++++++++-
>>>>>>>>>>  1 file changed, 21 insertions(+), 1 deletion(-)
>>>>>>>>>>
>>>>>>>>>> diff --git a/scripts/qemu.py b/scripts/qemu.py
>>>>>>>>>> index d9e24a0c1a..fca9b76990 100644
>>>>>>>>>> --- a/scripts/qemu.py
>>>>>>>>>> +++ b/scripts/qemu.py
>>>>>>>>>> @@ -36,6 +36,15 @@ CONSOLE_DEV_TYPES = {
>>>>>>>>>>      r'^s390-ccw-virtio.*': 'sclpconsole',
>>>>>>>>>>      }
>>>>>>>>>>  
>>>>>>>>>> +#: Maps archictures to the preferred machine type
>>>>>>>>>> +MACHINE_TYPES = {
>>>>>>>>>> +    r'^aarch64$': 'virt',
>>>>>>>>>> +    r'^ppc$': 'g3beige',
>>>>>>>>>> +    r'^ppc64$': 'pseries',
>>>>>>>>>> +    r'^s390x$': 's390-ccw-virtio',
>>>>>>>>>> +    r'^x86_64$': 'q35',
>>>>>>>>>
>>>>>>>>> Why choose Q35 rather than PC (the default)?
>>>>>>>>>
>>>>>>>>> I was wondering about how to generate variants/machines.json but this is
>>>>>>>>> definitively something we want to do via a QMP query.
>>>>>>>>>
>>>>>>>>> Eduardo what do you think?
>>>>>>>>>
>>>>>>>>
>>>>>>>> It was motivated by Eduardo's initiative to make q35 the default "across
>>>>>>>> the board".  He can confirm and give more details.
>>>>>>>
>>>>>>> Making Q35 the default on applications using QEMU and libvirt is
>>>>>>> something I'd like to happen.  But I think the simplest way to do
>>>>>>> that is to change the QEMU default.  This way you won't need this
>>>>>>> table on qemu.py: you can just use the default provided by QEMU.
>>>>>>>
>>>>>>
>>>>>> The idea is to bring consistency on how we're calling
>>>>>> "qemu-system-$(ARCH)", and at the same time apply the "explicit is
>>>>>> better than implicit" rule.
>>>>>>
>>>>>> The most important fact is that some targets do not (currently) have
>>>>>> "the default provided by QEMU", aarch64 is one of them.
>>>>>>
>>>>>> - Cleber.
>>>>>>
>>>>>
>>>>> So I ended up not relaying the question properly: should we default
>>>>> (even if explicitly adding "-machine") to "pc"?
>>>>
>>>> I think using the default machine-type (when QEMU has a default)
>>>> would be less surprising for users of the qemu.py API.
>>>>
>>>> Implicitly adding -machine when there's no default is also
>>>> surprising, but then it's a nice surprise: instead of crashing
>>>> you get a running VM.
>>>>
>>>> Now, there are two other questions related to this:
>>>>
>>>> If using 'pc' as default, should we always add -machine, or just
>>>> omit the machine-type name?  I think we should omit it unless the
>>>> caller asked for a specific machine-type name (because it would
>>>> be less surprising for users of the API).
>>>
>>> I agree with that.
>>>
>>
>> OK!
>>
>>>>
>>>> About our default testing configuration for acceptance tests:
>>>> should acceptance tests run against PC by default?  Should it
>>>> test Q35?  Should we test both PC and Q35?  I'm not sure what's
>>>> the answer, but I think these decisions shouldn't affect the
>>>> qemu.py API at all.
>>>
>>> If I'm going to submit contributions to some subsystem, I'd like to run
>>> all the tests that cover this subsystem, previous to annoy the maintainer.
>>>
>>> For example if a series target the "X86 Machines" subsystem, then I'd
>>> expect the JSON variant to test both PC and Q35.
>>>
>>
>> I agree, and we'll get there, but I'd rather do it in small steps.
> 
> Sure.
> 
>>
>> The reason is that we want every single FAIL/ERROR on the acceptance
>> tests to really flag a regression, so we need careful execution and
>> validation prior to increasing the "test matrix".
>>
>> At the same time, we need to be careful to not grow the default
>> acceptance tests execution to a point that people won't run it. I've
>> just heard similar feedback regarding Avocado-VT, that has *too many*
>> tests that make it impractical for the individual developer to run.
> 
> The problem here is not the number of tests but the set of tests selected.
> 
> A test should have a description of what it is testing, the covered
> devices/modes/...
> From this description it should be easy to add filtering rules.
> 
> From a developer point of view, I'll run the tests covering the areas I
> modified.
> A maintainer runs more extensive tests on his subsystems.
> 

And a CI system may run even more.  And QE will hopefully run an absurd
number of tests.

And, all of those groups will be working and collaborating on the same
code base, with no secret sauces.

"I have a dream", and that dream is developer A reporting a test
failure, and developer B checking the CI/QE database to find out that it
also happens on different scenarios - or only on a specific scenario.
IMO this can quickly point you towards the failure cause.

> Now if you plan a release, you are not an individual developer :)
> 

Yep, true.

>>
>> The plans we have for that is to define some sane test sets (probably
>> based on tags and/or variants files), and at the same time, rely on
>> combinatorial independent testing to reduce the number of test
>> variations to make it practical for the regular developer to run on his
>> environment.
> 
> OK, we'll get there! :)
> 
> (I don't want to reject people to contribute tests because "we already
> have too many".)
> 

Oh no, I agree with you.  I'm already envisioning a few things:

 * Proposing a set of tags
 * Combinatorial Independent Testing
 * Maybe breaking down the "tests/acceptance" directory into target and
non-target specific (think of "qemu-img" only tests)
 * Using the Avocado Job API to define the test suites (some old
example, still Avocado-VT related:
https://www.redhat.com/archives/avocado-devel/2018-April/msg00015.html)

Regards,
- Cleber.

> Regards,
> 
> Phil.
> 

Re: [Qemu-devel] [PATCH v2 4/7] scripts/qemu.py: set predefined machine type based on arch

[Cleber Rosa]

On 10/10/18 2:07 PM, Peter Maydell wrote:
> On 10 October 2018 at 18:52, Cleber Rosa <crosa@redhat.com> wrote:
>>
>>
>> On 10/10/18 12:23 PM, Peter Maydell wrote:
>>> On 10 October 2018 at 16:47, Cleber Rosa <crosa@redhat.com> wrote:
>>>> To make sure we're on the same page, we're still going to have default
>>>> machine types, based on the arch, for those targets that don't provide
>>>> one (aarch64 is one example).  Right?
>>>
>>> Does it make sense to define a default? The reason arm
>>> doesn't specify a default machine type is because you
>>> can't just run any old guest on any old machine type.
>>> You need to know "this guest image will run on machine
>>> type X", and run it on machine type X. This is like
>>> knowing you need to run a test on x86 PC and not
>>> on PPC spapr.
>>>
>>
>> While requiring tests to specify every single aspect of the environment
>> that will be used may be OK for low level unit tests, it puts a lot of
>> burden on higher level tests (which is supposed to be the vast majority
>> under tests/acceptance).
>>
>> From a test writer perspective, working on these higher level tests, it
>> may want to make sure that feature "X", unrelated to the target arch,
>> machine type, etc, "just works".  You man want to look at the "vnc.py"
>> test for a real world example.
> 
> OK, if it doesn't have a dependency on machine at all, it
> should state that somehow.
> 
>> Eduardo has suggested that "make check-acceptance" runs all (possible)
>> tests on all target archs by default.
> 
> Yeah; or we have some mechanism for trimming down the
> matrix of what we run. But I think it's better coverage
> if we have 3 tests ABC that don't depend on machine
> and 3 machines XYZ to run AX BY CZ than AX BX CX by
> specifying X as an arbitrary "default".
> 
> It looks like the 'vnc' test is just testing QEMU functionality,
> not anything that involves interacting with the guest or
> machine model? There's a good argument that that only really
> needs to be run once, not once per architecture.
> 
> You might also want to consider the "none" machine, which exists
> for bits of test infrastructure that aren't actually trying to
> run guests.
> 
>>> Would it make more sense for each test to specify
>>> which machine types it can work on?
>>>
>>
>> I think it does, but I believe in the black list approach, instead of
>> the white list.
>>
>> The reason for that is that I believe that majority of the tests under
>> "tests/acceptance" can be made to work on every target (which would be
>> the default).  So far, I've made sure tests behave correctly on the 5
>> arches included in the "archs.json" file in this series (x86_64, ppc64,
>> ppc, aarch64, s390x).
>>
>> To give a full disclosure, "boot_linux.py" (boots a linux kernel) is
>> x86_64 specific, and CANCELS when asked to be run on other archs.  But,
>> on the work I've done top of these series, it already works with ppc64
>> and aarch64.  Also, "boot_linux.py" sent in another series, (which boots
>> a full linux guest) is also being adapted to work on most of the target
>> archs.
> 
> Right, "boot Linux" is machine specific. The kernel/disk
> /etc that boots on aarch64 virt is probably not going to boot
> on the 64-bit xilinx board; and on 32-bit arm you definitely
> are going to want a different kernel in some places. This
> is likely to be true of most tests that actually try to run
> code in the guest.
> 

Agreed.

> We should aim to test the machines we care about (regardless
> of what architectures they are), rather than thinking about it
> in terms of "testing architectures X, Y, Z", I think.
> 

To me it's clear that:

 1) I lack a complete understanding of what "we care about"
 2) It's easier to start with something, and tweak it to taste

And TBH, I fully agree with Philippe in the sense that difference
developer/maintainer roles will require a different test "profile".

> I think you're going to need at least some whitelist functionality;
> otherwise half the tests are going to break every time we add
> a new machine (and "add every new machine to the blacklist for
> half the tests" doesn't scale very well).
> 

The whitelist approach is in effect, it's the reason I sent a
"archs.json" file, not with machine types, but with the archs that I've
tested with.

So, I'm going to push forward this series (a v3) with the same
simplified approach, that is, `make check-acceptance` will still run the
tests on a single target.

Then, once that is settled, we can decide on:

 1) `make check-acceptance` becomes "run on all target args/machine
types" and `make check-acceptance-$(ARCH)-$(MACHINE)` is introduced.
 2) `make check-acceptance-all` is introduced.

To me it's clear that there's a huge continuation to this discussion,
and that we should bite one piece at a time.

Thoughts?

Regards!
- Cleber.

> thanks
> -- PMM
> 

Re: [Qemu-devel] [PATCH v2 4/7] scripts/qemu.py: set predefined machine type based on arch

[Cleber Rosa]

On 10/10/18 11:47 AM, Cleber Rosa wrote:
> 
> 
> On 10/10/18 10:28 AM, Eduardo Habkost wrote:
>> On Wed, Oct 10, 2018 at 10:15:15AM -0400, Cleber Rosa wrote:
>>>
>>>
>>> On 10/10/18 9:59 AM, Cleber Rosa wrote:
>>>>
>>>>
>>>> On 10/10/18 9:46 AM, Eduardo Habkost wrote:
>>>>> On Wed, Oct 10, 2018 at 08:35:38AM -0400, Cleber Rosa wrote:
>>>>>>
>>>>>>
>>>>>> On 10/10/18 7:00 AM, Philippe Mathieu-Daudé wrote:
>>>>>>> On 10/10/2018 01:26, Cleber Rosa wrote:
>>>>>>>> Some targets require a machine type to be set, as there's no default
>>>>>>>> (aarch64 is one example).  To give a consistent interface to users of
>>>>>>>> this API, this changes set_machine() so that a predefined default can
>>>>>>>> be used, if one is not given.  The approach used is exactly the same
>>>>>>>> with the console device type.
>>>>>>>>
>>>>>>>> Also, even when there's a default machine type, for some purposes,
>>>>>>>> testing included, it's better if outside code is explicit about the
>>>>>>>> machine type, instead of relying on whatever is set internally.
>>>>>>>>
>>>>>>>> Signed-off-by: Cleber Rosa <crosa@redhat.com>
>>>>>>>> ---
>>>>>>>>  scripts/qemu.py | 22 +++++++++++++++++++++-
>>>>>>>>  1 file changed, 21 insertions(+), 1 deletion(-)
>>>>>>>>
>>>>>>>> diff --git a/scripts/qemu.py b/scripts/qemu.py
>>>>>>>> index d9e24a0c1a..fca9b76990 100644
>>>>>>>> --- a/scripts/qemu.py
>>>>>>>> +++ b/scripts/qemu.py
>>>>>>>> @@ -36,6 +36,15 @@ CONSOLE_DEV_TYPES = {
>>>>>>>>      r'^s390-ccw-virtio.*': 'sclpconsole',
>>>>>>>>      }
>>>>>>>>  
>>>>>>>> +#: Maps archictures to the preferred machine type
>>>>>>>> +MACHINE_TYPES = {
>>>>>>>> +    r'^aarch64$': 'virt',
>>>>>>>> +    r'^ppc$': 'g3beige',
>>>>>>>> +    r'^ppc64$': 'pseries',
>>>>>>>> +    r'^s390x$': 's390-ccw-virtio',
>>>>>>>> +    r'^x86_64$': 'q35',
>>>>>>>
>>>>>>> Why choose Q35 rather than PC (the default)?
>>>>>>>
>>>>>>> I was wondering about how to generate variants/machines.json but this is
>>>>>>> definitively something we want to do via a QMP query.
>>>>>>>
>>>>>>> Eduardo what do you think?
>>>>>>>
>>>>>>
>>>>>> It was motivated by Eduardo's initiative to make q35 the default "across
>>>>>> the board".  He can confirm and give more details.
>>>>>
>>>>> Making Q35 the default on applications using QEMU and libvirt is
>>>>> something I'd like to happen.  But I think the simplest way to do
>>>>> that is to change the QEMU default.  This way you won't need this
>>>>> table on qemu.py: you can just use the default provided by QEMU.
>>>>>
>>>>
>>>> The idea is to bring consistency on how we're calling
>>>> "qemu-system-$(ARCH)", and at the same time apply the "explicit is
>>>> better than implicit" rule.
>>>>
>>>> The most important fact is that some targets do not (currently) have
>>>> "the default provided by QEMU", aarch64 is one of them.
>>>>
>>>> - Cleber.
>>>>
>>>
>>> So I ended up not relaying the question properly: should we default
>>> (even if explicitly adding "-machine") to "pc"?
>>
>> I think using the default machine-type (when QEMU has a default)
>> would be less surprising for users of the qemu.py API.
>>
> 
> OK, agreed.
> 
>> Implicitly adding -machine when there's no default is also
>> surprising, but then it's a nice surprise: instead of crashing
>> you get a running VM.
>>
>> Now, there are two other questions related to this:
>>
>> If using 'pc' as default, should we always add -machine, or just
>> omit the machine-type name?  I think we should omit it unless the
>> caller asked for a specific machine-type name (because it would
>> be less surprising for users of the API).
>>
> 

Getting down to business, trying to apply those changes, I was faced
with a situation.  Actually, the same situation I faced a few months
ago.  Handling it was defered until it was *really* a blocker.
Basically the issue is: the set_console() method, which gives tests a
ready to use console, depends on knowing the machine type (see
CONSOLE_DEV_TYPES).

As a case study, let's look at "boot_console_linux.py":
 1) it sets the machine type explicitly
 2) it has nothing to do with the specific machine type
 3) the setting of a machine type is boiler plate code to set a console
 4) the console is used on the test's real purpose: verifying the Linux
kernel booted

Now, to be able to run the same test -- booting a Linux kernel -- on
*other target archs*, we need the same machinery.  Even more important:
to have similar tests we'll need to either abstract those features or
duplicate them.  This can be seen, at least in part, on the firmware
tests that Philippe sent to the list: they would also benefit from
having a console device ready to be used on the configured machine type[1]:

Assuming that we want to provide this type of machinery for free (or as
close as that) to the acceptance/functional tests, we need some source
of "known good" configuration for the targets we aim to support.

Let's restrict the discussion to the issue at hand, machine types, while
keeping in mind that the same pattern happened with devices types to use
as console before, and my experience running into default network device
types in further work (tests that interact with the guest by ssh'ing
into it).

The solutions I can think of are:

 1) run the target binary previous to the "real" run, and query
information -- this is what Avocado-VT does[2], and something I tried on
earlier versions of the acceptance tests infrastructure code

 2) attempt to get this information from the build system[3]

 3) hard code the "known" good configuration

I've previously worked on solutions along the lines of #1 and #2, but
the general feedback wasn't that positive, for valid reasons.  Eduardo
probably remembers this.

So, I'm tempted to try solution #3.  As much as duplicating target
defaults in qemu.py doesn't sound perfect, it seems to be the more
predictable and attainable solution at this point.

Thoughts?

Thanks!
- Cleber.

[1] - https://lists.gnu.org/archive/html/qemu-devel/2018-10/msg00591.html
[2] -
https://github.com/avocado-framework/avocado-vt/blob/65.0/virttest/utils_misc.py#L2105
[3] - http://lists.gnu.org/archive/html/qemu-devel/2017-07/msg06757.html

> OK.
> 
>> About our default testing configuration for acceptance tests:
>> should acceptance tests run against PC by default?  Should it
>> test Q35?  Should we test both PC and Q35?  I'm not sure what's
>> the answer, but I think these decisions shouldn't affect the
>> qemu.py API at all.
>>
> 
> OK.
> 
> To make sure we're on the same page, we're still going to have default
> machine types, based on the arch, for those targets that don't provide
> one (aarch64 is one example).  Right?
> 
> - Cleber.
> 

Re: [Qemu-devel] [PATCH v2 4/7] scripts/qemu.py: set predefined machine type based on arch

[Eduardo Habkost]

On Wed, Oct 10, 2018 at 08:17:26PM -0400, Cleber Rosa wrote:
> 
> 
> On 10/10/18 11:47 AM, Cleber Rosa wrote:
> > 
> > 
> > On 10/10/18 10:28 AM, Eduardo Habkost wrote:
> >> On Wed, Oct 10, 2018 at 10:15:15AM -0400, Cleber Rosa wrote:
> >>>
> >>>
> >>> On 10/10/18 9:59 AM, Cleber Rosa wrote:
> >>>>
> >>>>
> >>>> On 10/10/18 9:46 AM, Eduardo Habkost wrote:
> >>>>> On Wed, Oct 10, 2018 at 08:35:38AM -0400, Cleber Rosa wrote:
> >>>>>>
> >>>>>>
> >>>>>> On 10/10/18 7:00 AM, Philippe Mathieu-Daudé wrote:
> >>>>>>> On 10/10/2018 01:26, Cleber Rosa wrote:
> >>>>>>>> Some targets require a machine type to be set, as there's no default
> >>>>>>>> (aarch64 is one example).  To give a consistent interface to users of
> >>>>>>>> this API, this changes set_machine() so that a predefined default can
> >>>>>>>> be used, if one is not given.  The approach used is exactly the same
> >>>>>>>> with the console device type.
> >>>>>>>>
> >>>>>>>> Also, even when there's a default machine type, for some purposes,
> >>>>>>>> testing included, it's better if outside code is explicit about the
> >>>>>>>> machine type, instead of relying on whatever is set internally.
> >>>>>>>>
> >>>>>>>> Signed-off-by: Cleber Rosa <crosa@redhat.com>
> >>>>>>>> ---
> >>>>>>>>  scripts/qemu.py | 22 +++++++++++++++++++++-
> >>>>>>>>  1 file changed, 21 insertions(+), 1 deletion(-)
> >>>>>>>>
> >>>>>>>> diff --git a/scripts/qemu.py b/scripts/qemu.py
> >>>>>>>> index d9e24a0c1a..fca9b76990 100644
> >>>>>>>> --- a/scripts/qemu.py
> >>>>>>>> +++ b/scripts/qemu.py
> >>>>>>>> @@ -36,6 +36,15 @@ CONSOLE_DEV_TYPES = {
> >>>>>>>>      r'^s390-ccw-virtio.*': 'sclpconsole',
> >>>>>>>>      }
> >>>>>>>>  
> >>>>>>>> +#: Maps archictures to the preferred machine type
> >>>>>>>> +MACHINE_TYPES = {
> >>>>>>>> +    r'^aarch64$': 'virt',
> >>>>>>>> +    r'^ppc$': 'g3beige',
> >>>>>>>> +    r'^ppc64$': 'pseries',
> >>>>>>>> +    r'^s390x$': 's390-ccw-virtio',
> >>>>>>>> +    r'^x86_64$': 'q35',
> >>>>>>>
> >>>>>>> Why choose Q35 rather than PC (the default)?
> >>>>>>>
> >>>>>>> I was wondering about how to generate variants/machines.json but this is
> >>>>>>> definitively something we want to do via a QMP query.
> >>>>>>>
> >>>>>>> Eduardo what do you think?
> >>>>>>>
> >>>>>>
> >>>>>> It was motivated by Eduardo's initiative to make q35 the default "across
> >>>>>> the board".  He can confirm and give more details.
> >>>>>
> >>>>> Making Q35 the default on applications using QEMU and libvirt is
> >>>>> something I'd like to happen.  But I think the simplest way to do
> >>>>> that is to change the QEMU default.  This way you won't need this
> >>>>> table on qemu.py: you can just use the default provided by QEMU.
> >>>>>
> >>>>
> >>>> The idea is to bring consistency on how we're calling
> >>>> "qemu-system-$(ARCH)", and at the same time apply the "explicit is
> >>>> better than implicit" rule.
> >>>>
> >>>> The most important fact is that some targets do not (currently) have
> >>>> "the default provided by QEMU", aarch64 is one of them.
> >>>>
> >>>> - Cleber.
> >>>>
> >>>
> >>> So I ended up not relaying the question properly: should we default
> >>> (even if explicitly adding "-machine") to "pc"?
> >>
> >> I think using the default machine-type (when QEMU has a default)
> >> would be less surprising for users of the qemu.py API.
> >>
> > 
> > OK, agreed.
> > 
> >> Implicitly adding -machine when there's no default is also
> >> surprising, but then it's a nice surprise: instead of crashing
> >> you get a running VM.
> >>
> >> Now, there are two other questions related to this:
> >>
> >> If using 'pc' as default, should we always add -machine, or just
> >> omit the machine-type name?  I think we should omit it unless the
> >> caller asked for a specific machine-type name (because it would
> >> be less surprising for users of the API).
> >>
> > 
> 
> Getting down to business, trying to apply those changes, I was faced
> with a situation.  Actually, the same situation I faced a few months
> ago.  Handling it was defered until it was *really* a blocker.
> Basically the issue is: the set_console() method, which gives tests a
> ready to use console, depends on knowing the machine type (see
> CONSOLE_DEV_TYPES).
> 
> As a case study, let's look at "boot_console_linux.py":
>  1) it sets the machine type explicitly
>  2) it has nothing to do with the specific machine type
>  3) the setting of a machine type is boiler plate code to set a console
>  4) the console is used on the test's real purpose: verifying the Linux
> kernel booted
> 
> Now, to be able to run the same test -- booting a Linux kernel -- on
> *other target archs*, we need the same machinery.  Even more important:
> to have similar tests we'll need to either abstract those features or
> duplicate them.  This can be seen, at least in part, on the firmware
> tests that Philippe sent to the list: they would also benefit from
> having a console device ready to be used on the configured machine type[1]:
> 
> Assuming that we want to provide this type of machinery for free (or as
> close as that) to the acceptance/functional tests, we need some source
> of "known good" configuration for the targets we aim to support.
> 
> Let's restrict the discussion to the issue at hand, machine types, while
> keeping in mind that the same pattern happened with devices types to use
> as console before, and my experience running into default network device
> types in further work (tests that interact with the guest by ssh'ing
> into it).
> 
> The solutions I can think of are:
> 
>  1) run the target binary previous to the "real" run, and query
> information -- this is what Avocado-VT does[2], and something I tried on
> earlier versions of the acceptance tests infrastructure code
> 
>  2) attempt to get this information from the build system[3]
> 
>  3) hard code the "known" good configuration
> 
> I've previously worked on solutions along the lines of #1 and #2, but
> the general feedback wasn't that positive, for valid reasons.  Eduardo
> probably remembers this.

I don't remember seeing negative feedback for #1.  It sounds like
the best solution.

> 
> So, I'm tempted to try solution #3.  As much as duplicating target
> defaults in qemu.py doesn't sound perfect, it seems to be the more
> predictable and attainable solution at this point.

I consider #3 to be acceptable just as a temporary solution until
we implement #1.


> 
> Thoughts?
> 
> Thanks!
> - Cleber.
> 
> [1] - https://lists.gnu.org/archive/html/qemu-devel/2018-10/msg00591.html
> [2] -
> https://github.com/avocado-framework/avocado-vt/blob/65.0/virttest/utils_misc.py#L2105
> [3] - http://lists.gnu.org/archive/html/qemu-devel/2017-07/msg06757.html
> 
> > OK.
> > 
> >> About our default testing configuration for acceptance tests:
> >> should acceptance tests run against PC by default?  Should it
> >> test Q35?  Should we test both PC and Q35?  I'm not sure what's
> >> the answer, but I think these decisions shouldn't affect the
> >> qemu.py API at all.
> >>
> > 
> > OK.
> > 
> > To make sure we're on the same page, we're still going to have default
> > machine types, based on the arch, for those targets that don't provide
> > one (aarch64 is one example).  Right?
> > 
> > - Cleber.
> > 

-- 
Eduardo

Re: [Qemu-devel] [PATCH v2 4/7] scripts/qemu.py: set predefined machine type based on arch

[Cleber Rosa]

On 10/10/18 11:42 PM, Eduardo Habkost wrote:
> On Wed, Oct 10, 2018 at 08:17:26PM -0400, Cleber Rosa wrote:
>>
>>
>> On 10/10/18 11:47 AM, Cleber Rosa wrote:
>>>
>>>
>>> On 10/10/18 10:28 AM, Eduardo Habkost wrote:
>>>> On Wed, Oct 10, 2018 at 10:15:15AM -0400, Cleber Rosa wrote:
>>>>>
>>>>>
>>>>> On 10/10/18 9:59 AM, Cleber Rosa wrote:
>>>>>>
>>>>>>
>>>>>> On 10/10/18 9:46 AM, Eduardo Habkost wrote:
>>>>>>> On Wed, Oct 10, 2018 at 08:35:38AM -0400, Cleber Rosa wrote:
>>>>>>>>
>>>>>>>>
>>>>>>>> On 10/10/18 7:00 AM, Philippe Mathieu-Daudé wrote:
>>>>>>>>> On 10/10/2018 01:26, Cleber Rosa wrote:
>>>>>>>>>> Some targets require a machine type to be set, as there's no default
>>>>>>>>>> (aarch64 is one example).  To give a consistent interface to users of
>>>>>>>>>> this API, this changes set_machine() so that a predefined default can
>>>>>>>>>> be used, if one is not given.  The approach used is exactly the same
>>>>>>>>>> with the console device type.
>>>>>>>>>>
>>>>>>>>>> Also, even when there's a default machine type, for some purposes,
>>>>>>>>>> testing included, it's better if outside code is explicit about the
>>>>>>>>>> machine type, instead of relying on whatever is set internally.
>>>>>>>>>>
>>>>>>>>>> Signed-off-by: Cleber Rosa <crosa@redhat.com>
>>>>>>>>>> ---
>>>>>>>>>>  scripts/qemu.py | 22 +++++++++++++++++++++-
>>>>>>>>>>  1 file changed, 21 insertions(+), 1 deletion(-)
>>>>>>>>>>
>>>>>>>>>> diff --git a/scripts/qemu.py b/scripts/qemu.py
>>>>>>>>>> index d9e24a0c1a..fca9b76990 100644
>>>>>>>>>> --- a/scripts/qemu.py
>>>>>>>>>> +++ b/scripts/qemu.py
>>>>>>>>>> @@ -36,6 +36,15 @@ CONSOLE_DEV_TYPES = {
>>>>>>>>>>      r'^s390-ccw-virtio.*': 'sclpconsole',
>>>>>>>>>>      }
>>>>>>>>>>  
>>>>>>>>>> +#: Maps archictures to the preferred machine type
>>>>>>>>>> +MACHINE_TYPES = {
>>>>>>>>>> +    r'^aarch64$': 'virt',
>>>>>>>>>> +    r'^ppc$': 'g3beige',
>>>>>>>>>> +    r'^ppc64$': 'pseries',
>>>>>>>>>> +    r'^s390x$': 's390-ccw-virtio',
>>>>>>>>>> +    r'^x86_64$': 'q35',
>>>>>>>>>
>>>>>>>>> Why choose Q35 rather than PC (the default)?
>>>>>>>>>
>>>>>>>>> I was wondering about how to generate variants/machines.json but this is
>>>>>>>>> definitively something we want to do via a QMP query.
>>>>>>>>>
>>>>>>>>> Eduardo what do you think?
>>>>>>>>>
>>>>>>>>
>>>>>>>> It was motivated by Eduardo's initiative to make q35 the default "across
>>>>>>>> the board".  He can confirm and give more details.
>>>>>>>
>>>>>>> Making Q35 the default on applications using QEMU and libvirt is
>>>>>>> something I'd like to happen.  But I think the simplest way to do
>>>>>>> that is to change the QEMU default.  This way you won't need this
>>>>>>> table on qemu.py: you can just use the default provided by QEMU.
>>>>>>>
>>>>>>
>>>>>> The idea is to bring consistency on how we're calling
>>>>>> "qemu-system-$(ARCH)", and at the same time apply the "explicit is
>>>>>> better than implicit" rule.
>>>>>>
>>>>>> The most important fact is that some targets do not (currently) have
>>>>>> "the default provided by QEMU", aarch64 is one of them.
>>>>>>
>>>>>> - Cleber.
>>>>>>
>>>>>
>>>>> So I ended up not relaying the question properly: should we default
>>>>> (even if explicitly adding "-machine") to "pc"?
>>>>
>>>> I think using the default machine-type (when QEMU has a default)
>>>> would be less surprising for users of the qemu.py API.
>>>>
>>>
>>> OK, agreed.
>>>
>>>> Implicitly adding -machine when there's no default is also
>>>> surprising, but then it's a nice surprise: instead of crashing
>>>> you get a running VM.
>>>>
>>>> Now, there are two other questions related to this:
>>>>
>>>> If using 'pc' as default, should we always add -machine, or just
>>>> omit the machine-type name?  I think we should omit it unless the
>>>> caller asked for a specific machine-type name (because it would
>>>> be less surprising for users of the API).
>>>>
>>>
>>
>> Getting down to business, trying to apply those changes, I was faced
>> with a situation.  Actually, the same situation I faced a few months
>> ago.  Handling it was defered until it was *really* a blocker.
>> Basically the issue is: the set_console() method, which gives tests a
>> ready to use console, depends on knowing the machine type (see
>> CONSOLE_DEV_TYPES).
>>
>> As a case study, let's look at "boot_console_linux.py":
>>  1) it sets the machine type explicitly
>>  2) it has nothing to do with the specific machine type
>>  3) the setting of a machine type is boiler plate code to set a console
>>  4) the console is used on the test's real purpose: verifying the Linux
>> kernel booted
>>
>> Now, to be able to run the same test -- booting a Linux kernel -- on
>> *other target archs*, we need the same machinery.  Even more important:
>> to have similar tests we'll need to either abstract those features or
>> duplicate them.  This can be seen, at least in part, on the firmware
>> tests that Philippe sent to the list: they would also benefit from
>> having a console device ready to be used on the configured machine type[1]:
>>
>> Assuming that we want to provide this type of machinery for free (or as
>> close as that) to the acceptance/functional tests, we need some source
>> of "known good" configuration for the targets we aim to support.
>>
>> Let's restrict the discussion to the issue at hand, machine types, while
>> keeping in mind that the same pattern happened with devices types to use
>> as console before, and my experience running into default network device
>> types in further work (tests that interact with the guest by ssh'ing
>> into it).
>>
>> The solutions I can think of are:
>>
>>  1) run the target binary previous to the "real" run, and query
>> information -- this is what Avocado-VT does[2], and something I tried on
>> earlier versions of the acceptance tests infrastructure code
>>
>>  2) attempt to get this information from the build system[3]
>>
>>  3) hard code the "known" good configuration
>>
>> I've previously worked on solutions along the lines of #1 and #2, but
>> the general feedback wasn't that positive, for valid reasons.  Eduardo
>> probably remembers this.
> 
> I don't remember seeing negative feedback for #1.  It sounds like
> the best solution.
> 

IIRC, it was mostly related to the fact that the reliable way to query a
target information (instead of parsing a human oriented output) would be
to use QMP.

Then, the question of using QEMUMachine for that (and the possible
chicken-and-egg situations, having a different set of base args, etc)
led me into prototyping a simplified version:

https://github.com/clebergnu/qemu/commit/b769b3d969b9e29c644d12c30cd0bb7f312e4fbc#diff-07b9b989eb85128c2c8d2a8f17685698R46

Which would still be duplicating some code.  I'm not particularly happy
about either approaches TBH.

>>
>> So, I'm tempted to try solution #3.  As much as duplicating target
>> defaults in qemu.py doesn't sound perfect, it seems to be the more
>> predictable and attainable solution at this point.
> 
> I consider #3 to be acceptable just as a temporary solution until
> we implement #1.
> 
> 

So, with the extra information given here, would you recommend doing #3?
Or pause this, and work on a #1 of sorts?

Thanks!
- Cleber.

>>
>> Thoughts?
>>
>> Thanks!
>> - Cleber.
>>
>> [1] - https://lists.gnu.org/archive/html/qemu-devel/2018-10/msg00591.html
>> [2] -
>> https://github.com/avocado-framework/avocado-vt/blob/65.0/virttest/utils_misc.py#L2105
>> [3] - http://lists.gnu.org/archive/html/qemu-devel/2017-07/msg06757.html
>>
>>> OK.
>>>
>>>> About our default testing configuration for acceptance tests:
>>>> should acceptance tests run against PC by default?  Should it
>>>> test Q35?  Should we test both PC and Q35?  I'm not sure what's
>>>> the answer, but I think these decisions shouldn't affect the
>>>> qemu.py API at all.
>>>>
>>>
>>> OK.
>>>
>>> To make sure we're on the same page, we're still going to have default
>>> machine types, based on the arch, for those targets that don't provide
>>> one (aarch64 is one example).  Right?
>>>
>>> - Cleber.
>>>
> 

Re: [Qemu-devel] [PATCH v2 4/7] scripts/qemu.py: set predefined machine type based on arch

[Eduardo Habkost]

On Thu, Oct 11, 2018 at 12:43:06AM -0400, Cleber Rosa wrote:
> 
> 
> On 10/10/18 11:42 PM, Eduardo Habkost wrote:
> > On Wed, Oct 10, 2018 at 08:17:26PM -0400, Cleber Rosa wrote:
> >>
> >>
> >> On 10/10/18 11:47 AM, Cleber Rosa wrote:
> >>>
> >>>
> >>> On 10/10/18 10:28 AM, Eduardo Habkost wrote:
> >>>> On Wed, Oct 10, 2018 at 10:15:15AM -0400, Cleber Rosa wrote:
> >>>>>
> >>>>>
> >>>>> On 10/10/18 9:59 AM, Cleber Rosa wrote:
> >>>>>>
> >>>>>>
> >>>>>> On 10/10/18 9:46 AM, Eduardo Habkost wrote:
> >>>>>>> On Wed, Oct 10, 2018 at 08:35:38AM -0400, Cleber Rosa wrote:
> >>>>>>>>
> >>>>>>>>
> >>>>>>>> On 10/10/18 7:00 AM, Philippe Mathieu-Daudé wrote:
> >>>>>>>>> On 10/10/2018 01:26, Cleber Rosa wrote:
> >>>>>>>>>> Some targets require a machine type to be set, as there's no default
> >>>>>>>>>> (aarch64 is one example).  To give a consistent interface to users of
> >>>>>>>>>> this API, this changes set_machine() so that a predefined default can
> >>>>>>>>>> be used, if one is not given.  The approach used is exactly the same
> >>>>>>>>>> with the console device type.
> >>>>>>>>>>
> >>>>>>>>>> Also, even when there's a default machine type, for some purposes,
> >>>>>>>>>> testing included, it's better if outside code is explicit about the
> >>>>>>>>>> machine type, instead of relying on whatever is set internally.
> >>>>>>>>>>
> >>>>>>>>>> Signed-off-by: Cleber Rosa <crosa@redhat.com>
> >>>>>>>>>> ---
> >>>>>>>>>>  scripts/qemu.py | 22 +++++++++++++++++++++-
> >>>>>>>>>>  1 file changed, 21 insertions(+), 1 deletion(-)
> >>>>>>>>>>
> >>>>>>>>>> diff --git a/scripts/qemu.py b/scripts/qemu.py
> >>>>>>>>>> index d9e24a0c1a..fca9b76990 100644
> >>>>>>>>>> --- a/scripts/qemu.py
> >>>>>>>>>> +++ b/scripts/qemu.py
> >>>>>>>>>> @@ -36,6 +36,15 @@ CONSOLE_DEV_TYPES = {
> >>>>>>>>>>      r'^s390-ccw-virtio.*': 'sclpconsole',
> >>>>>>>>>>      }
> >>>>>>>>>>  
> >>>>>>>>>> +#: Maps archictures to the preferred machine type
> >>>>>>>>>> +MACHINE_TYPES = {
> >>>>>>>>>> +    r'^aarch64$': 'virt',
> >>>>>>>>>> +    r'^ppc$': 'g3beige',
> >>>>>>>>>> +    r'^ppc64$': 'pseries',
> >>>>>>>>>> +    r'^s390x$': 's390-ccw-virtio',
> >>>>>>>>>> +    r'^x86_64$': 'q35',
> >>>>>>>>>
> >>>>>>>>> Why choose Q35 rather than PC (the default)?
> >>>>>>>>>
> >>>>>>>>> I was wondering about how to generate variants/machines.json but this is
> >>>>>>>>> definitively something we want to do via a QMP query.
> >>>>>>>>>
> >>>>>>>>> Eduardo what do you think?
> >>>>>>>>>
> >>>>>>>>
> >>>>>>>> It was motivated by Eduardo's initiative to make q35 the default "across
> >>>>>>>> the board".  He can confirm and give more details.
> >>>>>>>
> >>>>>>> Making Q35 the default on applications using QEMU and libvirt is
> >>>>>>> something I'd like to happen.  But I think the simplest way to do
> >>>>>>> that is to change the QEMU default.  This way you won't need this
> >>>>>>> table on qemu.py: you can just use the default provided by QEMU.
> >>>>>>>
> >>>>>>
> >>>>>> The idea is to bring consistency on how we're calling
> >>>>>> "qemu-system-$(ARCH)", and at the same time apply the "explicit is
> >>>>>> better than implicit" rule.
> >>>>>>
> >>>>>> The most important fact is that some targets do not (currently) have
> >>>>>> "the default provided by QEMU", aarch64 is one of them.
> >>>>>>
> >>>>>> - Cleber.
> >>>>>>
> >>>>>
> >>>>> So I ended up not relaying the question properly: should we default
> >>>>> (even if explicitly adding "-machine") to "pc"?
> >>>>
> >>>> I think using the default machine-type (when QEMU has a default)
> >>>> would be less surprising for users of the qemu.py API.
> >>>>
> >>>
> >>> OK, agreed.
> >>>
> >>>> Implicitly adding -machine when there's no default is also
> >>>> surprising, but then it's a nice surprise: instead of crashing
> >>>> you get a running VM.
> >>>>
> >>>> Now, there are two other questions related to this:
> >>>>
> >>>> If using 'pc' as default, should we always add -machine, or just
> >>>> omit the machine-type name?  I think we should omit it unless the
> >>>> caller asked for a specific machine-type name (because it would
> >>>> be less surprising for users of the API).
> >>>>
> >>>
> >>
> >> Getting down to business, trying to apply those changes, I was faced
> >> with a situation.  Actually, the same situation I faced a few months
> >> ago.  Handling it was defered until it was *really* a blocker.
> >> Basically the issue is: the set_console() method, which gives tests a
> >> ready to use console, depends on knowing the machine type (see
> >> CONSOLE_DEV_TYPES).
> >>
> >> As a case study, let's look at "boot_console_linux.py":
> >>  1) it sets the machine type explicitly
> >>  2) it has nothing to do with the specific machine type
> >>  3) the setting of a machine type is boiler plate code to set a console
> >>  4) the console is used on the test's real purpose: verifying the Linux
> >> kernel booted
> >>
> >> Now, to be able to run the same test -- booting a Linux kernel -- on
> >> *other target archs*, we need the same machinery.  Even more important:
> >> to have similar tests we'll need to either abstract those features or
> >> duplicate them.  This can be seen, at least in part, on the firmware
> >> tests that Philippe sent to the list: they would also benefit from
> >> having a console device ready to be used on the configured machine type[1]:
> >>
> >> Assuming that we want to provide this type of machinery for free (or as
> >> close as that) to the acceptance/functional tests, we need some source
> >> of "known good" configuration for the targets we aim to support.
> >>
> >> Let's restrict the discussion to the issue at hand, machine types, while
> >> keeping in mind that the same pattern happened with devices types to use
> >> as console before, and my experience running into default network device
> >> types in further work (tests that interact with the guest by ssh'ing
> >> into it).
> >>
> >> The solutions I can think of are:
> >>
> >>  1) run the target binary previous to the "real" run, and query
> >> information -- this is what Avocado-VT does[2], and something I tried on
> >> earlier versions of the acceptance tests infrastructure code
> >>
> >>  2) attempt to get this information from the build system[3]
> >>
> >>  3) hard code the "known" good configuration
> >>
> >> I've previously worked on solutions along the lines of #1 and #2, but
> >> the general feedback wasn't that positive, for valid reasons.  Eduardo
> >> probably remembers this.
> > 
> > I don't remember seeing negative feedback for #1.  It sounds like
> > the best solution.
> > 
> 
> IIRC, it was mostly related to the fact that the reliable way to query a
> target information (instead of parsing a human oriented output) would be
> to use QMP.
> 
> Then, the question of using QEMUMachine for that (and the possible
> chicken-and-egg situations, having a different set of base args, etc)
> led me into prototyping a simplified version:
> 
> https://github.com/clebergnu/qemu/commit/b769b3d969b9e29c644d12c30cd0bb7f312e4fbc#diff-07b9b989eb85128c2c8d2a8f17685698R46
> 
> Which would still be duplicating some code.  I'm not particularly happy
> about either approaches TBH.
> 
> >>
> >> So, I'm tempted to try solution #3.  As much as duplicating target
> >> defaults in qemu.py doesn't sound perfect, it seems to be the more
> >> predictable and attainable solution at this point.
> > 
> > I consider #3 to be acceptable just as a temporary solution until
> > we implement #1.
> > 
> > 
> 
> So, with the extra information given here, would you recommend doing #3?
> Or pause this, and work on a #1 of sorts?

Getting more working tests is more important to me.  I won't mind
implementing #3 on the avocado_qemu module if implementing #1
first would delay the inclusion of tests on QEMU 3.1.

-- 
Eduardo

Re: [Qemu-devel] [PATCH v2 4/7] scripts/qemu.py: set predefined machine type based on arch

[Peter Maydell]

On 10 October 2018 at 20:54, Cleber Rosa <crosa@redhat.com> wrote:
> On 10/10/18 2:07 PM, Peter Maydell wrote:
>> We should aim to test the machines we care about (regardless
>> of what architectures they are), rather than thinking about it
>> in terms of "testing architectures X, Y, Z", I think.
>>
>
> To me it's clear that:
>
>  1) I lack a complete understanding of what "we care about"
>  2) It's easier to start with something, and tweak it to taste

Right. Effectively you're already defining a set of
"machines we care about" -- it's just that rather than
explicitly saying 'we care about x86-64 pc, i386 pc, aarch64 virt,
ppc spapr, ...' you're implicitly doing it by defining
a 'default machine' for various architectures and a list
of architectures. I think you should just list the machines
you're testing with, instead.

I don't mind if we start with a list of what we're testing
that's small (or even wrong!) -- we can expand and correct it
later. I'd just like the list to be the correct shape.

thanks
-- PMM

Re: [Qemu-devel] [PATCH v2 7/7] Acceptance Tests: change the handling of tests for specific archs

[Philippe Mathieu-Daudé]

Hi Cleber,

On 10/10/18 1:26 AM, Cleber Rosa wrote:
> With the introduction of a variants file that can run the same
> tests on various architectures, it makes sense to make most tests
> to be reusable on those environments.  The exception should be
> when a test is really testing a specific architecture feature.
> 
> With the change proposed here, on a command line such as:
> 
>   $ avocado run \
>      --json-variants-load=tests/acceptance/variants/arch.json \
>      -- tests/acceptance/
> 
> The boot_linux_console.py tests will appear as "CANCELED: Currently
> specific to the x86_64 arch", which is as a good thing when compared
> to being ignored by tags because:
> 
>  * The architecture specific parts can be addressed
>  * It will be run on the matching architecture (as opposed to always
>    being filtered out by the tags mechanism)
>  * CANCELED tests do no influence negatively the overall job results,
>    they're not considered an error or failure
> 
> Signed-off-by: Cleber Rosa <crosa@redhat.com>
> ---
>  tests/acceptance/boot_linux_console.py | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/tests/acceptance/boot_linux_console.py b/tests/acceptance/boot_linux_console.py
> index 58032f971c..ba3ac036da 100644
> --- a/tests/acceptance/boot_linux_console.py
> +++ b/tests/acceptance/boot_linux_console.py
> @@ -19,12 +19,13 @@ class BootLinuxConsole(Test):
>      and the kernel command line is properly passed from QEMU to the kernel
>  
>      :avocado: enable
> -    :avocado: tags=x86_64

Can we keep a such tag (in format 'arch:x86_64') ...

>      """
>  
>      timeout = 60
>  
>      def test(self):
> +        if self.arch != 'x86_64':
> +            self.cancel('Currently specific to the x86_64 target arch')

... and have this check generic? (Eventually via a QemuTest(Test) parent
class).

>          kernel_url = ('https://mirrors.kernel.org/fedora/releases/28/'
>                        'Everything/x86_64/os/images/pxeboot/vmlinuz')
>          kernel_hash = '238e083e114c48200f80d889f7e32eeb2793e02a'
> 

Re: [Qemu-devel] [PATCH v2 7/7] Acceptance Tests: change the handling of tests for specific archs

[Cleber Rosa]

On 10/13/18 7:08 AM, Philippe Mathieu-Daudé wrote:
> Hi Cleber,
> 
> On 10/10/18 1:26 AM, Cleber Rosa wrote:
>> With the introduction of a variants file that can run the same
>> tests on various architectures, it makes sense to make most tests
>> to be reusable on those environments.  The exception should be
>> when a test is really testing a specific architecture feature.
>>
>> With the change proposed here, on a command line such as:
>>
>>   $ avocado run \
>>      --json-variants-load=tests/acceptance/variants/arch.json \
>>      -- tests/acceptance/
>>
>> The boot_linux_console.py tests will appear as "CANCELED: Currently
>> specific to the x86_64 arch", which is as a good thing when compared
>> to being ignored by tags because:
>>
>>  * The architecture specific parts can be addressed
>>  * It will be run on the matching architecture (as opposed to always
>>    being filtered out by the tags mechanism)
>>  * CANCELED tests do no influence negatively the overall job results,
>>    they're not considered an error or failure
>>
>> Signed-off-by: Cleber Rosa <crosa@redhat.com>
>> ---
>>  tests/acceptance/boot_linux_console.py | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/tests/acceptance/boot_linux_console.py b/tests/acceptance/boot_linux_console.py
>> index 58032f971c..ba3ac036da 100644
>> --- a/tests/acceptance/boot_linux_console.py
>> +++ b/tests/acceptance/boot_linux_console.py
>> @@ -19,12 +19,13 @@ class BootLinuxConsole(Test):
>>      and the kernel command line is properly passed from QEMU to the kernel
>>  
>>      :avocado: enable
>> -    :avocado: tags=x86_64
> 
> Can we keep a such tag (in format 'arch:x86_64') ...
> 

Yes, sure.  That's how the best "standards" are born ;)

>>      """
>>  
>>      timeout = 60
>>  
>>      def test(self):
>> +        if self.arch != 'x86_64':
>> +            self.cancel('Currently specific to the x86_64 target arch')
> 
> ... and have this check generic? (Eventually via a QemuTest(Test) parent
> class).
> 

Yes, there's a lot of upcoming setup that can be made generic to the
base class.  Some of that is coming soon.

Thanks!
- Cleber.

>>          kernel_url = ('https://mirrors.kernel.org/fedora/releases/28/'
>>                        'Everything/x86_64/os/images/pxeboot/vmlinuz')
>>          kernel_hash = '238e083e114c48200f80d889f7e32eeb2793e02a'
>>