Re: [PATCH 0/4] xen: domain-tracked allocations, and fault injection

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Mon, 4 Jan 2021, Andrew Cooper wrote:
> On 04/01/2021 17:30, Stefano Stabellini wrote:
> > On Mon, 4 Jan 2021, Zheng, Fam wrote:
> >> On Mon, 2021-01-04 at 10:38 +0100, Roger Pau Monné wrote:
> >>> On Wed, Dec 23, 2020 at 12:10:43PM -0800, Stefano Stabellini wrote:
> >>>> On Wed, 23 Dec 2020, Andrew Cooper wrote:
> >>>>> On 23/12/2020 19:45, Stefano Stabellini wrote:
> >>>>>> On Wed, 23 Dec 2020, no-reply@xxxxxxxxxxx wrote:
> >>>>>>> Hi,
> >>>>>>>
> >>>>>>> Patchew automatically ran gitlab-ci pipeline with this patch
> >>>>>>> (series) applied, but the job failed. Maybe there's a bug in
> >>>>>>> the patches?
> >>>>>>>
> >>>>>>> You can find the link to the pipeline near the end of the
> >>>>>>> report below:
> >>>>>>>
> >>>>>>> Type: series
> >>>>>>> Message-id: 20201223163442.8840-1-andrew.cooper3@xxxxxxxxxx
> >>>>>>> Subject: [PATCH 0/4] xen: domain-tracked allocations, and
> >>>>>>> fault injection
> >>>>>>>
> >>>>>>> === TEST SCRIPT BEGIN ===
> >>>>>>> #!/bin/bash
> >>>>>>> sleep 10
> >>>>>>> patchew gitlab-pipeline-check -p xen-project/patchew/xen
> >>>>>>> === TEST SCRIPT END ===
> >>>>>> [...]
> >>>>>>
> >>>>>>> === OUTPUT BEGIN ===
> >>>>>>> [2020-12-23 16:38:43] Looking up pipeline...
> >>>>>>> [2020-12-23 16:38:43] Found pipeline 233889763:
> >>>>>>>
> >>>>>>>
> >> https://gitlab.com/xen-project/patchew/xen/-/pipelines/233889763
> >>>>>> This seems to be a genuine failure. Looking at the alpine-3.12-
> >>>>>> gcc-arm64
> >>>>>> build test, the build error is appended below. This is a link
> >>>>>> to the
> >>>>>> failed job: 
> >>>>>> https://gitlab.com/xen-project/patchew/xen/-/jobs/929842628
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>>> gcc  -DBUILD_ID -fno-strict-aliasing -std=gnu99 -Wall -Wstrict-
> >>>>>> prototypes -Wdeclaration-after-statement -Wno-unused-but-set-
> >>>>>> variable -Wno-unused-local-typedefs   -O2 -fomit-frame-pointer
> >>>>>> -D__XEN_INTERFACE_VERSION__=__XEN_LATEST_INTERFACE_VERSION__
> >>>>>> -MMD -MP -MF .xen-diag.o.d -D_LARGEFILE_SOURCE
> >>>>>> -D_LARGEFILE64_SOURCE   -Werror -include /builds/xen-
> >>>>>> project/patchew/xen/tools/misc/../../tools/config.h
> >>>>>> -I/builds/xen-
> >>>>>> project/patchew/xen/tools/misc/../../tools/include
> >>>>>> -I/builds/xen-
> >>>>>> project/patchew/xen/tools/misc/../../tools/include
> >>>>>> -D__XEN_TOOLS__ -I/builds/xen-
> >>>>>> project/patchew/xen/tools/misc/../../tools/include
> >>>>>> -I/builds/xen-
> >>>>>> project/patchew/xen/tools/misc/../../tools/include
> >>>>>> -I/builds/xen-
> >>>>>> project/patchew/xen/tools/misc/../../tools/include -Wno-
> >>>>>> declaration-after-statement  -c -o xen-diag.o xen-diag.c 
> >>>>>> xen-fault-ttl.c: In function 'main':
> >>>>>> xen-fault-ttl.c:25:14: error: 'struct xen_arch_domainconfig'
> >>>>>> has no member named 'emulation_flags'
> >>>>>>    25 |             .emulation_flags = XEN_X86_EMU_LAPIC,
> >>>>>>       |              ^~~~~~~~~~~~~~~
> >>>>>> xen-fault-ttl.c:25:32: error: 'XEN_X86_EMU_LAPIC' undeclared
> >>>>>> (first use in this function)
> >>>>>>    25 |             .emulation_flags = XEN_X86_EMU_LAPIC,
> >>>>>>       |                                ^~~~~~~~~~~~~~~~~
> >>>>>> xen-fault-ttl.c:25:32: note: each undeclared identifier is
> >>>>>> reported only once for each function it appears in
> >>>>>> make[4]: *** [/builds/xen-
> >>>>>> project/patchew/xen/tools/misc/../../tools/Rules.mk:144: xen-
> >>>>>> fault-ttl.o] Error 1
> >>>>>> make[4]: *** Waiting for unfinished jobs....
> >>>>>> make[4]: Leaving directory '/builds/xen-
> >>>>>> project/patchew/xen/tools/misc'
> >>>>>> make[3]: *** [/builds/xen-
> >>>>>> project/patchew/xen/tools/../tools/Rules.mk:160: subdir-
> >>>>>> install-misc] Error 2
> >>>>>> make[3]: Leaving directory '/builds/xen-
> >>>>>> project/patchew/xen/tools'
> >>>>>> make[2]: *** [/builds/xen-
> >>>>>> project/patchew/xen/tools/../tools/Rules.mk:155: subdirs-
> >>>>>> install] Error 2
> >>>>>> make[2]: Leaving directory '/builds/xen-
> >>>>>> project/patchew/xen/tools'
> >>>>>> make[1]: *** [Makefile:67: install] Error 2
> >>>>>> make[1]: Leaving directory '/builds/xen-
> >>>>>> project/patchew/xen/tools'
> >>>>>> make: *** [Makefile:134: install-tools] Error 2
> >>>>> Yeah - that is a real failure, which can be fixed with a little
> >>>>> bit of
> >>>>> ifdef-ary.  I'm confused as to why I didn't get that email
> >>>>> directly.
> >>>> It looks like patchew doesn't yet CC the original author?
> >>> Where do patchew emails go? I haven't seen any of them, and they
> >>> don't
> >>> seem to go to xen-devel.
> >> It's to limit the noise level. We intend to stablize the workflow a
> >> little more esp. wrt false positives before making every reply go to
> >> xen-devel. There's a few things to tweak in patchew.
> >>
> >> The next logical step should be including the patch author in the loop
> >> I think.
> > Let's do it.
> 
> Are we sure?  The false positive rate is still very high.
> 
> There are two separate x86 randconfig failures, an ARM randconfig
> failure, and an ARM smoke test failure.
> 
> None of these are fair to be raised as objections to a contributor, as
> it leaves the results very noisy.
> 
> I think we need to get to 0 known outstanding false positives before
> continuing.

Zero known false positives is clearly the goal, and I would consider it
the required quality before we make the results public CCing xen-devel.

However, I would have thought that we don't need to reach that level to
start CCing contributors. Often the failures are genuine and at the
moment one of us is manually forwarding the failure notifications to the
contributor, which is not ideal.

CCing the contributor with a warning notice that the CC-loop is not yet
stable looks like a good way to start. Maybe after I solve the QEMU
segfault issue in the Dom0less test that you reported this morning.