[PATCH] tests: qemu: Don't crash when capability file can't be parsed

[Martin Kletzander]

On Thu, Aug 12, 2021 at 04:55:04PM +0200, Peter Krempa wrote:
>In case the test directory contains invalid XML (this doesn't happen
>upstream, but can when developing, e.g. by forgetting git conflict
>markers) the tests would crash as in case when 'testQemuInfoSetArgs'
>fails we'd still invoke the test in qemuxml2argv and qemuxml2xml tests.
>
>Add a 'break' statement to avoid invocation of the test and add a debug
>message.
>
>Signed-off-by: Peter Krempa <pkrempa at redhat.com>
>---
> tests/qemuxml2argvtest.c | 4 +++-
> tests/qemuxml2xmltest.c  | 1 +
> tests/testutilsqemu.c    | 4 +++-
> 3 files changed, 7 insertions(+), 2 deletions(-)
>
>diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
>index b552f5deed..3f43e76842 100644
>--- a/tests/qemuxml2argvtest.c
>+++ b/tests/qemuxml2argvtest.c
>@@ -943,8 +943,10 @@ mymain(void)
>         }; \
>         info.qapiSchemaCache = qapiSchemaCache; \
>         if (testQemuInfoSetArgs(&info, capscache, capslatest, \
>-                                __VA_ARGS__, ARG_END) < 0) \
>+                                __VA_ARGS__, ARG_END) < 0) { \
>             ret = -1; \
>+            break; \
>+        } \

This makes the test not crash, but it is still not clearly visible in
the output what happened and with what tests, unless ran with
VIR_TEST_DEBUG=1.  It seems there should be some NULL-pointer check
somewhere under testCompareXMLToArgv and similar.  Unfortuntely making
testQemuInfoSetArgs part of the callback of virTestRun, in this case
testCompareXMLToArgv would be very cumbersome, but we should do at least
a little bit more because this patch itself does not seem to improve the
situation in my opinion.  See below:

Before this patch:

$ VIR_TEST_DEBUG=0 build/tests/qemuxml2argvtest
TEST: qemuxml2argvtest
       ........................................ 40
       ........................................ 80
       ........................................ 120
       ........................................ 160
       ........................................ 200
       ................................fish: Job 1, 'VIR_TEST_DEBUG=0
       build/tests/qe…' terminated by signal SIGSEGV (Address boundary
       error)

After this patch:

$ VIR_TEST_DEBUG=0 build/tests/qemuxml2argvtest
TEST: qemuxml2argvtest
       ........................................ 40
       ........................................ 80
       ........................................ 120
       ........................................ 160
       ........................................ 200
       ........................................ 240
       ........................................ 280
       ........................................ 320
       ........................................ 360
       ........................................ 400
       ........................................ 440
       ........................................ 480
       ........................................ 520
       ........................................ 560
       ........................................ 600
       ........................................ 640
       ........................................ 680
       ........................................ 720
       ........................................ 760
       ........................................ 800
       ........................................ 840
       ........................................ 880
       ........................................ 920
       ........................................ 960
       ........................................ 1000
       ........................................ 1040
       ........................................ 1080
       ........................................ 1120
       ................                         1136 FAIL

In the first case I can see more information right away, albeit the test
suite crashing.

What about a non-NULL check for info->qemuCaps in testUpdateQEMUCaps()
and/or (even better as that might actually be bug in our code) check in
virQEMUCapsSetArch() ?
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20210816/3df1e9c3/attachment-0001.sig>