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

[Martin Kletzander]

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 at samsung.com>
>> >
>> >Two utility functions are introduced for proper initialization and
>> >cleanup of the driver.
>> >
>> >Signed-off-by: Pavel Fedin <p.fedin at samsung.com>
>> >Signed-off-by: Ján Tomko <jtomko at redhat.com>
>> >---
>> > 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.
>
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20150915/da04ab19/attachment-0001.sig>