[PATCH 00/11] more misc fixes for 8.0 (tests, gdbstub, meta, docs)

[PATCH 00/11] more misc fixes for 8.0 (tests, gdbstub, meta, docs)

[Alex Bennée]

Here are a few more random fixes across the tree. All should be good
for the current phase of freeze although I can drop stuff if it causes
problems.

I've included another run at using the kaniko build tool and will see
if it improves the caching of things as it runs through my CI loop.

Alex.

Alex Bennée (6):
  scripts/coverage: initial coverage comparison script
  MAINTAINERS: add a section for policy documents
  qemu-options: finesse the recommendations around -blockdev
  metadata: add .git-blame-ignore-revs
  gitlab: fix typo
  tests/gitlab: use kaniko to build images

Daniel P. Berrangé (2):
  tests/qemu-iotests: explicitly invoke 'check' via 'python'
  tests/vm: use the default system python for NetBSD

Kautuk Consul (1):
  tests/requirements.txt: bump up avocado-framework version to 101.0

Marco Liebel (1):
  Use hexagon toolchain version 16.0.0

Philippe Mathieu-Daudé (1):
  gdbstub: Only build libgdb_user.fa / libgdb_softmmu.fa if necessary

 MAINTAINERS                                   |  18 +++
 .git-blame-ignore-revs                        |  21 ++++
 .gitlab-ci.d/base.yml                         |   2 +-
 .gitlab-ci.d/container-template.yml           |  22 ++--
 gdbstub/meson.build                           |   6 +-
 qemu-options.hx                               |   8 +-
 scripts/coverage/compare_gcov_json.py         | 118 ++++++++++++++++++
 .../dockerfiles/debian-hexagon-cross.docker   |   2 +-
 tests/qemu-iotests/meson.build                |   7 +-
 tests/requirements.txt                        |   2 +-
 tests/vm/netbsd                               |   3 +-
 11 files changed, 185 insertions(+), 24 deletions(-)
 create mode 100644 .git-blame-ignore-revs
 create mode 100755 scripts/coverage/compare_gcov_json.py

-- 
2.39.2

[PATCH 01/11] scripts/coverage: initial coverage comparison script

[Alex Bennée]

This is a very rough and ready first pass at comparing gcovr's json
output between two different runs. At the moment it will give you a
file level diff between two runs but hopefully it wont be too hard to
extend to give better insight.

After generating the coverage results you run with something like:

  ./scripts/coverage/compare_gcov_json.py \
    -a ./builds/gcov.config1/coverage.json \
    -b ./builds/gcov.config2/coverage.json

My hope is we can use this to remove some redundancy from testing as
well as evaluate if new tests are actually providing additional
coverage or just burning our precious CI time.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Cc: Kautuk Consul <kconsul@linux.vnet.ibm.com>

---
v2
  - add MAINTAINERS entry
---
 MAINTAINERS                           |   5 ++
 scripts/coverage/compare_gcov_json.py | 118 ++++++++++++++++++++++++++
 2 files changed, 123 insertions(+)
 create mode 100755 scripts/coverage/compare_gcov_json.py

diff --git a/MAINTAINERS b/MAINTAINERS
index ef45b5e71e..9e1a60ea24 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -3908,3 +3908,8 @@ Performance Tools and Tests
 M: Ahmed Karaman <ahmedkhaledkaraman@gmail.com>
 S: Maintained
 F: scripts/performance/
+
+Code Coverage Tools
+M: Alex Bennée <alex.bennee@linaro.org>
+S: Odd Fixes
+F: scripts/coverage/
diff --git a/scripts/coverage/compare_gcov_json.py b/scripts/coverage/compare_gcov_json.py
new file mode 100755
index 0000000000..451e2f8c00
--- /dev/null
+++ b/scripts/coverage/compare_gcov_json.py
@@ -0,0 +1,118 @@
+#!/usr/bin/env python3
+#
+# Compare output of two gcovr JSON reports and report differences. To
+# generate the required output first:
+#   - create two build dirs with --enable-gcov
+#   - run set of tests in each
+#   - run make coverage-html in each
+#   - run gcovr --json --exclude-unreachable-branches --print-summary -o coverage.json --root ../../ . *.p
+#
+# Author: Alex Bennée <alex.bennee@linaro.org>
+#
+# SPDX-License-Identifier: GPL-2.0-or-later
+#
+
+import argparse
+import json
+import sys
+from pathlib import Path
+
+def create_parser():
+    parser = argparse.ArgumentParser(
+        prog='compare_gcov_json',
+        description='analyse the differences in coverage between two runs')
+
+    parser.add_argument('-a', type=Path, default=None,
+                        help=('First file to check'))
+
+    parser.add_argument('-b', type=Path, default=None,
+                        help=('Second file to check'))
+
+    parser.add_argument('--verbose', action='store_true', default=False,
+                        help=('A minimal verbosity level that prints the '
+                              'overall result of the check/wait'))
+    return parser
+
+
+# See https://gcovr.com/en/stable/output/json.html#json-format-reference
+def load_json(json_file_path: Path, verbose = False) -> dict[str, set[int]]:
+
+    with open(json_file_path) as f:
+        data = json.load(f)
+
+    root_dir = json_file_path.absolute().parent
+    covered_lines = dict()
+
+    for filecov in data["files"]:
+        file_path = Path(filecov["file"])
+
+        # account for generated files - map into src tree
+        resolved_path = Path(file_path).absolute()
+        if resolved_path.is_relative_to(root_dir):
+            file_path = resolved_path.relative_to(root_dir)
+            # print(f"remapped {resolved_path} to {file_path}")
+
+        lines = filecov["lines"]
+
+        executed_lines = set(
+            linecov["line_number"]
+            for linecov in filecov["lines"]
+            if linecov["count"] != 0 and not linecov["gcovr/noncode"]
+        )
+
+        # if this file has any coverage add it to the system
+        if len(executed_lines) > 0:
+            if verbose:
+                print(f"file {file_path} {len(executed_lines)}/{len(lines)}")
+            covered_lines[str(file_path)] = executed_lines
+
+    return covered_lines
+
+def find_missing_files(first, second):
+    """
+    Return a list of files not covered in the second set
+    """
+    missing_files = []
+    for f in sorted(first):
+        file_a = first[f]
+        try:
+            file_b = second[f]
+        except KeyError:
+            missing_files.append(f)
+
+    return missing_files
+
+def main():
+    """
+    Script entry point
+    """
+    parser = create_parser()
+    args = parser.parse_args()
+
+    if not args.a or not args.b:
+        print("We need two files to compare")
+        sys.exit(1)
+
+    first_coverage = load_json(args.a, args.verbose)
+    second_coverage = load_json(args.b, args.verbose)
+
+    first_missing = find_missing_files(first_coverage,
+                                       second_coverage)
+
+    second_missing = find_missing_files(second_coverage,
+                                        first_coverage)
+
+    a_name = args.a.parent.name
+    b_name = args.b.parent.name
+
+    print(f"{b_name} missing coverage in {len(first_missing)} files")
+    for f in first_missing:
+        print(f"  {f}")
+
+    print(f"{a_name} missing coverage in {len(second_missing)} files")
+    for f in second_missing:
+        print(f"  {f}")
+
+
+if __name__ == '__main__':
+    main()
-- 
2.39.2

[PATCH 02/11] gdbstub: Only build libgdb_user.fa / libgdb_softmmu.fa if necessary

[Alex Bennée]

From: Philippe Mathieu-Daudé <philmd@linaro.org>

It is pointless to build libgdb_user.fa in a system-only build
(or libgdb_softmmu.fa in a user-only build). Besides, in some
restricted build configurations, some APIs might be restricted /
not available. Example in a KVM-only builds where TCG is disabled:

  $ ninja qemu-system-x86_64
  [99/2187] Compiling C object gdbstub/libgdb_user.fa.p/user.c.o
  FAILED: gdbstub/libgdb_user.fa.p/user.c.o
  ../../gdbstub/user.c: In function ‘gdb_breakpoint_insert’:
  ../../gdbstub/user.c:438:19: error: implicit declaration of function ‘cpu_breakpoint_insert’; did you mean ‘gdb_breakpoint_insert’? [-Werror=implicit-function-declaration]
    438 |             err = cpu_breakpoint_insert(cpu, addr, BP_GDB, NULL);
        |                   ^~~~~~~~~~~~~~~~~~~~~
        |                   gdb_breakpoint_insert
  ../../gdbstub/user.c:438:19: error: nested extern declaration of ‘cpu_breakpoint_insert’ [-Werror=nested-externs]
  ../../gdbstub/user.c: In function ‘gdb_breakpoint_remove’:
  ../../gdbstub/user.c:459:19: error: implicit declaration of function ‘cpu_breakpoint_remove’; did you mean ‘gdb_breakpoint_remove’? [-Werror=implicit-function-declaration]
    459 |             err = cpu_breakpoint_remove(cpu, addr, BP_GDB);
        |                   ^~~~~~~~~~~~~~~~~~~~~
        |                   gdb_breakpoint_remove
  ../../gdbstub/user.c:459:19: error: nested extern declaration of ‘cpu_breakpoint_remove’ [-Werror=nested-externs]
  cc1: all warnings being treated as errors
  ninja: build stopped: subcommand failed.

Fixes: 61b2e136db ("gdbstub: only compile gdbstub twice for whole build")
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-Id: <20230329161852.84992-1-philmd@linaro.org>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 gdbstub/meson.build | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/gdbstub/meson.build b/gdbstub/meson.build
index bd5c5cd67d..cdb4d28691 100644
--- a/gdbstub/meson.build
+++ b/gdbstub/meson.build
@@ -20,11 +20,13 @@ gdb_softmmu_ss = gdb_softmmu_ss.apply(config_host, strict: false)
 libgdb_user = static_library('gdb_user',
                              gdb_user_ss.sources() + genh,
                              name_suffix: 'fa',
-                             c_args: '-DCONFIG_USER_ONLY')
+                             c_args: '-DCONFIG_USER_ONLY',
+                             build_by_default: have_user)
 
 libgdb_softmmu = static_library('gdb_softmmu',
                                 gdb_softmmu_ss.sources() + genh,
-                                name_suffix: 'fa')
+                                name_suffix: 'fa',
+                                build_by_default: have_system)
 
 gdb_user = declare_dependency(link_whole: libgdb_user)
 user_ss.add(gdb_user)
-- 
2.39.2

[PATCH 03/11] MAINTAINERS: add a section for policy documents

[Alex Bennée]

We don't update these often but if you are the sort of person who
enjoys debating and tuning project policies you could now add yourself
as a reviewer here so you don't miss the next debate over tabs vs
spaces ;-)

Who's with me?

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Cc: Thomas Huth <thuth@redhat.com>
Cc: Daniel P. Berrangé <berrange@redhat.com>
Cc: Markus Armbruster <armbru@redhat.com>
Cc: Kashyap Chamarthy <kchamart@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Peter Maydell <peter.maydell@linaro.org>
Cc: Philippe Mathieu-Daudé <philmd@linaro.org>
Cc: Bernhard Beschow <shentey@gmail.com>

---
v2
  - s/your/you are/
  - add some willing victims
---
 MAINTAINERS | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 9e1a60ea24..2c173dbd96 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -64,6 +64,19 @@ L: qemu-devel@nongnu.org
 F: *
 F: */
 
