[libvirt] [PATCH 0/4] Implement mockup capabilities cache in QEMU tests
v2: https://www.redhat.com/archives/libvir-list/2015-September/msg00374.html v3: * split out the movement of existing initialization code to qemuTestDriverInit from the cache addition * qemuTestDriverInit now adds an empty capability cache, so most tests are unchanged * renamed 'cleanup' to 'error' in qemuTestDriverInit, since it's only called on error paths * fixed indentation issues to pass syntax-check * removed mode changes to 755 Pavel Fedin (4): tests: split out common qemu driver initialization Implement infrastracture for mocking up QEMU capabilities cache Use mockup cache Removed unneeded check src/qemu/qemu_capabilities.c | 16 ++++----- src/qemu/qemu_capspriv.h | 36 ++++++++++++++++++++ src/qemu/qemu_domain.c | 10 ++---- tests/domainsnapshotxml2xmltest.c | 10 ++---- tests/qemuagenttest.c | 11 +++--- tests/qemuargv2xmltest.c | 12 ++----- tests/qemuhotplugtest.c | 32 +++++++++--------- tests/qemuxml2argvtest.c | 17 +++++----- tests/qemuxml2xmltest.c | 8 ++--- tests/qemuxmlnstest.c | 17 +++++----- tests/testutilsqemu.c | 70 +++++++++++++++++++++++++++++++++++++++ tests/testutilsqemu.h | 7 ++++ 12 files changed, 170 insertions(+), 76 deletions(-) create mode 100644 src/qemu/qemu_capspriv.h -- 2.4.6
From: Pavel Fedin <p.fedin@samsung.com> Two utility functions are introduced for proper initialization and cleanup of the driver. Signed-off-by: Pavel Fedin <p.fedin@samsung.com> Signed-off-by: Ján Tomko <jtomko@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; if (VIR_ALLOC(testSnapshotXMLVariableLineRegex) < 0) goto cleanup; @@ -227,8 +224,7 @@ mymain(void) if (testSnapshotXMLVariableLineRegex) regfree(testSnapshotXMLVariableLineRegex); VIR_FREE(testSnapshotXMLVariableLineRegex); - virObjectUnref(driver.caps); - virObjectUnref(driver.xmlopt); + qemuTestDriverFree(&driver); return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE; } diff --git a/tests/qemuagenttest.c b/tests/qemuagenttest.c index 52cc834..1ebc030 100644 --- a/tests/qemuagenttest.c +++ b/tests/qemuagenttest.c @@ -31,6 +31,8 @@ #define VIR_FROM_THIS VIR_FROM_NONE +static virQEMUDriver driver; + static int testQemuAgentFSFreeze(const void *data) { @@ -909,7 +911,6 @@ static int mymain(void) { int ret = 0; - virDomainXMLOptionPtr xmlopt; #if !WITH_YAJL fputs("libvirt not compiled with yajl, skipping this test\n", stderr); @@ -917,13 +918,13 @@ mymain(void) #endif if (virThreadInitialize() < 0 || - !(xmlopt = virQEMUDriverCreateXMLConf(NULL))) + qemuTestDriverInit(&driver) < 0) return EXIT_FAILURE; virEventRegisterDefaultImpl(); -#define DO_TEST(name) \ - if (virtTestRun(# name, testQemuAgent ## name, xmlopt) < 0) \ +#define DO_TEST(name) \ + if (virtTestRun(# name, testQemuAgent ## name, driver.xmlopt) < 0) \ ret = -1 DO_TEST(FSFreeze); @@ -938,7 +939,7 @@ mymain(void) DO_TEST(Timeout); /* Timeout should always be called last */ - virObjectUnref(xmlopt); + qemuTestDriverFree(&driver); return (ret == 0) ? EXIT_SUCCESS : EXIT_FAILURE; } diff --git a/tests/qemuargv2xmltest.c b/tests/qemuargv2xmltest.c index ea85913..96453e5 100644 --- a/tests/qemuargv2xmltest.c +++ b/tests/qemuargv2xmltest.c @@ -145,15 +145,9 @@ mymain(void) { int ret = 0; - driver.config = virQEMUDriverConfigNew(false); - if (driver.config == NULL) + if (qemuTestDriverInit(&driver) < 0) return EXIT_FAILURE; - if ((driver.caps = testQemuCapsInit()) == NULL) - return EXIT_FAILURE; - - if (!(driver.xmlopt = virQEMUDriverCreateXMLConf(&driver))) - return EXIT_FAILURE; # define DO_TEST_FULL(name, flags) \ do { \ @@ -298,9 +292,7 @@ mymain(void) DO_TEST("machine-deakeywrap-off-argv"); DO_TEST("machine-keywrap-none-argv"); - virObjectUnref(driver.config); - virObjectUnref(driver.caps); - virObjectUnref(driver.xmlopt); + qemuTestDriverFree(&driver); return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE; } diff --git a/tests/qemuhotplugtest.c b/tests/qemuhotplugtest.c index 368a5e7..3cf7f36 100644 --- a/tests/qemuhotplugtest.c +++ b/tests/qemuhotplugtest.c @@ -338,14 +338,11 @@ mymain(void) #endif if (virThreadInitialize() < 0 || - !(driver.caps = testQemuCapsInit()) || - !(driver.xmlopt = virQEMUDriverCreateXMLConf(&driver))) + qemuTestDriverInit(&driver) < 0) return EXIT_FAILURE; virEventRegisterDefaultImpl(); - if (!(driver.config = virQEMUDriverConfigNew(false))) - return EXIT_FAILURE; VIR_FREE(driver.config->spiceListen); VIR_FREE(driver.config->vncListen); /* some dummy values from 'config file' */ @@ -486,9 +483,7 @@ mymain(void) "device_del", QMP_DEVICE_DELETED("scsi0-0-0-5") QMP_OK, "human-monitor-command", HMP("")); - virObjectUnref(driver.caps); - virObjectUnref(driver.xmlopt); - virObjectUnref(driver.config); + qemuTestDriverFree(&driver); return (ret == 0) ? EXIT_SUCCESS : EXIT_FAILURE; } diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index d4432df..cd12356 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -483,8 +483,7 @@ mymain(void) return EXIT_FAILURE; } - driver.config = virQEMUDriverConfigNew(false); - if (driver.config == NULL) + if (qemuTestDriverInit(&driver) < 0) return EXIT_FAILURE; driver.privileged = true; @@ -499,10 +498,6 @@ mymain(void) if (VIR_STRDUP_QUIET(driver.config->spiceTLSx509certdir, "/etc/pki/libvirt-spice") < 0) return EXIT_FAILURE; - if ((driver.caps = testQemuCapsInit()) == NULL) - return EXIT_FAILURE; - if (!(driver.xmlopt = virQEMUDriverCreateXMLConf(&driver))) - return EXIT_FAILURE; VIR_FREE(driver.config->stateDir); if (VIR_STRDUP_QUIET(driver.config->stateDir, "/nowhere") < 0) return EXIT_FAILURE; @@ -1761,9 +1756,7 @@ mymain(void) QEMU_CAPS_MACHINE_OPT, QEMU_CAPS_DRIVE, QEMU_CAPS_VIRTIO_SCSI, QEMU_CAPS_DEVICE, QEMU_CAPS_VIRTIO_CCW, QEMU_CAPS_VIRTIO_S390); - virObjectUnref(driver.config); - virObjectUnref(driver.caps); - virObjectUnref(driver.xmlopt); + qemuTestDriverFree(&driver); return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE; } diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index 5a20ebc..3552309 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -302,11 +302,10 @@ mymain(void) int ret = 0; struct testInfo info; - if ((driver.caps = testQemuCapsInit()) == NULL) + if (qemuTestDriverInit(&driver) < 0) return EXIT_FAILURE; - if (!(driver.xmlopt = virQEMUDriverCreateXMLConf(&driver))) - return EXIT_FAILURE; + driver.config->allowDiskFormatProbing = true; # define DO_TEST_FULL(name, is_different, when) \ do { \ @@ -631,8 +630,7 @@ mymain(void) DO_TEST("memory-hotplug-dimm"); DO_TEST("net-udp"); - virObjectUnref(driver.caps); - virObjectUnref(driver.xmlopt); + qemuTestDriverFree(&driver); return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE; } diff --git a/tests/qemuxmlnstest.c b/tests/qemuxmlnstest.c index a68e762..40e32dc 100644 --- a/tests/qemuxmlnstest.c +++ b/tests/qemuxmlnstest.c @@ -201,15 +201,12 @@ mymain(void) if (!abs_top_srcdir) abs_top_srcdir = abs_srcdir "/.."; - if (!(driver.config = virQEMUDriverConfigNew(false))) + if (qemuTestDriverInit(&driver) < 0) return EXIT_FAILURE; + VIR_FREE(driver.config->libDir); if (VIR_STRDUP_QUIET(driver.config->libDir, "/tmp") < 0) return EXIT_FAILURE; - if ((driver.caps = testQemuCapsInit()) == NULL) - return EXIT_FAILURE; - if (!(driver.xmlopt = virQEMUDriverCreateXMLConf(&driver))) - return EXIT_FAILURE; # define DO_TEST_FULL(name, migrateFrom, migrateFd, expectError, ...) \ do { \ @@ -251,9 +248,7 @@ mymain(void) DO_TEST("qemu-ns-commandline-ns0", false, NONE); DO_TEST("qemu-ns-commandline-ns1", false, NONE); - virObjectUnref(driver.config); - virObjectUnref(driver.caps); - virObjectUnref(driver.xmlopt); + qemuTestDriverFree(&driver); return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE; } diff --git a/tests/testutilsqemu.c b/tests/testutilsqemu.c index a2f4299..84dfa75 100644 --- a/tests/testutilsqemu.c +++ b/tests/testutilsqemu.c @@ -526,4 +526,34 @@ qemuTestParseCapabilities(const char *capsFile) xmlXPathFreeContext(ctxt); return NULL; } + +int qemuTestDriverInit(virQEMUDriver *driver) +{ + driver->config = virQEMUDriverConfigNew(false); + if (!driver->config) + return -ENOMEM; + + driver->caps = testQemuCapsInit(); + if (!driver->caps) + goto error; + + driver->xmlopt = virQEMUDriverCreateXMLConf(driver); + if (!driver->xmlopt) + goto error; + + return 0; + + error: + virObjectUnref(driver->caps); + virObjectUnref(driver->config); + virObjectUnref(driver->xmlopt); + return -ENOMEM; +} + +void qemuTestDriverFree(virQEMUDriver *driver) +{ + virObjectUnref(driver->xmlopt); + virObjectUnref(driver->caps); + virObjectUnref(driver->config); +} #endif diff --git a/tests/testutilsqemu.h b/tests/testutilsqemu.h index 0ec5dad..6c2d3b5 100644 --- a/tests/testutilsqemu.h +++ b/tests/testutilsqemu.h @@ -16,4 +16,6 @@ extern virCPUDefPtr cpuHaswell; void testQemuCapsSetCPU(virCapsPtr caps, virCPUDefPtr hostCPU); +int qemuTestDriverInit(virQEMUDriver *driver); +void qemuTestDriverFree(virQEMUDriver *driver); #endif -- 2.4.6
On Tue, Sep 15, 2015 at 10:05:19AM +0200, Ján Tomko wrote:
From: Pavel Fedin <p.fedin@samsung.com>
Two utility functions are introduced for proper initialization and cleanup of the driver.
Signed-off-by: Pavel Fedin <p.fedin@samsung.com> Signed-off-by: Ján Tomko <jtomko@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?
diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index 5a20ebc..3552309 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -302,11 +302,10 @@ mymain(void) int ret = 0; struct testInfo info;
- if ((driver.caps = testQemuCapsInit()) == NULL) + if (qemuTestDriverInit(&driver) < 0) return EXIT_FAILURE;
- if (!(driver.xmlopt = virQEMUDriverCreateXMLConf(&driver))) - return EXIT_FAILURE; + driver.config->allowDiskFormatProbing = true;
same here
diff --git a/tests/testutilsqemu.c b/tests/testutilsqemu.c index a2f4299..84dfa75 100644 --- a/tests/testutilsqemu.c +++ b/tests/testutilsqemu.c @@ -526,4 +526,34 @@ qemuTestParseCapabilities(const char *capsFile) xmlXPathFreeContext(ctxt); return NULL; } + +int qemuTestDriverInit(virQEMUDriver *driver) +{ + driver->config = virQEMUDriverConfigNew(false); + if (!driver->config) + return -ENOMEM; + + driver->caps = testQemuCapsInit(); + if (!driver->caps) + goto error; + + driver->xmlopt = virQEMUDriverCreateXMLConf(driver); + if (!driver->xmlopt) + goto error; + + return 0; + + error: + virObjectUnref(driver->caps); + virObjectUnref(driver->config); + virObjectUnref(driver->xmlopt);
qemuTestDriverFree would be nicer
+ return -ENOMEM;
also -1 would do here.
+} + +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.
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@samsung.com>
Two utility functions are introduced for proper initialization and cleanup of the driver.
Signed-off-by: Pavel Fedin <p.fedin@samsung.com> Signed-off-by: Ján Tomko <jtomko@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.
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.
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@samsung.com>
Two utility functions are introduced for proper initialization and cleanup of the driver.
Signed-off-by: Pavel Fedin <p.fedin@samsung.com> Signed-off-by: Ján Tomko <jtomko@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.
On 15.09.2015 10:05, Ján Tomko wrote:
From: Pavel Fedin <p.fedin@samsung.com>
Two utility functions are introduced for proper initialization and cleanup of the driver.
Signed-off-by: Pavel Fedin <p.fedin@samsung.com> Signed-off-by: Ján Tomko <jtomko@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;
Apart from what Martin already pointed out ...
if (VIR_ALLOC(testSnapshotXMLVariableLineRegex) < 0) goto cleanup; @@ -227,8 +224,7 @@ mymain(void) if (testSnapshotXMLVariableLineRegex) regfree(testSnapshotXMLVariableLineRegex); VIR_FREE(testSnapshotXMLVariableLineRegex); - virObjectUnref(driver.caps); - virObjectUnref(driver.xmlopt); + qemuTestDriverFree(&driver);
return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE; } diff --git a/tests/qemuagenttest.c b/tests/qemuagenttest.c index 52cc834..1ebc030 100644 --- a/tests/qemuagenttest.c +++ b/tests/qemuagenttest.c @@ -31,6 +31,8 @@
#define VIR_FROM_THIS VIR_FROM_NONE
+static virQEMUDriver driver; +
There's no need for this variable to be global. Put it into mymain() please.
static int testQemuAgentFSFreeze(const void *data) { @@ -909,7 +911,6 @@ static int mymain(void) { int ret = 0; - virDomainXMLOptionPtr xmlopt;
#if !WITH_YAJL fputs("libvirt not compiled with yajl, skipping this test\n", stderr); @@ -917,13 +918,13 @@ mymain(void) #endif
if (virThreadInitialize() < 0 || - !(xmlopt = virQEMUDriverCreateXMLConf(NULL))) + qemuTestDriverInit(&driver) < 0) return EXIT_FAILURE;
virEventRegisterDefaultImpl();
-#define DO_TEST(name) \ - if (virtTestRun(# name, testQemuAgent ## name, xmlopt) < 0) \ +#define DO_TEST(name) \ + if (virtTestRun(# name, testQemuAgent ## name, driver.xmlopt) < 0) \ ret = -1
DO_TEST(FSFreeze); @@ -938,7 +939,7 @@ mymain(void)
DO_TEST(Timeout); /* Timeout should always be called last */
- virObjectUnref(xmlopt); + qemuTestDriverFree(&driver);
return (ret == 0) ? EXIT_SUCCESS : EXIT_FAILURE; }
Michal
From: Pavel Fedin <p.fedin@samsung.com> The main purpose of this patch is to introduce test mode to virQEMUCapsCacheLookup(). This is done by adding a global variable, which effectively overrides binary name. This variable is supposed to be set by test suite. The second addition is qemuTestCapsCacheInsert() function which allows the test suite to actually populate the cache. Signed-off-by: Pavel Fedin <p.fedin@samsung.com> Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/qemu/qemu_capabilities.c | 16 +++++++--------- src/qemu/qemu_capspriv.h | 36 ++++++++++++++++++++++++++++++++++++ tests/testutilsqemu.c | 40 ++++++++++++++++++++++++++++++++++++++++ tests/testutilsqemu.h | 5 +++++ 4 files changed, 88 insertions(+), 9 deletions(-) create mode 100644 src/qemu/qemu_capspriv.h diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 4ad1bdb..9a819d2 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -42,6 +42,7 @@ #include "virstring.h" #include "qemu_hostdev.h" #include "qemu_domain.h" +#include "qemu_capspriv.h" #include <fcntl.h> #include <sys/stat.h> @@ -331,15 +332,6 @@ struct _virQEMUCaps { unsigned int *machineMaxCpus; }; -struct _virQEMUCapsCache { - virMutex lock; - virHashTablePtr binaries; - char *libDir; - char *cacheDir; - uid_t runUid; - gid_t runGid; -}; - struct virQEMUCapsSearchData { virArch arch; }; @@ -3718,11 +3710,17 @@ virQEMUCapsCacheNew(const char *libDir, return NULL; } +const char *qemuTestCapsName; virQEMUCapsPtr virQEMUCapsCacheLookup(virQEMUCapsCachePtr cache, const char *binary) { virQEMUCapsPtr ret = NULL; + + /* This is used only by test suite!!! */ + if (qemuTestCapsName) + binary = qemuTestCapsName; + virMutexLock(&cache->lock); ret = virHashLookup(cache->binaries, binary); if (ret && diff --git a/src/qemu/qemu_capspriv.h b/src/qemu/qemu_capspriv.h new file mode 100644 index 0000000..9288dbd --- /dev/null +++ b/src/qemu/qemu_capspriv.h @@ -0,0 +1,36 @@ +/* + * qemu_capspriv.h: private declarations for QEMU capabilities generation + * + * Copyright (C) 2015 Samsung Electronics Co. Ltd + * Copyright (C) 2015 Pavel Fedin + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * <http://www.gnu.org/licenses/>. + * + * Author: Pavel Fedin <p.fedin@samsung.com> + */ + +#ifndef __QEMU_CAPSPRIV_H__ +# define __QEMU_CAPSPRIV_H__ + +struct _virQEMUCapsCache { + virMutex lock; + virHashTablePtr binaries; + char *libDir; + char *cacheDir; + uid_t runUid; + gid_t runGid; +}; + +#endif diff --git a/tests/testutilsqemu.c b/tests/testutilsqemu.c index 84dfa75..275d22a 100644 --- a/tests/testutilsqemu.c +++ b/tests/testutilsqemu.c @@ -8,6 +8,7 @@ # include "cpu_conf.h" # include "qemu/qemu_driver.h" # include "qemu/qemu_domain.h" +# include "qemu/qemu_capspriv.h" # include "virstring.h" # define VIR_FROM_THIS VIR_FROM_QEMU @@ -527,6 +528,32 @@ qemuTestParseCapabilities(const char *capsFile) return NULL; } +int qemuTestCapsCacheInsert(virQEMUCapsCachePtr cache, const char *binary, + virQEMUCapsPtr caps) +{ + int ret; + + if (caps) { + /* Our caps were created artificially, so we don't want + * virQEMUCapsCacheFree() to attempt to deallocate them */ + virObjectRef(caps); + } else { + caps = virQEMUCapsNew(); + if (!caps) + return -ENOMEM; + } + + /* We can have repeating names for our test data sets, + * so make sure there's no old copy */ + virHashRemoveEntry(cache->binaries, binary); + + ret = virHashAddEntry(cache->binaries, binary, caps); + if (ret < 0) + virObjectUnref(caps); + + return ret; +} + int qemuTestDriverInit(virQEMUDriver *driver) { driver->config = virQEMUDriverConfigNew(false); @@ -537,13 +564,24 @@ int qemuTestDriverInit(virQEMUDriver *driver) if (!driver->caps) goto error; + /* Using /dev/null for libDir and cacheDir automatically produces errors + * upon attempt to use any of them */ + driver->qemuCapsCache = virQEMUCapsCacheNew("/dev/null", "/dev/null", 0, 0); + if (!driver->qemuCapsCache) + goto error; + driver->xmlopt = virQEMUDriverCreateXMLConf(driver); if (!driver->xmlopt) goto error; + qemuTestCapsName = "empty"; + if (qemuTestCapsCacheInsert(driver->qemuCapsCache, "empty", NULL) < 0) + goto error; + return 0; error: + virQEMUCapsCacheFree(driver->qemuCapsCache); virObjectUnref(driver->caps); virObjectUnref(driver->config); virObjectUnref(driver->xmlopt); @@ -552,8 +590,10 @@ int qemuTestDriverInit(virQEMUDriver *driver) void qemuTestDriverFree(virQEMUDriver *driver) { + virQEMUCapsCacheFree(driver->qemuCapsCache); virObjectUnref(driver->xmlopt); virObjectUnref(driver->caps); virObjectUnref(driver->config); } + #endif diff --git a/tests/testutilsqemu.h b/tests/testutilsqemu.h index 6c2d3b5..1d5dfc5 100644 --- a/tests/testutilsqemu.h +++ b/tests/testutilsqemu.h @@ -18,4 +18,9 @@ void testQemuCapsSetCPU(virCapsPtr caps, int qemuTestDriverInit(virQEMUDriver *driver); void qemuTestDriverFree(virQEMUDriver *driver); +int qemuTestCapsCacheInsert(virQEMUCapsCachePtr cache, const char *binary, + virQEMUCapsPtr caps); + +/* This variable is actually defined in src/qemu/qemu_capabilities.c */ +extern const char *qemuTestCapsName; #endif -- 2.4.6
On Tue, Sep 15, 2015 at 10:05:20AM +0200, Ján Tomko wrote:
From: Pavel Fedin <p.fedin@samsung.com>
The main purpose of this patch is to introduce test mode to virQEMUCapsCacheLookup(). This is done by adding a global variable, which effectively overrides binary name. This variable is supposed to be set by test suite.
The second addition is qemuTestCapsCacheInsert() function which allows the test suite to actually populate the cache.
Signed-off-by: Pavel Fedin <p.fedin@samsung.com> Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/qemu/qemu_capabilities.c | 16 +++++++--------- src/qemu/qemu_capspriv.h | 36 ++++++++++++++++++++++++++++++++++++ tests/testutilsqemu.c | 40 ++++++++++++++++++++++++++++++++++++++++ tests/testutilsqemu.h | 5 +++++ 4 files changed, 88 insertions(+), 9 deletions(-) create mode 100644 src/qemu/qemu_capspriv.h
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 4ad1bdb..9a819d2 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -42,6 +42,7 @@ #include "virstring.h" #include "qemu_hostdev.h" #include "qemu_domain.h" +#include "qemu_capspriv.h"
#include <fcntl.h> #include <sys/stat.h> @@ -331,15 +332,6 @@ struct _virQEMUCaps { unsigned int *machineMaxCpus; };
-struct _virQEMUCapsCache { - virMutex lock; - virHashTablePtr binaries; - char *libDir; - char *cacheDir; - uid_t runUid; - gid_t runGid; -}; - struct virQEMUCapsSearchData { virArch arch; }; @@ -3718,11 +3710,17 @@ virQEMUCapsCacheNew(const char *libDir, return NULL; }
+const char *qemuTestCapsName;
virQEMUCapsPtr virQEMUCapsCacheLookup(virQEMUCapsCachePtr cache, const char *binary) { virQEMUCapsPtr ret = NULL; + + /* This is used only by test suite!!! */ + if (qemuTestCapsName) + binary = qemuTestCapsName; + virMutexLock(&cache->lock); ret = virHashLookup(cache->binaries, binary); if (ret && diff --git a/src/qemu/qemu_capspriv.h b/src/qemu/qemu_capspriv.h new file mode 100644 index 0000000..9288dbd --- /dev/null +++ b/src/qemu/qemu_capspriv.h @@ -0,0 +1,36 @@ +/* + * qemu_capspriv.h: private declarations for QEMU capabilities generation + * + * Copyright (C) 2015 Samsung Electronics Co. Ltd + * Copyright (C) 2015 Pavel Fedin + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * <http://www.gnu.org/licenses/>. + * + * Author: Pavel Fedin <p.fedin@samsung.com> + */ + +#ifndef __QEMU_CAPSPRIV_H__ +# define __QEMU_CAPSPRIV_H__ + +struct _virQEMUCapsCache { + virMutex lock; + virHashTablePtr binaries; + char *libDir; + char *cacheDir; + uid_t runUid; + gid_t runGid; +}; + +#endif diff --git a/tests/testutilsqemu.c b/tests/testutilsqemu.c index 84dfa75..275d22a 100644 --- a/tests/testutilsqemu.c +++ b/tests/testutilsqemu.c @@ -8,6 +8,7 @@ # include "cpu_conf.h" # include "qemu/qemu_driver.h" # include "qemu/qemu_domain.h" +# include "qemu/qemu_capspriv.h" # include "virstring.h"
# define VIR_FROM_THIS VIR_FROM_QEMU @@ -527,6 +528,32 @@ qemuTestParseCapabilities(const char *capsFile) return NULL; }
+int qemuTestCapsCacheInsert(virQEMUCapsCachePtr cache, const char *binary, + virQEMUCapsPtr caps) +{ + int ret; + + if (caps) { + /* Our caps were created artificially, so we don't want + * virQEMUCapsCacheFree() to attempt to deallocate them */ + virObjectRef(caps); + } else { + caps = virQEMUCapsNew(); + if (!caps) + return -ENOMEM; + } + + /* We can have repeating names for our test data sets, + * so make sure there's no old copy */ + virHashRemoveEntry(cache->binaries, binary); + + ret = virHashAddEntry(cache->binaries, binary, caps); + if (ret < 0) + virObjectUnref(caps); + + return ret; +} + int qemuTestDriverInit(virQEMUDriver *driver) { driver->config = virQEMUDriverConfigNew(false); @@ -537,13 +564,24 @@ int qemuTestDriverInit(virQEMUDriver *driver) if (!driver->caps) goto error;
+ /* Using /dev/null for libDir and cacheDir automatically produces errors + * upon attempt to use any of them */ + driver->qemuCapsCache = virQEMUCapsCacheNew("/dev/null", "/dev/null", 0, 0); + if (!driver->qemuCapsCache) + goto error; + driver->xmlopt = virQEMUDriverCreateXMLConf(driver); if (!driver->xmlopt) goto error;
+ qemuTestCapsName = "empty"; + if (qemuTestCapsCacheInsert(driver->qemuCapsCache, "empty", NULL) < 0) + goto error; +
So this will be an example binary? Is that going to be used anywhere? I haven't received 3/4 yet, so I don't know. But if that is not going to be used, I would remove it, if it is going to be used, then I would add it when it is needed.
return 0;
error: + virQEMUCapsCacheFree(driver->qemuCapsCache);
this should follow the favor I asked for in 1/4, of course. ACK if you fix this and remove the empty capabilities (or agrue that they are needed)
virObjectUnref(driver->caps); virObjectUnref(driver->config); virObjectUnref(driver->xmlopt); @@ -552,8 +590,10 @@ int qemuTestDriverInit(virQEMUDriver *driver)
void qemuTestDriverFree(virQEMUDriver *driver) { + virQEMUCapsCacheFree(driver->qemuCapsCache); virObjectUnref(driver->xmlopt); virObjectUnref(driver->caps); virObjectUnref(driver->config); } + #endif diff --git a/tests/testutilsqemu.h b/tests/testutilsqemu.h index 6c2d3b5..1d5dfc5 100644 --- a/tests/testutilsqemu.h +++ b/tests/testutilsqemu.h @@ -18,4 +18,9 @@ void testQemuCapsSetCPU(virCapsPtr caps,
int qemuTestDriverInit(virQEMUDriver *driver); void qemuTestDriverFree(virQEMUDriver *driver); +int qemuTestCapsCacheInsert(virQEMUCapsCachePtr cache, const char *binary, + virQEMUCapsPtr caps); + +/* This variable is actually defined in src/qemu/qemu_capabilities.c */ +extern const char *qemuTestCapsName; #endif -- 2.4.6
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On 15.09.2015 10:05, Ján Tomko wrote:
From: Pavel Fedin <p.fedin@samsung.com>
The main purpose of this patch is to introduce test mode to virQEMUCapsCacheLookup(). This is done by adding a global variable, which effectively overrides binary name. This variable is supposed to be set by test suite.
The second addition is qemuTestCapsCacheInsert() function which allows the test suite to actually populate the cache.
Signed-off-by: Pavel Fedin <p.fedin@samsung.com> Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/qemu/qemu_capabilities.c | 16 +++++++--------- src/qemu/qemu_capspriv.h | 36 ++++++++++++++++++++++++++++++++++++ tests/testutilsqemu.c | 40 ++++++++++++++++++++++++++++++++++++++++ tests/testutilsqemu.h | 5 +++++ 4 files changed, 88 insertions(+), 9 deletions(-) create mode 100644 src/qemu/qemu_capspriv.h
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 4ad1bdb..9a819d2 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -42,6 +42,7 @@ #include "virstring.h" #include "qemu_hostdev.h" #include "qemu_domain.h" +#include "qemu_capspriv.h"
#include <fcntl.h> #include <sys/stat.h> @@ -331,15 +332,6 @@ struct _virQEMUCaps { unsigned int *machineMaxCpus; };
-struct _virQEMUCapsCache { - virMutex lock; - virHashTablePtr binaries; - char *libDir; - char *cacheDir; - uid_t runUid; - gid_t runGid; -}; - struct virQEMUCapsSearchData { virArch arch; }; @@ -3718,11 +3710,17 @@ virQEMUCapsCacheNew(const char *libDir, return NULL; }
+const char *qemuTestCapsName;
virQEMUCapsPtr virQEMUCapsCacheLookup(virQEMUCapsCachePtr cache, const char *binary) { virQEMUCapsPtr ret = NULL; + + /* This is used only by test suite!!! */ + if (qemuTestCapsName) + binary = qemuTestCapsName; + virMutexLock(&cache->lock); ret = virHashLookup(cache->binaries, binary); if (ret && diff --git a/src/qemu/qemu_capspriv.h b/src/qemu/qemu_capspriv.h new file mode 100644 index 0000000..9288dbd --- /dev/null +++ b/src/qemu/qemu_capspriv.h @@ -0,0 +1,36 @@ +/* + * qemu_capspriv.h: private declarations for QEMU capabilities generation + * + * Copyright (C) 2015 Samsung Electronics Co. Ltd + * Copyright (C) 2015 Pavel Fedin + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * <http://www.gnu.org/licenses/>. + * + * Author: Pavel Fedin <p.fedin@samsung.com> + */ +
Maybe we want to guard this file so that it cannot be included from a random .c file? Something like we do in ./src/util/vircommandpriv.h?
+#ifndef __QEMU_CAPSPRIV_H__ +# define __QEMU_CAPSPRIV_H__ + +struct _virQEMUCapsCache { + virMutex lock; + virHashTablePtr binaries; + char *libDir; + char *cacheDir; + uid_t runUid; + gid_t runGid; +}; + +#endif diff --git a/tests/testutilsqemu.c b/tests/testutilsqemu.c index 84dfa75..275d22a 100644 --- a/tests/testutilsqemu.c +++ b/tests/testutilsqemu.c @@ -8,6 +8,7 @@ # include "cpu_conf.h" # include "qemu/qemu_driver.h" # include "qemu/qemu_domain.h" +# include "qemu/qemu_capspriv.h" # include "virstring.h"
# define VIR_FROM_THIS VIR_FROM_QEMU @@ -527,6 +528,32 @@ qemuTestParseCapabilities(const char *capsFile) return NULL; }
+int qemuTestCapsCacheInsert(virQEMUCapsCachePtr cache, const char *binary, + virQEMUCapsPtr caps) +{ + int ret; + + if (caps) { + /* Our caps were created artificially, so we don't want + * virQEMUCapsCacheFree() to attempt to deallocate them */ + virObjectRef(caps); + } else { + caps = virQEMUCapsNew(); + if (!caps) + return -ENOMEM; + } + + /* We can have repeating names for our test data sets, + * so make sure there's no old copy */ + virHashRemoveEntry(cache->binaries, binary); + + ret = virHashAddEntry(cache->binaries, binary, caps); + if (ret < 0) + virObjectUnref(caps);
I think we want to set @qemuTestCapsName here: if (ret < 0) virObjectUnref(caps) else qemuTestCapsName = binary;
+ + return ret; +} + int qemuTestDriverInit(virQEMUDriver *driver) { driver->config = virQEMUDriverConfigNew(false); @@ -537,13 +564,24 @@ int qemuTestDriverInit(virQEMUDriver *driver) if (!driver->caps) goto error;
+ /* Using /dev/null for libDir and cacheDir automatically produces errors + * upon attempt to use any of them */ + driver->qemuCapsCache = virQEMUCapsCacheNew("/dev/null", "/dev/null", 0, 0); + if (!driver->qemuCapsCache) + goto error; + driver->xmlopt = virQEMUDriverCreateXMLConf(driver); if (!driver->xmlopt) goto error;
+ qemuTestCapsName = "empty"; + if (qemuTestCapsCacheInsert(driver->qemuCapsCache, "empty", NULL) < 0) + goto error; + return 0;
error: + virQEMUCapsCacheFree(driver->qemuCapsCache); virObjectUnref(driver->caps); virObjectUnref(driver->config); virObjectUnref(driver->xmlopt); @@ -552,8 +590,10 @@ int qemuTestDriverInit(virQEMUDriver *driver)
void qemuTestDriverFree(virQEMUDriver *driver) { + virQEMUCapsCacheFree(driver->qemuCapsCache); virObjectUnref(driver->xmlopt); virObjectUnref(driver->caps); virObjectUnref(driver->config); } + #endif diff --git a/tests/testutilsqemu.h b/tests/testutilsqemu.h index 6c2d3b5..1d5dfc5 100644 --- a/tests/testutilsqemu.h +++ b/tests/testutilsqemu.h @@ -18,4 +18,9 @@ void testQemuCapsSetCPU(virCapsPtr caps,
int qemuTestDriverInit(virQEMUDriver *driver); void qemuTestDriverFree(virQEMUDriver *driver); +int qemuTestCapsCacheInsert(virQEMUCapsCachePtr cache, const char *binary, + virQEMUCapsPtr caps); + +/* This variable is actually defined in src/qemu/qemu_capabilities.c */ +extern const char *qemuTestCapsName; #endif
Otherwise looking good. Michal
From: Pavel Fedin <p.fedin@samsung.com> Use the new API in order to correctly add capability sets to the cache before parsing XML files Signed-off-by: Pavel Fedin <p.fedin@samsung.com> --- tests/qemuhotplugtest.c | 23 +++++++++++++++-------- tests/qemuxml2argvtest.c | 6 ++++++ tests/qemuxmlnstest.c | 6 ++++++ 3 files changed, 27 insertions(+), 8 deletions(-) diff --git a/tests/qemuhotplugtest.c b/tests/qemuhotplugtest.c index 3cf7f36..109d820 100644 --- a/tests/qemuhotplugtest.c +++ b/tests/qemuhotplugtest.c @@ -57,7 +57,7 @@ static int qemuHotplugCreateObjects(virDomainXMLOptionPtr xmlopt, virDomainObjPtr *vm, const char *domxml, - bool event) + bool event, const char *testname) { int ret = -1; qemuDomainObjPrivatePtr priv = NULL; @@ -65,12 +65,6 @@ qemuHotplugCreateObjects(virDomainXMLOptionPtr xmlopt, if (!(*vm = virDomainObjNew(xmlopt))) goto cleanup; - if (!((*vm)->def = virDomainDefParseString(domxml, - driver.caps, - driver.xmlopt, - VIR_DOMAIN_DEF_PARSE_INACTIVE))) - goto cleanup; - priv = (*vm)->privateData; if (!(priv->qemuCaps = virQEMUCapsNew())) @@ -85,6 +79,18 @@ qemuHotplugCreateObjects(virDomainXMLOptionPtr xmlopt, if (event) virQEMUCapsSet(priv->qemuCaps, QEMU_CAPS_DEVICE_DEL_EVENT); + qemuTestCapsName = testname; + ret = qemuTestCapsCacheInsert(driver.qemuCapsCache, testname, + priv->qemuCaps); + if (ret < 0) + goto cleanup; + + if (!((*vm)->def = virDomainDefParseString(domxml, + driver.caps, + driver.xmlopt, + VIR_DOMAIN_DEF_PARSE_INACTIVE))) + goto cleanup; + if (qemuDomainAssignAddresses((*vm)->def, priv->qemuCaps, *vm) < 0) goto cleanup; @@ -243,7 +249,8 @@ testQemuHotplug(const void *data) vm = test->vm; } else { if (qemuHotplugCreateObjects(driver.xmlopt, &vm, domain_xml, - test->deviceDeletedEvent) < 0) + test->deviceDeletedEvent, + test->domain_filename) < 0) goto cleanup; } diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index cd12356..1fc767e 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -423,6 +423,12 @@ testCompareXMLToArgvHelper(const void *data) if (virQEMUCapsGet(info->extraFlags, QEMU_CAPS_ENABLE_FIPS)) flags |= FLAG_FIPS; + qemuTestCapsName = info->name; + result = qemuTestCapsCacheInsert(driver.qemuCapsCache, info->name, + info->extraFlags); + if (result < 0) + goto cleanup; + result = testCompareXMLToArgvFiles(xml, args, info->extraFlags, info->migrateFrom, info->migrateFd, flags); diff --git a/tests/qemuxmlnstest.c b/tests/qemuxmlnstest.c index 40e32dc..65bf1d3 100644 --- a/tests/qemuxmlnstest.c +++ b/tests/qemuxmlnstest.c @@ -179,6 +179,12 @@ testCompareXMLToArgvHelper(const void *data) abs_srcdir, info->name) < 0) goto cleanup; + qemuTestCapsName = info->name; + result = qemuTestCapsCacheInsert(driver.qemuCapsCache, info->name, + info->extraFlags); + if (result < 0) + goto cleanup; + result = testCompareXMLToArgvFiles(xml, args, info->extraFlags, info->migrateFrom, info->migrateFd, info->json, info->expectError); -- 2.4.6
On 15.09.2015 10:05, Ján Tomko wrote:
From: Pavel Fedin <p.fedin@samsung.com>
Use the new API in order to correctly add capability sets to the cache before parsing XML files
Signed-off-by: Pavel Fedin <p.fedin@samsung.com>
s/^/tests: / in $SUBJ.
--- tests/qemuhotplugtest.c | 23 +++++++++++++++-------- tests/qemuxml2argvtest.c | 6 ++++++ tests/qemuxmlnstest.c | 6 ++++++ 3 files changed, 27 insertions(+), 8 deletions(-)
diff --git a/tests/qemuhotplugtest.c b/tests/qemuhotplugtest.c index 3cf7f36..109d820 100644 --- a/tests/qemuhotplugtest.c +++ b/tests/qemuhotplugtest.c @@ -57,7 +57,7 @@ static int qemuHotplugCreateObjects(virDomainXMLOptionPtr xmlopt, virDomainObjPtr *vm, const char *domxml, - bool event) + bool event, const char *testname) { int ret = -1; qemuDomainObjPrivatePtr priv = NULL; @@ -65,12 +65,6 @@ qemuHotplugCreateObjects(virDomainXMLOptionPtr xmlopt, if (!(*vm = virDomainObjNew(xmlopt))) goto cleanup;
- if (!((*vm)->def = virDomainDefParseString(domxml, - driver.caps, - driver.xmlopt, - VIR_DOMAIN_DEF_PARSE_INACTIVE))) - goto cleanup; - priv = (*vm)->privateData;
if (!(priv->qemuCaps = virQEMUCapsNew())) @@ -85,6 +79,18 @@ qemuHotplugCreateObjects(virDomainXMLOptionPtr xmlopt, if (event) virQEMUCapsSet(priv->qemuCaps, QEMU_CAPS_DEVICE_DEL_EVENT);
+ qemuTestCapsName = testname; + ret = qemuTestCapsCacheInsert(driver.qemuCapsCache, testname, + priv->qemuCaps);
I think that @qemuTestCapsName should be set in qemuTestCapsCacheInsert(). On its successful return.
+ if (ret < 0) + goto cleanup; + + if (!((*vm)->def = virDomainDefParseString(domxml, + driver.caps, + driver.xmlopt, + VIR_DOMAIN_DEF_PARSE_INACTIVE))) + goto cleanup; + if (qemuDomainAssignAddresses((*vm)->def, priv->qemuCaps, *vm) < 0) goto cleanup;
@@ -243,7 +249,8 @@ testQemuHotplug(const void *data) vm = test->vm; } else { if (qemuHotplugCreateObjects(driver.xmlopt, &vm, domain_xml, - test->deviceDeletedEvent) < 0) + test->deviceDeletedEvent, + test->domain_filename) < 0) goto cleanup; }
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index cd12356..1fc767e 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -423,6 +423,12 @@ testCompareXMLToArgvHelper(const void *data) if (virQEMUCapsGet(info->extraFlags, QEMU_CAPS_ENABLE_FIPS)) flags |= FLAG_FIPS;
+ qemuTestCapsName = info->name; + result = qemuTestCapsCacheInsert(driver.qemuCapsCache, info->name, + info->extraFlags); + if (result < 0) + goto cleanup; + result = testCompareXMLToArgvFiles(xml, args, info->extraFlags, info->migrateFrom, info->migrateFd, flags); diff --git a/tests/qemuxmlnstest.c b/tests/qemuxmlnstest.c index 40e32dc..65bf1d3 100644 --- a/tests/qemuxmlnstest.c +++ b/tests/qemuxmlnstest.c @@ -179,6 +179,12 @@ testCompareXMLToArgvHelper(const void *data) abs_srcdir, info->name) < 0) goto cleanup;
+ qemuTestCapsName = info->name; + result = qemuTestCapsCacheInsert(driver.qemuCapsCache, info->name, + info->extraFlags); + if (result < 0) + goto cleanup; + result = testCompareXMLToArgvFiles(xml, args, info->extraFlags, info->migrateFrom, info->migrateFd, info->json, info->expectError);
Otherwise looking good. Michal
On Mon, Sep 21, 2015 at 03:03:20PM +0200, Michal Privoznik wrote:
On 15.09.2015 10:05, Ján Tomko wrote:
From: Pavel Fedin <p.fedin@samsung.com>
Use the new API in order to correctly add capability sets to the cache before parsing XML files
Signed-off-by: Pavel Fedin <p.fedin@samsung.com>
s/^/tests: / in $SUBJ.
--- tests/qemuhotplugtest.c | 23 +++++++++++++++-------- tests/qemuxml2argvtest.c | 6 ++++++ tests/qemuxmlnstest.c | 6 ++++++ 3 files changed, 27 insertions(+), 8 deletions(-)
diff --git a/tests/qemuhotplugtest.c b/tests/qemuhotplugtest.c index 3cf7f36..109d820 100644 --- a/tests/qemuhotplugtest.c +++ b/tests/qemuhotplugtest.c @@ -57,7 +57,7 @@ static int qemuHotplugCreateObjects(virDomainXMLOptionPtr xmlopt, virDomainObjPtr *vm, const char *domxml, - bool event) + bool event, const char *testname) { int ret = -1; qemuDomainObjPrivatePtr priv = NULL; @@ -65,12 +65,6 @@ qemuHotplugCreateObjects(virDomainXMLOptionPtr xmlopt, if (!(*vm = virDomainObjNew(xmlopt))) goto cleanup;
- if (!((*vm)->def = virDomainDefParseString(domxml, - driver.caps, - driver.xmlopt, - VIR_DOMAIN_DEF_PARSE_INACTIVE))) - goto cleanup; - priv = (*vm)->privateData;
if (!(priv->qemuCaps = virQEMUCapsNew())) @@ -85,6 +79,18 @@ qemuHotplugCreateObjects(virDomainXMLOptionPtr xmlopt, if (event) virQEMUCapsSet(priv->qemuCaps, QEMU_CAPS_DEVICE_DEL_EVENT);
+ qemuTestCapsName = testname; + ret = qemuTestCapsCacheInsert(driver.qemuCapsCache, testname, + priv->qemuCaps);
I think that @qemuTestCapsName should be set in qemuTestCapsCacheInsert(). On its successful return.
It makes sense since we always overwrite it. Anything else? Jan
On 22.09.2015 08:27, Ján Tomko wrote:
On Mon, Sep 21, 2015 at 03:03:20PM +0200, Michal Privoznik wrote:
On 15.09.2015 10:05, Ján Tomko wrote:
From: Pavel Fedin <p.fedin@samsung.com>
Use the new API in order to correctly add capability sets to the cache before parsing XML files
Signed-off-by: Pavel Fedin <p.fedin@samsung.com>
s/^/tests: / in $SUBJ.
--- tests/qemuhotplugtest.c | 23 +++++++++++++++-------- tests/qemuxml2argvtest.c | 6 ++++++ tests/qemuxmlnstest.c | 6 ++++++ 3 files changed, 27 insertions(+), 8 deletions(-)
diff --git a/tests/qemuhotplugtest.c b/tests/qemuhotplugtest.c index 3cf7f36..109d820 100644 --- a/tests/qemuhotplugtest.c +++ b/tests/qemuhotplugtest.c @@ -57,7 +57,7 @@ static int qemuHotplugCreateObjects(virDomainXMLOptionPtr xmlopt, virDomainObjPtr *vm, const char *domxml, - bool event) + bool event, const char *testname) { int ret = -1; qemuDomainObjPrivatePtr priv = NULL; @@ -65,12 +65,6 @@ qemuHotplugCreateObjects(virDomainXMLOptionPtr xmlopt, if (!(*vm = virDomainObjNew(xmlopt))) goto cleanup;
- if (!((*vm)->def = virDomainDefParseString(domxml, - driver.caps, - driver.xmlopt, - VIR_DOMAIN_DEF_PARSE_INACTIVE))) - goto cleanup; - priv = (*vm)->privateData;
if (!(priv->qemuCaps = virQEMUCapsNew())) @@ -85,6 +79,18 @@ qemuHotplugCreateObjects(virDomainXMLOptionPtr xmlopt, if (event) virQEMUCapsSet(priv->qemuCaps, QEMU_CAPS_DEVICE_DEL_EVENT);
+ qemuTestCapsName = testname; + ret = qemuTestCapsCacheInsert(driver.qemuCapsCache, testname, + priv->qemuCaps);
I think that @qemuTestCapsName should be set in qemuTestCapsCacheInsert(). On its successful return.
It makes sense since we always overwrite it.
Anything else?
No, I think if you fix this locally, you're good to push it. ACK with my suggestion worked in. Michal
From: Pavel Fedin <p.fedin@samsung.com> Since test suite now correctly creates capabilities cache, the hack is not needed any more. Signed-off-by: Pavel Fedin <p.fedin@samsung.com> Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/qemu/qemu_domain.c | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index c8b0ccd..26ca12d 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -1039,10 +1039,7 @@ qemuDomainDefPostParse(virDomainDefPtr def, return ret; - /* This condition is actually a (temporary) hack for test suite which - * does not create capabilities cache */ - if (driver && driver->qemuCapsCache) - qemuCaps = virQEMUCapsCacheLookup(driver->qemuCapsCache, def->emulator); + qemuCaps = virQEMUCapsCacheLookup(driver->qemuCapsCache, def->emulator); /* Add implicit PCI root controller if the machine has one */ switch (def->os.arch) { @@ -1241,10 +1238,7 @@ qemuDomainDeviceDefPostParse(virDomainDeviceDefPtr dev, if (driver) cfg = virQEMUDriverGetConfig(driver); - /* This condition is actually a (temporary) hack for test suite which - * does not create capabilities cache */ - if (driver && driver->qemuCapsCache) - qemuCaps = virQEMUCapsCacheLookup(driver->qemuCapsCache, def->emulator); + qemuCaps = virQEMUCapsCacheLookup(driver->qemuCapsCache, def->emulator); if (dev->type == VIR_DOMAIN_DEVICE_NET && dev->data.net->type != VIR_DOMAIN_NET_TYPE_HOSTDEV && -- 2.4.6
On 15.09.2015 10:05, Ján Tomko wrote:
From: Pavel Fedin <p.fedin@samsung.com>
Since test suite now correctly creates capabilities cache, the hack is not needed any more.
Signed-off-by: Pavel Fedin <p.fedin@samsung.com> Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/qemu/qemu_domain.c | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index c8b0ccd..26ca12d 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -1039,10 +1039,7 @@ qemuDomainDefPostParse(virDomainDefPtr def, return ret;
- /* This condition is actually a (temporary) hack for test suite which - * does not create capabilities cache */ - if (driver && driver->qemuCapsCache) - qemuCaps = virQEMUCapsCacheLookup(driver->qemuCapsCache, def->emulator); + qemuCaps = virQEMUCapsCacheLookup(driver->qemuCapsCache, def->emulator);
/* Add implicit PCI root controller if the machine has one */ switch (def->os.arch) { @@ -1241,10 +1238,7 @@ qemuDomainDeviceDefPostParse(virDomainDeviceDefPtr dev, if (driver) cfg = virQEMUDriverGetConfig(driver);
- /* This condition is actually a (temporary) hack for test suite which - * does not create capabilities cache */ - if (driver && driver->qemuCapsCache) - qemuCaps = virQEMUCapsCacheLookup(driver->qemuCapsCache, def->emulator); + qemuCaps = virQEMUCapsCacheLookup(driver->qemuCapsCache, def->emulator);
if (dev->type == VIR_DOMAIN_DEVICE_NET && dev->data.net->type != VIR_DOMAIN_NET_TYPE_HOSTDEV &&
ACK Michal
participants (3)
-
Ján Tomko -
Martin Kletzander -
Michal Privoznik