[PATCH 0/4] Jobs based on custom runners: add CentOS Stream 8

[PATCH 0/4] Jobs based on custom runners: add CentOS Stream 8

[Cleber Rosa]

This builds on top the "GitLab Custom Runners and Jobs (was: QEMU
Gating CI)" series, showing an example of how other entities can
add their own custom jobs to the GitLab CI pipeline.

First of all, it may be useful to see an actual pipeline (and the
reespective job introduced here) combined with the jobs introduced
on "GitLab Custom Runners and Jobs (was: QEMU Gating CI)":

 * https://gitlab.com/cleber.gnu/qemu/-/pipelines/316527166
 * https://gitlab.com/cleber.gnu/qemu/-/jobs/1325976765

The runner (the machine and job) is to be managed by Red Hat, and
adds, at the very least, bare metal x86_64 KVM testing capabilities to
the QEMU pipeline.  This brings extra coverage for some unittests, and
the ability to run the acceptance tests dependent on KVM.

The runner is already completely set up and registered to the
https://gitlab.com/qemu-project/qemu project instance, and jobs will
be triggered according to the same rules for the jobs introduced on
"GitLab Custom Runners and Jobs (was: QEMU Gating CI)", that is,
but pushes to the staging branch.  Still, the job is set with mode
"allow failures", so it should not disrupt the existing pipeline.
Once its reliability is proved (rules and service levels are to be
determined), that can be reverted.

Even though the formal method of tracking machine/job maintainers have
not been formalized, it should be known that the contacts/admins for
this machine and job are:

 - Cleber Rosa
   <crosa@redhat.com>
   clebergnu on #qemu

 - Willian Rampazzo
   <willianr@redhat.com>
   willianr on #qemu

Based-on: <20210608031425.833536-1-crosa@redhat.com>

Cleber Rosa (4):
  block.c: fix compilation error on possible unitialized variable
  Python QEMU utils: introduce a generic feature list
  Acceptance Tests: introduce method to require a feature and option
  Jobs based on custom runners: add CentOS Stream 8

 .gitlab-ci.d/custom-runners.yml           |  29 ++++
 block.c                                   |   2 +-
 python/qemu/utils/__init__.py             |   2 +
 python/qemu/utils/accel.py                |  15 +-
 python/qemu/utils/feature.py              |  44 +++++
 scripts/ci/org.centos/stream/README       |   2 +
 scripts/ci/org.centos/stream/configure    | 190 ++++++++++++++++++++++
 scripts/ci/setup/build-environment.yml    |  38 +++++
 tests/acceptance/avocado_qemu/__init__.py |  29 +++-
 tests/acceptance/multiprocess.py          |   1 +
 10 files changed, 337 insertions(+), 15 deletions(-)
 create mode 100644 python/qemu/utils/feature.py
 create mode 100644 scripts/ci/org.centos/stream/README
 create mode 100755 scripts/ci/org.centos/stream/configure

-- 
2.25.4

[PATCH 1/4] block.c: fix compilation error on possible unitialized variable

[Cleber Rosa]

GCC from CentOS Stream 8 is erroring out on a possibily unitialized
varible.

Full version info for the compiler used:

 gcc (GCC) 8.5.0 20210514 (Red Hat 8.5.0-1)

Signed-off-by: Cleber Rosa <crosa@redhat.com>
---
 block.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/block.c b/block.c
index 3f456892d0..08f29e6b65 100644
--- a/block.c
+++ b/block.c
@@ -4866,7 +4866,7 @@ static int bdrv_replace_node_common(BlockDriverState *from,
     Transaction *tran = tran_new();
     g_autoptr(GHashTable) found = NULL;
     g_autoptr(GSList) refresh_list = NULL;
-    BlockDriverState *to_cow_parent;
+    BlockDriverState *to_cow_parent = NULL;
     int ret;
 
     if (detach_subchain) {
-- 
2.25.4

[PATCH 2/4] Python QEMU utils: introduce a generic feature list

[Cleber Rosa]

Which can be used to check for any "feature" that is available as a
QEMU command line option, and that will return its list of available
options.

This is a generalization of the list_accel() utility function, which
is itself re-implemented in terms of the more generic feature.

Signed-off-by: Cleber Rosa <crosa@redhat.com>
---
 python/qemu/utils/__init__.py |  2 ++
 python/qemu/utils/accel.py    | 15 ++----------
 python/qemu/utils/feature.py  | 44 +++++++++++++++++++++++++++++++++++
 3 files changed, 48 insertions(+), 13 deletions(-)
 create mode 100644 python/qemu/utils/feature.py

diff --git a/python/qemu/utils/__init__.py b/python/qemu/utils/__init__.py
index 7f1a5138c4..1d0789eaa2 100644
--- a/python/qemu/utils/__init__.py
+++ b/python/qemu/utils/__init__.py
@@ -20,12 +20,14 @@
 
 # pylint: disable=import-error
 from .accel import kvm_available, list_accel, tcg_available
+from .feature import list_feature
 
 
 __all__ = (
     'get_info_usernet_hostfwd_port',
     'kvm_available',
     'list_accel',
+    'list_feature',
     'tcg_available',
 )
 
diff --git a/python/qemu/utils/accel.py b/python/qemu/utils/accel.py
index 297933df2a..b5bb80c6d3 100644
--- a/python/qemu/utils/accel.py
+++ b/python/qemu/utils/accel.py
@@ -14,13 +14,11 @@
 # the COPYING file in the top-level directory.
 #
 
-import logging
 import os
-import subprocess
 from typing import List, Optional
 
+from qemu.utils.feature import list_feature
 
-LOG = logging.getLogger(__name__)
 
 # Mapping host architecture to any additional architectures it can
 # support which often includes its 32 bit cousin.
@@ -39,16 +37,7 @@ def list_accel(qemu_bin: str) -> List[str]:
     @raise Exception: if failed to run `qemu -accel help`
     @return a list of accelerator names.
     """
-    if not qemu_bin:
-        return []
-    try:
-        out = subprocess.check_output([qemu_bin, '-accel', 'help'],
-                                      universal_newlines=True)
-    except:
-        LOG.debug("Failed to get the list of accelerators in %s", qemu_bin)
-        raise
-    # Skip the first line which is the header.
-    return [acc.strip() for acc in out.splitlines()[1:]]
+    return list_feature(qemu_bin, 'accel')
 
 
 def kvm_available(target_arch: Optional[str] = None,
diff --git a/python/qemu/utils/feature.py b/python/qemu/utils/feature.py
new file mode 100644
index 0000000000..b4a5f929ab
--- /dev/null
+++ b/python/qemu/utils/feature.py
@@ -0,0 +1,44 @@
+"""
+QEMU feature module:
+
+This module provides a utility for discovering the availability of
+generic features.
+"""
+# Copyright (C) 2022 Red Hat Inc.
+#
+# Authors:
+#  Cleber Rosa <crosa@redhat.com>
+#
+# This work is licensed under the terms of the GNU GPL, version 2.  See
+# the COPYING file in the top-level directory.
+#
+
+import logging
+import subprocess
+from typing import List
+
+
+LOG = logging.getLogger(__name__)
+
+
+def list_feature(qemu_bin: str, feature: str) -> List[str]:
+    """
+    List available options the QEMU binary for a given feature type.
+
+    By calling a "qemu $feature -help" and parsing its output.
+
+    @param qemu_bin (str): path to the QEMU binary.
+    @param feature (str): feature name, matching the command line option.
+    @raise Exception: if failed to run `qemu -feature help`
+    @return a list of available options for the given feature.
+    """
+    if not qemu_bin:
+        return []
+    try:
+        out = subprocess.check_output([qemu_bin, '-%s' % feature, 'help'],
+                                      universal_newlines=True)
+    except:
+        LOG.debug("Failed to get the list of %s(s) in %s", feature, qemu_bin)
+        raise
+    # Skip the first line which is the header.
+    return [item.split(' ', 1)[0] for item in out.splitlines()[1:]]
-- 
2.25.4

[PATCH 3/4] Acceptance Tests: introduce method to require a feature and option

[Cleber Rosa]

In this context, and according to the qemu.utils.list_feature() utility
function, a feature is something is available as a QEMU command line
option that can take the "help" value.

This builds on top of that utility function, and allows test writers
to require, for instance, the "x-remote" (option) machine type
(feature).

This example is actually applied here to the reespective test, given
that the option is conditionally built, and the test will ERROR
without it.

Signed-off-by: Cleber Rosa <crosa@redhat.com>
---
 tests/acceptance/avocado_qemu/__init__.py | 29 ++++++++++++++++++++++-
 tests/acceptance/multiprocess.py          |  1 +
 2 files changed, 29 insertions(+), 1 deletion(-)

diff --git a/tests/acceptance/avocado_qemu/__init__.py b/tests/acceptance/avocado_qemu/__init__.py
index 93c4b9851f..432caff4e6 100644
--- a/tests/acceptance/avocado_qemu/__init__.py
+++ b/tests/acceptance/avocado_qemu/__init__.py
@@ -11,6 +11,7 @@
 import logging
 import os
 import shutil
+import subprocess
 import sys
 import uuid
 import tempfile
@@ -45,7 +46,8 @@
 from qemu.utils import (
     get_info_usernet_hostfwd_port,
     kvm_available,
-    tcg_available,
+    list_feature,
+    tcg_available
 )
 
 def is_readable_executable_file(path):
@@ -182,6 +184,31 @@ def _get_unique_tag_val(self, tag_name):
             return vals.pop()
         return None
 
+    def require_feature(self, feature, option=None):
+        """
+        Requires a feature to be available for the test to continue
+
+        It takes into account the currently set qemu binary, and only checks
+        for by running a "qemu -$feature help" command.  If the specific option
+        is given, it checks if it's listed for the given feature.
+
+        If the check fails, the test is canceled.
+
+        :param feature: name of a QEMU feature, such as "accel" or "machine"
+        :type feature: str
+        :param option: the specific value for the feature.  For instance,
+                       if feature is "machine", option can be "q35".
+        type option: str
+        """
+        try:
+            options_available = list_feature(self.qemu_bin, feature)
+        except subprocess.CalledProcessError:
+            self.cancel('Feature "%s" does not appear to be present.' % feature)
+        if option is not None:
+            if option not in options_available:
+                self.cancel('Feature "%s" does not have "%s" as an option' %
+                            (feature, option))
+
     def require_accelerator(self, accelerator):
         """
         Requires an accelerator to be available for the test to continue
diff --git a/tests/acceptance/multiprocess.py b/tests/acceptance/multiprocess.py
index 96627f022a..4d8a40a510 100644
--- a/tests/acceptance/multiprocess.py
+++ b/tests/acceptance/multiprocess.py
@@ -22,6 +22,7 @@ def do_test(self, kernel_url, initrd_url, kernel_command_line,
                 machine_type):
         """Main test method"""
         self.require_accelerator('kvm')
+        self.require_feature('machine', 'x-remote')
 
         # Create socketpair to connect proxy and remote processes
         proxy_sock, remote_sock = socket.socketpair(socket.AF_UNIX,
-- 
2.25.4

[PATCH 4/4] Jobs based on custom runners: add CentOS Stream 8

[Cleber Rosa]

This introduces three different parts of a job designed to run
on a custom runner managed by Red Hat.  The goals include:

 a) serve as a model for other organizations that want to onboard
    their own runners, with their specific platforms, build
    configuration and tests.

 b) bring awareness to the differences between upstream QEMU and the
    version available under CentOS Stream, which is "A preview of
    upcoming Red Hat Enterprise Linux minor and major releases.".

 c) becase of b), it should be easier to identify and reduce the gap
    between Red Hat's downstream and upstream QEMU.