+Project policy and developer guides
+R: Alex Bennée <alex.bennee@linaro.org>
+R: Daniel P. Berrangé <berrange@redhat.com>
+R: Thomas Huth <thuth@redhat.com>
+R: Markus Armbruster <armbru@redhat.com>
+W: https://www.qemu.org/docs/master/devel/index.html
+S: Odd Fixes
+F: docs/devel/style.rst
+F: docs/devel/code-of-conduct.rst
+F: docs/devel/conflict-resolution.rst
+F: docs/devel/submitting-a-patch.rst
+F: docs/devel/submitting-a-pull-request.rst
+
 Responsible Disclosure, Reporting Security Issues
 -------------------------------------------------
 W: https://wiki.qemu.org/SecurityProcess
-- 
2.39.2

[PATCH 04/11] qemu-options: finesse the recommendations around -blockdev

[Alex Bennée]

We are a bit premature in recommending -blockdev/-device as the best
way to configure block devices, especially in the common case.
Improve the language to hopefully make things clearer.

Suggested-by: Michael Tokarev <mjt@tls.msk.ru>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 qemu-options.hx | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/qemu-options.hx b/qemu-options.hx
index 59bdf67a2c..9a69ed838e 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -1143,10 +1143,14 @@ have gone through several iterations as the feature set and complexity
 of the block layer have grown. Many online guides to QEMU often
 reference older and deprecated options, which can lead to confusion.
 
-The recommended modern way to describe disks is to use a combination of
+The most explicit way to describe disks is to use a combination of
 ``-device`` to specify the hardware device and ``-blockdev`` to
 describe the backend. The device defines what the guest sees and the
-backend describes how QEMU handles the data.
+backend describes how QEMU handles the data. The ``-drive`` option
+combines the device and backend into a single command line options
+which is useful in the majority of cases. Older options like ``-hda``
+bake in a lot of assumptions from the days when QEMU was emulating a
+legacy PC, they are not recommended for modern configurations.
 
 ERST
 
-- 
2.39.2

[PATCH 05/11] metadata: add .git-blame-ignore-revs

[Alex Bennée]

Someone mentioned this on IRC so I thought I would try it out with a
few commits that are pure code style fixes.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Message-Id: <20230318115657.1345921-1-alex.bennee@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Tested-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 .git-blame-ignore-revs | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)
 create mode 100644 .git-blame-ignore-revs

diff --git a/.git-blame-ignore-revs b/.git-blame-ignore-revs
new file mode 100644
index 0000000000..fa01e77a1e
--- /dev/null
+++ b/.git-blame-ignore-revs
@@ -0,0 +1,21 @@
+#
+# List of code-formatting clean ups the git blame can ignore
+#
+#   git blame --ignore-revs-file .git-blame-ignore-revs
+#
+# or
+#
+#   git config blame.ignoreRevsFile .git-blame-ignore-revs
+#
+
+# gdbstub: clean-up indents
+ad9e4585b3c7425759d3eea697afbca71d2c2082
+
+# e1000e: fix code style
+0eadd56bf53ab196a16d492d7dd31c62e1c24c32
+
+# target/riscv: coding style fixes
+8c7feddddd9218b407792120bcfda0347ed16205
+
+# replace TABs with spaces
++48805df9c22a0700fba4b3b548fafaa21726ca68
-- 
2.39.2

[PATCH 06/11] Use hexagon toolchain version 16.0.0

[Alex Bennée]

From: Marco Liebel <quic_mliebel@quicinc.com>

Signed-off-by: Marco Liebel <quic_mliebel@quicinc.com>
Reviewed-by: Brian Cain <bcain@quicinc.com>
Message-Id: <20230329142108.1199509-1-quic_mliebel@quicinc.com>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 tests/docker/dockerfiles/debian-hexagon-cross.docker | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/docker/dockerfiles/debian-hexagon-cross.docker b/tests/docker/dockerfiles/debian-hexagon-cross.docker
index 5308ccb8fe..b99d99f943 100644
--- a/tests/docker/dockerfiles/debian-hexagon-cross.docker
+++ b/tests/docker/dockerfiles/debian-hexagon-cross.docker
@@ -27,7 +27,7 @@ RUN apt-get update && \
 
 
 ENV TOOLCHAIN_INSTALL /opt
-ENV TOOLCHAIN_RELEASE 15.0.3
+ENV TOOLCHAIN_RELEASE 16.0.0
 ENV TOOLCHAIN_BASENAME "clang+llvm-${TOOLCHAIN_RELEASE}-cross-hexagon-unknown-linux-musl"
 ENV TOOLCHAIN_URL https://codelinaro.jfrog.io/artifactory/codelinaro-toolchain-for-hexagon/v${TOOLCHAIN_RELEASE}/${TOOLCHAIN_BASENAME}.tar.xz
 
-- 
2.39.2

[PATCH 07/11] tests/qemu-iotests: explicitly invoke 'check' via 'python'

[Alex Bennée]

From: Daniel P. Berrangé <berrange@redhat.com>

The 'check' script will use "#!/usr/bin/env python3" by default
to locate python, but this doesn't work in distros which lack a
bare 'python3' binary like NetBSD.

We need to explicitly invoke 'check' by referring to the 'python'
variable in meson, which resolves to the detected python binary
that QEMU intends to use.

This fixes a regression introduced by

  commit 51ab5f8bd795d8980351f8531e54995ff9e6d163
  Author: Daniel P. Berrangé <berrange@redhat.com>
  Date:   Wed Mar 15 17:43:23 2023 +0000

    iotests: register each I/O test separately with meson

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Message-Id: <20230329124539.822022-1-berrange@redhat.com>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 tests/qemu-iotests/meson.build | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/tests/qemu-iotests/meson.build b/tests/qemu-iotests/meson.build
index a162f683ef..9735071a29 100644
--- a/tests/qemu-iotests/meson.build
+++ b/tests/qemu-iotests/meson.build
@@ -47,19 +47,20 @@ foreach format, speed: qemu_iotests_formats
   endif
 
   rc = run_command(
-      [qemu_iotests_check_cmd] + args + ['-n'],
+      [python, qemu_iotests_check_cmd] + args + ['-n'],
       check: true,
   )
 
   foreach item: rc.stdout().strip().split()
