Re: [xen-unstable bisection] complete test-amd64-i386-xl-shadow

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Mon, Sep 07, 2020 at 10:21:36AM +0200, Jan Beulich wrote:
> On 07.09.2020 00:48, osstest service owner wrote:
> > branch xen-unstable
> > xenbranch xen-unstable
> > job test-amd64-i386-xl-shadow
> > testid guest-saverestore
> > 
> > Tree: linux git://xenbits.xen.org/linux-pvops.git
> > Tree: linuxfirmware git://xenbits.xen.org/osstest/linux-firmware.git
> > Tree: qemu git://xenbits.xen.org/qemu-xen-traditional.git
> > Tree: qemuu git://xenbits.xen.org/qemu-xen.git
> > Tree: xen git://xenbits.xen.org/xen.git
> > 
> > *** Found and reproduced problem changeset ***
> > 
> >   Bug is in tree:  xen git://xenbits.xen.org/xen.git
> >   Bug introduced:  696c273f3d9a169911308fb7e0a702a3eb6a150d
> >   Bug not present: a609b6577f7867db4be1470130b7b3c686398c4f
> >   Last fail repro: http://logs.test-lab.xenproject.org/osstest/logs/153833/
> > 
> > 
> >   commit 696c273f3d9a169911308fb7e0a702a3eb6a150d
> >   Author: Jan Beulich <jbeulich@xxxxxxxx>
> >   Date:   Fri Sep 4 11:13:01 2020 +0200
> >   
> >       x86: generalize padding field handling
> >       
> >       The original intention was to ignore padding fields, but the pattern
> >       matched only ones whose names started with an underscore. Also match
> >       fields whose names are in line with the C spec by not having a leading
> >       underscore. (Note that the leading ^ in the sed regexps was pointless
> >       and hence get dropped.)
> 
> I conclude this needs to be reverted, and there was a thinko of mine
> involved here: Avoiding translation of padding fields would be okay
> only when their values don't get checked in the native handler. We
> effectively have a not written down (afaict) rule here that _pad*
> fields get ignored (and hence don't need translation), while pad*
> fields may not be ignored and hence may need translation. I don't
> like this state, but I also can't think of a good way out of it, at
> least not just yet.

I think his stems from the fact that we don't have a rule whether
explicit padding fields in structs should be zeroed. IIRC there are
hypercalls that would check for padding fields to be 0, while others
don't.

At this point I assume we can only implement the least restrictive
one, which is to not force padding fields to be zeroed?

This would have the side effect that they cannot be later used to
introduce additional fields to the struct without signaling the
version in use.

Roger.