Re: [PATCH for-4.16 3/4] test/tsx: set grant version for created domains

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

On Mon, Nov 15, 2021 at 12:43:34PM +0000, Andrew Cooper wrote:
> On 15/11/2021 12:17, Roger Pau Monne wrote:
> > Set the grant table version for the created domains to use version 1,
> > as such tests domains don't require the usage of the grant table at
> > all. A TODO note is added to switch those dummy domains to not have a
> > grant table at all when possible. 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>
> > Reported-by: Jan Beulich <jbeulich@xxxxxxxx>
> > Signed-off-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>
> > ---
> > Cc: Ian Jackson <iwj@xxxxxxxxxxxxxx>
> >
> > 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.
> 
> What do you mean unusable?  Other than this, the test works.

I mean, it's unusable because I broke it with the gnttab change, and
that's it's current status unless this patch is applied.

> > ---
> >  tools/tests/tsx/test-tsx.c | 4 ++++
> >  1 file changed, 4 insertions(+)
> >
> > diff --git a/tools/tests/tsx/test-tsx.c b/tools/tests/tsx/test-tsx.c
> > index fab99c135e..f1dcff4c30 100644
> > --- a/tools/tests/tsx/test-tsx.c
> > +++ b/tools/tests/tsx/test-tsx.c
> > @@ -444,6 +444,8 @@ static void test_guests(void)
> >          struct xen_domctl_createdomain c = {
> >              .max_vcpus = 1,
> >              .max_grant_frames = 1,
> > +            /* TODO: switch to 0 once support for no grant table is added. 
> > */
> 
> I'd avoid these TODOs.  It's test code, so really doesn't matter too much.

I'm fine with this, when looking at the test code I didn't see any
reason why gnttab was needed, so I thought it would be a fine use-case
for opting out to grant table. As you say it's a test case, so it
doesn't matter much.

Do you want me to repost with the comments removed?

Thanks, Roger.