-      args = ['-tap', '-' + format, item,
+      args = [qemu_iotests_check_cmd,
+              '-tap', '-' + format, item,
               '--source-dir', meson.current_source_dir(),
               '--build-dir', meson.current_build_dir()]
       # Some individual tests take as long as 45 seconds
       # Bump the timeout to 3 minutes for some headroom
       # on slow machines to minimize spurious failures
       test('io-' + format + '-' + item,
-           qemu_iotests_check_cmd,
+           python,
            args: args,
            depends: qemu_iotests_binaries,
            env: qemu_iotests_env,
-- 
2.39.2

[PATCH 08/11] tests/vm: use the default system python for NetBSD

[Alex Bennée]

From: Daniel P. Berrangé <berrange@redhat.com>

Currently our NetBSD VM recipe requests instal of the python37 package
and explicitly tells QEMU to use that version of python. Since the
NetBSD base ISO was updated to version 9.3 though, the default system
python version is 3.9 which is sufficiently new for QEMU to rely on.
Rather than requesting an older python, just test against the default
system python which is what most users will have.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Message-Id: <20230329124601.822209-1-berrange@redhat.com>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 tests/vm/netbsd | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/tests/vm/netbsd b/tests/vm/netbsd
index aa54338dfa..0b9536ca17 100755
--- a/tests/vm/netbsd
+++ b/tests/vm/netbsd
@@ -30,7 +30,6 @@ class NetBSDVM(basevm.BaseVM):
         "git-base",
         "pkgconf",
         "xz",
-        "python37",
         "ninja-build",
 
         # gnu tools
@@ -66,7 +65,7 @@ class NetBSDVM(basevm.BaseVM):
         mkdir src build; cd src;
         tar -xf /dev/rld1a;
         cd ../build
-        ../src/configure --python=python3.7 --disable-opengl {configure_opts};
+        ../src/configure --disable-opengl {configure_opts};
         gmake --output-sync -j{jobs} {target} {verbose};
     """
     poweroff = "/sbin/poweroff"
-- 
2.39.2

[PATCH 09/11] tests/requirements.txt: bump up avocado-framework version to 101.0

[Alex Bennée]

From: Kautuk Consul <kconsul@linux.vnet.ibm.com>

Avocado version 101.0 has a fix to re-compute the checksum
of an asset file if the algorithm used in the *-CHECKSUM
file isn't the same as the one being passed to it by the
avocado user (i.e. the avocado_qemu python module).
In the earlier avocado versions this fix wasn't there due
to which if the checksum wouldn't match the earlier
checksum (calculated by a different algorithm), the avocado
code would start downloading a fresh image from the internet
URL thus making the test-cases take longer to execute.

Bump up the avocado-framework version to 101.0.

Signed-off-by: Kautuk Consul <kconsul@linux.vnet.ibm.com>
Tested-by: Hariharan T S <hariharan.ts@linux.vnet.ibm.com>
Message-Id: <20230327115030.3418323-2-kconsul@linux.vnet.ibm.com>
---
 tests/requirements.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/requirements.txt b/tests/requirements.txt
index 0ba561b6bd..a6f73da681 100644
--- a/tests/requirements.txt
+++ b/tests/requirements.txt
@@ -2,5 +2,5 @@
 # in the tests/venv Python virtual environment. For more info,
 # refer to: https://pip.pypa.io/en/stable/user_guide/#id1
 # Note that qemu.git/python/ is always implicitly installed.
-avocado-framework==88.1
+avocado-framework==101.0
 pycdlib==1.11.0
-- 
2.39.2

[PATCH 10/11] gitlab: fix typo

[Alex Bennée]

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 .gitlab-ci.d/base.yml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/.gitlab-ci.d/base.yml b/.gitlab-ci.d/base.yml
index 0274228de8..2fbb58d2a3 100644
--- a/.gitlab-ci.d/base.yml
+++ b/.gitlab-ci.d/base.yml
@@ -75,5 +75,5 @@
     - if: '$QEMU_CI != "2" && $CI_PROJECT_NAMESPACE != "qemu-project"'
       when: manual
 
-    # Jobs can run if any jobs they depend on were successfull
+    # Jobs can run if any jobs they depend on were successful
     - when: on_success
-- 
2.39.2

[PATCH 11/11] tests/gitlab: use kaniko to build images

[Alex Bennée]

Apparently the docker-in-docker approach has some flaws including
needing privileged mode to run and being quite slow. An alternative
approach is to use Google's kaniko tool. It also works across
different gitlab executors.

Following the gitlab example code we drop all the direct docker calls
and usage of the script and make a direct call to kaniko and hope the
images are cacheable by others.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Message-Id: <20230224180857.1050220-8-alex.bennee@linaro.org>

---
v2
  - add danpb's --cache suggestions
---
 .gitlab-ci.d/container-template.yml | 22 ++++++++++------------
 1 file changed, 10 insertions(+), 12 deletions(-)

diff --git a/.gitlab-ci.d/container-template.yml b/.gitlab-ci.d/container-template.yml
index 519b8a9482..cd8e0a1ff6 100644
--- a/.gitlab-ci.d/container-template.yml
+++ b/.gitlab-ci.d/container-template.yml
@@ -1,21 +1,19 @@
 .container_job_template:
   extends: .base_job_template
-  image: docker:stable
+  image:
+    name: gcr.io/kaniko-project/executor:v1.9.0-debug
+    entrypoint: [""]
   stage: containers
-  services:
-    - docker:dind
   before_script:
     - export TAG="$CI_REGISTRY_IMAGE/qemu/$NAME:latest"
     - export COMMON_TAG="$CI_REGISTRY/qemu-project/qemu/qemu/$NAME:latest"
-    - apk add python3
-    - docker info
-    - docker login $CI_REGISTRY -u "$CI_REGISTRY_USER" -p "$CI_REGISTRY_PASSWORD"
   script:
     - echo "TAG:$TAG"
     - echo "COMMON_TAG:$COMMON_TAG"
-    - docker build --tag "$TAG" --cache-from "$TAG" --cache-from "$COMMON_TAG"
-      --build-arg BUILDKIT_INLINE_CACHE=1
-      -f "tests/docker/dockerfiles/$NAME.docker" "."
-    - docker push "$TAG"
-  after_script:
-    - docker logout
+    - /kaniko/executor
+          --reproducible
+          --context "${CI_PROJECT_DIR}"
+          --cache=true
+          --cache-repo "${COMMON_TAG}"
+          --dockerfile "${CI_PROJECT_DIR}/tests/docker/dockerfiles/$NAME.docker"
+          --destination "${TAG}"
-- 
2.39.2

Re: [PATCH 11/11] tests/gitlab: use kaniko to build images

[Daniel P. Berrangé]

On Thu, Mar 30, 2023 at 11:11:41AM +0100, Alex Bennée wrote:
> Apparently the docker-in-docker approach has some flaws including
> needing privileged mode to run and being quite slow. An alternative
> approach is to use Google's kaniko tool. It also works across
> different gitlab executors.
> 
> Following the gitlab example code we drop all the direct docker calls
> and usage of the script and make a direct call to kaniko and hope the
> images are cacheable by others.
> 
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> Message-Id: <20230224180857.1050220-8-alex.bennee@linaro.org>
> 
> ---
> v2
>   - add danpb's --cache suggestions
> ---
>  .gitlab-ci.d/container-template.yml | 22 ++++++++++------------
>  1 file changed, 10 insertions(+), 12 deletions(-)
> 
> diff --git a/.gitlab-ci.d/container-template.yml b/.gitlab-ci.d/container-template.yml
> index 519b8a9482..cd8e0a1ff6 100644
> --- a/.gitlab-ci.d/container-template.yml
> +++ b/.gitlab-ci.d/container-template.yml
> @@ -1,21 +1,19 @@
>  .container_job_template:
>    extends: .base_job_template
> -  image: docker:stable
> +  image:
> +    name: gcr.io/kaniko-project/executor:v1.9.0-debug
> +    entrypoint: [""]
>    stage: containers
> -  services:
> -    - docker:dind
>    before_script:
>      - export TAG="$CI_REGISTRY_IMAGE/qemu/$NAME:latest"
>      - export COMMON_TAG="$CI_REGISTRY/qemu-project/qemu/qemu/$NAME:latest"
> -    - apk add python3
> -    - docker info
> -    - docker login $CI_REGISTRY -u "$CI_REGISTRY_USER" -p "$CI_REGISTRY_PASSWORD"
>    script:
>      - echo "TAG:$TAG"
>      - echo "COMMON_TAG:$COMMON_TAG"
> -    - docker build --tag "$TAG" --cache-from "$TAG" --cache-from "$COMMON_TAG"
> -      --build-arg BUILDKIT_INLINE_CACHE=1
> -      -f "tests/docker/dockerfiles/$NAME.docker" "."
> -    - docker push "$TAG"
> -  after_script:
> -    - docker logout
> +    - /kaniko/executor
> +          --reproducible
> +          --context "${CI_PROJECT_DIR}"
> +          --cache=true
> +          --cache-repo "${COMMON_TAG}"

IIRC with docker if we told it to cache we would have to first have done
a  'docker pull $COMMON_TAG' as it wouldn't pull down the image if
it was not already local. I'm fuzzy on whether kaniko has the same
need or not ?  I guess we were broken already in that respect as
we already uses --cache-from with docker without a docker pull

> +          --dockerfile "${CI_PROJECT_DIR}/tests/docker/dockerfiles/$NAME.docker"
> +          --destination "${TAG}"


With 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: [PATCH 10/11] gitlab: fix typo

[Philippe Mathieu-Daudé]

On 30/3/23 12:11, Alex Bennée wrote:
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> ---
>   .gitlab-ci.d/base.yml | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Re: [PATCH 11/11] tests/gitlab: use kaniko to build images

[Daniel P. Berrangé]

On Thu, Mar 30, 2023 at 11:17:41AM +0100, Daniel P. Berrangé wrote:
> On Thu, Mar 30, 2023 at 11:11:41AM +0100, Alex Bennée wrote:
> > Apparently the docker-in-docker approach has some flaws including
> > needing privileged mode to run and being quite slow. An alternative
> > approach is to use Google's kaniko tool. It also works across
> > different gitlab executors.
> > 
> > Following the gitlab example code we drop all the direct docker calls
> > and usage of the script and make a direct call to kaniko and hope the
> > images are cacheable by others.
> > 
> > Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> > Message-Id: <20230224180857.1050220-8-alex.bennee@linaro.org>
> > 
> > ---
> > v2
> >   - add danpb's --cache suggestions
> > ---
> >  .gitlab-ci.d/container-template.yml | 22 ++++++++++------------
> >  1 file changed, 10 insertions(+), 12 deletions(-)
> > 
> > diff --git a/.gitlab-ci.d/container-template.yml b/.gitlab-ci.d/container-template.yml
> > index 519b8a9482..cd8e0a1ff6 100644
> > --- a/.gitlab-ci.d/container-template.yml
> > +++ b/.gitlab-ci.d/container-template.yml
> > @@ -1,21 +1,19 @@
> >  .container_job_template:
> >    extends: .base_job_template
> > -  image: docker:stable
> > +  image:
> > +    name: gcr.io/kaniko-project/executor:v1.9.0-debug
> > +    entrypoint: [""]
> >    stage: containers
> > -  services:
> > -    - docker:dind
> >    before_script:
> >      - export TAG="$CI_REGISTRY_IMAGE/qemu/$NAME:latest"
> >      - export COMMON_TAG="$CI_REGISTRY/qemu-project/qemu/qemu/$NAME:latest"
> > -    - apk add python3
> > -    - docker info
> > -    - docker login $CI_REGISTRY -u "$CI_REGISTRY_USER" -p "$CI_REGISTRY_PASSWORD"
> >    script:
> >      - echo "TAG:$TAG"
> >      - echo "COMMON_TAG:$COMMON_TAG"
> > -    - docker build --tag "$TAG" --cache-from "$TAG" --cache-from "$COMMON_TAG"
> > -      --build-arg BUILDKIT_INLINE_CACHE=1
> > -      -f "tests/docker/dockerfiles/$NAME.docker" "."
> > -    - docker push "$TAG"
> > -  after_script:
> > -    - docker logout
> > +    - /kaniko/executor
> > +          --reproducible
> > +          --context "${CI_PROJECT_DIR}"
> > +          --cache=true
> > +          --cache-repo "${COMMON_TAG}"
> 
> IIRC with docker if we told it to cache we would have to first have done
> a  'docker pull $COMMON_TAG' as it wouldn't pull down the image if
> it was not already local. I'm fuzzy on whether kaniko has the same
> need or not ?  I guess we were broken already in that respect as
> we already uses --cache-from with docker without a docker pull

Oh never mind, because we're not docker-in-docker, we can't pull the
image tag down locally, and as discussed on IRC, caching works in a
very different way. kaniko wants to be able to push & pull in the
cache-repo itself.

I'm inclined to think we're better off ignoring layer caching and instead
focus on entirely skipping execution of kaniko if we know the dockerfile
has not changed eg something along the lines of:

   manifest=$(curl ....some registry URL to fetch image metadata)
   oldchecksum=$(...extract a LABEL from metadata container dockerfile sha256)
   newchecksum=$(sha256sum tests/docker/dockerfiles/$NAME.docker)

   if test $oldchecksum != $newchecksum -o -n $QEMU_FORCE_REBUILD"
   then
      - /kaniko/executor
            --reproducible
            --context "${CI_PROJECT_DIR}"
            --dockerfile "${CI_PROJECT_DIR}/tests/docker/dockerfiles/$NAME.docker"
	    --label DKR_CHECKSUM=$newchecksum
            --destination "${TAG}"
   fi


And then have a weekly pipeline on sundays that sets QEMU_FORCE_REBUILD=1
so that we pick up changes from the distro base images, and/or package
repes regularly.

With 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: [PATCH 03/11] MAINTAINERS: add a section for policy documents

[Thomas Huth]

On 30/03/2023 12.11, Alex Bennée wrote:
> We don't update these often but if you are the sort of person who
> enjoys debating and tuning project policies you could now add yourself
> as a reviewer here so you don't miss the next debate over tabs vs
> spaces ;-)
> 
> Who's with me?
> 
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> Cc: Thomas Huth <thuth@redhat.com>
> Cc: Daniel P. Berrangé <berrange@redhat.com>
> Cc: Markus Armbruster <armbru@redhat.com>
> Cc: Kashyap Chamarthy <kchamart@redhat.com>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Peter Maydell <peter.maydell@linaro.org>
> Cc: Philippe Mathieu-Daudé <philmd@linaro.org>
> Cc: Bernhard Beschow <shentey@gmail.com>
> 
> ---
> v2
>    - s/your/you are/
>    - add some willing victims
> ---
>   MAINTAINERS | 13 +++++++++++++
>   1 file changed, 13 insertions(+)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 9e1a60ea24..2c173dbd96 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -64,6 +64,19 @@ L: qemu-devel@nongnu.org
>   F: *
>   F: */
>   
> +Project policy and developer guides
> +R: Alex Bennée <alex.bennee@linaro.org>
> +R: Daniel P. Berrangé <berrange@redhat.com>
> +R: Thomas Huth <thuth@redhat.com>
> +R: Markus Armbruster <armbru@redhat.com>
> +W: https://www.qemu.org/docs/master/devel/index.html
> +S: Odd Fixes
> +F: docs/devel/style.rst
> +F: docs/devel/code-of-conduct.rst
> +F: docs/devel/conflict-resolution.rst
> +F: docs/devel/submitting-a-patch.rst
> +F: docs/devel/submitting-a-pull-request.rst

Reviewed-by: Thomas Huth <thuth@redhat.com>

Re: [PATCH 04/11] qemu-options: finesse the recommendations around -blockdev

[Thomas Huth]

On 30/03/2023 12.11, Alex Bennée wrote:
> We are a bit premature in recommending -blockdev/-device as the best
> way to configure block devices, especially in the common case.
> Improve the language to hopefully make things clearer.
> 
> Suggested-by: Michael Tokarev <mjt@tls.msk.ru>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> ---
>   qemu-options.hx | 8 ++++++--
>   1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/qemu-options.hx b/qemu-options.hx
> index 59bdf67a2c..9a69ed838e 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -1143,10 +1143,14 @@ have gone through several iterations as the feature set and complexity
>   of the block layer have grown. Many online guides to QEMU often
>   reference older and deprecated options, which can lead to confusion.
>   
> -The recommended modern way to describe disks is to use a combination of
> +The most explicit way to describe disks is to use a combination of
>   ``-device`` to specify the hardware device and ``-blockdev`` to
>   describe the backend. The device defines what the guest sees and the
> -backend describes how QEMU handles the data.
> +backend describes how QEMU handles the data. The ``-drive`` option
> +combines the device and backend into a single command line options
> +which is useful in the majority of cases. Older options like ``-hda``
> +bake in a lot of assumptions from the days when QEMU was emulating a
> +legacy PC, they are not recommended for modern configurations.

Reviewed-by: Thomas Huth <thuth@redhat.com>

Re: [PATCH 05/11] metadata: add .git-blame-ignore-revs

[Thomas Huth]

On 30/03/2023 12.11, Alex Bennée wrote:
> Someone mentioned this on IRC so I thought I would try it out with a
> few commits that are pure code style fixes.
> 
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> Message-Id: <20230318115657.1345921-1-alex.bennee@linaro.org>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> Tested-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   .git-blame-ignore-revs | 21 +++++++++++++++++++++
>   1 file changed, 21 insertions(+)
>   create mode 100644 .git-blame-ignore-revs
> 
> diff --git a/.git-blame-ignore-revs b/.git-blame-ignore-revs
> new file mode 100644
> index 0000000000..fa01e77a1e
> --- /dev/null
> +++ b/.git-blame-ignore-revs
> @@ -0,0 +1,21 @@
> +#
> +# List of code-formatting clean ups the git blame can ignore
> +#
> +#   git blame --ignore-revs-file .git-blame-ignore-revs
> +#
> +# or
> +#
> +#   git config blame.ignoreRevsFile .git-blame-ignore-revs
> +#
> +
> +# gdbstub: clean-up indents
> +ad9e4585b3c7425759d3eea697afbca71d2c2082
> +
> +# e1000e: fix code style
> +0eadd56bf53ab196a16d492d7dd31c62e1c24c32
> +
> +# target/riscv: coding style fixes
> +8c7feddddd9218b407792120bcfda0347ed16205
> +
> +# replace TABs with spaces
> ++48805df9c22a0700fba4b3b548fafaa21726ca68

Looks like there is a superfluous "+" at the beginning of the last line?

  Thomas

Re: [PATCH 07/11] tests/qemu-iotests: explicitly invoke 'check' via 'python'

[Thomas Huth]

On 30/03/2023 12.11, Alex Bennée wrote:
> From: Daniel P. Berrangé <berrange@redhat.com>
> 
> The 'check' script will use "#!/usr/bin/env python3" by default
> to locate python, but this doesn't work in distros which lack a
> bare 'python3' binary like NetBSD.
> 
> We need to explicitly invoke 'check' by referring to the 'python'
> variable in meson, which resolves to the detected python binary
> that QEMU intends to use.
> 
> This fixes a regression introduced by
> 
>    commit 51ab5f8bd795d8980351f8531e54995ff9e6d163
>    Author: Daniel P. Berrangé <berrange@redhat.com>
>    Date:   Wed Mar 15 17:43:23 2023 +0000
> 
>      iotests: register each I/O test separately with meson
> 
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> Message-Id: <20230329124539.822022-1-berrange@redhat.com>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> ---
>   tests/qemu-iotests/meson.build | 7 ++++---
>   1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/tests/qemu-iotests/meson.build b/tests/qemu-iotests/meson.build
> index a162f683ef..9735071a29 100644
> --- a/tests/qemu-iotests/meson.build
> +++ b/tests/qemu-iotests/meson.build
> @@ -47,19 +47,20 @@ foreach format, speed: qemu_iotests_formats
>     endif
>   
>     rc = run_command(
> -      [qemu_iotests_check_cmd] + args + ['-n'],
> +      [python, qemu_iotests_check_cmd] + args + ['-n'],
>         check: true,
>     )
>   
>     foreach item: rc.stdout().strip().split()
> -      args = ['-tap', '-' + format, item,
> +      args = [qemu_iotests_check_cmd,
> +              '-tap', '-' + format, item,
>                 '--source-dir', meson.current_source_dir(),
>                 '--build-dir', meson.current_build_dir()]
>         # Some individual tests take as long as 45 seconds
>         # Bump the timeout to 3 minutes for some headroom
>         # on slow machines to minimize spurious failures
>         test('io-' + format + '-' + item,
> -           qemu_iotests_check_cmd,
> +           python,
>              args: args,
>              depends: qemu_iotests_binaries,
>              env: qemu_iotests_env,

Reviewed-by: Thomas Huth <thuth@redhat.com>

Re: [PATCH 08/11] tests/vm: use the default system python for NetBSD

[Thomas Huth]

On 30/03/2023 12.11, Alex Bennée wrote:
> From: Daniel P. Berrangé <berrange@redhat.com>
> 
> Currently our NetBSD VM recipe requests instal of the python37 package
> and explicitly tells QEMU to use that version of python. Since the
> NetBSD base ISO was updated to version 9.3 though, the default system
> python version is 3.9 which is sufficiently new for QEMU to rely on.
> Rather than requesting an older python, just test against the default
> system python which is what most users will have.
> 
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> Message-Id: <20230329124601.822209-1-berrange@redhat.com>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> ---
>   tests/vm/netbsd | 3 +--
>   1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/tests/vm/netbsd b/tests/vm/netbsd
> index aa54338dfa..0b9536ca17 100755
> --- a/tests/vm/netbsd
> +++ b/tests/vm/netbsd
> @@ -30,7 +30,6 @@ class NetBSDVM(basevm.BaseVM):
>           "git-base",
>           "pkgconf",
>           "xz",
> -        "python37",
>           "ninja-build",
>   
>           # gnu tools
> @@ -66,7 +65,7 @@ class NetBSDVM(basevm.BaseVM):
>           mkdir src build; cd src;
>           tar -xf /dev/rld1a;
>           cd ../build
> -        ../src/configure --python=python3.7 --disable-opengl {configure_opts};
> +        ../src/configure --disable-opengl {configure_opts};
>           gmake --output-sync -j{jobs} {target} {verbose};
>       """
>       poweroff = "/sbin/poweroff"

Reviewed-by: Thomas Huth <thuth@redhat.com>

Re: [PATCH 10/11] gitlab: fix typo

[Thomas Huth]

On 30/03/2023 12.11, Alex Bennée wrote:
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> ---
>   .gitlab-ci.d/base.yml | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/.gitlab-ci.d/base.yml b/.gitlab-ci.d/base.yml
> index 0274228de8..2fbb58d2a3 100644
> --- a/.gitlab-ci.d/base.yml
> +++ b/.gitlab-ci.d/base.yml
> @@ -75,5 +75,5 @@
>       - if: '$QEMU_CI != "2" && $CI_PROJECT_NAMESPACE != "qemu-project"'
>         when: manual
>   
> -    # Jobs can run if any jobs they depend on were successfull
> +    # Jobs can run if any jobs they depend on were successful
>       - when: on_success

Reviewed-by: Thomas Huth <thuth@redhat.com>

Re: [PATCH 09/11] tests/requirements.txt: bump up avocado-framework version to 101.0

[Thomas Huth]

On 30/03/2023 12.11, Alex Bennée wrote:
> From: Kautuk Consul <kconsul@linux.vnet.ibm.com>
> 
> Avocado version 101.0 has a fix to re-compute the checksum
> of an asset file if the algorithm used in the *-CHECKSUM
> file isn't the same as the one being passed to it by the
> avocado user (i.e. the avocado_qemu python module).
> In the earlier avocado versions this fix wasn't there due
> to which if the checksum wouldn't match the earlier
> checksum (calculated by a different algorithm), the avocado
> code would start downloading a fresh image from the internet
> URL thus making the test-cases take longer to execute.
> 
> Bump up the avocado-framework version to 101.0.
> 
> Signed-off-by: Kautuk Consul <kconsul@linux.vnet.ibm.com>
> Tested-by: Hariharan T S <hariharan.ts@linux.vnet.ibm.com>
> Message-Id: <20230327115030.3418323-2-kconsul@linux.vnet.ibm.com>
> ---
>   tests/requirements.txt | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tests/requirements.txt b/tests/requirements.txt
> index 0ba561b6bd..a6f73da681 100644
> --- a/tests/requirements.txt
> +++ b/tests/requirements.txt
> @@ -2,5 +2,5 @@
>   # in the tests/venv Python virtual environment. For more info,
>   # refer to: https://pip.pypa.io/en/stable/user_guide/#id1
>   # Note that qemu.git/python/ is always implicitly installed.
> -avocado-framework==88.1
> +avocado-framework==101.0
>   pycdlib==1.11.0

Did you check whether the same amount of avocado tests still works as 
before? ... last time I tried to bump the version, a lot of things were 
failing, and I think Cleber was recently working  on fixing things, but I 
haven't heart anything back from him yet that it would be OK to bump to a 
newer version now ...
So upgrading to a new version of Avocado during the softfreeze sounds 
somewhat risky to me right now - I'd appreciate if we could do that after 
the release instead.

  Thomas

Re: [PATCH 09/11] tests/requirements.txt: bump up avocado-framework version to 101.0

[Alex Bennée]

Thomas Huth <thuth@redhat.com> writes:

> On 30/03/2023 12.11, Alex Bennée wrote:
>> From: Kautuk Consul <kconsul@linux.vnet.ibm.com>
>> Avocado version 101.0 has a fix to re-compute the checksum
>> of an asset file if the algorithm used in the *-CHECKSUM
>> file isn't the same as the one being passed to it by the
>> avocado user (i.e. the avocado_qemu python module).
>> In the earlier avocado versions this fix wasn't there due
>> to which if the checksum wouldn't match the earlier
>> checksum (calculated by a different algorithm), the avocado
>> code would start downloading a fresh image from the internet
>> URL thus making the test-cases take longer to execute.
>> Bump up the avocado-framework version to 101.0.
>> Signed-off-by: Kautuk Consul <kconsul@linux.vnet.ibm.com>
>> Tested-by: Hariharan T S <hariharan.ts@linux.vnet.ibm.com>
>> Message-Id: <20230327115030.3418323-2-kconsul@linux.vnet.ibm.com>
>> ---
>>   tests/requirements.txt | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>> diff --git a/tests/requirements.txt b/tests/requirements.txt
>> index 0ba561b6bd..a6f73da681 100644
>> --- a/tests/requirements.txt
>> +++ b/tests/requirements.txt
>> @@ -2,5 +2,5 @@
>>   # in the tests/venv Python virtual environment. For more info,
>>   # refer to: https://pip.pypa.io/en/stable/user_guide/#id1
>>   # Note that qemu.git/python/ is always implicitly installed.
>> -avocado-framework==88.1
>> +avocado-framework==101.0
>>   pycdlib==1.11.0
>
> Did you check whether the same amount of avocado tests still works as
> before? ... last time I tried to bump the version, a lot of things
> were failing, and I think Cleber was recently working  on fixing
> things, but I haven't heart anything back from him yet that it would
> be OK to bump to a newer version now ...

I ran it on my default build and the only failure was:

 (008/222) tests/avocado/boot_linux.py:BootLinuxS390X.test_s390_ccw_virtio_tcg: INTERRUPTED: timeout (240.01 s)

which passed on a retry. But now I realise with failfast it skipped a bunch:

RESULTS    : PASS 46 | ERROR 0 | FAIL 0 | SKIP 174 | WARN 0 | INTERRUPT 1 | CANCEL 1
JOB TIME   : 290.26 s

> So upgrading to a new version of Avocado during the softfreeze sounds
> somewhat risky to me right now - I'd appreciate if we could do that
> after the release instead.

Sure. I was hoping we would speed up avocado a little by avoiding double downloads.

>
>  Thomas


-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro

Re: [PATCH 09/11] tests/requirements.txt: bump up avocado-framework version to 101.0

[Thomas Huth]

On 30/03/2023 14.12, Alex Bennée wrote:
> 
> Thomas Huth <thuth@redhat.com> writes:
> 
>> On 30/03/2023 12.11, Alex Bennée wrote:
>>> From: Kautuk Consul <kconsul@linux.vnet.ibm.com>
>>> Avocado version 101.0 has a fix to re-compute the checksum
>>> of an asset file if the algorithm used in the *-CHECKSUM
>>> file isn't the same as the one being passed to it by the
>>> avocado user (i.e. the avocado_qemu python module).
>>> In the earlier avocado versions this fix wasn't there due
>>> to which if the checksum wouldn't match the earlier
>>> checksum (calculated by a different algorithm), the avocado
>>> code would start downloading a fresh image from the internet
>>> URL thus making the test-cases take longer to execute.
>>> Bump up the avocado-framework version to 101.0.
>>> Signed-off-by: Kautuk Consul <kconsul@linux.vnet.ibm.com>
>>> Tested-by: Hariharan T S <hariharan.ts@linux.vnet.ibm.com>
>>> Message-Id: <20230327115030.3418323-2-kconsul@linux.vnet.ibm.com>
>>> ---
>>>    tests/requirements.txt | 2 +-
>>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>> diff --git a/tests/requirements.txt b/tests/requirements.txt
>>> index 0ba561b6bd..a6f73da681 100644
>>> --- a/tests/requirements.txt
>>> +++ b/tests/requirements.txt
>>> @@ -2,5 +2,5 @@
>>>    # in the tests/venv Python virtual environment. For more info,
>>>    # refer to: https://pip.pypa.io/en/stable/user_guide/#id1
>>>    # Note that qemu.git/python/ is always implicitly installed.
>>> -avocado-framework==88.1
>>> +avocado-framework==101.0
>>>    pycdlib==1.11.0
>>
>> Did you check whether the same amount of avocado tests still works as
>> before? ... last time I tried to bump the version, a lot of things
>> were failing, and I think Cleber was recently working  on fixing
>> things, but I haven't heart anything back from him yet that it would
>> be OK to bump to a newer version now ...
> 
> I ran it on my default build and the only failure was:
> 
>   (008/222) tests/avocado/boot_linux.py:BootLinuxS390X.test_s390_ccw_virtio_tcg: INTERRUPTED: timeout (240.01 s)
> 
> which passed on a retry. But now I realise with failfast it skipped a bunch:

That one is also failing for me here when I apply the patch. Without the 
patch, the test is working fine. I think this needs more careful testing 
first - e.g. the tests are run in parallel now by default, which breaks a 
lot of our timeout settings.

  Thomas

Re: [PATCH 11/11] tests/gitlab: use kaniko to build images

[Thomas Huth]

On 30/03/2023 12.11, Alex Bennée wrote:
> Apparently the docker-in-docker approach has some flaws including
> needing privileged mode to run and being quite slow. An alternative
> approach is to use Google's kaniko tool. It also works across
> different gitlab executors.
> 
> Following the gitlab example code we drop all the direct docker calls
> and usage of the script and make a direct call to kaniko and hope the
> images are cacheable by others.
> 
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> Message-Id: <20230224180857.1050220-8-alex.bennee@linaro.org>
> 
> ---
> v2
>    - add danpb's --cache suggestions
> ---
>   .gitlab-ci.d/container-template.yml | 22 ++++++++++------------
>   1 file changed, 10 insertions(+), 12 deletions(-)
> 
> diff --git a/.gitlab-ci.d/container-template.yml b/.gitlab-ci.d/container-template.yml
> index 519b8a9482..cd8e0a1ff6 100644
> --- a/.gitlab-ci.d/container-template.yml
> +++ b/.gitlab-ci.d/container-template.yml
> @@ -1,21 +1,19 @@
>   .container_job_template:
>     extends: .base_job_template
> -  image: docker:stable
> +  image:
> +    name: gcr.io/kaniko-project/executor:v1.9.0-debug
> +    entrypoint: [""]
>     stage: containers
> -  services:
> -    - docker:dind
>     before_script:
>       - export TAG="$CI_REGISTRY_IMAGE/qemu/$NAME:latest"
>       - export COMMON_TAG="$CI_REGISTRY/qemu-project/qemu/qemu/$NAME:latest"
> -    - apk add python3
> -    - docker info
> -    - docker login $CI_REGISTRY -u "$CI_REGISTRY_USER" -p "$CI_REGISTRY_PASSWORD"
>     script:
>       - echo "TAG:$TAG"
>       - echo "COMMON_TAG:$COMMON_TAG"
> -    - docker build --tag "$TAG" --cache-from "$TAG" --cache-from "$COMMON_TAG"
> -      --build-arg BUILDKIT_INLINE_CACHE=1
> -      -f "tests/docker/dockerfiles/$NAME.docker" "."
> -    - docker push "$TAG"
> -  after_script:
> -    - docker logout
> +    - /kaniko/executor
> +          --reproducible
> +          --context "${CI_PROJECT_DIR}"
> +          --cache=true
> +          --cache-repo "${COMMON_TAG}"
> +          --dockerfile "${CI_PROJECT_DIR}/tests/docker/dockerfiles/$NAME.docker"
> +          --destination "${TAG}"

Acked-by: Thomas Huth <thuth@redhat.com>

Re: [PATCH 01/11] scripts/coverage: initial coverage comparison script

[Thomas Huth]

On 30/03/2023 12.11, Alex Bennée wrote:
> This is a very rough and ready first pass at comparing gcovr's json
> output between two different runs. At the moment it will give you a
> file level diff between two runs but hopefully it wont be too hard to
> extend to give better insight.
> 
> After generating the coverage results you run with something like:
> 
>    ./scripts/coverage/compare_gcov_json.py \
>      -a ./builds/gcov.config1/coverage.json \
>      -b ./builds/gcov.config2/coverage.json
> 
> My hope is we can use this to remove some redundancy from testing as
> well as evaluate if new tests are actually providing additional
> coverage or just burning our precious CI time.
> 
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>

FWIW:
Acked-by: Thomas Huth <thuth@redhat.com>

Re: [PATCH 03/11] MAINTAINERS: add a section for policy documents

[Markus Armbruster]

Alex Bennée <alex.bennee@linaro.org> writes:

> We don't update these often but if you are the sort of person who
> enjoys debating and tuning project policies you could now add yourself
> as a reviewer here so you don't miss the next debate over tabs vs
> spaces ;-)
>
> Who's with me?
>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> Cc: Thomas Huth <thuth@redhat.com>
> Cc: Daniel P. Berrangé <berrange@redhat.com>
> Cc: Markus Armbruster <armbru@redhat.com>
> Cc: Kashyap Chamarthy <kchamart@redhat.com>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Peter Maydell <peter.maydell@linaro.org>
> Cc: Philippe Mathieu-Daudé <philmd@linaro.org>
> Cc: Bernhard Beschow <shentey@gmail.com>

Reviewed-by: Markus Armbruster <armbru@redhat.com>

Re: [PATCH 03/11] MAINTAINERS: add a section for policy documents

[Warner Losh]

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

Alex,

On Thu, Mar 30, 2023 at 12:11 PM Alex Bennée <alex.bennee@linaro.org> wrote:

> We don't update these often but if you are the sort of person who
> enjoys debating and tuning project policies you could now add yourself
> as a reviewer here so you don't miss the next debate over tabs vs
> spaces ;-)
>
> Who's with me?
>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> Cc: Thomas Huth <thuth@redhat.com>
> Cc: Daniel P. Berrangé <berrange@redhat.com>
> Cc: Markus Armbruster <armbru@redhat.com>
> Cc: Kashyap Chamarthy <kchamart@redhat.com>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Peter Maydell <peter.maydell@linaro.org>
> Cc: Philippe Mathieu-Daudé <philmd@linaro.org>
> Cc: Bernhard Beschow <shentey@gmail.com>
>

