On Wed, Sep 22, 2021 at 08:16:21PM -0400, John Snow wrote:
> Add a warning for when 'iotests' runs against a qemu namespace that
> isn't the one in the source tree. This might occur if you have
> (accidentally) installed the Python namespace package to your local
> packages.
IIUC, it is/was a valid use case to run the iotests on arbitrary
QEMU outside the source tree ie a distro packaged binary set.
Are we not allowing the tests/qemu-iotests/* content to be
run outside the context of the main source tree for this
purpose ?
eg consider if Fedora/RHEL builds put tests/qemu-iotests/* into
a 'qemu-iotests' RPM, which was installed and used with a distro
package python-qemu ?
(1) "isn't the one in the source tree" is imprecise language here. The real key is that it must be the QEMU namespace located at "
testenv.py/../../../python/qemu". This usually means the source tree, but it'd work in any identically structured filesystem hierarchy.
(2) The preceding patch might help illustrate this. At present the iotests expect to be able to find the 'qemu' python packages by navigating directories starting from their own location (and not CWD). What patch 1 does is to centralize the logic that has to go "searching" for the python packages into the init_directories method in testenv.py so that each individual test doesn't have to repeat it.
i.e. before patch 1, iotests.py does this:
sys.path.append(os.path.join(os.path.dirname(__file__), '..', '..', 'python'))
so we'll always crawl to ../../python from wherever iotests.py is.
After patch 1, we're going to crawl to the same location, but starting from testenv.py instead. testenv.py and iotests.py are in the same directory, so I expect whatever worked before (and in whatever environment) will continue working after. I can't say with full certainty in what circumstances we support running iotests, but I don't think I have introduced any new limitation with this.
> (I'm not going to say that this is because I bit myself with this,
> but. You can fill in the blanks.)
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
> tests/qemu-iotests/testenv.py | 21 ++++++++++++++++++++-
> 1 file changed, 20 insertions(+), 1 deletion(-)
>
> diff --git a/tests/qemu-iotests/testenv.py b/tests/qemu-iotests/testenv.py
> index 88104dace90..8a43b193af5 100644
> --- a/tests/qemu-iotests/testenv.py
> +++ b/tests/qemu-iotests/testenv.py
> @@ -16,6 +16,8 @@
> # along with this program. If not, see <http://www.gnu.org/licenses/>.
> #
>
> +import importlib.util
> +import logging
> import os
> import sys
> import tempfile
> @@ -25,7 +27,7 @@
> import random
> import subprocess
> import glob
> -from typing import List, Dict, Any, Optional, ContextManager
> +from typing import List, Dict, Any, Optional, ContextManager, cast
>
> DEF_GDB_OPTIONS = 'localhost:12345'
>
> @@ -112,6 +114,22 @@ def init_directories(self) -> None:
> # Path where qemu goodies live in this source tree.
> qemu_srctree_path = Path(__file__, '../../../python').resolve()
>
> + # warn if we happen to be able to find 'qemu' packages from an
> + # unexpected location (i.e. the package is already installed in
> + # the user's environment)
> + qemu_spec = importlib.util.find_spec('qemu.qmp')
> + if qemu_spec:
> + spec_path = Path(cast(str, qemu_spec.origin))
> + try:
> + _ = spec_path.relative_to(qemu_srctree_path)
> + except ValueError:
> + self._logger.warning(
> + "WARNING: 'qemu' package will be imported from outside "
> + "the source tree!")
> + self._logger.warning(
> + "Importing from: '%s'",
> + spec_path.parents[1])
It feels to me like the scenario we're blocking here is actally
the scenario we want to aim for.
It isn't blocking it, it's just a warning. At present, iotests expects that it's using the version of the python packages that are in the source tree / tarball / whatever. Since I converted those packages to be installable to your environment, I introduced an ambiguity about which version iotests is actually using: the version you installed, or the version in the source tree? This is just merely a warning to let you know that iotests is technically doing something new. ("Hey, you're running some other python code than what iotests has done for the past ten years. Are you cool with that?")
You're right, though: my actual end-goal (after a lot of pending cleanups I have in-flight, I am getting to it ...!) is to eventually pivot iotests to always using qemu as an installed package and wean it off of using directory-crawling to find its dependencies. We are close to doing that. When we do transition to that, the warning can be dropped as there will no longer be an ambiguity over which version it is using. There are some questions for me to ponder first over exactly how the environment setup should be accomplished, though.