Re: [PATCH v5 13/16] tests: remove "make check-venv" target

["Alex Bennée" <alex.bennee@linaro.org>]

John Snow <jsnow@redhat.com> writes:

> On Thu, Feb 5, 2026 at 12:09 PM Alex Bennée <alex.bennee@linaro.org> wrote:
>>
>> John Snow <jsnow@redhat.com> writes:
>>
>> > With the qemu.qmp and qemu.machine dependencies now installed by default
>> > at configure time and additional dependencies required by functional
>> > testing installed on demand, there is no longer any reason to have an
>> > explicit target.
>> >
>> > FIXME: This forces image regeneration for vm tests whenever Make
>> > determines that the image needs to be rebuilt; which is a regression
>> > over the previous behavior.
>> >
>> > Signed-off-by: John Snow <jsnow@redhat.com>
>> > ---
>> >  tests/Makefile.include    | 22 ++--------------------
>> >  tests/vm/Makefile.include | 24 +++++++-----------------
>> >  2 files changed, 9 insertions(+), 37 deletions(-)
>> >
>> > diff --git a/tests/Makefile.include b/tests/Makefile.include
>> > index f28c9e329aa..2a203e23718 100644
>> > --- a/tests/Makefile.include
>> > +++ b/tests/Makefile.include
>> > @@ -21,7 +21,6 @@ ifneq ($(filter $(all-check-targets), check-softfloat),)
>> >  endif
>> >       @echo
>> >       @echo " $(MAKE) check-report.junit.xml   Generates an aggregated XML test report"
>> > -     @echo " $(MAKE) check-venv               Creates a Python venv for tests"
>> >       @echo " $(MAKE) check-clean              Clean the tests and related data"
>> >       @echo
>> >       @echo "The following are useful for CI builds"
>> > @@ -92,33 +91,16 @@ clean-tcg: $(CLEAN_TCG_TARGET_RULES)
>> >  .PHONY: distclean-tcg
>> >  distclean-tcg: $(DISTCLEAN_TCG_TARGET_RULES)
>> >
>> > -# Python venv for running tests
>> > -
>> > -.PHONY: check-venv
>> > -
>> >  # Build up our target list from the filtered list of ninja targets
>> >  TARGETS=$(patsubst libqemu-%.a, %, $(filter libqemu-%.a, $(ninja-targets)))
>> >
>> > -TESTS_VENV_TOKEN=$(BUILD_DIR)/pyvenv/tests.group
>> > -
>> > -quiet-venv-pip = $(quiet-@)$(call quiet-command-run, \
>> > -    $(PYTHON) -m pip -q --disable-pip-version-check $1, \
>> > -    "VENVPIP","$1")
>> > -
>> > -$(TESTS_VENV_TOKEN): $(SRC_PATH)/pythondeps.toml
>> > -     $(call quiet-venv-pip,install -e "$(SRC_PATH)/python/")
>> > -     $(MKVENV_ENSUREGROUP) $< tooling functests
>> > -     $(call quiet-command, touch $@)
>> > -
>> > -check-venv: $(TESTS_VENV_TOKEN)
>> > -
>> >  FUNCTIONAL_TARGETS=$(patsubst %-softmmu,check-functional-%, $(filter %-softmmu,$(TARGETS)))
>> >  .PHONY: $(FUNCTIONAL_TARGETS)
>> > -$(FUNCTIONAL_TARGETS): check-venv
>> > +$(FUNCTIONAL_TARGETS):
>> >       @$(MAKE) SPEED=thorough $(subst -functional,-func,$@)
>> >
>> >  .PHONY: check-functional
>> > -check-functional: check-venv
>> > +check-functional:
>> >       @$(NINJA) precache-functional
>> >       @$(PYTHON) $(SRC_PATH)/scripts/clean_functional_cache.py
>> >       @QEMU_TEST_NO_DOWNLOAD=1 $(MAKE) SPEED=thorough check-func check-func-quick
>> > diff --git a/tests/vm/Makefile.include b/tests/vm/Makefile.include
>> > index 14188bba1c6..095ec2eefa3 100644
>> > --- a/tests/vm/Makefile.include
>> > +++ b/tests/vm/Makefile.include
>> > @@ -1,14 +1,5 @@
>> >  # Makefile for VM tests
>> >
>> > -# Hack to allow running in an unconfigured build tree
>> > -ifeq ($(realpath $(SRC_PATH)),$(realpath .))
>> > -VM_PYTHON = PYTHONPATH=$(SRC_PATH)/python /usr/bin/env python3
>> > -VM_VENV =
>> > -else
>> > -VM_PYTHON = $(PYTHON)
>> > -VM_VENV = check-venv
>> > -endif
>> > -
>>
>> It's a shame to loose this because the build directory should have no
>> influence on what we build in the VM. Surely if we have qmp installed on
>> the system (and therefor its deps) we should still want the ability to
>> build in the src dir. Currently I get:
>>
>>   ➜  make vm-build-netbsd V=1
>>   tests/vm/netbsd  --debug     --source-path . --image "/home/alex/.cache/qemu-vm/images/netbsd.img" --force --build-image /home/alex/.cache/qemu-vm/images/netbsd.img
>>   Traceback (most recent call last):
>>     File "/home/alex/lsrc/qemu.git/tests/vm/netbsd", line 19, in <module>
>>       import basevm
>>     File "/home/alex/lsrc/qemu.git/tests/vm/basevm.py", line 32, in <module>
>>       from qemu.machine import QEMUMachine
>>   ModuleNotFoundError: No module named 'qemu'
>>   make: *** [tests/vm/Makefile.include:86: /home/alex/.cache/qemu-vm/images/netbsd.img] Error 1
>
> Ah, I see.... it used to be possible to run the VM tests *without a
> build directory at all*.
>
> So, this used to work because of this bit in tests/vm/Makefile.include:
>
> # Hack to allow running in an unconfigured build tree
> ifeq ($(realpath $(SRC_PATH)),$(realpath .))
> VM_PYTHON = PYTHONPATH=$(SRC_PATH)/python /usr/bin/env python3
> VM_VENV =
> else
> VM_PYTHON = $(PYTHON)
> VM_VENV = check-venv
> endif
>
> What this did was effectively treat qemu.git/python/ as an installed
> package directory and picked the most likely culprit for your actual
> python interpreter location. (Note that the configure script is a bit
> more thorough in picking a python interpreter to use that is bypassed
> here.) It's possible we can continue to do this, however, it will
> begin requiring that you just-so-happen to have qemu.qmp available in
> your python environment. The list of dependencies you might need for
> this to work successfully may increase as the years go by, so this is
> a little bit "porcelain". I'm not sure I like this, just because I
> don't wanna be on the hook for mysterious failures down the line.
>
> Otherwise, to get all of the dependencies and python configuration
> managed for you, you have to do this:
>
> mkdir build && pushd build && ../configure && make vm-test-netbsd
>
> ... But you don't actually have to build, and in fact you don't even
> really need to actually run configure either, so, hm.... yeah, how
> about this, using a new "vm-venv" folder in the source tree as the
> virtual environment location, using a dependency hook like check-venv
> but now localized specifically for VM tests benefit and only when it
> is run from the source tree:
>
> jsnow@jsnow-thinkpadp16vgen1 ~/s/qemu (python_drop_qmp_pyvenv)> make
> vm-build-netbsd
> /usr/bin/env python3 ./python/scripts/mkvenv.py create vm-venv
> mkvenv: Creating non-isolated virtual environment at 'vm-venv'
> /home/jsnow/src/qemu/vm-venv/bin/python3
> ./vm-venv/bin/python3 ./python/scripts/mkvenv.py ensuregroup --online
> ./pythondeps.toml tooling functests
> mkvenv: checking for qemu.qmp>=0.0.5
> mkvenv: checking for pip>=21.3.1
> mkvenv: checking for setuptools>=53.0.0
> mkvenv: checking for wheel>=0.36.2
> mkvenv: checking for pygdbmi>=0.11.0.0
> mkvenv: installing qemu.qmp==0.0.5
> mkvenv: installing /home/jsnow/src/qemu/python
>     VM-IMAGE netbsd
> ### Preparing iso and disk image ...
> Formatting '/home/jsnow/.cache/qemu-vm/images/netbsd.img.tmp',
> fmt=qcow2 cluster_size=65536 extended_l2=off compression_type=zlib
> size=21474836480 lazy_refcounts=off refcount_bits=16
>
> This will take care of dependencies and everything for you and won't
> interfere with normal configured builds at all. Only downside is a new
> temporary folder that clutters up the source root that you'll have to
> delete now and then.

