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

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

[Roger Pau Monne]

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@citrix.com>
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
---
Cc: Ian Jackson <iwj@xenproject.org>

This patch only modifies a test, so it should be safe to commit as
it's not going to cause any changes to the hypervisor or the tools.
Worse that could happen is it makes the test even more broken, but
it's already unusable.
---
 tools/tests/resource/test-resource.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/tools/tests/resource/test-resource.c b/tools/tests/resource/test-resource.c
index 988f96f7c1..658dd52aed 100644
--- 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
-- 
2.33.0

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

[Jan Beulich]

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@citrix.com>
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>

Technically
Reviewed-by: Jan Beulich <jbeulich@suse.com>

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),

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

Jan

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

[Roger Pau Monné]

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@citrix.com>
> > Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> 
> Technically
> Reviewed-by: Jan Beulich <jbeulich@suse.com>
> 
> 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.

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

[Ian Jackson]

Roger Pau Monne writes ("[PATCH for-4.16] tests/resource: set grant version for created domains"):
> 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@citrix.com>
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>

Release-Acked-by: Ian Jackson <iwj@xenproject.org>

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

[Jan Beulich]

On 16.11.2021 15:40, Ian Jackson wrote:
> Roger Pau Monne writes ("[PATCH for-4.16] tests/resource: set grant version for created domains"):
>> 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@citrix.com>
>> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> 
> Release-Acked-by: Ian Jackson <iwj@xenproject.org>

I was meaning to commit this, but you've replied to the singleton v1
patch which is now in the middle of a 4-patch series. I can't very
well assume (silently) that you've meant your ack to apply to that
series - please clarify.

Jan