| Message ID | d03ce75683ad3e16bfb14f3f65b66fcae20531ab.1532467269.git.crobinso@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | conf: Replace SKIP_OSTYPE with SKIP_VALIDATE | expand |
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
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)
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