Can we just add it to .gitignore or do we want it to be visible so users
don't forget?

> Otherwise it uses the same dependency resolution
> as a normal full-blooded configure. Only difference is that it uses a
> simpler heuristic for which python to use (/usr/bin/env python3).
>
> Sound good?

Yes that sounds good.

> --js
>
>>
>> >  .PHONY: vm-build-all vm-clean-all
>> >
>> >  EFI_AARCH64 = $(wildcard $(BUILD_DIR)/pc-bios/edk2-aarch64-code.fd)
>> > @@ -90,11 +81,10 @@ vm-clean-all:
>> >
>> >  $(IMAGES_DIR)/%.img: $(SRC_PATH)/tests/vm/% \
>> >                       $(SRC_PATH)/tests/vm/basevm.py \
>> > -                     $(SRC_PATH)/tests/vm/Makefile.include \
>> > -                     $(VM_VENV)
>> > +                     $(SRC_PATH)/tests/vm/Makefile.include
>> >       @mkdir -p $(IMAGES_DIR)
>> >       $(call quiet-command, \
>> > -             $(VM_PYTHON) $< \
>> > +             $(PYTHON) $< \
>> >               $(if $(V)$(DEBUG), --debug) \
>> >               $(if $(GENISOIMAGE),--genisoimage $(GENISOIMAGE)) \
>> >               $(if $(QEMU_LOCAL),--build-path $(BUILD_DIR)) \
>> > @@ -102,14 +92,14 @@ $(IMAGES_DIR)/%.img:     $(SRC_PATH)/tests/vm/% \
>> >               $(if $(LOG_CONSOLE),--log-console) \
>> >               --source-path $(SRC_PATH) \
>> >               --image "$@" \
>> > -             $(if $(filter-out check-venv, $?), --force) \
>> > +             --force \
>>
>> I think this is fine, as danp says if the deps are touched we should
>> re-build the VM.
>>
>> >               --build-image $@, \
>> >               "  VM-IMAGE $*")
>> >
>> >  # Build in VM $(IMAGE)
>> > -vm-build-%: $(IMAGES_DIR)/%.img $(VM_VENV)
>> > +vm-build-%: $(IMAGES_DIR)/%.img
>> >       $(call quiet-command, \
>> > -             $(VM_PYTHON) $(SRC_PATH)/tests/vm/$* \
>> > +             $(PYTHON) $(SRC_PATH)/tests/vm/$* \
>> >               $(if $(V)$(DEBUG), --debug) \
>> >               $(if $(DEBUG), --interactive) \
>> >               $(if $(J),--jobs $(J)) \
>> > @@ -133,9 +123,9 @@ vm-boot-serial-%: $(IMAGES_DIR)/%.img
>> >               -device virtio-net-pci,netdev=vnet \
>> >       || true
>> >
>> > -vm-boot-ssh-%: $(IMAGES_DIR)/%.img $(VM_VENV)
>> > +vm-boot-ssh-%: $(IMAGES_DIR)/%.img
>> >       $(call quiet-command, \
>> > -             $(VM_PYTHON) $(SRC_PATH)/tests/vm/$* \
>> > +             $(PYTHON) $(SRC_PATH)/tests/vm/$* \
>> >               $(if $(J),--jobs $(J)) \
>> >               $(if $(V)$(DEBUG), --debug) \
>> >               $(if $(QEMU_LOCAL),--build-path $(BUILD_DIR)) \
>>
>> --
>> Alex Bennée
>> Virtualisation Tech Lead @ Linaro
>>

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro