Re: [PATCH 1/4] tests: split out common qemu driver initialization

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Sep 15, 2015 at 01:59:54PM +0200, Ján Tomko wrote:
On Tue, Sep 15, 2015 at 10:40:36AM +0200, Martin Kletzander wrote:
On Tue, Sep 15, 2015 at 10:05:19AM +0200, Ján Tomko wrote:
>From: Pavel Fedin <p.fedin@xxxxxxxxxxx>
>
>Two utility functions are introduced for proper initialization and
>cleanup of the driver.
>
>Signed-off-by: Pavel Fedin <p.fedin@xxxxxxxxxxx>
>Signed-off-by: Ján Tomko <jtomko@xxxxxxxxxx>
>---
> tests/domainsnapshotxml2xmltest.c | 10 +++-------
> tests/qemuagenttest.c             | 11 ++++++-----
> tests/qemuargv2xmltest.c          | 12 ++----------
> tests/qemuhotplugtest.c           |  9 ++-------
> tests/qemuxml2argvtest.c          | 11 ++---------
> tests/qemuxml2xmltest.c           |  8 +++-----
> tests/qemuxmlnstest.c             | 11 +++--------
> tests/testutilsqemu.c             | 30 ++++++++++++++++++++++++++++++
> tests/testutilsqemu.h             |  2 ++
> 9 files changed, 53 insertions(+), 51 deletions(-)
>
>diff --git a/tests/domainsnapshotxml2xmltest.c b/tests/domainsnapshotxml2xmltest.c
>index 3955a19..b66af3e 100644
>--- a/tests/domainsnapshotxml2xmltest.c
>+++ b/tests/domainsnapshotxml2xmltest.c
>@@ -152,13 +152,10 @@ mymain(void)
> {
>     int ret = 0;
>
>-    if ((driver.caps = testQemuCapsInit()) == NULL)
>+    if (qemuTestDriverInit(&driver) < 0)
>         return EXIT_FAILURE;
>
>-    if (!(driver.xmlopt = virQEMUDriverCreateXMLConf(&driver))) {
>-        virObjectUnref(driver.caps);
>-        return EXIT_FAILURE;
>-    }
>+    driver.config->allowDiskFormatProbing = true;
>

Why is this needed?


These tests did not have a driver.config before, but they do after
switching to qemuTestDriverInit.

In qemuDomainDeviceDefPostParse, the code setting the format to raw when
format probing is off is wrapped in if (cfg). After this patch, it no
longer gets skipped.

The alternative would be to adjust the test files to work without
probing.


Although I would normally vote for that, I'm kind of on the edge.
Setting it to "true" by default is somewhat easier to fix, from what I
checked, but that's not we want since that's not reasonable production
default.  Could you at least put a TODO or XXX as a comment before the
' = true' line?  ACK with that (at least we can track it even though
it's not something crucial.

>diff --git a/tests/testutilsqemu.c b/tests/testutilsqemu.c
>index a2f4299..84dfa75 100644
>--- a/tests/testutilsqemu.c
>+++ b/tests/testutilsqemu.c

>+
>+    return 0;
>+
>+ error:
>+    virObjectUnref(driver->caps);
>+    virObjectUnref(driver->config);
>+    virObjectUnref(driver->xmlopt);

qemuTestDriverFree would be nicer

>+    return -ENOMEM;

also -1 would do here.


I have squashed these two changes to my local branch.

Jan

>+}
>+
>+void qemuTestDriverFree(virQEMUDriver *driver)
>+{
>+    virObjectUnref(driver->xmlopt);
>+    virObjectUnref(driver->caps);
>+    virObjectUnref(driver->config);
>+}
> #endif

ACK with allowDiskFormatProbing removed and the two mentioned nits
fixed, otherwise please explain the format probing if you want that
it too.


Attachment: signature.asc
Description: PGP signature

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

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]