[libvirt] [PATCH 0/9] qemuxml2argvtest cleanups
While looking at how slow this test has gotten, I found a lost test case, some redundancies. Thankfully, not running the HostModel functions twice helped, but there is still room for improvement: we spend a lot of time in the CPU driver comparing the same strings over and over again and with over 300 entries in virQEMUCaps enum, we can reduce some of these string comparisons in the future to cope with more and more QEMU versions being tested. Ján Tomko (9): tests: drop 'drive' from qemuxml2startup tests tests: add macro for dealing with exclusive flags tests: introduce macro for qemu XML->startup XML tests: only run startup XML tests if requested tests: report errors in QEMU XML->startup XML tests tests: drop redundant virQEMUCapsFilterByMachineType tests: do not mangle real qemu caps in xml2argvtest tests: turn skipLegacyCPUs into a flag qemu: remove unnecessary virQEMUCapsFreeHostCPUModel src/qemu/qemu_capabilities.c | 25 +------ src/qemu/qemu_capspriv.h | 5 -- tests/qemuxml2argvtest.c | 76 +++++++++++++--------- .../{disk-drive-shared.xml => disk-shared.xml} | 0 4 files changed, 49 insertions(+), 57 deletions(-) rename tests/qemuxml2startupxmloutdata/{disk-drive-shared.xml => disk-shared.xml} (100%) -- 2.16.4
Commit 0bdb704 renamed the corresponding xml->argv tests, but due to the optimistic nature of xml->startup xml testing, this test was quietly skipped. Signed-off-by: Ján Tomko <jtomko@redhat.com> --- .../qemuxml2startupxmloutdata/{disk-drive-shared.xml => disk-shared.xml} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename tests/qemuxml2startupxmloutdata/{disk-drive-shared.xml => disk-shared.xml} (100%) diff --git a/tests/qemuxml2startupxmloutdata/disk-drive-shared.xml b/tests/qemuxml2startupxmloutdata/disk-shared.xml similarity index 100% rename from tests/qemuxml2startupxmloutdata/disk-drive-shared.xml rename to tests/qemuxml2startupxmloutdata/disk-shared.xml -- 2.16.4
On 09/09/2018 04:10 AM, Ján Tomko wrote:
Commit 0bdb704 renamed the corresponding xml->argv tests, but due to the optimistic nature of xml->startup xml testing, this test was quietly skipped.
Signed-off-by: Ján Tomko <jtomko@redhat.com> --- .../qemuxml2startupxmloutdata/{disk-drive-shared.xml => disk-shared.xml} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename tests/qemuxml2startupxmloutdata/{disk-drive-shared.xml => disk-shared.xml} (100%)
diff --git a/tests/qemuxml2startupxmloutdata/disk-drive-shared.xml b/tests/qemuxml2startupxmloutdata/disk-shared.xml similarity index 100% rename from tests/qemuxml2startupxmloutdata/disk-drive-shared.xml rename to tests/qemuxml2startupxmloutdata/disk-shared.xml
ACK Michal
We can reject some non-sensical combinations with an error message, once we add flags for them. Introduce a macro to make this easier. Signed-off-by: Ján Tomko <jtomko@redhat.com> --- tests/qemuxml2argvtest.c | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 04b6c00eba..b04cf7c923 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -467,6 +467,21 @@ testCompareXMLToStartupXML(const void *data) } +# define TEST_EXCLUSIVE_FLAGS(FLAG1, FLAG2) \ + if ((testFlags & FLAG1) && (testFlags & FLAG2)) { \ + VIR_TEST_DEBUG("Flags %s and %s are mutually exclusive\n", \ + #FLAG1, #FLAG2); \ + return -1; \ + } + + +static int +testCheckExclusiveFlags(int testFlags ATTRIBUTE_UNUSED) +{ + return 0; +} + + static int testCompareXMLToArgv(const void *data) { @@ -509,6 +524,9 @@ testCompareXMLToArgv(const void *data) if (virQEMUCapsGet(info->qemuCaps, QEMU_CAPS_ENABLE_FIPS)) flags |= FLAG_FIPS; + if (testCheckExclusiveFlags(info->flags) < 0) + goto cleanup; + if (qemuTestCapsCacheInsert(driver.qemuCapsCache, info->qemuCaps) < 0) goto cleanup; -- 2.16.4
On 09/09/2018 04:10 AM, Ján Tomko wrote:
We can reject some non-sensical combinations with an error message, once we add flags for them.
Introduce a macro to make this easier.
Signed-off-by: Ján Tomko <jtomko@redhat.com> --- tests/qemuxml2argvtest.c | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+)
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 04b6c00eba..b04cf7c923 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -467,6 +467,21 @@ testCompareXMLToStartupXML(const void *data) }
+# define TEST_EXCLUSIVE_FLAGS(FLAG1, FLAG2) \ + if ((testFlags & FLAG1) && (testFlags & FLAG2)) { \ + VIR_TEST_DEBUG("Flags %s and %s are mutually exclusive\n", \ + #FLAG1, #FLAG2); \ + return -1; \ + } + +
What's wrong with VIR_EXCLUSIVE_FLAGS_RET()? Michal
Use this macro to indicate the intention to also run the XML->startup XML test. It sets the newly introduced FLAG_STEAL_VM flag, which is the new witness for the XML->argv test to leave the VM object behind. This will allow us to report proper errors in XML->startup tests. Signed-off-by: Ján Tomko <jtomko@redhat.com> --- tests/qemuxml2argvtest.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index b04cf7c923..4d16dc58c1 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -293,6 +293,7 @@ typedef enum { FLAG_EXPECT_FAILURE = 1 << 0, FLAG_EXPECT_PARSE_ERROR = 1 << 1, FLAG_FIPS = 1 << 2, + FLAG_STEAL_VM = 1 << 3, } virQemuXML2ArgvTestFlags; struct testInfo { @@ -648,7 +649,7 @@ testCompareXMLToArgv(const void *data) ret = 0; } - if (!(flags & FLAG_EXPECT_FAILURE) && ret == 0) + if (flags & FLAG_STEAL_VM) VIR_STEAL_PTR(info->vm, vm); cleanup: @@ -858,6 +859,9 @@ mymain(void) # define DO_TEST_GIC(name, gic, ...) \ DO_TEST_FULL(name, NULL, -1, 0, 0, gic, __VA_ARGS__) +# define DO_TEST_WITH_STARTUP(name, ...) \ + DO_TEST_FULL(name, NULL, -1, FLAG_STEAL_VM, 0, GIC_NONE, __VA_ARGS__) + # define DO_TEST_FAILURE(name, ...) \ DO_TEST_FULL(name, NULL, -1, FLAG_EXPECT_FAILURE, \ 0, GIC_NONE, __VA_ARGS__) @@ -1097,7 +1101,7 @@ mymain(void) DO_TEST_PARSE_ERROR("disk-fmt-cow", NONE); DO_TEST_PARSE_ERROR("disk-fmt-dir", NONE); DO_TEST_PARSE_ERROR("disk-fmt-iso", NONE); - DO_TEST("disk-shared", NONE); + DO_TEST_WITH_STARTUP("disk-shared", NONE); DO_TEST_CAPS_VER("disk-shared", "2.12.0"); DO_TEST_CAPS_LATEST("disk-shared"); DO_TEST_PARSE_ERROR("disk-shared-qcow", NONE); -- 2.16.4
On 09/09/2018 04:10 AM, Ján Tomko wrote:
Use this macro to indicate the intention to also run the XML->startup XML test.
It sets the newly introduced FLAG_STEAL_VM flag, which is the new witness for the XML->argv test to leave the VM object behind.
This will allow us to report proper errors in XML->startup tests.
Signed-off-by: Ján Tomko <jtomko@redhat.com> --- tests/qemuxml2argvtest.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)
ACK Michal
Use the recently introduced flag as a witness. This reduces the apparent number of test cases to the real number of test cases. Note that this does not suffer from the same problem as commit 70255fa was fixing, because the condition for running virTestRun does not depend on results of previous tests. Signed-off-by: Ján Tomko <jtomko@redhat.com> --- tests/qemuxml2argvtest.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 4d16dc58c1..54eede5b38 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -479,6 +479,8 @@ testCompareXMLToStartupXML(const void *data) static int testCheckExclusiveFlags(int testFlags ATTRIBUTE_UNUSED) { + TEST_EXCLUSIVE_FLAGS(FLAG_STEAL_VM, FLAG_EXPECT_FAILURE); + TEST_EXCLUSIVE_FLAGS(FLAG_STEAL_VM, FLAG_EXPECT_PARSE_ERROR); return 0; } @@ -846,7 +848,8 @@ mymain(void) if (virTestRun("QEMU XML-2-ARGV " name, \ testCompareXMLToArgv, &info) < 0) \ ret = -1; \ - if (virTestRun("QEMU XML-2-startup-XML " name, \ + if (((flags) & FLAG_STEAL_VM) && \ + virTestRun("QEMU XML-2-startup-XML " name, \ testCompareXMLToStartupXML, &info) < 0) \ ret = -1; \ virObjectUnref(info.qemuCaps); \ -- 2.16.4
On 09/09/2018 04:10 AM, Ján Tomko wrote:
Use the recently introduced flag as a witness. This reduces the apparent number of test cases to the real number of test cases.
Note that this does not suffer from the same problem as commit 70255fa was fixing, because the condition for running virTestRun does not depend on results of previous tests.
Signed-off-by: Ján Tomko <jtomko@redhat.com> --- tests/qemuxml2argvtest.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 4d16dc58c1..54eede5b38 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -479,6 +479,8 @@ testCompareXMLToStartupXML(const void *data) static int testCheckExclusiveFlags(int testFlags ATTRIBUTE_UNUSED)
testFlags are no long unused, are they ;-)
{ + TEST_EXCLUSIVE_FLAGS(FLAG_STEAL_VM, FLAG_EXPECT_FAILURE); + TEST_EXCLUSIVE_FLAGS(FLAG_STEAL_VM, FLAG_EXPECT_PARSE_ERROR);
Again, I vote for VIR_EXCLUSIVE_FLAGS*().
return 0; }
@@ -846,7 +848,8 @@ mymain(void) if (virTestRun("QEMU XML-2-ARGV " name, \ testCompareXMLToArgv, &info) < 0) \ ret = -1; \ - if (virTestRun("QEMU XML-2-startup-XML " name, \ + if (((flags) & FLAG_STEAL_VM) && \ + virTestRun("QEMU XML-2-startup-XML " name, \ testCompareXMLToStartupXML, &info) < 0) \ ret = -1; \ virObjectUnref(info.qemuCaps); \
Michal
Now that the function is only run if requested by the FLAG_STEAL_VM flag, we know that missing data is an error, not a request to skip the test. The existence of the output file is now checked by virTestCompareToFile, which allows usage of the VIR_TEST_REGENERATE_OUTPUT=1 env variable to generate new test cases. Signed-off-by: Ján Tomko <jtomko@redhat.com> --- tests/qemuxml2argvtest.c | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 54eede5b38..56c4c18850 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -444,18 +444,15 @@ testCompareXMLToStartupXML(const void *data) char *actual = NULL; int ret = -1; - if (!info->vm) - return EXIT_AM_SKIP; + if (!info->vm) { + VIR_TEST_DEBUG("VM object missing. Did the args conversion succeed?"); + return -1; + } if (virAsprintf(&xml, "%s/qemuxml2startupxmloutdata/%s.xml", abs_srcdir, info->name) < 0) goto cleanup; - if (!virFileExists(xml)) { - ret = EXIT_AM_SKIP; - goto cleanup; - } - if (!(actual = virDomainDefFormat(info->vm->def, NULL, format_flags))) goto cleanup; -- 2.16.4
On 09/09/2018 04:10 AM, Ján Tomko wrote:
Now that the function is only run if requested by the FLAG_STEAL_VM flag, we know that missing data is an error, not a request to skip the test.
The existence of the output file is now checked by virTestCompareToFile, which allows usage of the VIR_TEST_REGENERATE_OUTPUT=1 env variable to generate new test cases.
Signed-off-by: Ján Tomko <jtomko@redhat.com> --- tests/qemuxml2argvtest.c | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-)
ACK Michal
Introduced by commit <af204232>. Made redundant by commit 1e9a083 which switched to using qemuProcessCreatePretendCmd, where capabilities are filtered in qemuProcessInit after being fetched from the cache. Signed-off-by: Ján Tomko <jtomko@redhat.com> --- tests/qemuxml2argvtest.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 56c4c18850..16427b3c85 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -426,8 +426,6 @@ testUpdateQEMUCaps(const struct testInfo *info, virQEMUCapsInitHostCPUModel(info->qemuCaps, caps->host.arch, VIR_DOMAIN_VIRT_QEMU); - virQEMUCapsFilterByMachineType(info->qemuCaps, vm->def->os.machine); - ret = 0; cleanup: -- 2.16.4
On 09/09/2018 04:10 AM, Ján Tomko wrote:
Introduced by commit <af204232>.
Made redundant by commit 1e9a083 which switched to using qemuProcessCreatePretendCmd, where capabilities are filtered in qemuProcessInit after being fetched from the cache.
Signed-off-by: Ján Tomko <jtomko@redhat.com> --- tests/qemuxml2argvtest.c | 2 -- 1 file changed, 2 deletions(-)
ACK Michal
None of the things testUpdateQEMUCaps adjusts are applicable for tests that use the DO_TEST_CAPS macros, i.e. real QEMU capabilities parsed from the XML files: The architecture must be chosen before we even open the caps file, CPU models are already present and the expensive HostModel computation was already done in virQEMUCapsLoadCache. Introduce FLAG_REAL_CAPS and skip the whole testUpdateQEMUCaps function for DO_TEST_CAPS. This speeds up the test by 25 % Signed-off-by: Ján Tomko <jtomko@redhat.com> --- tests/qemuxml2argvtest.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 16427b3c85..36fa99392a 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -294,6 +294,7 @@ typedef enum { FLAG_EXPECT_PARSE_ERROR = 1 << 1, FLAG_FIPS = 1 << 2, FLAG_STEAL_VM = 1 << 3, + FLAG_REAL_CAPS = 1 << 4, } virQemuXML2ArgvTestFlags; struct testInfo { @@ -568,7 +569,8 @@ testCompareXMLToArgv(const void *data) if (qemuProcessPrepareMonitorChr(&monitor_chr, priv->libDir) < 0) goto cleanup; - if (testUpdateQEMUCaps(info, vm, driver.caps) < 0) + if (!(info->flags & FLAG_REAL_CAPS) && + testUpdateQEMUCaps(info, vm, driver.caps) < 0) goto cleanup; log = virTestLogContentAndReset(); @@ -785,7 +787,7 @@ mymain(void) do { \ static struct testInfo info = { \ name, "." suffix, NULL, migrateFrom, migrateFrom ? 7 : -1,\ - (flags), parseFlags, false, NULL \ + (flags | FLAG_REAL_CAPS), parseFlags, false, NULL \ }; \ info.skipLegacyCPUs = skipLegacyCPUs; \ if (!(info.qemuCaps = qemuTestParseCapabilitiesArch(virArchFromString(arch), \ -- 2.16.4
On 09/09/2018 04:10 AM, Ján Tomko wrote:
None of the things testUpdateQEMUCaps adjusts are applicable for tests that use the DO_TEST_CAPS macros, i.e. real QEMU capabilities parsed from the XML files:
The architecture must be chosen before we even open the caps file, CPU models are already present and the expensive HostModel computation was already done in virQEMUCapsLoadCache.
Introduce FLAG_REAL_CAPS and skip the whole testUpdateQEMUCaps function for DO_TEST_CAPS.
This speeds up the test by 25 %
Signed-off-by: Ján Tomko <jtomko@redhat.com> --- tests/qemuxml2argvtest.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)
ACK Michal
Make it obvious when it is used intentionally and error out when used in combination with real capabilities. Signed-off-by: Ján Tomko <jtomko@redhat.com> --- tests/qemuxml2argvtest.c | 23 ++++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 36fa99392a..1a137399d4 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -295,6 +295,7 @@ typedef enum { FLAG_FIPS = 1 << 2, FLAG_STEAL_VM = 1 << 3, FLAG_REAL_CAPS = 1 << 4, + FLAG_SKIP_LEGACY_CPUS = 1 << 5, } virQemuXML2ArgvTestFlags; struct testInfo { @@ -305,7 +306,6 @@ struct testInfo { int migrateFd; unsigned int flags; unsigned int parseFlags; - bool skipLegacyCPUs; virDomainObjPtr vm; }; @@ -414,7 +414,8 @@ testUpdateQEMUCaps(const struct testInfo *info, virQEMUCapsInitQMPBasicArch(info->qemuCaps); - if (testAddCPUModels(info->qemuCaps, info->skipLegacyCPUs) < 0) + if (testAddCPUModels(info->qemuCaps, + !!(info->flags & FLAG_SKIP_LEGACY_CPUS)) < 0) goto cleanup; virQEMUCapsFreeHostCPUModel(info->qemuCaps, caps->host.arch, @@ -477,6 +478,7 @@ testCheckExclusiveFlags(int testFlags ATTRIBUTE_UNUSED) { TEST_EXCLUSIVE_FLAGS(FLAG_STEAL_VM, FLAG_EXPECT_FAILURE); TEST_EXCLUSIVE_FLAGS(FLAG_STEAL_VM, FLAG_EXPECT_PARSE_ERROR); + TEST_EXCLUSIVE_FLAGS(FLAG_REAL_CAPS, FLAG_SKIP_LEGACY_CPUS); return 0; } @@ -673,7 +675,6 @@ mymain(void) { int ret = 0, i; char *fakerootdir; - bool skipLegacyCPUs = false; const char *archs[] = { "aarch64", "ppc64", @@ -787,9 +788,8 @@ mymain(void) do { \ static struct testInfo info = { \ name, "." suffix, NULL, migrateFrom, migrateFrom ? 7 : -1,\ - (flags | FLAG_REAL_CAPS), parseFlags, false, NULL \ + (flags | FLAG_REAL_CAPS), parseFlags, NULL \ }; \ - info.skipLegacyCPUs = skipLegacyCPUs; \ if (!(info.qemuCaps = qemuTestParseCapabilitiesArch(virArchFromString(arch), \ capsfile))) \ return EXIT_FAILURE; \ @@ -836,9 +836,8 @@ mymain(void) do { \ static struct testInfo info = { \ name, NULL, NULL, migrateFrom, migrateFd, (flags), parseFlags, \ - false, NULL \ + NULL \ }; \ - info.skipLegacyCPUs = skipLegacyCPUs; \ if (testInitQEMUCaps(&info, gic) < 0) \ return EXIT_FAILURE; \ virQEMUCapsSetList(info.qemuCaps, __VA_ARGS__, QEMU_CAPS_LAST); \ @@ -1749,10 +1748,12 @@ mymain(void) DO_TEST("cpu-numa-memshared", QEMU_CAPS_OBJECT_MEMORY_FILE); DO_TEST("cpu-host-model", NONE); DO_TEST("cpu-host-model-vendor", NONE); - skipLegacyCPUs = true; - DO_TEST("cpu-host-model-fallback", NONE); - DO_TEST_FAILURE("cpu-host-model-nofallback", NONE); - skipLegacyCPUs = false; + DO_TEST_FULL("cpu-host-model-fallback", NULL, -1, + FLAG_SKIP_LEGACY_CPUS, 0, + GIC_NONE, NONE); + DO_TEST_FULL("cpu-host-model-nofallback", NULL, -1, + FLAG_SKIP_LEGACY_CPUS | FLAG_EXPECT_FAILURE, + 0, GIC_NONE, NONE); DO_TEST("cpu-host-passthrough", QEMU_CAPS_KVM); DO_TEST_FAILURE("cpu-qemu-host-passthrough", QEMU_CAPS_KVM); -- 2.16.4
On 09/09/2018 04:10 AM, Ján Tomko wrote:
Make it obvious when it is used intentionally and error out when used in combination with real capabilities.
Signed-off-by: Ján Tomko <jtomko@redhat.com> --- tests/qemuxml2argvtest.c | 23 ++++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-)
ACK Michal
After removing the host CPU model re-computation, this function is no longer necessary. This reverts commits: commit d0498881a04dddd772f9f63b03de80fb4c33d090 virQEMUCapsFreeHostCPUModel: Don't always free host cpuData commit 5276ec712a44b3680569a096e8fe56a925f0d495 testUpdateQEMUCaps: Don't leak host cpuData Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/qemu/qemu_capabilities.c | 25 ++----------------------- src/qemu/qemu_capspriv.h | 5 ----- tests/qemuxml2argvtest.c | 5 ----- 3 files changed, 2 insertions(+), 33 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 08cf822b88..69663de1ce 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -1526,19 +1526,12 @@ virQEMUCapsHostCPUDataCopy(virQEMUCapsHostCPUDataPtr dst, static void -virQEMUCapsHostCPUDataClearModels(virQEMUCapsHostCPUDataPtr cpuData) +virQEMUCapsHostCPUDataClear(virQEMUCapsHostCPUDataPtr cpuData) { + qemuMonitorCPUModelInfoFree(cpuData->info); virCPUDefFree(cpuData->reported); virCPUDefFree(cpuData->migratable); virCPUDefFree(cpuData->full); -} - - -static void -virQEMUCapsHostCPUDataClear(virQEMUCapsHostCPUDataPtr cpuData) -{ - qemuMonitorCPUModelInfoFree(cpuData->info); - virQEMUCapsHostCPUDataClearModels(cpuData); memset(cpuData, 0, sizeof(*cpuData)); } @@ -2984,20 +2977,6 @@ virQEMUCapsNewHostCPUModel(void) } -void -virQEMUCapsFreeHostCPUModel(virQEMUCapsPtr qemuCaps, - virArch hostArch, - virDomainVirtType type) -{ - virQEMUCapsHostCPUDataPtr cpuData = virQEMUCapsGetHostCPUData(qemuCaps, type); - - if (!virQEMUCapsGuestIsNative(hostArch, qemuCaps->arch)) - return; - - virQEMUCapsHostCPUDataClearModels(cpuData); -} - - void virQEMUCapsInitHostCPUModel(virQEMUCapsPtr qemuCaps, virArch hostArch, diff --git a/src/qemu/qemu_capspriv.h b/src/qemu/qemu_capspriv.h index cb5e0dd9a9..8d1a40fe74 100644 --- a/src/qemu/qemu_capspriv.h +++ b/src/qemu/qemu_capspriv.h @@ -56,11 +56,6 @@ void virQEMUCapsSetArch(virQEMUCapsPtr qemuCaps, virArch arch); -void -virQEMUCapsFreeHostCPUModel(virQEMUCapsPtr qemuCaps, - virArch hostArch, - virDomainVirtType type); - void virQEMUCapsInitHostCPUModel(virQEMUCapsPtr qemuCaps, virArch hostArch, diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 1a137399d4..867e0d569f 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -418,11 +418,6 @@ testUpdateQEMUCaps(const struct testInfo *info, !!(info->flags & FLAG_SKIP_LEGACY_CPUS)) < 0) goto cleanup; - virQEMUCapsFreeHostCPUModel(info->qemuCaps, caps->host.arch, - VIR_DOMAIN_VIRT_KVM); - virQEMUCapsFreeHostCPUModel(info->qemuCaps, caps->host.arch, - VIR_DOMAIN_VIRT_QEMU); - virQEMUCapsInitHostCPUModel(info->qemuCaps, caps->host.arch, VIR_DOMAIN_VIRT_KVM); virQEMUCapsInitHostCPUModel(info->qemuCaps, caps->host.arch, -- 2.16.4
On 09/09/2018 04:10 AM, Ján Tomko wrote:
After removing the host CPU model re-computation, this function is no longer necessary.
This reverts commits: commit d0498881a04dddd772f9f63b03de80fb4c33d090 virQEMUCapsFreeHostCPUModel: Don't always free host cpuData commit 5276ec712a44b3680569a096e8fe56a925f0d495 testUpdateQEMUCaps: Don't leak host cpuData
Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/qemu/qemu_capabilities.c | 25 ++----------------------- src/qemu/qemu_capspriv.h | 5 ----- tests/qemuxml2argvtest.c | 5 ----- 3 files changed, 2 insertions(+), 33 deletions(-)
ACK Michal
participants (2)
-
Ján Tomko -
Michal Privoznik