The components themselves to achieve this custom job are:

 1) build environment configuration: documentation and a playbook for
    a base Enterprise Linux 8 system (also applicable to CentOS
    Stream), which other users can run on their system to get the
    environment suitable for building QEMU.

 2) QEMU build configuration: how QEMU will be built to match, as
    closely as possible, the binaries built and packaged on CentOS
    stream 8.

 3) job definition: GitLab CI jobs that will dispatch the build/test
    job to the machine specifically configured according to #1.

Signed-off-by: Cleber Rosa <crosa@redhat.com>
---
 .gitlab-ci.d/custom-runners.yml        |  29 ++++
 scripts/ci/org.centos/stream/README    |   2 +
 scripts/ci/org.centos/stream/configure | 190 +++++++++++++++++++++++++
 scripts/ci/setup/build-environment.yml |  38 +++++
 4 files changed, 259 insertions(+)
 create mode 100644 scripts/ci/org.centos/stream/README
 create mode 100755 scripts/ci/org.centos/stream/configure

diff --git a/.gitlab-ci.d/custom-runners.yml b/.gitlab-ci.d/custom-runners.yml
index 061d3cdfed..ee5143995e 100644
--- a/.gitlab-ci.d/custom-runners.yml
+++ b/.gitlab-ci.d/custom-runners.yml
@@ -220,3 +220,32 @@ ubuntu-20.04-aarch64-notcg:
  - ../configure --disable-libssh --disable-tcg
  - make --output-sync -j`nproc`
  - make --output-sync -j`nproc` check V=1
+
+centos-stream-8-x86_64:
+ allow_failure: true
+ needs: []
+ stage: build
+ tags:
+ - centos_stream_8
+ - x86_64
+ rules:
+ - if: '$CI_COMMIT_BRANCH =~ /^staging/'
+ artifacts:
+   name: "$CI_JOB_NAME-$CI_COMMIT_REF_SLUG"
+   when: on_failure
+   expire_in: 7 days
+   paths:
+     - build/tests/results/latest/results.xml
+     - build/tests/results/latest/test-results
+   reports:
+     junit: build/tests/results/latest/results.xml
+ script:
+ - mkdir build
+ - cd build
+ - ../scripts/ci/org.centos/stream/configure
+ - make --output-sync -j`nproc`
+ - make --output-sync -j`nproc` check V=1
+ - make get-vm-images
+ # Only run tests that are either marked explicitly for KVM and x86_64
+ # or tests that are supposed to be valid for all targets
+ - ./tests/venv/bin/avocado run --job-results-dir=tests/results/ --filter-by-tags-include-empty --filter-by-tags-include-empty-key -t accel:kvm,arch:x86_64 -- tests/acceptance/
diff --git a/scripts/ci/org.centos/stream/README b/scripts/ci/org.centos/stream/README
new file mode 100644
index 0000000000..f99bda99b8
--- /dev/null
+++ b/scripts/ci/org.centos/stream/README
@@ -0,0 +1,2 @@
+This directory contains scripts for generating a build of QEMU that
+closely matches the CentOS Stream builds of the qemu-kvm package.
diff --git a/scripts/ci/org.centos/stream/configure b/scripts/ci/org.centos/stream/configure
new file mode 100755
index 0000000000..1e7207faec
--- /dev/null
+++ b/scripts/ci/org.centos/stream/configure
@@ -0,0 +1,190 @@
+#!/bin/sh -e
+../configure \
+--prefix="/usr" \
+--libdir="/usr/lib64" \
+--datadir="/usr/share" \
+--sysconfdir="/etc" \
+--interp-prefix=/usr/qemu-%M \
+--localstatedir="/var" \
+--docdir="/usr/share/doc" \
+--libexecdir="/usr/libexec" \
+--extra-ldflags="-Wl,--build-id -Wl,-z,relro -Wl,-z,now" \
+--extra-cflags="-O2 -g -pipe -Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -Wp,-D_GLIBCXX_ASSERTIONS -fexceptions -fstack-protector-strong -grecord-gcc-switches -specs=/usr/lib/rpm/redhat/redhat-hardened-cc1 -specs=/usr/lib/rpm/redhat/redhat-annobin-cc1 -m64 -mtune=generic -fasynchronous-unwind-tables -fstack-clash-protection -fcf-protection" \
+--with-suffix="qemu-kvm" \
+--firmwarepath=/usr/share/qemu-firmware \
+--meson="/usr/bin/meson" \
+--target-list="x86_64-softmmu" \
+--block-drv-rw-whitelist=qcow2,raw,file,host_device,nbd,iscsi,rbd,blkdebug,luks,null-co,nvme,copy-on-read,throttle,gluster \
+--audio-drv-list= \
+--block-drv-ro-whitelist=vmdk,vhdx,vpc,https,ssh \
+--with-coroutine=ucontext \
+--with-git=git \
+--tls-priority=@QEMU,SYSTEM \
+--disable-attr \
+--disable-auth-pam \
+--disable-avx2 \
+--disable-avx512f \
+--disable-bochs \
+--disable-brlapi \
+--disable-bsd-user \
+--disable-bzip2 \
+--disable-cap-ng \
+--disable-capstone \
+--disable-cfi \
+--disable-cfi-debug \
+--disable-cloop \
+--disable-cocoa \
+--disable-coroutine-pool \
+--disable-crypto-afalg \
+--disable-curl \
+--disable-curses \
+--disable-debug-info \
+--disable-debug-mutex \
+--disable-debug-tcg \
+--disable-dmg \
+--disable-docs \
+--disable-fdt \
+--disable-fuse \
+--disable-fuse-lseek \
+--disable-gcrypt \
+--disable-gio \
+--disable-glusterfs \
+--disable-gnutls \
+--disable-gtk \
+--disable-guest-agent \
+--disable-guest-agent-msi \
+--disable-hax \
+--disable-hvf \
+--disable-iconv \
+--disable-jemalloc \
+--disable-kvm \
+--disable-libdaxctl \
+--disable-libiscsi \
+--disable-libnfs \
+--disable-libpmem \
+--disable-libssh \
+--disable-libudev \
+--disable-libusb \
+--disable-libxml2 \
+--disable-linux-aio \
+--disable-linux-io-uring \
+--disable-linux-user \
+--disable-live-block-migration \
+--disable-lto \
+--disable-lzfse \
+--disable-lzo \
+--disable-malloc-trim \
+--disable-membarrier \
+--disable-modules \
+--disable-module-upgrades \
+--disable-mpath \
+--disable-multiprocess \
+--disable-netmap \
+--disable-nettle \
+--disable-numa \
+--disable-opengl \
+--disable-parallels \
+--disable-pie \
+--disable-pvrdma \
+--disable-qcow1 \
+--disable-qed \
+--disable-qom-cast-debug \
+--disable-rbd \
+--disable-rdma \
+--disable-replication \
+--disable-rng-none \
+--disable-safe-stack \
+--disable-sanitizers \
+--disable-sdl \
+--disable-sdl-image \
+--disable-seccomp \
+--disable-smartcard \
+--disable-snappy \
+--disable-sparse \
+--disable-spice \
+--disable-strip \
+--disable-system \
+--disable-tcg \
+--disable-tcmalloc \
+--disable-tools \
+--disable-tpm \
+--disable-u2f \
+--disable-usb-redir \
+--disable-user \
+--disable-vde \
+--disable-vdi \
+--disable-vhost-crypto \
+--disable-vhost-kernel \
+--disable-vhost-net \
+--disable-vhost-scsi \
+--disable-vhost-user \
+--disable-vhost-user-blk-server \
+--disable-vhost-vdpa \
+--disable-vhost-vsock \
+--disable-virglrenderer \
+--disable-virtfs \
+--disable-virtiofsd \
+--disable-vnc \
+--disable-vnc-jpeg \
+--disable-vnc-png \
+--disable-vnc-sasl \
+--disable-vte \
+--disable-vvfat \
+--disable-werror \
+--disable-whpx \
+--disable-xen \
+--disable-xen-pci-passthrough \
+--disable-xfsctl \
+--disable-xkbcommon \
+--disable-zstd \
+--enable-attr \
+--enable-avx2 \
+--enable-cap-ng \
+--enable-capstone \
+--enable-coroutine-pool \
+--enable-curl \
+--enable-debug-info \
+--enable-docs \
+--enable-gcrypt \
+--enable-glusterfs \
+--enable-gnutls \
+--enable-guest-agent \
+--enable-iconv \
+--enable-kvm \
+--enable-libiscsi \
+--enable-libpmem \
+--enable-libssh \
+--enable-libusb \
+--enable-libudev \
+--enable-linux-aio \
+--enable-lzo \
+--enable-malloc-trim \
+--enable-modules \
+--enable-mpath \
+--enable-numa \
+--enable-opengl \
+--enable-pie \
+--enable-rbd \
+--enable-rdma \
+--enable-seccomp \
+--enable-snappy \
+--enable-smartcard \
+--enable-spice \
+--enable-system \
+--enable-tcg \
+--enable-tools \
+--enable-tpm \
+--enable-trace-backend=dtrace \
+--enable-usb-redir \
+--enable-virtiofsd \
+--enable-vhost-kernel \
+--enable-vhost-net \
+--enable-vhost-user \
+--enable-vhost-user-blk-server \
+--enable-vhost-vdpa \
+--enable-vhost-vsock \
+--enable-vnc \
+--enable-vnc-png \
+--enable-vnc-sasl \
+--enable-werror \
+--enable-xkbcommon
diff --git a/scripts/ci/setup/build-environment.yml b/scripts/ci/setup/build-environment.yml
index 664f2f0519..b1e01b1025 100644
--- a/scripts/ci/setup/build-environment.yml
+++ b/scripts/ci/setup/build-environment.yml
@@ -96,3 +96,41 @@
       when:
         - ansible_facts['distribution'] == 'Ubuntu'
         - ansible_facts['distribution_version'] == '20.04'
+
+    - name: Install basic packages to build QEMU on EL8
+      dnf:
+        # This list of packages start with tests/docker/dockerfiles/centos8.docker
+        # but only include files that are common to all distro variants and present
+        # in the standard repos (no add-ons)
+        name:
+          - bzip2
+          - bzip2-devel
+          - dbus-daemon
+          - diffutils
+          - gcc
+          - gcc-c++
+          - genisoimage
+          - gettext
+          - git
+          - glib2-devel
+          - libaio-devel
+          - libepoxy-devel
+          - libgcrypt-devel
+          - lzo-devel
+          - make
+          - mesa-libEGL-devel
+          - nettle-devel
+          - nmap-ncat
+          - perl-Test-Harness
+          - pixman-devel
+          - python36
+          - rdma-core-devel
+          - spice-glib-devel
+          - spice-server
+          - systemtap-sdt-devel
+          - tar
+          - zlib-devel
+        state: present
+      when:
+        - ansible_facts['distribution_file_variety'] == 'RedHat'
+        - ansible_facts['distribution_version'] == '8'
-- 
2.25.4

Re: [PATCH 2/4] Python QEMU utils: introduce a generic feature list

