Re: [PATCH] tests/vpci: install test

[Jan Beulich <jbeulich@xxxxxxxx>]

On 13.03.2023 12:15, Roger Pau Monné wrote:
> On Mon, Mar 13, 2023 at 11:43:43AM +0100, Jan Beulich wrote:
>> On 13.03.2023 11:31, Roger Pau Monné wrote:
>>> On Fri, Mar 10, 2023 at 03:32:41PM +0100, Jan Beulich wrote:
>>>> On 10.03.2023 14:38, Roger Pau Monné wrote:
>>>>> On Fri, Mar 10, 2023 at 12:06:29PM +0100, Jan Beulich wrote:
>>>>>> On 09.03.2023 17:58, Roger Pau Monne wrote:
>>>>>>> Introduce an install target, like it's used by other tests.  This
>>>>>>> allows running the test on the installed systems, which is easier than
>>>>>>> running it during the build phase when dealing with automated testing.
>>>>>>> Strictly speaking the vpci test doesn't require to be run on a Xen
>>>>>>> host currently, but that allows easier integration with logic that
>>>>>>> runs the rest of the tests.
>>>>>>
>>>>>> I accept that as a possible way of looking at things, but personally I
>>>>>> remain unconvinced of this model. To me what is installed should be of
>>>>>> value to users. If there was a properly separated directory where all
>>>>>> (and only) tests were put, I might agree with installing. (Nevertheless
>>>>>> this isn't an objection, merely a remark.)
>>>>>>
>>>>>>> While there also adjust the makefile to use $(RM), and rename the
>>>>>>> resulting binary to use a dash instead of an underscore (again to
>>>>>>> match the rest of the tests).
>>>>>>>
>>>>>>> Since the resulting test binary is now part of the distribution CC
>>>>>>> must be used instead of HOSTCC.
>>>>>>
>>>>>> This breaks the run: goal, doesn't it? If the new mode is wanted, I
>>>>>> think the two kinds of binaries (and rules) need separating (maybe a
>>>>>> way can be found to avoid duplicating the rules, which would seem
>>>>>> desirable).
>>>>>
>>>>> The run rule is not hooked up in any of the upper level makefile logic,
>>>>
>>>> What about the run-tests-% goal in the top level Makefile?
>>>
>>> Urg, I wasn't aware of that target.  I assume just removing the `run`
>>> target from the vpci makefile would be an acceptable solution then.
>>
>> I'm afraid I wouldn't view this as acceptable. I would very much like
>> to retain these run: goals, as I view it as important that such tests
>> be possible to run easily and right from the build area. What might be
>> acceptable to me is if ...
>>
>>> It's still the user that needs to explicitly call run-tests-vpci, so
>>> it would better know that HOSTCC == CC before attempting that.
>>
>> ... the run: rune would be enclosed in "ifeq ($(CC),$(HOSTCC))". Yet
>> even that is fragile. For tests like this I view it as secondary to
>> be runnable on the destination architecture, and hence I continue to
>> think that if installing such tests is really wanted, binaries for
>> host and target should be properly separated.
> 
> vpci test is special in this regard when compared to the rest of the
> tests that do make use of the hypercall interface to a degree, and
> hence are not expected to be run from the build host as they require
> to be run from a Xen domain.

Right, which is why I said "for tests like this" (which includes in
particular also the x86 emulator harness).

> I think the benefit of having the test run part of XenRT is greater
> than the downfall of installing a test as part of the distribution.
> 
> I've added a guard to the `run` target in order to check that HOSTCC
> == CC, hope that is enough.

With that at least I won't further object to the change.

Jan