Re: [RFC PATCH 1/1] qtest/migration: Support more than one QEMU binary

[Juan Quintela]

Fabiano Rosas <farosas@suse.de> wrote:
> Daniel P. Berrangé <berrange@redhat.com> writes:
>
>> On Tue, Oct 03, 2023 at 05:24:50PM +0200, Philippe Mathieu-Daudé wrote:
[...]

>> $ cat myqemu.dkr 
>> FROM fedora:38
>>
>> RUN dnf -y install qemu-kvm
>>
>> $ podman build -f myqemu.dkr --tag myqemu .
>>
>> $ cat > myqemu <<EOF
>> #!/bin/sh
>> exec podman run --volume /tmp=/tmp --security-opt label=disable myqemu 
>> qemu-system-x86_64 "$@"
>>
>> $ chmod +x myqemu
>>
>> $ QTEST_QEMU_BINARY=./myqemu.sh  ./build/tests/qtest/rtc-test
>
> I'm favor of this. I usually set that variable to something like 'gdb
> --args ...' and it works just fine.
>
>> Except we fail on the last step, because bind mounts don't make UNIX domain
>> sockets accessible. So we can see the /tmp/qtest-$PID.sock in the container,
>> but it can't be used.
>>
>> UNIX domain sockets in the filesystem are tied to the mount namespace, and
>> podman/docker inherantly creates a new mount namespace making the UNIX
>> domani socket inaccessible.
>>
>> UNIX domain sockets in the abstract namespace, however, are tied to the
>> network namespace, so if you used podman --network host, they should be
>> accessible.
>>
>> libqtest could be changed to use abstract UNIX domain sockets on Linux
>> only, and likely unlock the use of podman for QEMU.

That is one idea, but why can't we convince a container to compile
_both_ qemus?

I am not familiar with containers, but:
- We already know how to compile a qemu inside a container
- We can teach it to compile $HEAD and v8.0.0 (or whatever)

And do the test inside, right?

On the other hand, your approach has the advantage that one can test
opensuse qemu against fedora qemu, and similar.  Not sure how useful is
that, though.

[lots of code to find common machine types]

I think that it is just easier to pass the machine type we want to test
to whatever script we have.  Specially where [sane] architectures like
arm don't have a default machine type (no, I haven't double checked if
that has changed lately).

>> IMHO, we should just create a new qtest_init_env variant, that is the
>> same as qtest_init, but accepts an environment variable name to use as
>> an override.
>>
>> eg
>>
>>    qtest_init_env("QTEST_QEMU_BINARY_SRC", extra_args)

That was going to be my suggestion.

>> it would look for $QTEST_QEMU_BINARY_SRC and if not found automatically
>> fallback to $QTEST_QEMU_BINARY.
>>
>
> This was initially intended to be an off-tree patch that I could use to
> test migration compatibility, so I avoided touching libqtest. Now that I
> learned this might be of interest we can make it less hackish.
>
>> I don't think there's any need to explicitly forbid setting both
>> QTEST_QEMU_BINARY_SRC and QTEST_QEMU_BINARY_DST at the same time.
>>
>
> This is a little biased on my usage of migration-test from the command
> line. Adding one of SRC|DST is easier when coming/going from a test that
> already uses QTEST_QEMU_BINARY. No big deal, of course.
>
> We'll just have to do something about qtest_get_machines(),
> qtest_has_device() and qtest_get_arch(), which expect QTEST_QEMU_BINARY
> to be present.

I think we can do the same trick here:

qtest_has_device_env("VARIABLE") and let qemu_has_device() just call it
with QTEST_QEMU_BINARY.  Same for the others.

It is more, if we can do it easy, we can do that qtest_init_env() checks
in the case that variable is not QTEST_QEMU_BINARY that this one is also
set, and assume in the rest of the code that options are compatible
between whatever we passed an QTEST_QEMU_BINARY, that way we don't need
to change anything else.  Well, just put a big warning in a comment
saying that using qtest_init_env() means that you know what you are
doing.

Later, Juan.