Reviewed-by: Warner Losh <imp@bsdimp.com>

Since I'm not on the list, I approve :). I had enough dealing with code of
conduct issues with FreeBSD to last for any number of years...

Warner


> ---
> v2
>   - s/your/you are/
>   - add some willing victims
> ---
>  MAINTAINERS | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 9e1a60ea24..2c173dbd96 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -64,6 +64,19 @@ L: qemu-devel@nongnu.org
>  F: *
>  F: */
>
> +Project policy and developer guides
> +R: Alex Bennée <alex.bennee@linaro.org>
> +R: Daniel P. Berrangé <berrange@redhat.com>
> +R: Thomas Huth <thuth@redhat.com>
> +R: Markus Armbruster <armbru@redhat.com>
> +W: https://www.qemu.org/docs/master/devel/index.html
> +S: Odd Fixes
> +F: docs/devel/style.rst
> +F: docs/devel/code-of-conduct.rst
> +F: docs/devel/conflict-resolution.rst
> +F: docs/devel/submitting-a-patch.rst
> +F: docs/devel/submitting-a-pull-request.rst
> +
>  Responsible Disclosure, Reporting Security Issues
>  -------------------------------------------------
>  W: https://wiki.qemu.org/SecurityProcess
> --
> 2.39.2
>
>

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

Re: [PATCH 03/11] MAINTAINERS: add a section for policy documents

[Kashyap Chamarthy]

On Thu, Mar 30, 2023 at 11:11:33AM +0100, Alex Bennée wrote:
> We don't update these often but if you are the sort of person who
> enjoys debating and tuning project policies you could now add yourself
> as a reviewer here so you don't miss the next debate over tabs vs
> spaces ;-)
> 
> Who's with me?

Happy to chip in wherever I can be useful :-)

Reviewed-by: Kashyap Chamarthy <kchamart@redhat.com> 

> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> Cc: Thomas Huth <thuth@redhat.com>
> Cc: Daniel P. Berrangé <berrange@redhat.com>
> Cc: Markus Armbruster <armbru@redhat.com>
> Cc: Kashyap Chamarthy <kchamart@redhat.com>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Peter Maydell <peter.maydell@linaro.org>
> Cc: Philippe Mathieu-Daudé <philmd@linaro.org>
> Cc: Bernhard Beschow <shentey@gmail.com>
>
> ---
> v2
>   - s/your/you are/
>   - add some willing victims
> ---
>  MAINTAINERS | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 9e1a60ea24..2c173dbd96 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -64,6 +64,19 @@ L: qemu-devel@nongnu.org
>  F: *
>  F: */
>  
> +Project policy and developer guides
> +R: Alex Bennée <alex.bennee@linaro.org>
> +R: Daniel P. Berrangé <berrange@redhat.com>
> +R: Thomas Huth <thuth@redhat.com>
> +R: Markus Armbruster <armbru@redhat.com>
> +W: https://www.qemu.org/docs/master/devel/index.html
> +S: Odd Fixes
> +F: docs/devel/style.rst
> +F: docs/devel/code-of-conduct.rst
> +F: docs/devel/conflict-resolution.rst
> +F: docs/devel/submitting-a-patch.rst
> +F: docs/devel/submitting-a-pull-request.rst
> +
>  Responsible Disclosure, Reporting Security Issues
>  -------------------------------------------------
>  W: https://wiki.qemu.org/SecurityProcess
> -- 
> 2.39.2
> 

-- 
/kashyap

Re: [PATCH 11/11] tests/gitlab: use kaniko to build images

[Alex Bennée]

Daniel P. Berrangé <berrange@redhat.com> writes:

> On Thu, Mar 30, 2023 at 11:17:41AM +0100, Daniel P. Berrangé wrote:
>> On Thu, Mar 30, 2023 at 11:11:41AM +0100, Alex Bennée wrote:
>> > Apparently the docker-in-docker approach has some flaws including
>> > needing privileged mode to run and being quite slow. An alternative
>> > approach is to use Google's kaniko tool. It also works across
>> > different gitlab executors.
>> > 
>> > Following the gitlab example code we drop all the direct docker calls
>> > and usage of the script and make a direct call to kaniko and hope the
>> > images are cacheable by others.
>> > 
>> > Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>> > Message-Id: <20230224180857.1050220-8-alex.bennee@linaro.org>
>> > 
>> > ---
>> > v2
>> >   - add danpb's --cache suggestions
>> > ---
>> >  .gitlab-ci.d/container-template.yml | 22 ++++++++++------------
>> >  1 file changed, 10 insertions(+), 12 deletions(-)
>> > 
>> > diff --git a/.gitlab-ci.d/container-template.yml b/.gitlab-ci.d/container-template.yml
>> > index 519b8a9482..cd8e0a1ff6 100644
>> > --- a/.gitlab-ci.d/container-template.yml
>> > +++ b/.gitlab-ci.d/container-template.yml
>> > @@ -1,21 +1,19 @@
>> >  .container_job_template:
>> >    extends: .base_job_template
>> > -  image: docker:stable
>> > +  image:
>> > +    name: gcr.io/kaniko-project/executor:v1.9.0-debug
>> > +    entrypoint: [""]
>> >    stage: containers
>> > -  services:
>> > -    - docker:dind
>> >    before_script:
>> >      - export TAG="$CI_REGISTRY_IMAGE/qemu/$NAME:latest"
>> >      - export COMMON_TAG="$CI_REGISTRY/qemu-project/qemu/qemu/$NAME:latest"
>> > -    - apk add python3
>> > -    - docker info
>> > -    - docker login $CI_REGISTRY -u "$CI_REGISTRY_USER" -p "$CI_REGISTRY_PASSWORD"
>> >    script:
>> >      - echo "TAG:$TAG"
>> >      - echo "COMMON_TAG:$COMMON_TAG"
>> > -    - docker build --tag "$TAG" --cache-from "$TAG" --cache-from "$COMMON_TAG"
>> > -      --build-arg BUILDKIT_INLINE_CACHE=1
>> > -      -f "tests/docker/dockerfiles/$NAME.docker" "."
>> > -    - docker push "$TAG"
>> > -  after_script:
>> > -    - docker logout
>> > +    - /kaniko/executor
>> > +          --reproducible
>> > +          --context "${CI_PROJECT_DIR}"
>> > +          --cache=true
>> > +          --cache-repo "${COMMON_TAG}"
>> 
>> IIRC with docker if we told it to cache we would have to first have done
>> a  'docker pull $COMMON_TAG' as it wouldn't pull down the image if
>> it was not already local. I'm fuzzy on whether kaniko has the same
>> need or not ?  I guess we were broken already in that respect as
>> we already uses --cache-from with docker without a docker pull
>
> Oh never mind, because we're not docker-in-docker, we can't pull the
> image tag down locally, and as discussed on IRC, caching works in a
> very different way. kaniko wants to be able to push & pull in the
> cache-repo itself.
>
> I'm inclined to think we're better off ignoring layer caching and instead
> focus on entirely skipping execution of kaniko if we know the dockerfile
> has not changed eg something along the lines of:
>
>    manifest=$(curl ....some registry URL to fetch image metadata)
>    oldchecksum=$(...extract a LABEL from metadata container dockerfile sha256)
>    newchecksum=$(sha256sum tests/docker/dockerfiles/$NAME.docker)
>
>    if test $oldchecksum != $newchecksum -o -n $QEMU_FORCE_REBUILD"
>    then
>       - /kaniko/executor
>             --reproducible
>             --context "${CI_PROJECT_DIR}"
>             --dockerfile "${CI_PROJECT_DIR}/tests/docker/dockerfiles/$NAME.docker"
> 	    --label DKR_CHECKSUM=$newchecksum
>             --destination "${TAG}"
>    fi
>
>
> And then have a weekly pipeline on sundays that sets QEMU_FORCE_REBUILD=1
> so that we pick up changes from the distro base images, and/or package
> repes regularly.

Hmm this appears to be a dead end. I got to this:

--8<---------------cut here---------------start------------->8---
tests/gitlab: use kaniko to build images

Apparently the docker-in-docker approach has some flaws including
needing privileged mode to run and being quite slow. An alternative
approach is to use Google's kaniko tool. It also works across
different gitlab executors.

Following the gitlab example code we drop all the direct docker calls
and usage of the script and make a direct call to kaniko and hope the
images are cacheable by others.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Message-Id: <20230224180857.1050220-8-alex.bennee@linaro.org>

---
v2
  - add danpb's --cache suggestions
v3
  - don't include :latest in tag
  - allow kaniko to infer local registry location, drop COMMON_TAG
  - add registry login details
  - version bump
  - don't push cache layers

1 file changed, 13 insertions(+), 14 deletions(-)
.gitlab-ci.d/container-template.yml | 27 +++++++++++++--------------

modified   .gitlab-ci.d/container-template.yml
@@ -1,21 +1,20 @@
 .container_job_template:
   extends: .base_job_template
-  image: docker:stable
+  image:
+    name: gcr.io/kaniko-project/executor:v1.9.2-debug
+    entrypoint: [""]
   stage: containers
-  services:
-    - docker:dind
   before_script:
-    - export TAG="$CI_REGISTRY_IMAGE/qemu/$NAME:latest"
-    - export COMMON_TAG="$CI_REGISTRY/qemu-project/qemu/qemu/$NAME:latest"
-    - apk add python3
-    - docker info
-    - docker login $CI_REGISTRY -u "$CI_REGISTRY_USER" -p "$CI_REGISTRY_PASSWORD"
+    - export TAG="$CI_REGISTRY_IMAGE/qemu/$NAME"
   script:
     - echo "TAG:$TAG"
     - echo "COMMON_TAG:$COMMON_TAG"
-    - docker build --tag "$TAG" --cache-from "$TAG" --cache-from "$COMMON_TAG"
-      --build-arg BUILDKIT_INLINE_CACHE=1
-      -f "tests/docker/dockerfiles/$NAME.docker" "."
-    - docker push "$TAG"
-  after_script:
-    - docker logout
+    - echo "{\"auths\":{\"${CI_REGISTRY}\":{\"auth\":\"$(echo -n ${CI_REGISTRY_USER}:${CI_REGISTRY_PASSWORD} | base64)\"}}}" > /kaniko/.docker/config.json
+    - /kaniko/executor
+          --reproducible
+          --context "${CI_PROJECT_DIR}"
+          --cache=true
+          --reproducible
+          --no-push-cache
+          --dockerfile "${CI_PROJECT_DIR}/tests/docker/dockerfiles/$NAME.docker"
+          --destination "${TAG}"
--8<---------------cut here---------------end--------------->8---

However the builds are failing so I think I just need to drop this and
move on.

>
> With regards,
> Daniel


-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro

Re: [PATCH 09/11] tests/requirements.txt: bump up avocado-framework version to 101.0

[Thomas Huth]

On 30/03/2023 14.21, Thomas Huth wrote:
> On 30/03/2023 14.12, Alex Bennée wrote:
>>
>> Thomas Huth <thuth@redhat.com> writes:
>>
>>> On 30/03/2023 12.11, Alex Bennée wrote:
>>>> From: Kautuk Consul <kconsul@linux.vnet.ibm.com>
>>>> Avocado version 101.0 has a fix to re-compute the checksum
>>>> of an asset file if the algorithm used in the *-CHECKSUM
>>>> file isn't the same as the one being passed to it by the
>>>> avocado user (i.e. the avocado_qemu python module).
>>>> In the earlier avocado versions this fix wasn't there due
>>>> to which if the checksum wouldn't match the earlier
>>>> checksum (calculated by a different algorithm), the avocado
>>>> code would start downloading a fresh image from the internet
>>>> URL thus making the test-cases take longer to execute.
>>>> Bump up the avocado-framework version to 101.0.
>>>> Signed-off-by: Kautuk Consul <kconsul@linux.vnet.ibm.com>
>>>> Tested-by: Hariharan T S <hariharan.ts@linux.vnet.ibm.com>
>>>> Message-Id: <20230327115030.3418323-2-kconsul@linux.vnet.ibm.com>
>>>> ---
>>>>    tests/requirements.txt | 2 +-
>>>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>>> diff --git a/tests/requirements.txt b/tests/requirements.txt
>>>> index 0ba561b6bd..a6f73da681 100644
>>>> --- a/tests/requirements.txt
>>>> +++ b/tests/requirements.txt
>>>> @@ -2,5 +2,5 @@
>>>>    # in the tests/venv Python virtual environment. For more info,
>>>>    # refer to: https://pip.pypa.io/en/stable/user_guide/#id1
>>>>    # Note that qemu.git/python/ is always implicitly installed.
>>>> -avocado-framework==88.1
>>>> +avocado-framework==101.0
>>>>    pycdlib==1.11.0
>>>
>>> Did you check whether the same amount of avocado tests still works as
>>> before? ... last time I tried to bump the version, a lot of things
>>> were failing, and I think Cleber was recently working  on fixing
>>> things, but I haven't heart anything back from him yet that it would
>>> be OK to bump to a newer version now ...
>>
>> I ran it on my default build and the only failure was:
>>
>>   (008/222) 
>> tests/avocado/boot_linux.py:BootLinuxS390X.test_s390_ccw_virtio_tcg: 
>> INTERRUPTED: timeout (240.01 s)
>>
>> which passed on a retry. But now I realise with failfast it skipped a bunch:
> 
> That one is also failing for me here when I apply the patch. Without the 
> patch, the test is working fine. I think this needs more careful testing 
> first - e.g. the tests are run in parallel now by default, which breaks a 
> lot of our timeout settings.

FWIW, I think we likely want something like this added to this patch,
so we avoid to run those tests in parallel (unless requested with -jX):

diff a/tests/Makefile.include b/tests/Makefile.include
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -138,12 +138,15 @@ get-vm-image-fedora-31-%: check-venv
  # download all vm images, according to defined targets
  get-vm-images: check-venv $(patsubst %,get-vm-image-fedora-31-%, $(FEDORA_31_DOWNLOAD))
  
+JOBS_OPTION=$(lastword -j1 $(filter-out -j, $(filter -j%, $(MAKEFLAGS))))
+
  check-avocado: check-venv $(TESTS_RESULTS_DIR) get-vm-images
         $(call quiet-command, \
              $(TESTS_PYTHON) -m avocado \
              --show=$(AVOCADO_SHOW) run --job-results-dir=$(TESTS_RESULTS_DIR) \
              $(if $(AVOCADO_TAGS),, --filter-by-tags-include-empty \
                         --filter-by-tags-include-empty-key) \
+            --max-parallel-tasks $(JOBS_OPTION:-j%=%) \
              $(AVOCADO_CMDLINE_TAGS) \
              $(if $(GITLAB_CI),,--failfast) $(AVOCADO_TESTS), \
              "AVOCADO", "tests/avocado")

That way we can avoid the timeout problems unless we found a
proper solution for those.

  Thomas

Re: [PATCH 04/11] qemu-options: finesse the recommendations around -blockdev

[Michael Tokarev]

30.03.2023 13:11, Alex Bennée wrote:
> We are a bit premature in recommending -blockdev/-device as the best
> way to configure block devices, especially in the common case.
> Improve the language to hopefully make things clearer.
> 
> Suggested-by: Michael Tokarev <mjt@tls.msk.ru>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> ---
>   qemu-options.hx | 8 ++++++--
>   1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/qemu-options.hx b/qemu-options.hx
> index 59bdf67a2c..9a69ed838e 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -1143,10 +1143,14 @@ have gone through several iterations as the feature set and complexity
>   of the block layer have grown. Many online guides to QEMU often
>   reference older and deprecated options, which can lead to confusion.
>   
> -The recommended modern way to describe disks is to use a combination of
> +The most explicit way to describe disks is to use a combination of
>   ``-device`` to specify the hardware device and ``-blockdev`` to
>   describe the backend. The device defines what the guest sees and the
> -backend describes how QEMU handles the data.
> +backend describes how QEMU handles the data. The ``-drive`` option
> +combines the device and backend into a single command line options
> +which is useful in the majority of cases. Older options like ``-hda``
> +bake in a lot of assumptions from the days when QEMU was emulating a
> +legacy PC, they are not recommended for modern configurations.