[Wainer dos Santos Moschetta]

Hi,

On 6/8/21 11:09 AM, Cleber Rosa wrote:
> Which can be used to check for any "feature" that is available as a
> QEMU command line option, and that will return its list of available
> options.
>
> This is a generalization of the list_accel() utility function, which
> is itself re-implemented in terms of the more generic feature.
>
> Signed-off-by: Cleber Rosa <crosa@redhat.com>
> ---
>   python/qemu/utils/__init__.py |  2 ++
>   python/qemu/utils/accel.py    | 15 ++----------
>   python/qemu/utils/feature.py  | 44 +++++++++++++++++++++++++++++++++++
>   3 files changed, 48 insertions(+), 13 deletions(-)
>   create mode 100644 python/qemu/utils/feature.py
>
> diff --git a/python/qemu/utils/__init__.py b/python/qemu/utils/__init__.py
> index 7f1a5138c4..1d0789eaa2 100644
> --- a/python/qemu/utils/__init__.py
> +++ b/python/qemu/utils/__init__.py
> @@ -20,12 +20,14 @@
>   
>   # pylint: disable=import-error
>   from .accel import kvm_available, list_accel, tcg_available
> +from .feature import list_feature
>   
>   
>   __all__ = (
>       'get_info_usernet_hostfwd_port',
>       'kvm_available',
>       'list_accel',
> +    'list_feature',
>       'tcg_available',
>   )
>   
> diff --git a/python/qemu/utils/accel.py b/python/qemu/utils/accel.py
> index 297933df2a..b5bb80c6d3 100644
> --- a/python/qemu/utils/accel.py
> +++ b/python/qemu/utils/accel.py
> @@ -14,13 +14,11 @@
>   # the COPYING file in the top-level directory.
>   #
>   
> -import logging
>   import os
> -import subprocess
>   from typing import List, Optional
>   
> +from qemu.utils.feature import list_feature
>   
> -LOG = logging.getLogger(__name__)
>   
>   # Mapping host architecture to any additional architectures it can
>   # support which often includes its 32 bit cousin.
> @@ -39,16 +37,7 @@ def list_accel(qemu_bin: str) -> List[str]:
>       @raise Exception: if failed to run `qemu -accel help`
>       @return a list of accelerator names.
>       """
> -    if not qemu_bin:
> -        return []
> -    try:
> -        out = subprocess.check_output([qemu_bin, '-accel', 'help'],
> -                                      universal_newlines=True)
> -    except:
> -        LOG.debug("Failed to get the list of accelerators in %s", qemu_bin)
> -        raise
> -    # Skip the first line which is the header.
> -    return [acc.strip() for acc in out.splitlines()[1:]]
> +    return list_feature(qemu_bin, 'accel')
>   
>   
>   def kvm_available(target_arch: Optional[str] = None,
> diff --git a/python/qemu/utils/feature.py b/python/qemu/utils/feature.py
> new file mode 100644
> index 0000000000..b4a5f929ab
> --- /dev/null
> +++ b/python/qemu/utils/feature.py
> @@ -0,0 +1,44 @@
> +"""
> +QEMU feature module:
> +
> +This module provides a utility for discovering the availability of
> +generic features.
> +"""
> +# Copyright (C) 2022 Red Hat Inc.
Cleber, please, tell me how is the future like! :)
> +#
> +# Authors:
> +#  Cleber Rosa <crosa@redhat.com>
> +#
> +# This work is licensed under the terms of the GNU GPL, version 2.  See
> +# the COPYING file in the top-level directory.
> +#
> +
> +import logging
> +import subprocess
> +from typing import List
> +
> +
> +LOG = logging.getLogger(__name__)
> +
> +
> +def list_feature(qemu_bin: str, feature: str) -> List[str]:
> +    """
> +    List available options the QEMU binary for a given feature type.
> +
> +    By calling a "qemu $feature -help" and parsing its output.

I understand we need a mean to easily cancel the test if given feature 
is not present. However, I'm unsure this generic list_feature() is what 
we need.

The `-accel help` returns a simple list of strings (besides the header, 
which is dismissed). Whereas `-machine help` returns what could be 
parsed as a tuple of (name, description).

Another example is `-cpu help` which will print a similar list as 
`-machine`, plus a section with CPUID flags.

If confess I still don't have a better idea, although I feel it will 
require a OO design.

Thanks!

- Wainer

> +
> +    @param qemu_bin (str): path to the QEMU binary.
> +    @param feature (str): feature name, matching the command line option.
> +    @raise Exception: if failed to run `qemu -feature help`
> +    @return a list of available options for the given feature.
> +    """
> +    if not qemu_bin:
> +        return []
> +    try:
> +        out = subprocess.check_output([qemu_bin, '-%s' % feature, 'help'],
> +                                      universal_newlines=True)
> +    except:
> +        LOG.debug("Failed to get the list of %s(s) in %s", feature, qemu_bin)
> +        raise
> +    # Skip the first line which is the header.
> +    return [item.split(' ', 1)[0] for item in out.splitlines()[1:]]

Re: [PATCH 2/4] Python QEMU utils: introduce a generic feature list

[Cleber Rosa Junior]

[-- Attachment #1: Type: text/plain, Size: 6325 bytes --]

On Tue, Jun 8, 2021 at 5:42 PM Wainer dos Santos Moschetta <
wainersm@redhat.com> wrote:

> Hi,
>
> On 6/8/21 11:09 AM, Cleber Rosa wrote:
> > Which can be used to check for any "feature" that is available as a
> > QEMU command line option, and that will return its list of available
> > options.
> >
> > This is a generalization of the list_accel() utility function, which
> > is itself re-implemented in terms of the more generic feature.
> >
> > Signed-off-by: Cleber Rosa <crosa@redhat.com>
> > ---
> >   python/qemu/utils/__init__.py |  2 ++
> >   python/qemu/utils/accel.py    | 15 ++----------
> >   python/qemu/utils/feature.py  | 44 +++++++++++++++++++++++++++++++++++
> >   3 files changed, 48 insertions(+), 13 deletions(-)
> >   create mode 100644 python/qemu/utils/feature.py
> >
> > diff --git a/python/qemu/utils/__init__.py
> b/python/qemu/utils/__init__.py
> > index 7f1a5138c4..1d0789eaa2 100644
> > --- a/python/qemu/utils/__init__.py
> > +++ b/python/qemu/utils/__init__.py
> > @@ -20,12 +20,14 @@
> >
> >   # pylint: disable=import-error
> >   from .accel import kvm_available, list_accel, tcg_available
> > +from .feature import list_feature
> >
> >
> >   __all__ = (
> >       'get_info_usernet_hostfwd_port',
> >       'kvm_available',
> >       'list_accel',
> > +    'list_feature',
> >       'tcg_available',
> >   )
> >
> > diff --git a/python/qemu/utils/accel.py b/python/qemu/utils/accel.py
> > index 297933df2a..b5bb80c6d3 100644
> > --- a/python/qemu/utils/accel.py
> > +++ b/python/qemu/utils/accel.py
> > @@ -14,13 +14,11 @@
> >   # the COPYING file in the top-level directory.
> >   #
> >
> > -import logging
> >   import os
> > -import subprocess
> >   from typing import List, Optional
> >
> > +from qemu.utils.feature import list_feature
> >
> > -LOG = logging.getLogger(__name__)
> >
> >   # Mapping host architecture to any additional architectures it can
> >   # support which often includes its 32 bit cousin.
> > @@ -39,16 +37,7 @@ def list_accel(qemu_bin: str) -> List[str]:
> >       @raise Exception: if failed to run `qemu -accel help`
> >       @return a list of accelerator names.
> >       """
> > -    if not qemu_bin:
> > -        return []
> > -    try:
> > -        out = subprocess.check_output([qemu_bin, '-accel', 'help'],
> > -                                      universal_newlines=True)
> > -    except:
> > -        LOG.debug("Failed to get the list of accelerators in %s",
> qemu_bin)
> > -        raise
> > -    # Skip the first line which is the header.
> > -    return [acc.strip() for acc in out.splitlines()[1:]]
> > +    return list_feature(qemu_bin, 'accel')
> >
> >
> >   def kvm_available(target_arch: Optional[str] = None,
> > diff --git a/python/qemu/utils/feature.py b/python/qemu/utils/feature.py
> > new file mode 100644
> > index 0000000000..b4a5f929ab
> > --- /dev/null
> > +++ b/python/qemu/utils/feature.py
> > @@ -0,0 +1,44 @@
> > +"""
> > +QEMU feature module:
> > +
> > +This module provides a utility for discovering the availability of
> > +generic features.
> > +"""
> > +# Copyright (C) 2022 Red Hat Inc.
> Cleber, please, tell me how is the future like! :)
>

I grabbed a sports almanac.  That's all I can say. :)

Now seriously, thanks for spotting the typo.


> > +#
> > +# Authors:
> > +#  Cleber Rosa <crosa@redhat.com>
> > +#
> > +# This work is licensed under the terms of the GNU GPL, version 2.  See
> > +# the COPYING file in the top-level directory.
> > +#
> > +
> > +import logging
> > +import subprocess
> > +from typing import List
> > +
> > +
> > +LOG = logging.getLogger(__name__)
> > +
> > +
> > +def list_feature(qemu_bin: str, feature: str) -> List[str]:
> > +    """
> > +    List available options the QEMU binary for a given feature type.
> > +
> > +    By calling a "qemu $feature -help" and parsing its output.
>
> I understand we need a mean to easily cancel the test if given feature
> is not present. However, I'm unsure this generic list_feature() is what
> we need.
>
> The `-accel help` returns a simple list of strings (besides the header,
> which is dismissed). Whereas `-machine help` returns what could be
> parsed as a tuple of (name, description).
>
> Another example is `-cpu help` which will print a similar list as
> `-machine`, plus a section with CPUID flags.
>
>
I made sure it worked with both "accel" and "machine", but you're right
about many other "-$feature help" that won't conform to the mapping
("-chardev help" is probably the only other one that should work).  What I
thought about was to keep the same list_feature(), but make its parsing of
items flexible.  Then it could be reused for other list_$feature() like
methods.  At the same time, it could be an opportunity to standardize a bit
more of the "help" outputs.

For instance, I think it would make sense for "cpu" to keep showing the
amount of information it shows, but:

1) The first item could be the name of the relevant "option" (the cpu
model) for that feature (cpu), instead of, say, "x86". Basically reversing
the order of first and second, or first and third fields.
2) Everything *after* an empty line would be extra information, not
necessarily an option available for that feature.

But, this is most probably not going to be achieved in the short term.

What I can do here, is to hide list_feature(), say call it _list_feature()
so that we don't duplicate code, and expose a list_machine().

Let me know what you think.

Thanks,
- Cleber.


> If confess I still don't have a better idea, although I feel it will
> require a OO design.
>
> Thanks!
>
> - Wainer
>
> > +
> > +    @param qemu_bin (str): path to the QEMU binary.
> > +    @param feature (str): feature name, matching the command line
> option.
> > +    @raise Exception: if failed to run `qemu -feature help`
> > +    @return a list of available options for the given feature.
> > +    """
> > +    if not qemu_bin:
> > +        return []
> > +    try:
> > +        out = subprocess.check_output([qemu_bin, '-%s' % feature,
> 'help'],
> > +                                      universal_newlines=True)
> > +    except:
> > +        LOG.debug("Failed to get the list of %s(s) in %s", feature,
> qemu_bin)
> > +        raise
> > +    # Skip the first line which is the header.
> > +    return [item.split(' ', 1)[0] for item in out.splitlines()[1:]]
>
>

