[3/7] tests: qemuhotplug: Fix segfault when XML loading fails

Message ID d03ce75683ad3e16bfb14f3f65b66fcae20531ab.1532467269.git.crobinso@redhat.com
State New
Headers show
Series
  • conf: Replace SKIP_OSTYPE with SKIP_VALIDATE
Related show

Commit Message

Cole Robinson July 24, 2018, 9:23 p.m.
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(+)

-- 
2.17.1

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

Comments

Michal Prívozník July 26, 2018, 6:44 a.m. | #1
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
Cole Robinson July 26, 2018, 2:05 p.m. | #2
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

Patch

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)