Re: [Xen-devel] [PATCH] domain_create: honour global grant/maptrack frame limits...

[Jürgen Groß <jgross@xxxxxxxx>]

On 26.11.19 12:30, Paul Durrant wrote:
> On Wed, 13 Nov 2019 at 13:55, Paul Durrant <pdurrant@xxxxxxxxxx> wrote:
> > ...when their values are larger than the per-domain configured limits.
> >
> > Signed-off-by: Paul Durrant <pdurrant@xxxxxxxxxx>
> > ---
> > Cc: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> > Cc: George Dunlap <George.Dunlap@xxxxxxxxxxxxx>
> > Cc: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>
> > Cc: Jan Beulich <jbeulich@xxxxxxxx>
> > Cc: Julien Grall <julien@xxxxxxx>
> > Cc: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
> > Cc: Stefano Stabellini <sstabellini@xxxxxxxxxx>
> > Cc: Wei Liu <wl@xxxxxxx>
> >
> > After mining through commits it is still unclear to me exactly when Xen
> > stopped honouring the global values, but I really think this commit should
> > be back-ported to stable trees as it was a behavioural change that can
> > cause domUs to fail in non-obvious ways.
> Any other opinions on this? AFAICT questions is still open:
>
> - Do we consider not honouring the command line values to be a
> regression (since domUs that would have worked before will no longer
> work after a basic upgrade of Xen)?
>
>    Paul
>
> > ---
> >   xen/common/domain.c | 14 ++++++++++++--
> >   1 file changed, 12 insertions(+), 2 deletions(-)
> >
> > diff --git a/xen/common/domain.c b/xen/common/domain.c
> > index 611116c7fc..aad6d55b82 100644
> > --- a/xen/common/domain.c
> > +++ b/xen/common/domain.c
> > @@ -335,6 +335,7 @@ struct domain *domain_create(domid_t domid,
> >       enum { INIT_watchdog = 1u<<1,
> >              INIT_evtchn = 1u<<3, INIT_gnttab = 1u<<4, INIT_arch = 1u<<5 };
> >       int err, init_status = 0;
> > +    unsigned int max_grant_frames, max_maptrack_frames;
> >
> >       if ( config && (err = sanitise_domain_config(config)) )
> >           return ERR_PTR(err);
> > @@ -456,8 +457,17 @@ struct domain *domain_create(domid_t domid,
> >               goto fail;
> >           init_status |= INIT_evtchn;
> >
> > -        if ( (err = grant_table_init(d, config->max_grant_frames,
> > -                                     config->max_maptrack_frames)) != 0 )
> > +        /*
> > +         * Make sure that the configured values don't reduce any
> > +         * global command line override.
> > +         */
> > +        max_grant_frames = max(config->max_grant_frames,
> > +                               opt_max_grant_frames);
> > +        max_maptrack_frames = max(config->max_maptrack_frames,
> > +                                  opt_max_maptrack_frames);
> > +
> > +        if ( (err = grant_table_init(d, max_grant_frames,
> > +                                     max_maptrack_frames)) != 0 )
So basically the per-domain settings are ignored.

They are not allowed to be smaller than the global limits (due to
using max()).

They are not allowed to be larger than the global limits (due to the
test in grant_table_init().

That is _not_ the purpose of being able to control the settings per
domain.


Juergen

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel