Re: [PATCH for-4.16] tests/resource: set grant version for created domains

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

On Mon, Nov 15, 2021 at 12:02:53PM +0100, Jan Beulich wrote:
> On 15.11.2021 11:51, Roger Pau Monne wrote:
> > Set the grant table version for the created domains to use version 1,
> > as that's the used by the test cases. Without setting the grant
> > version the domains for the tests cannot be created.
> > 
> > Fixes: 7379f9e10a ('gnttab: allow setting max version per-domain')
> > Reported-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> > Signed-off-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>
> 
> Technically
> Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>
> 
> However, two remarks:
> 
> > --- a/tools/tests/resource/test-resource.c
> > +++ b/tools/tests/resource/test-resource.c
> > @@ -120,6 +120,7 @@ static void test_domain_configurations(void)
> >              .create = {
> >                  .max_vcpus = 2,
> >                  .max_grant_frames = 40,
> > +                .grant_opts = 1,
> >              },
> >          },
> >          {
> > @@ -128,6 +129,7 @@ static void test_domain_configurations(void)
> >                  .flags = XEN_DOMCTL_CDF_hvm,
> >                  .max_vcpus = 2,
> >                  .max_grant_frames = 40,
> > +                .grant_opts = 1,
> >                  .arch = {
> >                      .emulation_flags = XEN_X86_EMU_LAPIC,
> >                  },
> > @@ -140,6 +142,7 @@ static void test_domain_configurations(void)
> >                  .flags = XEN_DOMCTL_CDF_hvm | XEN_DOMCTL_CDF_hap,
> >                  .max_vcpus = 2,
> >                  .max_grant_frames = 40,
> > +                .grant_opts = 1,
> >              },
> >          },
> >  #endif
> 
> The literal 1-s here are really odd to read already now. It would get
> worse if some flags were specified later on and then used here, ending
> in e.g.
> 
>                 .grant_opts = XEN_DOMCTL_CDG_feature | 1,
> 
> Imo there really ought to be a wrapper macro, such that use sites
> will at the same time have documented what this 1 is about:
> 
>                 .grant_opts = XEN_DOMCTL_CDG_version(1),

OK. I better add one now before we start gaining more of those.

> 
> And then I guess tools/tests/tsx/test-tsx.c needs similar adjustment.

Yes, and the python bindings will also need an adjustment.

Thanks, Roger.