[-- Attachment #2: Type: text/html, Size: 8617 bytes --]

Re: [PATCH 1/4] block.c: fix compilation error on possible unitialized variable

[Thomas Huth]

On 08/06/2021 16.09, Cleber Rosa wrote:
> GCC from CentOS Stream 8 is erroring out on a possibily unitialized
> varible.
> 
> Full version info for the compiler used:
> 
>   gcc (GCC) 8.5.0 20210514 (Red Hat 8.5.0-1)
> 
> Signed-off-by: Cleber Rosa <crosa@redhat.com>
> ---
>   block.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/block.c b/block.c
> index 3f456892d0..08f29e6b65 100644
> --- a/block.c
> +++ b/block.c
> @@ -4866,7 +4866,7 @@ static int bdrv_replace_node_common(BlockDriverState *from,
>       Transaction *tran = tran_new();
>       g_autoptr(GHashTable) found = NULL;
>       g_autoptr(GSList) refresh_list = NULL;
> -    BlockDriverState *to_cow_parent;
> +    BlockDriverState *to_cow_parent = NULL;
>       int ret;

Already reported here:

  https://lists.gnu.org/archive/html/qemu-devel/2021-05/msg01229.html

  Thomas

Re: [PATCH 4/4] Jobs based on custom runners: add CentOS Stream 8

[Cleber Rosa Junior]

On Tue, Jun 8, 2021 at 10:10 AM Cleber Rosa <crosa@redhat.com> wrote:
>
> This introduces three different parts of a job designed to run
> on a custom runner managed by Red Hat.  The goals include:
>
>  a) serve as a model for other organizations that want to onboard
>     their own runners, with their specific platforms, build
>     configuration and tests.
>
>  b) bring awareness to the differences between upstream QEMU and the
>     version available under CentOS Stream, which is "A preview of
>     upcoming Red Hat Enterprise Linux minor and major releases.".
>
>  c) becase of b), it should be easier to identify and reduce the gap
>     between Red Hat's downstream and upstream QEMU.
>
> The components themselves to achieve this custom job are:
>
>  1) build environment configuration: documentation and a playbook for
>     a base Enterprise Linux 8 system (also applicable to CentOS
>     Stream), which other users can run on their system to get the
>     environment suitable for building QEMU.
>
>  2) QEMU build configuration: how QEMU will be built to match, as
>     closely as possible, the binaries built and packaged on CentOS
>     stream 8.
>
>  3) job definition: GitLab CI jobs that will dispatch the build/test
>     job to the machine specifically configured according to #1.
>
> Signed-off-by: Cleber Rosa <crosa@redhat.com>
> ---
>  .gitlab-ci.d/custom-runners.yml        |  29 ++++
>  scripts/ci/org.centos/stream/README    |   2 +
>  scripts/ci/org.centos/stream/configure | 190 +++++++++++++++++++++++++
>  scripts/ci/setup/build-environment.yml |  38 +++++
>  4 files changed, 259 insertions(+)
>  create mode 100644 scripts/ci/org.centos/stream/README
>  create mode 100755 scripts/ci/org.centos/stream/configure
>
> diff --git a/.gitlab-ci.d/custom-runners.yml b/.gitlab-ci.d/custom-runners.yml
> index 061d3cdfed..ee5143995e 100644
> --- a/.gitlab-ci.d/custom-runners.yml
> +++ b/.gitlab-ci.d/custom-runners.yml
> @@ -220,3 +220,32 @@ ubuntu-20.04-aarch64-notcg:
>   - ../configure --disable-libssh --disable-tcg
>   - make --output-sync -j`nproc`
>   - make --output-sync -j`nproc` check V=1
> +
> +centos-stream-8-x86_64:
> + allow_failure: true
> + needs: []
> + stage: build
> + tags:
> + - centos_stream_8
> + - x86_64
> + rules:
> + - if: '$CI_COMMIT_BRANCH =~ /^staging/'
> + artifacts:
> +   name: "$CI_JOB_NAME-$CI_COMMIT_REF_SLUG"
> +   when: on_failure
> +   expire_in: 7 days
> +   paths:
> +     - build/tests/results/latest/results.xml
> +     - build/tests/results/latest/test-results
> +   reports:
> +     junit: build/tests/results/latest/results.xml
> + script:
> + - mkdir build
> + - cd build
> + - ../scripts/ci/org.centos/stream/configure
> + - make --output-sync -j`nproc`
> + - make --output-sync -j`nproc` check V=1
> + - make get-vm-images
> + # Only run tests that are either marked explicitly for KVM and x86_64
> + # or tests that are supposed to be valid for all targets
> + - ./tests/venv/bin/avocado run --job-results-dir=tests/results/ --filter-by-tags-include-empty --filter-by-tags-include-empty-key -t accel:kvm,arch:x86_64 -- tests/acceptance/
> diff --git a/scripts/ci/org.centos/stream/README b/scripts/ci/org.centos/stream/README
> new file mode 100644
> index 0000000000..f99bda99b8
> --- /dev/null
> +++ b/scripts/ci/org.centos/stream/README
> @@ -0,0 +1,2 @@
> +This directory contains scripts for generating a build of QEMU that
> +closely matches the CentOS Stream builds of the qemu-kvm package.
> diff --git a/scripts/ci/org.centos/stream/configure b/scripts/ci/org.centos/stream/configure
> new file mode 100755
> index 0000000000..1e7207faec
> --- /dev/null
> +++ b/scripts/ci/org.centos/stream/configure
> @@ -0,0 +1,190 @@
> +#!/bin/sh -e
> +../configure \
> +--prefix="/usr" \
> +--libdir="/usr/lib64" \
> +--datadir="/usr/share" \
> +--sysconfdir="/etc" \
> +--interp-prefix=/usr/qemu-%M \
> +--localstatedir="/var" \
> +--docdir="/usr/share/doc" \
> +--libexecdir="/usr/libexec" \
> +--extra-ldflags="-Wl,--build-id -Wl,-z,relro -Wl,-z,now" \
> +--extra-cflags="-O2 -g -pipe -Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -Wp,-D_GLIBCXX_ASSERTIONS -fexceptions -fstack-protector-strong -grecord-gcc-switches -specs=/usr/lib/rpm/redhat/redhat-hardened-cc1 -specs=/usr/lib/rpm/redhat/redhat-annobin-cc1 -m64 -mtune=generic -fasynchronous-unwind-tables -fstack-clash-protection -fcf-protection" \
> +--with-suffix="qemu-kvm" \
> +--firmwarepath=/usr/share/qemu-firmware \
> +--meson="/usr/bin/meson" \
> +--target-list="x86_64-softmmu" \
> +--block-drv-rw-whitelist=qcow2,raw,file,host_device,nbd,iscsi,rbd,blkdebug,luks,null-co,nvme,copy-on-read,throttle,gluster \
> +--audio-drv-list= \
> +--block-drv-ro-whitelist=vmdk,vhdx,vpc,https,ssh \
> +--with-coroutine=ucontext \
> +--with-git=git \
> +--tls-priority=@QEMU,SYSTEM \
> +--disable-attr \
> +--disable-auth-pam \
> +--disable-avx2 \
> +--disable-avx512f \
> +--disable-bochs \
> +--disable-brlapi \
> +--disable-bsd-user \
> +--disable-bzip2 \
> +--disable-cap-ng \
> +--disable-capstone \
> +--disable-cfi \
> +--disable-cfi-debug \
> +--disable-cloop \
> +--disable-cocoa \
> +--disable-coroutine-pool \
> +--disable-crypto-afalg \
> +--disable-curl \
> +--disable-curses \
> +--disable-debug-info \
> +--disable-debug-mutex \
> +--disable-debug-tcg \
> +--disable-dmg \
> +--disable-docs \
> +--disable-fdt \
> +--disable-fuse \
> +--disable-fuse-lseek \
> +--disable-gcrypt \
> +--disable-gio \
> +--disable-glusterfs \
> +--disable-gnutls \
> +--disable-gtk \
> +--disable-guest-agent \
> +--disable-guest-agent-msi \
> +--disable-hax \
> +--disable-hvf \
> +--disable-iconv \
> +--disable-jemalloc \
> +--disable-kvm \
> +--disable-libdaxctl \
> +--disable-libiscsi \
> +--disable-libnfs \
> +--disable-libpmem \
> +--disable-libssh \
> +--disable-libudev \
> +--disable-libusb \
> +--disable-libxml2 \
> +--disable-linux-aio \
> +--disable-linux-io-uring \
> +--disable-linux-user \
> +--disable-live-block-migration \
> +--disable-lto \
> +--disable-lzfse \
> +--disable-lzo \
> +--disable-malloc-trim \
> +--disable-membarrier \
> +--disable-modules \
> +--disable-module-upgrades \
> +--disable-mpath \
> +--disable-multiprocess \
> +--disable-netmap \
> +--disable-nettle \
> +--disable-numa \
> +--disable-opengl \
> +--disable-parallels \
> +--disable-pie \
> +--disable-pvrdma \
> +--disable-qcow1 \
> +--disable-qed \
> +--disable-qom-cast-debug \
> +--disable-rbd \
> +--disable-rdma \
> +--disable-replication \
> +--disable-rng-none \
> +--disable-safe-stack \
> +--disable-sanitizers \
> +--disable-sdl \
> +--disable-sdl-image \
> +--disable-seccomp \
> +--disable-smartcard \
> +--disable-snappy \
> +--disable-sparse \
> +--disable-spice \
> +--disable-strip \
> +--disable-system \
> +--disable-tcg \
> +--disable-tcmalloc \
> +--disable-tools \
> +--disable-tpm \
> +--disable-u2f \
> +--disable-usb-redir \
> +--disable-user \
> +--disable-vde \
> +--disable-vdi \
> +--disable-vhost-crypto \
> +--disable-vhost-kernel \
> +--disable-vhost-net \
> +--disable-vhost-scsi \
> +--disable-vhost-user \
> +--disable-vhost-user-blk-server \
> +--disable-vhost-vdpa \
> +--disable-vhost-vsock \
> +--disable-virglrenderer \
> +--disable-virtfs \
> +--disable-virtiofsd \
> +--disable-vnc \
> +--disable-vnc-jpeg \
> +--disable-vnc-png \
> +--disable-vnc-sasl \
> +--disable-vte \
> +--disable-vvfat \
> +--disable-werror \
> +--disable-whpx \
> +--disable-xen \
> +--disable-xen-pci-passthrough \
> +--disable-xfsctl \
> +--disable-xkbcommon \
> +--disable-zstd \
> +--enable-attr \
> +--enable-avx2 \
> +--enable-cap-ng \
> +--enable-capstone \
> +--enable-coroutine-pool \
> +--enable-curl \
> +--enable-debug-info \
> +--enable-docs \
> +--enable-gcrypt \
> +--enable-glusterfs \
> +--enable-gnutls \
> +--enable-guest-agent \
> +--enable-iconv \
> +--enable-kvm \
> +--enable-libiscsi \
> +--enable-libpmem \
> +--enable-libssh \
> +--enable-libusb \
> +--enable-libudev \
> +--enable-linux-aio \
> +--enable-lzo \
> +--enable-malloc-trim \
> +--enable-modules \
> +--enable-mpath \
> +--enable-numa \
> +--enable-opengl \
> +--enable-pie \
> +--enable-rbd \
> +--enable-rdma \
> +--enable-seccomp \
> +--enable-snappy \
> +--enable-smartcard \
> +--enable-spice \
> +--enable-system \
> +--enable-tcg \
> +--enable-tools \
> +--enable-tpm \
> +--enable-trace-backend=dtrace \
> +--enable-usb-redir \
> +--enable-virtiofsd \
> +--enable-vhost-kernel \
> +--enable-vhost-net \
> +--enable-vhost-user \
> +--enable-vhost-user-blk-server \
> +--enable-vhost-vdpa \
> +--enable-vhost-vsock \
> +--enable-vnc \
> +--enable-vnc-png \
> +--enable-vnc-sasl \
> +--enable-werror \
> +--enable-xkbcommon
> diff --git a/scripts/ci/setup/build-environment.yml b/scripts/ci/setup/build-environment.yml
> index 664f2f0519..b1e01b1025 100644
> --- a/scripts/ci/setup/build-environment.yml
> +++ b/scripts/ci/setup/build-environment.yml
> @@ -96,3 +96,41 @@
>        when:
>          - ansible_facts['distribution'] == 'Ubuntu'
>          - ansible_facts['distribution_version'] == '20.04'
> +
> +    - name: Install basic packages to build QEMU on EL8
> +      dnf:
> +        # This list of packages start with tests/docker/dockerfiles/centos8.docker
> +        # but only include files that are common to all distro variants and present
> +        # in the standard repos (no add-ons)
> +        name:
> +          - bzip2
> +          - bzip2-devel
> +          - dbus-daemon
> +          - diffutils
> +          - gcc
> +          - gcc-c++
> +          - genisoimage
> +          - gettext
> +          - git
> +          - glib2-devel
> +          - libaio-devel
> +          - libepoxy-devel
> +          - libgcrypt-devel
> +          - lzo-devel
> +          - make
> +          - mesa-libEGL-devel
> +          - nettle-devel
> +          - nmap-ncat
> +          - perl-Test-Harness
> +          - pixman-devel
> +          - python36
> +          - rdma-core-devel
> +          - spice-glib-devel
> +          - spice-server
> +          - systemtap-sdt-devel
> +          - tar
> +          - zlib-devel
> +        state: present
> +      when:
> +        - ansible_facts['distribution_file_variety'] == 'RedHat'
> +        - ansible_facts['distribution_version'] == '8'
> --
> 2.25.4
>

An apology and a heads up to reviewers: I left the CentOS Stream
specific repo/packages playbook unstaged, and the README should be
more informative about it.  So here it is:

diff --git a/scripts/ci/org.centos/stream/README
b/scripts/ci/org.centos/stream/README
index f99bda99b8..a6f0566145 100644
--- a/scripts/ci/org.centos/stream/README
+++ b/scripts/ci/org.centos/stream/README
@@ -1,2 +1,11 @@
 This directory contains scripts for generating a build of QEMU that
 closely matches the CentOS Stream builds of the qemu-kvm package.
+
+To have your the environment ready to configure, build QEMU and run
+its tests, please:
+
+ * apply the generic "build-environment.yml" playbook located at
+   scripts/ci/setup
+
+ * apply the "build-environment.yml" in this directory, which is
+   specific to CentOS Stream.
diff --git a/scripts/ci/org.centos/stream/build-environment.yml
b/scripts/ci/org.centos/stream/build-environment.yml
new file mode 100644
index 0000000000..832bba8bc6
--- /dev/null
+++ b/scripts/ci/org.centos/stream/build-environment.yml
@@ -0,0 +1,51 @@
+---
+- name: Installation of extra packages to build QEMU
+  hosts: all
+  tasks:
+    - name: Extra check for CentOS Stream 8
+      lineinfile:
+        path: /etc/redhat-release
+        line: CentOS Stream release 8
+        state: present
+      check_mode: yes
+      register: centos_stream_8
+
+    - name: Enable PowerTools repo on CentOS Stream 8
+      ini_file:
+        path: /etc/yum.repos.d/CentOS-Stream-PowerTools.repo
+        section: powertools
+        option: enabled
+        value: "1"
+      when:
+        - ansible_facts['distribution'] == 'CentOS'
+        - ansible_facts['distribution_major_version'] == '8'
+        - centos_stream_8
+
+    - name: Install basic packages to build QEMU on CentOS Stream 8
+      dnf:
+        name:
+          - device-mapper-multipath-devel
+          - glusterfs-api-devel
+          - gnutls-devel
+          - libcap-ng-devel
+          - libcurl-devel
+          - libiscsi-devel
+          - libpmem-devel
+          - librados-devel
+          - librbd-devel
+          - libseccomp-devel
+          - libssh-devel
+          - libxkbcommon-devel
+          - meson
+          - ninja-build
+          - numactl-devel
+          - python3-sphinx
+          - redhat-rpm-config
+          - snappy-devel
+          - spice-server-devel
+          - systemd-devel
+        state: present
+      when:
+        - ansible_facts['distribution'] == 'CentOS'
+        - ansible_facts['distribution_major_version'] == '8'
+        - centos_stream_8

Re: [PATCH 0/4] Jobs based on custom runners: add CentOS Stream 8

[Willian Rampazzo]

On Tue, Jun 8, 2021 at 11:09 AM Cleber Rosa <crosa@redhat.com> wrote:
>
> This builds on top the "GitLab Custom Runners and Jobs (was: QEMU
> Gating CI)" series, showing an example of how other entities can
> add their own custom jobs to the GitLab CI pipeline.
>
> First of all, it may be useful to see an actual pipeline (and the
> reespective job introduced here) combined with the jobs introduced
> on "GitLab Custom Runners and Jobs (was: QEMU Gating CI)":
>
>  * https://gitlab.com/cleber.gnu/qemu/-/pipelines/316527166
>  * https://gitlab.com/cleber.gnu/qemu/-/jobs/1325976765
>
> The runner (the machine and job) is to be managed by Red Hat, and
> adds, at the very least, bare metal x86_64 KVM testing capabilities to
> the QEMU pipeline.  This brings extra coverage for some unittests, and
> the ability to run the acceptance tests dependent on KVM.
>
> The runner is already completely set up and registered to the
> https://gitlab.com/qemu-project/qemu project instance, and jobs will
> be triggered according to the same rules for the jobs introduced on
> "GitLab Custom Runners and Jobs (was: QEMU Gating CI)", that is,
> but pushes to the staging branch.  Still, the job is set with mode
> "allow failures", so it should not disrupt the existing pipeline.
> Once its reliability is proved (rules and service levels are to be
> determined), that can be reverted.
>
> Even though the formal method of tracking machine/job maintainers have
> not been formalized, it should be known that the contacts/admins for
> this machine and job are:
>
>  - Cleber Rosa
>    <crosa@redhat.com>
>    clebergnu on #qemu
>
>  - Willian Rampazzo
>    <willianr@redhat.com>
>    willianr on #qemu

Acked-by: Willian Rampazzo <willianr@redhat.com>

Re: [PATCH 4/4] Jobs based on custom runners: add CentOS Stream 8

[Willian Rampazzo]

On Tue, Jun 8, 2021 at 11:10 AM Cleber Rosa <crosa@redhat.com> wrote:
>
> This introduces three different parts of a job designed to run
> on a custom runner managed by Red Hat.  The goals include:
>
>  a) serve as a model for other organizations that want to onboard
>     their own runners, with their specific platforms, build
>     configuration and tests.
>
>  b) bring awareness to the differences between upstream QEMU and the
>     version available under CentOS Stream, which is "A preview of
>     upcoming Red Hat Enterprise Linux minor and major releases.".
>
>  c) becase of b), it should be easier to identify and reduce the gap
>     between Red Hat's downstream and upstream QEMU.
>
> The components themselves to achieve this custom job are:
>
>  1) build environment configuration: documentation and a playbook for
>     a base Enterprise Linux 8 system (also applicable to CentOS
>     Stream), which other users can run on their system to get the
>     environment suitable for building QEMU.
>
>  2) QEMU build configuration: how QEMU will be built to match, as
>     closely as possible, the binaries built and packaged on CentOS
>     stream 8.
>
>  3) job definition: GitLab CI jobs that will dispatch the build/test
>     job to the machine specifically configured according to #1.
>
> Signed-off-by: Cleber Rosa <crosa@redhat.com>
> ---
>  .gitlab-ci.d/custom-runners.yml        |  29 ++++
>  scripts/ci/org.centos/stream/README    |   2 +
>  scripts/ci/org.centos/stream/configure | 190 +++++++++++++++++++++++++
>  scripts/ci/setup/build-environment.yml |  38 +++++
>  4 files changed, 259 insertions(+)
>  create mode 100644 scripts/ci/org.centos/stream/README
>  create mode 100755 scripts/ci/org.centos/stream/configure
>
> diff --git a/.gitlab-ci.d/custom-runners.yml b/.gitlab-ci.d/custom-runners.yml
> index 061d3cdfed..ee5143995e 100644
> --- a/.gitlab-ci.d/custom-runners.yml
> +++ b/.gitlab-ci.d/custom-runners.yml
> @@ -220,3 +220,32 @@ ubuntu-20.04-aarch64-notcg:
>   - ../configure --disable-libssh --disable-tcg
>   - make --output-sync -j`nproc`
>   - make --output-sync -j`nproc` check V=1
> +
> +centos-stream-8-x86_64:
> + allow_failure: true
> + needs: []
> + stage: build
> + tags:
> + - centos_stream_8
> + - x86_64

What happens if another organization wants to add its own custom
runner with its own set of tests based on centos stream 8? My
suggestion is to add an organization tag to the custom runners. If
this job runs tests important to Red Hat, we should name it and tag
the runner with it.

Unless Red Hat is willing to add other tests that are interesting to
other organizations and run it on its custom runner. If that is the
case, who should check those tests in case of failure?

