Re: [PATCH v2 5/5] automation: add x86-64 livepatching test

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Wed, 13 Dec 2023, Roger Pau Monné wrote:
> On Wed, Nov 29, 2023 at 07:03:10PM -0800, Stefano Stabellini wrote:
> > On Tue, 28 Nov 2023, Roger Pau Monne wrote:
> > > Introduce a new gitlab tests for livepatching, using 
> > > livepatch-build-tools,
> > > which better reflects how downstreams build live patches rather than the
> > > in-tree tests.
> > > 
> > > The tests applies the dummy in-tree patch example, checks that the patch 
> > > is
> > > applied correctly and then reverts and unloads it.
> > > 
> > > Signed-off-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>
> > > ---
> > >  automation/gitlab-ci/build.yaml               |  8 +++
> > >  automation/gitlab-ci/test.yaml                |  8 +++
> > >  automation/scripts/build                      | 21 ++++++
> > >  .../scripts/qemu-alpine-x86_64-livepatch.sh   | 68 +++++++++++++++++++
> > >  4 files changed, 105 insertions(+)
> > >  create mode 100755 automation/scripts/qemu-alpine-x86_64-livepatch.sh
> > > 
> > > diff --git a/automation/gitlab-ci/build.yaml 
> > > b/automation/gitlab-ci/build.yaml
> > > index 32af30ccedc9..22026df51b87 100644
> > > --- a/automation/gitlab-ci/build.yaml
> > > +++ b/automation/gitlab-ci/build.yaml
> > > @@ -358,6 +358,14 @@ alpine-3.18-gcc-debug:
> > >    variables:
> > >      CONTAINER: alpine:3.18
> > >  
> > > +alpine-3.18-gcc-livepatch:
> > > +  extends: .gcc-x86-64-build
> > > +  variables:
> > > +    CONTAINER: alpine:3.18
> > > +    LIVEPATCH: y
> > > +    EXTRA_XEN_CONFIG: |
> > > +      CONFIG_LIVEPATCH=y
> > > +
> > >  debian-stretch-gcc-debug:
> > >    extends: .gcc-x86-64-build-debug
> > >    variables:
> > > diff --git a/automation/gitlab-ci/test.yaml 
> > > b/automation/gitlab-ci/test.yaml
> > > index 6aabdb9d156f..58a90be5ed0e 100644
> > > --- a/automation/gitlab-ci/test.yaml
> > > +++ b/automation/gitlab-ci/test.yaml
> > > @@ -459,3 +459,11 @@ qemu-smoke-ppc64le-powernv9-gcc:
> > >    needs:
> > >      - qemu-system-ppc64-8.1.0-ppc64-export
> > >      - debian-bullseye-gcc-ppc64le-debug
> > > +
> > > +qemu-alpine-x86_64-gcc-livepatch:
> > > +  extends: .qemu-x86-64
> > > +  script:
> > > +    - ./automation/scripts/qemu-alpine-x86_64-livepatch.sh 2>&1 | tee 
> > > ${LOGFILE}
> > > +  needs:
> > > +    - *x86-64-test-needs
> > > +    - alpine-3.18-gcc-livepatch
> > > diff --git a/automation/scripts/build b/automation/scripts/build
> > > index b3c71fb6fb60..0a0a6dceb08c 100755
> > > --- a/automation/scripts/build
> > > +++ b/automation/scripts/build
> > > @@ -103,3 +103,24 @@ else
> > >      cp -r dist binaries/
> > >      if [[ -f xen/xen ]] ; then cp xen/xen binaries/xen; fi
> > >  fi
> > > +
> > > +if [[ "$LIVEPATCH" == "y" ]]; then
> > > +    # Build a test livepatch using livepatch-build-tools.
> > > +
> > > +    if [[ "$XEN_TARGET_ARCH" != "x86_64" ]]; then
> > > +        exit 1
> > > +    fi
> > > +
> > > +    # git diff --no-index returns 0 if no differences, otherwise 1.
> > > +    git diff --no-index --output=test.patch xen/arch/x86/test-smc-lp.c \
> > > +                                            
> > > xen/arch/x86/test-smc-lp-alt.c && exit 1
> > > +
> > > +    BUILDID=$(readelf -Wn xen/xen-syms | sed -n -e 's/^.*Build ID: //p')
> > > +
> > > +    git clone https://xenbits.xen.org/git-http/livepatch-build-tools.git
> > > +    cd livepatch-build-tools
> > > +    make
> > > +    ./livepatch-build -s ../ -p ../test.patch -o out -c ../xen/.config \
> > > +        --depends $BUILDID --xen-depends $BUILDID
> > > +    cp out/test.livepatch ../binaries/test.livepatch
> > > +fi
> > 
> > I realize this is a matter of taste but if possible I would move this to
> > qemu-alpine-x86_64-livepatch.sh not to make the build script too
> > complex.
> 
> I've attempted that, but there are some issues.  First, the
> elfutils-dev package would need to be added to the test container,
> checkout livepatch-build-tools.git from the test script, and do the
> differential build in the test script, so all the Xen hypervisor build
> dependencies would also be needed in the test container.
> 
> > Otherwise, plase create automation/scripts/livepatch and move this code
> > there. You can call automation/scripts/livepatch from
> > automation/scripts/build.
> 
> Unless you have a strong desire to pursue building the livepatch in
> the test step, I will go with the route proposed here, and split the
> livepatch build into automation/scripts/build-livepatch.

I am OK with this.


> > Other than that, this is great! I'll let other review the livepatch
> > specific changes in this series
> 
> Thanks, will post a new version soon.
> 
> Roger.
>