Reviewed-by: Michael Tokarev <mjt@tls.msk.ru>

This is a very welcome change, in my opinion.

Thank you Alex!

/mjt

Re: [PATCH 04/11] qemu-options: finesse the recommendations around -blockdev

[Markus Armbruster]

Alex Bennée <alex.bennee@linaro.org> writes:

> We are a bit premature in recommending -blockdev/-device as the best
> way to configure block devices, especially in the common case.
> Improve the language to hopefully make things clearer.
>
> Suggested-by: Michael Tokarev <mjt@tls.msk.ru>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> ---
>  qemu-options.hx | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/qemu-options.hx b/qemu-options.hx
> index 59bdf67a2c..9a69ed838e 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -1143,10 +1143,14 @@ have gone through several iterations as the feature set and complexity
>  of the block layer have grown. Many online guides to QEMU often
>  reference older and deprecated options, which can lead to confusion.
>  
> -The recommended modern way to describe disks is to use a combination of
> +The most explicit way to describe disks is to use a combination of
>  ``-device`` to specify the hardware device and ``-blockdev`` to
>  describe the backend. The device defines what the guest sees and the
> -backend describes how QEMU handles the data.
> +backend describes how QEMU handles the data. The ``-drive`` option
> +combines the device and backend into a single command line options
> +which is useful in the majority of cases.

-drive may look simpler from afar, but it really is a hot mess.  Sadly,
we can't get rid of it until we find a replacement for configuring
onboard block devices.  We might be able to clean it up some if we
accept compatibility breaks.  A new convenience option would be less
confusing, I guess.

>                                            Older options like ``-hda``
> +bake in a lot of assumptions from the days when QEMU was emulating a
> +legacy PC, they are not recommended for modern configurations.
>  
>  ERST

These older options and the non-option argument are simple macros for
-drive:

    IMG-FILE                    -drive index=0,file=IMG-FILE,media=disk
    -hda IMG-FILE               -drive index=0,file=IMG-FILE,media=disk
    -hdb IMG-FILE               -drive index=1,file=IMG-FILE,media=disk
    -hdc IMG-FILE               -drive index=2,file=IMG-FILE,media=disk
    -hdd IMG-FILE               -drive index=3,file=IMG-FILE,media=disk
    -cdrom IMG-FILE             -drive index=2,file=IMG-FILE,media=cdrom
    -fda IMG-FILE               -drive if=floppy,index=0,file=IMG-FILE
    -fdb IMG-FILE               -drive if=floppy,index=1,file=IMG-FILE
    -mtdblock IMG-FILE          -drive if=mtd,file=IMG-FILE
    -sd IMG-FILE                -drive if=sd,file=IMG-FILE
    -pflash IMG-FILE            -drive if=pflash,file=IMG-FILE

What assumptions do you have in mind?

I think you need at least Kevin's Acked-by for this.

Re: [PATCH 03/11] MAINTAINERS: add a section for policy documents

[Philippe Mathieu-Daudé]

On 30/3/23 12:11, Alex Bennée wrote:
> We don't update these often but if you are the sort of person who
> enjoys debating and tuning project policies you could now add yourself
> as a reviewer here so you don't miss the next debate over tabs vs
> spaces ;-)
> 
> Who's with me?
> 
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> Cc: Thomas Huth <thuth@redhat.com>
> Cc: Daniel P. Berrangé <berrange@redhat.com>
> Cc: Markus Armbruster <armbru@redhat.com>
> Cc: Kashyap Chamarthy <kchamart@redhat.com>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Peter Maydell <peter.maydell@linaro.org>
> Cc: Philippe Mathieu-Daudé <philmd@linaro.org>
> Cc: Bernhard Beschow <shentey@gmail.com>
> 
> ---
> v2
>    - s/your/you are/
>    - add some willing victims
> ---
>   MAINTAINERS | 13 +++++++++++++
>   1 file changed, 13 insertions(+)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 9e1a60ea24..2c173dbd96 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -64,6 +64,19 @@ L: qemu-devel@nongnu.org
>   F: *
>   F: */
>   
> +Project policy and developer guides
> +R: Alex Bennée <alex.bennee@linaro.org>
> +R: Daniel P. Berrangé <berrange@redhat.com>
> +R: Thomas Huth <thuth@redhat.com>
> +R: Markus Armbruster <armbru@redhat.com>

Please add me too.

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

> +W: https://www.qemu.org/docs/master/devel/index.html
> +S: Odd Fixes
> +F: docs/devel/style.rst
> +F: docs/devel/code-of-conduct.rst
> +F: docs/devel/conflict-resolution.rst
> +F: docs/devel/submitting-a-patch.rst
> +F: docs/devel/submitting-a-pull-request.rst
> +
>   Responsible Disclosure, Reporting Security Issues
>   -------------------------------------------------
>   W: https://wiki.qemu.org/SecurityProcess

Re: [PATCH 04/11] qemu-options: finesse the recommendations around -blockdev

[Alex Bennée]

Markus Armbruster <armbru@redhat.com> writes:

> Alex Bennée <alex.bennee@linaro.org> writes:
>
>> We are a bit premature in recommending -blockdev/-device as the best
>> way to configure block devices, especially in the common case.
>> Improve the language to hopefully make things clearer.
>>
>> Suggested-by: Michael Tokarev <mjt@tls.msk.ru>
>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>> ---
>>  qemu-options.hx | 8 ++++++--
>>  1 file changed, 6 insertions(+), 2 deletions(-)
>>
>> diff --git a/qemu-options.hx b/qemu-options.hx
>> index 59bdf67a2c..9a69ed838e 100644
>> --- a/qemu-options.hx
>> +++ b/qemu-options.hx
>> @@ -1143,10 +1143,14 @@ have gone through several iterations as the feature set and complexity
>>  of the block layer have grown. Many online guides to QEMU often
>>  reference older and deprecated options, which can lead to confusion.
>>  
>> -The recommended modern way to describe disks is to use a combination of
>> +The most explicit way to describe disks is to use a combination of
>>  ``-device`` to specify the hardware device and ``-blockdev`` to
>>  describe the backend. The device defines what the guest sees and the
>> -backend describes how QEMU handles the data.
>> +backend describes how QEMU handles the data. The ``-drive`` option
>> +combines the device and backend into a single command line options
>> +which is useful in the majority of cases.
>
> -drive may look simpler from afar, but it really is a hot mess.  Sadly,
> we can't get rid of it until we find a replacement for configuring
> onboard block devices.  We might be able to clean it up some if we
> accept compatibility breaks.  A new convenience option would be less
> confusing, I guess.

This is only a partial revert of the original wording which others have
pointed out was a little too prescriptive. I believe the case of
snapshot was one where a pure device/blockdev command line is hard to
use. 

>>                                            Older options like ``-hda``
>> +bake in a lot of assumptions from the days when QEMU was emulating a
>> +legacy PC, they are not recommended for modern configurations.
>>  
>>  ERST
>
> These older options and the non-option argument are simple macros for
> -drive:
>
>     IMG-FILE                    -drive index=0,file=IMG-FILE,media=disk
>     -hda IMG-FILE               -drive index=0,file=IMG-FILE,media=disk
>     -hdb IMG-FILE               -drive index=1,file=IMG-FILE,media=disk
>     -hdc IMG-FILE               -drive index=2,file=IMG-FILE,media=disk
>     -hdd IMG-FILE               -drive index=3,file=IMG-FILE,media=disk
>     -cdrom IMG-FILE             -drive index=2,file=IMG-FILE,media=cdrom
>     -fda IMG-FILE               -drive if=floppy,index=0,file=IMG-FILE
>     -fdb IMG-FILE               -drive if=floppy,index=1,file=IMG-FILE
>     -mtdblock IMG-FILE          -drive if=mtd,file=IMG-FILE
>     -sd IMG-FILE                -drive if=sd,file=IMG-FILE
>     -pflash IMG-FILE            -drive if=pflash,file=IMG-FILE
>
> What assumptions do you have in mind?

I was under the impression things like -hda wouldn't work say on an Arm
machine because you don't know what sort of interface you might be
using and -hda implies IDE. Where is this macro substitution done?

> I think you need at least Kevin's Acked-by for this.

In the ideal world I could convince the block maintainers to write a new
section to the manual that explains the theory behind the block
subsystem and how things interact and are put together. Until then this
is merely a sticking plaster to make the manual a little more
authoritative than then numerous example command lines our users blindly
copy from online blog posts.

Of course we could* always ask our new AI overlords:

  1. `qemu-system-x86_64 -drive file=disk.img,format=raw,if=virtio`
  In this command, we are running QEMU on an x86_64 CPU architecture and creating a virtual block device using the file `disk.img` as its storage. The format of the file is set to `raw`, which means that it will be accessed directly as a block device. We are using the `virtio` interface to connect the block device to the virtual machine.

  2. `qemu-system-x86_64 -drive file=disk.img,format=qcow2,if=virtio`
  This command is similar to the first one, but the format of the `disk.img` file is set to `qcow2`. This means that the file is a disk image in the QEMU Copy-On-Write format, which allows for more efficient storage of virtual disks. We are still using the `virtio` interface to connect the device to the VM.

  3. `qemu-system-x86_64 -drive file=/dev/sda,if=virtio`
  In this command, we are using a physical block device as the backend for the virtual block device. The device we are using is `/dev/sda`, which is typically the first hard disk on a Linux system. We are still using the `virtio` interface to connect the device to the VM.

  4. `qemu-system-x86_64 -device ide-hd,drive=mydrive -drive file=disk.img,format=qcow2,id=mydrive`
  This command sets up two block devices for the virtual machine. The first device is connected using the `ide-hd` device type, and is connected to a virtual drive called `mydrive`. The second device is a `qcow2` disk image file, also using the virtual drive `mydrive`. The `id` property on the second command tells QEMU to use the virtual drive with the identifier `mydrive` as the backend for this block device.

  The differences between these commands lie in the backend layout of the virtual devices. In the first two commands, we are using file-based storage for the devices, with different formats. In the third command, we are using a physical block device as the backend. In the fourth command, we are using a combination of virtual and file-based storage, with virtual drives connected to individual devices. The choice of backend layout will depend on the specific requirements of the virtual machine and its workload.

  5. `qemu-system-arm -machine virt,gic-version=3 -cpu cortex-a57 -drive file=disk1.qcow2,if=none,id=drive0 -device virtio-blk-device,drive=drive0 -blockdev node-name=my-snapshot,driver=qcow2,file=snapshot.qcow2,backing=drive0 -device virtio-blk-device,drive=my-snapshot`
  In this command, we are using an "ARM virtual machine" and creating a block device using `disk1.qcow2` file as its storage with `if=none` parameter to not attach it directly to the device. Then, we create a virtual device of `virtio-blk-device` type and connecting `drive0` to it using the `-device` option. 

  Next, we are creating a new block device named `my-snapshot` with the `qcow2` driver using `-blockdev` and specifying `file=snapshot.qcow2` as its storage. We are also specifying `backing=drive0`, which means the new block device is a snapshot of `drive0`. Finally, we are creating another virtual device of `virtio-blk-device` type and connecting `my-snapshot` to it using the `-device` option.

  This command creates a snapshot of the original `disk1.qcow2` image, which allows us to create clones of the original disk that are based on different points in time.

  6. `qemu-system-ppc64 -M pseries -drive file=disk1.raw,if=none,id=drive0 -blockdev node-name=my-backup,driver=file,cache.direct=on,cache.no-flush=off,filename=backup.raw -blockdev node-name=my-snapshot,driver=qcow2,file=snapshot.qcow2,backing=my-backup -device virtio-blk-device,drive=my-snapshot`
  In this command, we are using a "PowerPC64 virtual machine" machine type and creating a block device using `disk1.raw` as its storage with `if=none` parameter to not attach it directly to the device. 

  Next, we are creating a backup block device using the `file` driver, and set `cache.direct` to `on` and `cache.no-flush` to `off`, which means that the write operations will go directly to the `backup.raw` image file and reduces the risk of data loss. 

  Then, we create a new block device named `my-snapshot` with the `qcow2` driver using `-blockdev` and specifying `file=snapshot.qcow2` as its storage. We are also specifying `backing=my-backup`, which means the new block device is a snapshot of `my-backup`. Finally, we are creating a virtual device of `virtio-blk-device` type and connecting `my-snapshot` to it using the `-device` option.

  This command creates a snapshot of the `my-backup` block device, which can be used to recover data if the original disk becomes corrupted or lost. The `cache` options are used to ensure that write operations are written directly to the backup file, rather than being buffered in memory and potentially lost in the event of a system crash.

