[v1,3/7] ts-xen-build: Install livepatch regressions tests. [and 1 more messages]

Commit Message

Konrad Rzeszutek Wilk Nov. 21, 2016, 9:28 p.m. UTC

On Thu, Nov 17, 2016 at 11:49:19AM +0000, Ian Jackson wrote:
> Konrad Rzeszutek Wilk writes ("[PATCH v1 3/7] ts-xen-build: Install livepatch regressions tests."):
> > That come with the Xen git tree (see xen/test/).
> 
> I think this and the "build them" patch should be combined.
> 
> > +    buildcmd_stamped_logged(600, 'xen', 'tests-install', '',<<END,'') if $dokconfig;
> 
> Can you keep the lines down to 75 characters or less please ?
> 
> > +        if test -d xen/test; then
> > +           mkdir -p dist/install/usr/lib/debug
> > +           livepatch_files=`find xen/test/livepatch -name '*.livepatch' -print`
> > +           cp \$livepatch_files dist/install/usr/lib/debug
> 
> Should this not be in the xen.git Makefiles ?

Jan didn't like it (as part of the normal 'install' stanza).

I could add it in xen/test/Makefile, but I had a hard time executing
anything inside 'xen' sub-directories by themselves, aka:

 make -C xen/test install

As the 'xen/test/livepatch/Makefile' does:
include $(XEN_ROOT)/Config.mk

(and other) and the XEN_ROOT is not available unless you run it from
within 'xen' directory.

Which means I would have to add a new top-level target, such as:

 make -C xen test_install

or such. But then it is not exactly sure where one would install
the "tests"? /usr/lib/debug? /usr/lib/xen/debug/ ?

I figured it would be easier if it was left unimplemented and folks
just copied the files out of there.

 
> 
> Also, the result of this is that the tests end up in the tools output
> because you haven't fixed `divide'.  Background: each osstest
> invocation of ts-xen-build produces two primary deliverables: `' and
> `xen' aka `dist' and `xendist'.
> 
> I think, but I'm not sure, that these patches contain hypervisor code
> and should be in `xendist'.

In the cover letter you mentioned that it may be good to have an
xenlptdist.tar.gz which would only contain the livepatch test-cases.

And then we could use the existence of that file as a check for
the hypervisor having the support?

If I squash this patch in this one:


It should in theory (testing it now) do the right thing. Now just need
to figure out how to gate the execution of ts-livepatch on the existence
of that file (in a non-hackish way).

> 
> Thanks,
> Ian.
> 
> _______________________________________________
> Xen-devel mailing list
> [e-mail]
> https://lists.xen.org/xen-devel

Comments

Ian Jackson Dec. 12, 2016, 4:12 p.m. UTC | #1

Konrad Rzeszutek Wilk writes ("Re: [Xen-devel] [PATCH v1 3/7] ts-xen-build: Install livepatch regressions tests. [and 1 more messages]"):
> On Thu, Nov 17, 2016 at 11:49:19AM +0000, Ian Jackson wrote:
> > Konrad Rzeszutek Wilk writes ("[PATCH v1 3/7] ts-xen-build: Install livepatch regressions tests."):
> > > +        if test -d xen/test; then
> > > +           mkdir -p dist/install/usr/lib/debug
> > > +           livepatch_files=`find xen/test/livepatch -name '*.livepatch' -print`
> > > +           cp \$livepatch_files dist/install/usr/lib/debug
> > 
> > Should this not be in the xen.git Makefiles ?
> 
> Jan didn't like it (as part of the normal 'install' stanza).

I think I agree with Jan that it should not be included in the result
of `make install'.

But can it not be a new toplevel target ?  Jan, would that be OK with
you ?

Normally this kind of file-searching and -installing logic is best
done in the xen.git Makefiles (or indeed in the Makefiles of whatever
is being tested), because that way xen.git can change what these files
are, how they are laid out in the source code, and so on, without
disturbing osstest.

What you do above makes the xen.git source tree layout part of the
public interface.  (Or, perhaps, causes osstest to falsely assume that
it is part of the public interface...)

> Which means I would have to add a new top-level target, such as:
>  make -C xen test_install
> or such. But then it is not exactly sure where one would install
> the "tests"? /usr/lib/debug? /usr/lib/xen/debug/ ?

Yes, something like that.  I don't think the path matters very much.
I think something like /usr[/local]/lib/xen/debug is probably best.

> > Also, the result of this is that the tests end up in the tools output
> > because you haven't fixed `divide'.  Background: each osstest
> > invocation of ts-xen-build produces two primary deliverables: `' and
> > `xen' aka `dist' and `xendist'.
> > 
> > I think, but I'm not sure, that these patches contain hypervisor code
> > and should be in `xendist'.
> 
> In the cover letter you mentioned that it may be good to have an
> xenlptdist.tar.gz which would only contain the livepatch test-cases.

Yes.

> And then we could use the existence of that file as a check for
> the hypervisor having the support?

You mean the existence of that output.  Earlier I wrote:

 ] * If you split your hypervisor test patches into their own dist (eg
 ]   `xenlptdist' for `live patch test'), and do not generate that
 ]    output when the Xen build didn't support it, the existing machinery
 ]    will spot that your live tests are blocked on earlier Xen, and
 ]    report that as a single non-regression test failure "never pass"
 ]    for earlier Xens - and it will do that before it allocates
 ]    a host.

> It should in theory (testing it now) do the right thing. Now just need
> to figure out how to gate the execution of ts-livepatch on the existence
> of that file (in a non-hackish way).

What I mean is that you do not need to do anything.  Your new test job
will automatically be "started" on every Xen branch, after the build
jobs it refers to have completed.  But the first thing it does, before
it even goes as far as trying to find a host to run on, is to check
whether the builds it uses succeeded.

I think (and I asserted above) that if your xenlptdist output (or
whatever you want to call it) is not generated at all, by
ts-xen-build, this check will fail, and the attempt to run livepatch
tests will be abandoned.

Ie all will be as it should be.

Ian.

Patch

diff --git a/ts-xen-build b/ts-xen-build
index 1b36b9c..1137947 100755
--- a/ts-xen-build
+++ b/ts-xen-build
@@ -170,11 +170,11 @@  END
                 $make_prefix make -C xen tests
             fi
 END
-    buildcmd_stamped_logged(600, 'xen', 'tests-install', '',<<END,'') if $dokconfig;
+    buildcmd_stamped_logged(600, 'xen', 'xenlpt-install', '',<<END,'') if $dokconfig;
         if test -d xen/test; then
-           mkdir -p dist/install/usr/lib/debug
+           mkdir -p dist/xenlptinstall/usr/lib/debug
            livepatch_files=`find xen/test/livepatch -name '*.livepatch' -print`
-           cp \$livepatch_files dist/install/usr/lib/debug
+           cp \$livepatch_files dist/xenlptinstall/usr/lib/debug
         fi
 END
 }