> + rules:
> + - if: '$CI_COMMIT_BRANCH =~ /^staging/'
> + artifacts:
> +   name: "$CI_JOB_NAME-$CI_COMMIT_REF_SLUG"
> +   when: on_failure
> +   expire_in: 7 days
> +   paths:
> +     - build/tests/results/latest/results.xml
> +     - build/tests/results/latest/test-results
> +   reports:
> +     junit: build/tests/results/latest/results.xml
> + script:
> + - mkdir build
> + - cd build
> + - ../scripts/ci/org.centos/stream/configure
> + - make --output-sync -j`nproc`
> + - make --output-sync -j`nproc` check V=1
> + - make get-vm-images
> + # Only run tests that are either marked explicitly for KVM and x86_64
> + # or tests that are supposed to be valid for all targets
> + - ./tests/venv/bin/avocado run --job-results-dir=tests/results/ --filter-by-tags-include-empty --filter-by-tags-include-empty-key -t accel:kvm,arch:x86_64 -- tests/acceptance/
> diff --git a/scripts/ci/org.centos/stream/README b/scripts/ci/org.centos/stream/README
> new file mode 100644
> index 0000000000..f99bda99b8
> --- /dev/null
> +++ b/scripts/ci/org.centos/stream/README
> @@ -0,0 +1,2 @@
> +This directory contains scripts for generating a build of QEMU that
> +closely matches the CentOS Stream builds of the qemu-kvm package.
> diff --git a/scripts/ci/org.centos/stream/configure b/scripts/ci/org.centos/stream/configure
> new file mode 100755
> index 0000000000..1e7207faec
> --- /dev/null
> +++ b/scripts/ci/org.centos/stream/configure
> @@ -0,0 +1,190 @@
> +#!/bin/sh -e
> +../configure \
> +--prefix="/usr" \
> +--libdir="/usr/lib64" \
> +--datadir="/usr/share" \
> +--sysconfdir="/etc" \
> +--interp-prefix=/usr/qemu-%M \
> +--localstatedir="/var" \
> +--docdir="/usr/share/doc" \
> +--libexecdir="/usr/libexec" \
> +--extra-ldflags="-Wl,--build-id -Wl,-z,relro -Wl,-z,now" \
> +--extra-cflags="-O2 -g -pipe -Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -Wp,-D_GLIBCXX_ASSERTIONS -fexceptions -fstack-protector-strong -grecord-gcc-switches -specs=/usr/lib/rpm/redhat/redhat-hardened-cc1 -specs=/usr/lib/rpm/redhat/redhat-annobin-cc1 -m64 -mtune=generic -fasynchronous-unwind-tables -fstack-clash-protection -fcf-protection" \
> +--with-suffix="qemu-kvm" \
> +--firmwarepath=/usr/share/qemu-firmware \
> +--meson="/usr/bin/meson" \
> +--target-list="x86_64-softmmu" \
> +--block-drv-rw-whitelist=qcow2,raw,file,host_device,nbd,iscsi,rbd,blkdebug,luks,null-co,nvme,copy-on-read,throttle,gluster \
> +--audio-drv-list= \
> +--block-drv-ro-whitelist=vmdk,vhdx,vpc,https,ssh \
> +--with-coroutine=ucontext \
> +--with-git=git \
> +--tls-priority=@QEMU,SYSTEM \
> +--disable-attr \
> +--disable-auth-pam \
> +--disable-avx2 \
> +--disable-avx512f \
> +--disable-bochs \
> +--disable-brlapi \
> +--disable-bsd-user \
> +--disable-bzip2 \
> +--disable-cap-ng \
> +--disable-capstone \
> +--disable-cfi \
> +--disable-cfi-debug \
> +--disable-cloop \
> +--disable-cocoa \
> +--disable-coroutine-pool \
> +--disable-crypto-afalg \
> +--disable-curl \
> +--disable-curses \
> +--disable-debug-info \
> +--disable-debug-mutex \
> +--disable-debug-tcg \
> +--disable-dmg \
> +--disable-docs \
> +--disable-fdt \
> +--disable-fuse \
> +--disable-fuse-lseek \
> +--disable-gcrypt \
> +--disable-gio \
> +--disable-glusterfs \
> +--disable-gnutls \
> +--disable-gtk \
> +--disable-guest-agent \
> +--disable-guest-agent-msi \
> +--disable-hax \
> +--disable-hvf \
> +--disable-iconv \
> +--disable-jemalloc \
> +--disable-kvm \
> +--disable-libdaxctl \
> +--disable-libiscsi \
> +--disable-libnfs \
> +--disable-libpmem \
> +--disable-libssh \
> +--disable-libudev \
> +--disable-libusb \
> +--disable-libxml2 \
> +--disable-linux-aio \
> +--disable-linux-io-uring \
> +--disable-linux-user \
> +--disable-live-block-migration \
> +--disable-lto \
> +--disable-lzfse \
> +--disable-lzo \
> +--disable-malloc-trim \
> +--disable-membarrier \
> +--disable-modules \
> +--disable-module-upgrades \
> +--disable-mpath \
> +--disable-multiprocess \
> +--disable-netmap \
> +--disable-nettle \
> +--disable-numa \
> +--disable-opengl \
> +--disable-parallels \
> +--disable-pie \
> +--disable-pvrdma \
> +--disable-qcow1 \
> +--disable-qed \
> +--disable-qom-cast-debug \
> +--disable-rbd \
> +--disable-rdma \
> +--disable-replication \
> +--disable-rng-none \
> +--disable-safe-stack \
> +--disable-sanitizers \
> +--disable-sdl \
> +--disable-sdl-image \
> +--disable-seccomp \
> +--disable-smartcard \
> +--disable-snappy \
> +--disable-sparse \
> +--disable-spice \
> +--disable-strip \
> +--disable-system \
> +--disable-tcg \
> +--disable-tcmalloc \
> +--disable-tools \
> +--disable-tpm \
> +--disable-u2f \
> +--disable-usb-redir \
> +--disable-user \
> +--disable-vde \
> +--disable-vdi \
> +--disable-vhost-crypto \
> +--disable-vhost-kernel \
> +--disable-vhost-net \
> +--disable-vhost-scsi \
> +--disable-vhost-user \
> +--disable-vhost-user-blk-server \
> +--disable-vhost-vdpa \
> +--disable-vhost-vsock \
> +--disable-virglrenderer \
> +--disable-virtfs \
> +--disable-virtiofsd \
> +--disable-vnc \
> +--disable-vnc-jpeg \
> +--disable-vnc-png \
> +--disable-vnc-sasl \
> +--disable-vte \
> +--disable-vvfat \
> +--disable-werror \
> +--disable-whpx \
> +--disable-xen \
> +--disable-xen-pci-passthrough \
> +--disable-xfsctl \
> +--disable-xkbcommon \
> +--disable-zstd \
> +--enable-attr \
> +--enable-avx2 \
> +--enable-cap-ng \
> +--enable-capstone \
> +--enable-coroutine-pool \
> +--enable-curl \
> +--enable-debug-info \
> +--enable-docs \
> +--enable-gcrypt \
> +--enable-glusterfs \
> +--enable-gnutls \
> +--enable-guest-agent \
> +--enable-iconv \
> +--enable-kvm \
> +--enable-libiscsi \
> +--enable-libpmem \
> +--enable-libssh \
> +--enable-libusb \
> +--enable-libudev \
> +--enable-linux-aio \
> +--enable-lzo \
> +--enable-malloc-trim \
> +--enable-modules \
> +--enable-mpath \
> +--enable-numa \
> +--enable-opengl \
> +--enable-pie \
> +--enable-rbd \
> +--enable-rdma \
> +--enable-seccomp \
> +--enable-snappy \
> +--enable-smartcard \
> +--enable-spice \
> +--enable-system \
> +--enable-tcg \
> +--enable-tools \
> +--enable-tpm \
> +--enable-trace-backend=dtrace \
> +--enable-usb-redir \
> +--enable-virtiofsd \
> +--enable-vhost-kernel \
> +--enable-vhost-net \
> +--enable-vhost-user \
> +--enable-vhost-user-blk-server \
> +--enable-vhost-vdpa \
> +--enable-vhost-vsock \
> +--enable-vnc \
> +--enable-vnc-png \
> +--enable-vnc-sasl \
> +--enable-werror \
> +--enable-xkbcommon
> diff --git a/scripts/ci/setup/build-environment.yml b/scripts/ci/setup/build-environment.yml
> index 664f2f0519..b1e01b1025 100644
> --- a/scripts/ci/setup/build-environment.yml
> +++ b/scripts/ci/setup/build-environment.yml
> @@ -96,3 +96,41 @@
>        when:
>          - ansible_facts['distribution'] == 'Ubuntu'
>          - ansible_facts['distribution_version'] == '20.04'
> +
> +    - name: Install basic packages to build QEMU on EL8
> +      dnf:
> +        # This list of packages start with tests/docker/dockerfiles/centos8.docker
> +        # but only include files that are common to all distro variants and present
> +        # in the standard repos (no add-ons)
> +        name:
> +          - bzip2
> +          - bzip2-devel
> +          - dbus-daemon
> +          - diffutils
> +          - gcc
> +          - gcc-c++
> +          - genisoimage
> +          - gettext
> +          - git
> +          - glib2-devel
> +          - libaio-devel
> +          - libepoxy-devel
> +          - libgcrypt-devel
> +          - lzo-devel
> +          - make
> +          - mesa-libEGL-devel
> +          - nettle-devel
> +          - nmap-ncat
> +          - perl-Test-Harness
> +          - pixman-devel
> +          - python36
> +          - rdma-core-devel
> +          - spice-glib-devel
> +          - spice-server
> +          - systemtap-sdt-devel
> +          - tar
> +          - zlib-devel
> +        state: present
> +      when:
> +        - ansible_facts['distribution_file_variety'] == 'RedHat'
> +        - ansible_facts['distribution_version'] == '8'

I had set up 3 different systems. One with Ubuntu 20.04, one with
Centos Stream 8, and one with Red Hat 8.4. Running the
`build-environment.yml` playbook all steps succeed, except `TASK
[Install basic packages to build QEMU on EL8]`. There seems to be a
problem with the DNF parameter:

TASK [Install basic packages to build QEMU on EL8]
************************************************************************************************
fatal: [192.168.122.41]: FAILED! => {"changed": false, "msg":
"Unsupported parameters for (dnf) module: when Supported parameters
include: allow_downgrade, autoremove, bugfix, conf_file,
disable_excludes, disable_gpg_check, disable_plugin, disablerepo,
download_dir, download_only, enable_plugin, enablerepo, exclude,
install_repoquery, install_weak_deps, installroot, list, lock_timeout,
name, releasever, security, skip_broken, state, update_cache,
update_only, validate_certs"}
fatal: [192.168.122.236]: FAILED! => {"changed": false, "msg":
"Unsupported parameters for (dnf) module: when Supported parameters
include: allow_downgrade, autoremove, bugfix, conf_file,
disable_excludes, disable_gpg_check, disable_plugin, disablerepo,
download_dir, download_only, enable_plugin, enablerepo, exclude,
install_repoquery, install_weak_deps, installroot, list, lock_timeout,
name, releasever, security, skip_broken, state, update_cache,
update_only, validate_certs"}
fatal: [192.168.122.168]: FAILED! => {"changed": false, "msg":
"Unsupported parameters for (dnf) module: when Supported parameters
include: allow_downgrade, autoremove, bugfix, conf_file,
disable_excludes, disable_gpg_check, disable_plugin, disablerepo,
download_dir, download_only, enable_plugin, enablerepo, exclude,
install_repoquery, install_weak_deps, installroot, list, lock_timeout,
name, releasever, security, skip_broken, state, update_cache,
update_only, validate_certs"}

> --
> 2.25.4
>

Re: [PATCH 2/4] Python QEMU utils: introduce a generic feature list

[Willian Rampazzo]

On Tue, Jun 8, 2021 at 8:55 PM Cleber Rosa Junior <crosa@redhat.com> wrote:
>
>
>
> On Tue, Jun 8, 2021 at 5:42 PM Wainer dos Santos Moschetta <wainersm@redhat.com> wrote:
>>
>> Hi,
>>
>> On 6/8/21 11:09 AM, Cleber Rosa wrote:
>> > Which can be used to check for any "feature" that is available as a
>> > QEMU command line option, and that will return its list of available
>> > options.
>> >
>> > This is a generalization of the list_accel() utility function, which
>> > is itself re-implemented in terms of the more generic feature.
>> >
>> > Signed-off-by: Cleber Rosa <crosa@redhat.com>
>> > ---
>> >   python/qemu/utils/__init__.py |  2 ++
>> >   python/qemu/utils/accel.py    | 15 ++----------
>> >   python/qemu/utils/feature.py  | 44 +++++++++++++++++++++++++++++++++++
>> >   3 files changed, 48 insertions(+), 13 deletions(-)
>> >   create mode 100644 python/qemu/utils/feature.py
>> >
>> > diff --git a/python/qemu/utils/__init__.py b/python/qemu/utils/__init__.py
>> > index 7f1a5138c4..1d0789eaa2 100644
>> > --- a/python/qemu/utils/__init__.py
>> > +++ b/python/qemu/utils/__init__.py
>> > @@ -20,12 +20,14 @@
>> >
>> >   # pylint: disable=import-error
>> >   from .accel import kvm_available, list_accel, tcg_available
>> > +from .feature import list_feature
>> >
>> >
>> >   __all__ = (
>> >       'get_info_usernet_hostfwd_port',
>> >       'kvm_available',
>> >       'list_accel',
>> > +    'list_feature',
>> >       'tcg_available',
>> >   )
>> >
>> > diff --git a/python/qemu/utils/accel.py b/python/qemu/utils/accel.py
>> > index 297933df2a..b5bb80c6d3 100644
>> > --- a/python/qemu/utils/accel.py
>> > +++ b/python/qemu/utils/accel.py
>> > @@ -14,13 +14,11 @@
>> >   # the COPYING file in the top-level directory.
>> >   #
>> >
>> > -import logging
>> >   import os
>> > -import subprocess
>> >   from typing import List, Optional
>> >
>> > +from qemu.utils.feature import list_feature
>> >
>> > -LOG = logging.getLogger(__name__)
>> >
>> >   # Mapping host architecture to any additional architectures it can
>> >   # support which often includes its 32 bit cousin.
>> > @@ -39,16 +37,7 @@ def list_accel(qemu_bin: str) -> List[str]:
>> >       @raise Exception: if failed to run `qemu -accel help`
>> >       @return a list of accelerator names.
>> >       """
>> > -    if not qemu_bin:
>> > -        return []
>> > -    try:
>> > -        out = subprocess.check_output([qemu_bin, '-accel', 'help'],
>> > -                                      universal_newlines=True)
>> > -    except:
>> > -        LOG.debug("Failed to get the list of accelerators in %s", qemu_bin)
>> > -        raise
>> > -    # Skip the first line which is the header.
>> > -    return [acc.strip() for acc in out.splitlines()[1:]]
>> > +    return list_feature(qemu_bin, 'accel')
>> >
>> >
>> >   def kvm_available(target_arch: Optional[str] = None,
>> > diff --git a/python/qemu/utils/feature.py b/python/qemu/utils/feature.py
>> > new file mode 100644
>> > index 0000000000..b4a5f929ab
>> > --- /dev/null
>> > +++ b/python/qemu/utils/feature.py
>> > @@ -0,0 +1,44 @@
>> > +"""
>> > +QEMU feature module:
>> > +
>> > +This module provides a utility for discovering the availability of
>> > +generic features.
>> > +"""
>> > +# Copyright (C) 2022 Red Hat Inc.
>> Cleber, please, tell me how is the future like! :)
>
>
> I grabbed a sports almanac.  That's all I can say. :)
>
> Now seriously, thanks for spotting the typo.
>
>>
>> > +#
>> > +# Authors:
>> > +#  Cleber Rosa <crosa@redhat.com>
>> > +#
>> > +# This work is licensed under the terms of the GNU GPL, version 2.  See
>> > +# the COPYING file in the top-level directory.
>> > +#
>> > +
>> > +import logging
>> > +import subprocess
>> > +from typing import List
>> > +
>> > +
>> > +LOG = logging.getLogger(__name__)
>> > +
>> > +
>> > +def list_feature(qemu_bin: str, feature: str) -> List[str]:
>> > +    """
>> > +    List available options the QEMU binary for a given feature type.
>> > +
>> > +    By calling a "qemu $feature -help" and parsing its output.
>>
>> I understand we need a mean to easily cancel the test if given feature
>> is not present. However, I'm unsure this generic list_feature() is what
>> we need.
>>
>> The `-accel help` returns a simple list of strings (besides the header,
>> which is dismissed). Whereas `-machine help` returns what could be
>> parsed as a tuple of (name, description).
>>
>> Another example is `-cpu help` which will print a similar list as
>> `-machine`, plus a section with CPUID flags.
>>
>
> I made sure it worked with both "accel" and "machine", but you're right about many other "-$feature help" that won't conform to the mapping ("-chardev help" is probably the only other one that should work).  What I thought about was to keep the same list_feature(), but make its parsing of items flexible.  Then it could be reused for other list_$feature() like methods.  At the same time, it could be an opportunity to standardize a bit more of the "help" outputs.
>
> For instance, I think it would make sense for "cpu" to keep showing the amount of information it shows, but:
>
> 1) The first item could be the name of the relevant "option" (the cpu model) for that feature (cpu), instead of, say, "x86". Basically reversing the order of first and second, or first and third fields.
> 2) Everything *after* an empty line would be extra information, not necessarily an option available for that feature.
>
> But, this is most probably not going to be achieved in the short term.
>
> What I can do here, is to hide list_feature(), say call it _list_feature() so that we don't duplicate code, and expose a list_machine().

I have reviewed the code and it looks good to me, but +1 for
`list_machine()` and other `list_<specific>` functions. We can handle
different cases on its own function and make it easier to use.

>
> Let me know what you think.
>
> Thanks,
> - Cleber.
>
>>
>> If confess I still don't have a better idea, although I feel it will
>> require a OO design.
>>
>> Thanks!
>>
>> - Wainer
>>
>> > +
>> > +    @param qemu_bin (str): path to the QEMU binary.
>> > +    @param feature (str): feature name, matching the command line option.
>> > +    @raise Exception: if failed to run `qemu -feature help`
>> > +    @return a list of available options for the given feature.
>> > +    """
>> > +    if not qemu_bin:
>> > +        return []
>> > +    try:
>> > +        out = subprocess.check_output([qemu_bin, '-%s' % feature, 'help'],
>> > +                                      universal_newlines=True)
>> > +    except:
>> > +        LOG.debug("Failed to get the list of %s(s) in %s", feature, qemu_bin)
>> > +        raise
>> > +    # Skip the first line which is the header.
>> > +    return [item.split(' ', 1)[0] for item in out.splitlines()[1:]]
>>

Re: [PATCH 3/4] Acceptance Tests: introduce method to require a feature and option

[Willian Rampazzo]

On Tue, Jun 8, 2021 at 11:09 AM Cleber Rosa <crosa@redhat.com> wrote:
>
> In this context, and according to the qemu.utils.list_feature() utility
> function, a feature is something is available as a QEMU command line
> option that can take the "help" value.
>
> This builds on top of that utility function, and allows test writers
> to require, for instance, the "x-remote" (option) machine type
> (feature).
>
> This example is actually applied here to the reespective test, given
> that the option is conditionally built, and the test will ERROR
> without it.
>
> Signed-off-by: Cleber Rosa <crosa@redhat.com>
> ---
>  tests/acceptance/avocado_qemu/__init__.py | 29 ++++++++++++++++++++++-
>  tests/acceptance/multiprocess.py          |  1 +
>  2 files changed, 29 insertions(+), 1 deletion(-)
>
> diff --git a/tests/acceptance/avocado_qemu/__init__.py b/tests/acceptance/avocado_qemu/__init__.py
> index 93c4b9851f..432caff4e6 100644
> --- a/tests/acceptance/avocado_qemu/__init__.py
> +++ b/tests/acceptance/avocado_qemu/__init__.py
> @@ -11,6 +11,7 @@
>  import logging
>  import os
>  import shutil
> +import subprocess
>  import sys
>  import uuid
>  import tempfile
> @@ -45,7 +46,8 @@
>  from qemu.utils import (
>      get_info_usernet_hostfwd_port,
>      kvm_available,
> -    tcg_available,
> +    list_feature,
> +    tcg_available
>  )
>
>  def is_readable_executable_file(path):
> @@ -182,6 +184,31 @@ def _get_unique_tag_val(self, tag_name):
>              return vals.pop()
>          return None
>
> +    def require_feature(self, feature, option=None):
> +        """
> +        Requires a feature to be available for the test to continue
> +
> +        It takes into account the currently set qemu binary, and only checks
> +        for by running a "qemu -$feature help" command.  If the specific option
> +        is given, it checks if it's listed for the given feature.
> +
> +        If the check fails, the test is canceled.
> +
> +        :param feature: name of a QEMU feature, such as "accel" or "machine"
> +        :type feature: str
> +        :param option: the specific value for the feature.  For instance,
> +                       if feature is "machine", option can be "q35".
> +        type option: str
> +        """
> +        try:
> +            options_available = list_feature(self.qemu_bin, feature)

Looking at this code, the previous patch makes more sense the way it is now :)

Maybe, splitting it into multiple `list_` functions will make the code
here more complex.

Anyway, I'll let you decide.

Reviewed-by: Willian Rampazzo <willianr@redhat.com>

Re: [PATCH 2/4] Python QEMU utils: introduce a generic feature list

[Willian Rampazzo]

On Tue, Jun 8, 2021 at 11:09 AM Cleber Rosa <crosa@redhat.com> wrote:
>
> Which can be used to check for any "feature" that is available as a
> QEMU command line option, and that will return its list of available
> options.
>
> This is a generalization of the list_accel() utility function, which
> is itself re-implemented in terms of the more generic feature.
>
> Signed-off-by: Cleber Rosa <crosa@redhat.com>
> ---
>  python/qemu/utils/__init__.py |  2 ++
>  python/qemu/utils/accel.py    | 15 ++----------
>  python/qemu/utils/feature.py  | 44 +++++++++++++++++++++++++++++++++++
>  3 files changed, 48 insertions(+), 13 deletions(-)
>  create mode 100644 python/qemu/utils/feature.py
>

Based on my comments from the next patch of this series:

Reviewed-by: Willian Rampazzo <willianr@redhat.com>

Re: [PATCH 2/4] Python QEMU utils: introduce a generic feature list

[Wainer dos Santos Moschetta]

[-- Attachment #1: Type: text/plain, Size: 7393 bytes --]

Hi,

On 6/8/21 8:55 PM, Cleber Rosa Junior wrote:
>
>
> On Tue, Jun 8, 2021 at 5:42 PM Wainer dos Santos Moschetta 
> <wainersm@redhat.com <mailto:wainersm@redhat.com>> wrote:
>
>     Hi,
>
>     On 6/8/21 11:09 AM, Cleber Rosa wrote:
>     > Which can be used to check for any "feature" that is available as a
>     > QEMU command line option, and that will return its list of available
>     > options.
>     >
>     > This is a generalization of the list_accel() utility function, which
>     > is itself re-implemented in terms of the more generic feature.
>     >
>     > Signed-off-by: Cleber Rosa <crosa@redhat.com
>     <mailto:crosa@redhat.com>>
>     > ---
>     >   python/qemu/utils/__init__.py |  2 ++
>     >   python/qemu/utils/accel.py    | 15 ++----------
>     >   python/qemu/utils/feature.py  | 44
>     +++++++++++++++++++++++++++++++++++
>     >   3 files changed, 48 insertions(+), 13 deletions(-)
>     >   create mode 100644 python/qemu/utils/feature.py
>     >
>     > diff --git a/python/qemu/utils/__init__.py
>     b/python/qemu/utils/__init__.py
>     > index 7f1a5138c4..1d0789eaa2 100644
>     > --- a/python/qemu/utils/__init__.py
>     > +++ b/python/qemu/utils/__init__.py
>     > @@ -20,12 +20,14 @@
>     >
>     >   # pylint: disable=import-error
>     >   from .accel import kvm_available, list_accel, tcg_available
>     > +from .feature import list_feature
>     >
>     >
>     >   __all__ = (
>     >       'get_info_usernet_hostfwd_port',
>     >       'kvm_available',
>     >       'list_accel',
>     > +    'list_feature',
>     >       'tcg_available',
>     >   )
>     >
>     > diff --git a/python/qemu/utils/accel.py b/python/qemu/utils/accel.py
>     > index 297933df2a..b5bb80c6d3 100644
>     > --- a/python/qemu/utils/accel.py
>     > +++ b/python/qemu/utils/accel.py
>     > @@ -14,13 +14,11 @@
>     >   # the COPYING file in the top-level directory.
>     >   #
>     >
>     > -import logging
>     >   import os
>     > -import subprocess
>     >   from typing import List, Optional
>     >
>     > +from qemu.utils.feature import list_feature
>     >
>     > -LOG = logging.getLogger(__name__)
>     >
>     >   # Mapping host architecture to any additional architectures it can
>     >   # support which often includes its 32 bit cousin.
>     > @@ -39,16 +37,7 @@ def list_accel(qemu_bin: str) -> List[str]:
>     >       @raise Exception: if failed to run `qemu -accel help`
>     >       @return a list of accelerator names.
>     >       """
>     > -    if not qemu_bin:
>     > -        return []
>     > -    try:
>     > -        out = subprocess.check_output([qemu_bin, '-accel', 'help'],
>     > - universal_newlines=True)
>     > -    except:
>     > -        LOG.debug("Failed to get the list of accelerators in
>     %s", qemu_bin)
>     > -        raise
>     > -    # Skip the first line which is the header.
>     > -    return [acc.strip() for acc in out.splitlines()[1:]]
>     > +    return list_feature(qemu_bin, 'accel')
>     >
>     >
>     >   def kvm_available(target_arch: Optional[str] = None,
>     > diff --git a/python/qemu/utils/feature.py
>     b/python/qemu/utils/feature.py
>     > new file mode 100644
>     > index 0000000000..b4a5f929ab
>     > --- /dev/null
>     > +++ b/python/qemu/utils/feature.py
>     > @@ -0,0 +1,44 @@
>     > +"""
>     > +QEMU feature module:
>     > +
>     > +This module provides a utility for discovering the availability of
>     > +generic features.
>     > +"""
>     > +# Copyright (C) 2022 Red Hat Inc.
>     Cleber, please, tell me how is the future like! :)
>
>
> I grabbed a sports almanac.  That's all I can say. :)
>
> Now seriously, thanks for spotting the typo.
>
>     > +#
>     > +# Authors:
>     > +#  Cleber Rosa <crosa@redhat.com <mailto:crosa@redhat.com>>
>     > +#
>     > +# This work is licensed under the terms of the GNU GPL, version
>     2.  See
>     > +# the COPYING file in the top-level directory.
>     > +#
>     > +
>     > +import logging
>     > +import subprocess
>     > +from typing import List
>     > +
>     > +
>     > +LOG = logging.getLogger(__name__)
>     > +
>     > +
>     > +def list_feature(qemu_bin: str, feature: str) -> List[str]:
>     > +    """
>     > +    List available options the QEMU binary for a given feature
>     type.
>     > +
>     > +    By calling a "qemu $feature -help" and parsing its output.
>
>     I understand we need a mean to easily cancel the test if given
>     feature
>     is not present. However, I'm unsure this generic list_feature() is
>     what
>     we need.
>
>     The `-accel help` returns a simple list of strings (besides the
>     header,
>     which is dismissed). Whereas `-machine help` returns what could be
>     parsed as a tuple of (name, description).
>
>     Another example is `-cpu help` which will print a similar list as
>     `-machine`, plus a section with CPUID flags.
>
>
> I made sure it worked with both "accel" and "machine", but you're 
> right about many other "-$feature help" that won't conform to the 
> mapping ("-chardev help" is probably the only other one that should 
> work).  What I thought about was to keep the same list_feature(), but 
> make its parsing of items flexible.  Then it could be reused for other 
> list_$feature() like methods.  At the same time, it could be an 
> opportunity to standardize a bit more of the "help" outputs.
>
> For instance, I think it would make sense for "cpu" to keep showing 
> the amount of information it shows, but:
>
> 1) The first item could be the name of the relevant "option" (the cpu 
> model) for that feature (cpu), instead of, say, "x86". Basically 
> reversing the order of first and second, or first and third fields.
> 2) Everything *after* an empty line would be extra information, not 
> necessarily an option available for that feature.
>
> But, this is most probably not going to be achieved in the short term.
>
> What I can do here, is to hide list_feature(), say call it 
> _list_feature() so that we don't duplicate code, and expose a 
> list_machine().

I prefer that implementation, with list_machine(), list_accel()...etc. 
Allow me another suggestion: maybe rename list_feature() to 
feature_help() (or something similar).

- Wainer

>
> Let me know what you think.
>
> Thanks,
> - Cleber.
>
>     If confess I still don't have a better idea, although I feel it will
>     require a OO design.
>
>     Thanks!
>
>     - Wainer
>
>     > +
>     > +    @param qemu_bin (str): path to the QEMU binary.
>     > +    @param feature (str): feature name, matching the command
>     line option.
>     > +    @raise Exception: if failed to run `qemu -feature help`
>     > +    @return a list of available options for the given feature.
>     > +    """
>     > +    if not qemu_bin:
>     > +        return []
>     > +    try:
>     > +        out = subprocess.check_output([qemu_bin, '-%s' %
>     feature, 'help'],
>     > + universal_newlines=True)
>     > +    except:
>     > +        LOG.debug("Failed to get the list of %s(s) in %s",
>     feature, qemu_bin)
>     > +        raise
>     > +    # Skip the first line which is the header.
>     > +    return [item.split(' ', 1)[0] for item in out.splitlines()[1:]]
>

[-- Attachment #2: Type: text/html, Size: 12089 bytes --]

Re: [PATCH 2/4] Python QEMU utils: introduce a generic feature list

[John Snow]

On 6/8/21 10:09 AM, Cleber Rosa wrote:
> Which can be used to check for any "feature" that is available as a
> QEMU command line option, and that will return its list of available
> options.
> 
> This is a generalization of the list_accel() utility function, which
> is itself re-implemented in terms of the more generic feature.
> 
> Signed-off-by: Cleber Rosa <crosa@redhat.com>
> ---
>   python/qemu/utils/__init__.py |  2 ++
>   python/qemu/utils/accel.py    | 15 ++----------
>   python/qemu/utils/feature.py  | 44 +++++++++++++++++++++++++++++++++++
>   3 files changed, 48 insertions(+), 13 deletions(-)
>   create mode 100644 python/qemu/utils/feature.py
> 
> diff --git a/python/qemu/utils/__init__.py b/python/qemu/utils/__init__.py
> index 7f1a5138c4..1d0789eaa2 100644
> --- a/python/qemu/utils/__init__.py
> +++ b/python/qemu/utils/__init__.py
> @@ -20,12 +20,14 @@
>   
>   # pylint: disable=import-error
>   from .accel import kvm_available, list_accel, tcg_available
> +from .feature import list_feature
>   
>   
>   __all__ = (
>       'get_info_usernet_hostfwd_port',
>       'kvm_available',
>       'list_accel',
> +    'list_feature',
>       'tcg_available',
>   )
>   
> diff --git a/python/qemu/utils/accel.py b/python/qemu/utils/accel.py
> index 297933df2a..b5bb80c6d3 100644
> --- a/python/qemu/utils/accel.py
> +++ b/python/qemu/utils/accel.py
> @@ -14,13 +14,11 @@
>   # the COPYING file in the top-level directory.
>   #
>   
> -import logging
>   import os
> -import subprocess
>   from typing import List, Optional
>   
> +from qemu.utils.feature import list_feature
>   
> -LOG = logging.getLogger(__name__)
>   
>   # Mapping host architecture to any additional architectures it can
>   # support which often includes its 32 bit cousin.
> @@ -39,16 +37,7 @@ def list_accel(qemu_bin: str) -> List[str]:
>       @raise Exception: if failed to run `qemu -accel help`
>       @return a list of accelerator names.
>       """
> -    if not qemu_bin:
> -        return []
> -    try:
> -        out = subprocess.check_output([qemu_bin, '-accel', 'help'],
> -                                      universal_newlines=True)
> -    except:
> -        LOG.debug("Failed to get the list of accelerators in %s", qemu_bin)
> -        raise
> -    # Skip the first line which is the header.
> -    return [acc.strip() for acc in out.splitlines()[1:]]
> +    return list_feature(qemu_bin, 'accel')
>   
>   
>   def kvm_available(target_arch: Optional[str] = None,
> diff --git a/python/qemu/utils/feature.py b/python/qemu/utils/feature.py
> new file mode 100644
> index 0000000000..b4a5f929ab
> --- /dev/null
> +++ b/python/qemu/utils/feature.py
> @@ -0,0 +1,44 @@
> +"""
> +QEMU feature module:
> +
> +This module provides a utility for discovering the availability of
> +generic features.
> +"""
> +# Copyright (C) 2022 Red Hat Inc.
> +#
> +# Authors:
> +#  Cleber Rosa <crosa@redhat.com>
> +#
> +# This work is licensed under the terms of the GNU GPL, version 2.  See
> +# the COPYING file in the top-level directory.
> +#
> +
> +import logging
> +import subprocess
> +from typing import List
> +
> +
> +LOG = logging.getLogger(__name__)
> +
> +
> +def list_feature(qemu_bin: str, feature: str) -> List[str]:
> +    """
> +    List available options the QEMU binary for a given feature type.
> +
> +    By calling a "qemu $feature -help" and parsing its output.
> +
> +    @param qemu_bin (str): path to the QEMU binary.
> +    @param feature (str): feature name, matching the command line option.
> +    @raise Exception: if failed to run `qemu -feature help`
> +    @return a list of available options for the given feature.
> +    """
> +    if not qemu_bin:
> +        return []
> +    try:
> +        out = subprocess.check_output([qemu_bin, '-%s' % feature, 'help'],
> +                                      universal_newlines=True)
> +    except:
> +        LOG.debug("Failed to get the list of %s(s) in %s", feature, qemu_bin)
> +        raise
> +    # Skip the first line which is the header.
> +    return [item.split(' ', 1)[0] for item in out.splitlines()[1:]]
> 

It's messy stuff, but all of machine.py is pretty messy stuff right now. 
No real qualms with more messy stuff going into qemu.utils for the time 
being.

Eventually, we will want to come up with a more universal way to 
interrogate features present in QEMU binaries. Using introspection data 
or QMP queries would be my preferred (and ideally SOLE) way to detect 
QEMU features.

But that's something to worry about later, I suppose.

As long as it passes the CI and doesn't break any tests, I'll toss you 
my ACK here and trust your judgment:

Acked-by: John Snow <jsnow@redhat.com>

--js