Some tests use the same VM state multiple times in a row. But if we
failed loading the VM XML, subsequent tests crash on the NULL def
pointer

Signed-off-by: Cole Robinson <crobinso@redhat.com>
---
I hit this with failing tests while writing this series

 tests/qemuhotplugtest.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tests/qemuhotplugtest.c b/tests/qemuhotplugtest.c
index 674ba92b27..4f9e127f88 100644
--- a/tests/qemuhotplugtest.c
+++ b/tests/qemuhotplugtest.c
@@ -268,6 +268,8 @@ testQemuHotplug(const void *data)
 
     if (test->vm) {
         vm = test->vm;
+        if (!vm->def)
+            goto cleanup;
     } else {
         if (qemuHotplugCreateObjects(driver.xmlopt, &vm, domain_xml,
                                      test->deviceDeletedEvent) < 0)
-- 
2.17.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

On 07/24/2018 11:23 PM, Cole Robinson wrote:
> Some tests use the same VM state multiple times in a row. But if we
> failed loading the VM XML, subsequent tests crash on the NULL def
> pointer
> 
> Signed-off-by: Cole Robinson <crobinso@redhat.com>
> ---
> I hit this with failing tests while writing this series
> 
>  tests/qemuhotplugtest.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/tests/qemuhotplugtest.c b/tests/qemuhotplugtest.c
> index 674ba92b27..4f9e127f88 100644
> --- a/tests/qemuhotplugtest.c
> +++ b/tests/qemuhotplugtest.c
> @@ -268,6 +268,8 @@ testQemuHotplug(const void *data)
>  
>      if (test->vm) {
>          vm = test->vm;
> +        if (!vm->def)
> +            goto cleanup;
>      } else {
>          if (qemuHotplugCreateObjects(driver.xmlopt, &vm, domain_xml,
>                                       test->deviceDeletedEvent) < 0)
> 

I wonder if we should fprintf(stderr, "test skipped because of an
failure in dependant test"); or something among these lines. The idea
being it's easier to debug. Look at all places where we 'goto cleanup'.
At least libvirt error is reported there (virAsprintf reports OOM,
virTestLoadFile reports error too, etc.).

Michal

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

On 07/26/2018 02:44 AM, Michal Privoznik wrote:
> On 07/24/2018 11:23 PM, Cole Robinson wrote:
>> Some tests use the same VM state multiple times in a row. But if we
>> failed loading the VM XML, subsequent tests crash on the NULL def
>> pointer
>>
>> Signed-off-by: Cole Robinson <crobinso@redhat.com>
>> ---
>> I hit this with failing tests while writing this series
>>
>>  tests/qemuhotplugtest.c | 2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/tests/qemuhotplugtest.c b/tests/qemuhotplugtest.c
>> index 674ba92b27..4f9e127f88 100644
>> --- a/tests/qemuhotplugtest.c
>> +++ b/tests/qemuhotplugtest.c
>> @@ -268,6 +268,8 @@ testQemuHotplug(const void *data)
>>  
>>      if (test->vm) {
>>          vm = test->vm;
>> +        if (!vm->def)
>> +            goto cleanup;
>>      } else {
>>          if (qemuHotplugCreateObjects(driver.xmlopt, &vm, domain_xml,
>>                                       test->deviceDeletedEvent) < 0)
>>
> 
> I wonder if we should fprintf(stderr, "test skipped because of an
> failure in dependant test"); or something among these lines. The idea
> being it's easier to debug. Look at all places where we 'goto cleanup'.
> At least libvirt error is reported there (virAsprintf reports OOM,
> virTestLoadFile reports error too, etc.).
> 

I'll use VIR_TEST_VERBOSE() which will give similarish behavior to
virReportError in this case: output will only be shown if
VIR_TEST_DEBUG=1 or similar is used

Thanks,
Cole

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list