But even these stochastic parrots gloss over the meaning of such
vagaries as if=none.

* just because we could doesn't mean we should

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro

Re: [PATCH 04/11] qemu-options: finesse the recommendations around -blockdev

[Markus Armbruster]

Alex Bennée <alex.bennee@linaro.org> writes:

> Markus Armbruster <armbru@redhat.com> writes:
>
>> Alex Bennée <alex.bennee@linaro.org> writes:
>>
>>> We are a bit premature in recommending -blockdev/-device as the best
>>> way to configure block devices, especially in the common case.
>>> Improve the language to hopefully make things clearer.
>>>
>>> Suggested-by: Michael Tokarev <mjt@tls.msk.ru>
>>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>>> ---
>>>  qemu-options.hx | 8 ++++++--
>>>  1 file changed, 6 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/qemu-options.hx b/qemu-options.hx
>>> index 59bdf67a2c..9a69ed838e 100644
>>> --- a/qemu-options.hx
>>> +++ b/qemu-options.hx
>>> @@ -1143,10 +1143,14 @@ have gone through several iterations as the feature set and complexity
>>>  of the block layer have grown. Many online guides to QEMU often
>>>  reference older and deprecated options, which can lead to confusion.
>>>  
>>> -The recommended modern way to describe disks is to use a combination of
>>> +The most explicit way to describe disks is to use a combination of
>>>  ``-device`` to specify the hardware device and ``-blockdev`` to
>>>  describe the backend. The device defines what the guest sees and the
>>> -backend describes how QEMU handles the data.
>>> +backend describes how QEMU handles the data. The ``-drive`` option
>>> +combines the device and backend into a single command line options
>>> +which is useful in the majority of cases.
>>
>> -drive may look simpler from afar, but it really is a hot mess.  Sadly,
>> we can't get rid of it until we find a replacement for configuring
>> onboard block devices.  We might be able to clean it up some if we
>> accept compatibility breaks.  A new convenience option would be less
>> confusing, I guess.
>
> This is only a partial revert of the original wording which others have
> pointed out was a little too prescriptive. I believe the case of
> snapshot was one where a pure device/blockdev command line is hard to
> use. 
>
>>>                                            Older options like ``-hda``
>>> +bake in a lot of assumptions from the days when QEMU was emulating a
>>> +legacy PC, they are not recommended for modern configurations.
>>>  
>>>  ERST
>>
>> These older options and the non-option argument are simple macros for
>> -drive:
>>
>>     IMG-FILE                    -drive index=0,file=IMG-FILE,media=disk
>>     -hda IMG-FILE               -drive index=0,file=IMG-FILE,media=disk
>>     -hdb IMG-FILE               -drive index=1,file=IMG-FILE,media=disk
>>     -hdc IMG-FILE               -drive index=2,file=IMG-FILE,media=disk
>>     -hdd IMG-FILE               -drive index=3,file=IMG-FILE,media=disk
>>     -cdrom IMG-FILE             -drive index=2,file=IMG-FILE,media=cdrom
>>     -fda IMG-FILE               -drive if=floppy,index=0,file=IMG-FILE
>>     -fdb IMG-FILE               -drive if=floppy,index=1,file=IMG-FILE
>>     -mtdblock IMG-FILE          -drive if=mtd,file=IMG-FILE
>>     -sd IMG-FILE                -drive if=sd,file=IMG-FILE
>>     -pflash IMG-FILE            -drive if=pflash,file=IMG-FILE
>>
>> What assumptions do you have in mind?
>
> I was under the impression things like -hda wouldn't work say on an Arm
> machine because you don't know what sort of interface you might be
> using and -hda implies IDE. Where is this macro substitution done?

qemu_init() calls drive_add() for all these options.

drive_add(TYPE, INDEX, FILE, OPTSTR) creates a QemuOpts in group
"drive".  It sets "if" to if_name[TYPE] unless TYPE is IF_DEFAULT,
"index" to INDEX unless it's negative, and "file" to FILE unless it's
null.  Then it parses OPTSTR on top.

For -hdX, the call looks like

                drive_add(IF_DEFAULT, popt->index - QEMU_OPTION_hda, optarg,
                          HD_OPTS);

We pass IF_DEFAULT, so "if" remains unset.  "index" is set to 0 for
-hda, 1, for -hdb and so forth.  "file" is set to the option argument.
Since HD_OPTS is "media=disk", we set "media" to "disk".

The QemuOpts in config group "drive" get passed to drive_new() via
drive_init_func().  Unset "if" defaults to the current machine's class's
block_default_type.

If a machine doesn't set this member explicitly, it remains zero, which
is IF_NONE.  Documented in blockdev.h:

    typedef enum {
        IF_DEFAULT = -1,            /* for use with drive_add() only */
        /*
         * IF_NONE must be zero, because we want MachineClass member
--->     * block_default_type to default-initialize to IF_NONE
         */
        IF_NONE = 0,
        IF_IDE, IF_SCSI, IF_FLOPPY, IF_PFLASH, IF_MTD, IF_SD, IF_VIRTIO, IF_XEN,
        IF_COUNT
    } BlockInterfaceType;

Questions?

>> I think you need at least Kevin's Acked-by for this.
>
> In the ideal world I could convince the block maintainers to write a new
> section to the manual that explains the theory behind the block
> subsystem and how things interact and are put together. Until then this
> is merely a sticking plaster to make the manual a little more
> authoritative than then numerous example command lines our users blindly
> copy from online blog posts.
>
> Of course we could* always ask our new AI overlords:
>
[...]

> * just because we could doesn't mean we should

Heh!

Re: [PATCH 04/11] qemu-options: finesse the recommendations around -blockdev

[Thomas Huth]

On 03/04/2023 16.55, Markus Armbruster wrote:
> Alex Bennée <alex.bennee@linaro.org> writes:
> 
>> Markus Armbruster <armbru@redhat.com> writes:
>>
>>> Alex Bennée <alex.bennee@linaro.org> writes:
...
>> I was under the impression things like -hda wouldn't work say on an Arm
>> machine because you don't know what sort of interface you might be
>> using and -hda implies IDE. Where is this macro substitution done?
> 
> qemu_init() calls drive_add() for all these options.
> 
> drive_add(TYPE, INDEX, FILE, OPTSTR) creates a QemuOpts in group
> "drive".  It sets "if" to if_name[TYPE] unless TYPE is IF_DEFAULT,
> "index" to INDEX unless it's negative, and "file" to FILE unless it's
> null.  Then it parses OPTSTR on top.
> 
> For -hdX, the call looks like
> 
>                  drive_add(IF_DEFAULT, popt->index - QEMU_OPTION_hda, optarg,
>                            HD_OPTS);
> 
> We pass IF_DEFAULT, so "if" remains unset.  "index" is set to 0 for
> -hda, 1, for -hdb and so forth.  "file" is set to the option argument.
> Since HD_OPTS is "media=disk", we set "media" to "disk".
> 
> The QemuOpts in config group "drive" get passed to drive_new() via
> drive_init_func().  Unset "if" defaults to the current machine's class's
> block_default_type.
> 
> If a machine doesn't set this member explicitly, it remains zero, which
> is IF_NONE.  Documented in blockdev.h:
> 
>      typedef enum {
>          IF_DEFAULT = -1,            /* for use with drive_add() only */
>          /*
>           * IF_NONE must be zero, because we want MachineClass member
> --->     * block_default_type to default-initialize to IF_NONE
>           */
>          IF_NONE = 0,
>          IF_IDE, IF_SCSI, IF_FLOPPY, IF_PFLASH, IF_MTD, IF_SD, IF_VIRTIO, IF_XEN,
>          IF_COUNT
>      } BlockInterfaceType;
> 
> Questions?

How's the average user supposed to know that? Our qemu-options.hx just says: 
"-hda/-hdb file  use 'file' as IDE hard disk 0/1 image"...

  Thomas

Re: [PATCH 04/11] qemu-options: finesse the recommendations around -blockdev

[Markus Armbruster]

Thomas Huth <thuth@redhat.com> writes:

> On 03/04/2023 16.55, Markus Armbruster wrote:
>> Alex Bennée <alex.bennee@linaro.org> writes:
>> 
>>> Markus Armbruster <armbru@redhat.com> writes:
>>>
>>>> Alex Bennée <alex.bennee@linaro.org> writes:
> ...
>>> I was under the impression things like -hda wouldn't work say on an Arm
>>> machine because you don't know what sort of interface you might be
>>> using and -hda implies IDE. Where is this macro substitution done?
>> 
>> qemu_init() calls drive_add() for all these options.
>> 
>> drive_add(TYPE, INDEX, FILE, OPTSTR) creates a QemuOpts in group
>> "drive".  It sets "if" to if_name[TYPE] unless TYPE is IF_DEFAULT,
>> "index" to INDEX unless it's negative, and "file" to FILE unless it's
>> null.  Then it parses OPTSTR on top.
>> 
>> For -hdX, the call looks like
>> 
>>                  drive_add(IF_DEFAULT, popt->index - QEMU_OPTION_hda, optarg,
>>                            HD_OPTS);
>> 
>> We pass IF_DEFAULT, so "if" remains unset.  "index" is set to 0 for
>> -hda, 1, for -hdb and so forth.  "file" is set to the option argument.
>> Since HD_OPTS is "media=disk", we set "media" to "disk".
>> 
>> The QemuOpts in config group "drive" get passed to drive_new() via
>> drive_init_func().  Unset "if" defaults to the current machine's class's
>> block_default_type.
>> 
>> If a machine doesn't set this member explicitly, it remains zero, which
>> is IF_NONE.  Documented in blockdev.h:
>> 
>>      typedef enum {
>>          IF_DEFAULT = -1,            /* for use with drive_add() only */
>>          /*
>>           * IF_NONE must be zero, because we want MachineClass member
>> --->     * block_default_type to default-initialize to IF_NONE
>>           */
>>          IF_NONE = 0,
>>          IF_IDE, IF_SCSI, IF_FLOPPY, IF_PFLASH, IF_MTD, IF_SD, IF_VIRTIO, IF_XEN,
>>          IF_COUNT
>>      } BlockInterfaceType;
>> 
>> Questions?
>
> How's the average user supposed to know that? Our qemu-options.hx just says: 
> "-hda/-hdb file  use 'file' as IDE hard disk 0/1 image"...

Ancient doc bug.  Should have been updated in commit 2d0d2837dcf
(Support default block interfaces per QEMUMachine) back in 2012.