Xenstore's unit test fails on read and write of big numbers if
quota-maxsize is set to a lower number than those test cases use.

Output a special warning instead of a failure message in such cases
and make the error non-fatal to the unit test.

Signed-off-by: Kevin Stefanov <kevin.stefanov@citrix.com>
---
CC: Ian Jackson <iwj@xenproject.org>
CC: Wei Liu <wl@xen.org>
CC: Juergen Gross <jgross@suse.com>
CC: Julien Grall <julien@xen.org>
CC: Andrew Cooper <andrew.cooper3@citrix.com>

v2: Adhere to coding style, use E2BIG instead of 7, set ret to 0
---
 tools/tests/xenstore/test-xenstore.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/tools/tests/xenstore/test-xenstore.c b/tools/tests/xenstore/test-xenstore.c
index d3574b3fa2..f8423e568e 100644
--- a/tools/tests/xenstore/test-xenstore.c
+++ b/tools/tests/xenstore/test-xenstore.c
@@ -110,8 +110,17 @@ static int call_test(struct test *tst, int iters, bool no_clock)
             break;
     }
 
+    /* Make E2BIG non-fatal to the test */
     if ( ret )
-        printf("%-10s: failed (ret = %d, stage %s)\n", tst->name, ret, stage);
+    {
+	if ( ret == E2BIG )
+        {
+            printf("%-10s: Not run - argument list too long\n", tst->name);
+            ret = 0;
+        }
+        else      
+            printf("%-10s: failed (ret = %d, stage %s)\n", tst->name, ret, stage);
+    }
     else if ( !no_clock )
     {
         printf("%-10s:", tst->name);
-- 
2.25.1

Kevin Stefanov writes ("[PATCH v2] tools/tests: Make E2BIG non-fatal to xenstore unit test"):
> Xenstore's unit test fails on read and write of big numbers if
> quota-maxsize is set to a lower number than those test cases use.
> 
> Output a special warning instead of a failure message in such cases
> and make the error non-fatal to the unit test.

I realise that I am late to this, but I'm not sure I agree with the
basic principle of this change.  In general tolerating particular
errors in a test, and simply abandoning the test if they occcur, is
normally not the best approach.

Questions that come to my mind (and which aren't answered in the
commit message and probably should be) include:

Why does test-xenstore using these large numbers for its tests ?
Why would you run the tests with a quota too low for the tests ?
Might this test change not in principle miss genuine bugs ?

Ian.

On 15.10.21 15:16, Ian Jackson wrote:
> Kevin Stefanov writes ("[PATCH v2] tools/tests: Make E2BIG non-fatal to xenstore unit test"):
>> Xenstore's unit test fails on read and write of big numbers if
>> quota-maxsize is set to a lower number than those test cases use.
>>
>> Output a special warning instead of a failure message in such cases
>> and make the error non-fatal to the unit test.
> 
> I realise that I am late to this, but I'm not sure I agree with the
> basic principle of this change.  In general tolerating particular
> errors in a test, and simply abandoning the test if they occcur, is
> normally not the best approach.
> 
> Questions that come to my mind (and which aren't answered in the
> commit message and probably should be) include:
> 
> Why does test-xenstore using these large numbers for its tests ?

For testing large data packets.

> Why would you run the tests with a quota too low for the tests ?

Good question.

> Might this test change not in principle miss genuine bugs ?

Yes, e.g. if a test returns E2BIG even if it shouldn't.

So I agree to being more cautious here.

Maybe a parameter could be added to limit the allowed data size?
Then the "large data" test could be adjusted to not send more data
than allowed (it should be noted that the node size quota is
including data, names of children, and access right entries, so
the pure node data should be selected with some spare size in
mind, e.g. 100 bytes smaller than the quota).


Juergen

On 15.10.2021 14:14, Kevin Stefanov wrote:
> --- a/tools/tests/xenstore/test-xenstore.c
> +++ b/tools/tests/xenstore/test-xenstore.c
> @@ -110,8 +110,17 @@ static int call_test(struct test *tst, int iters, bool no_clock)
>              break;
>      }
>  
> +    /* Make E2BIG non-fatal to the test */
>      if ( ret )
> -        printf("%-10s: failed (ret = %d, stage %s)\n", tst->name, ret, stage);
> +    {
> +	if ( ret == E2BIG )

Just fyi that it looks like a hard tab has slipped in here.

Jan