When preparing qemuCaps for test cases the following is
happening:

qemuTestParseCapabilitiesArch() is called, which calls
virQEMUCapsLoadCache() which in turn calls
virQEMUCapsInitHostCPUModel() which sets qemuCaps->kvmCPU and
qemuCaps->tcgCPU.

But then the code tries to update the capabilities:

testCompareXMLToArgv() calls testUpdateQEMUCaps() which calls
virQEMUCapsInitHostCPUModel() again overwriting previously
allocated memory. The solution is to free host cpuData in
testUpdateQEMUCaps().

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---

Technically this is v2 of [1] but since it implements completely
different approach I'm sending it as v1.

1: https://www.redhat.com/archives/libvir-list/2018-May/msg02260.html

 src/qemu/qemu_capabilities.c | 21 +++++++++++++++++++--
 src/qemu/qemu_capspriv.h     |  4 ++++
 tests/qemuxml2argvtest.c     |  3 +++
 3 files changed, 26 insertions(+), 2 deletions(-)

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index e2e76e4dd8..ea1ff520d8 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -1516,12 +1516,19 @@ virQEMUCapsHostCPUDataCopy(virQEMUCapsHostCPUDataPtr dst,
 
 
 static void
-virQEMUCapsHostCPUDataClear(virQEMUCapsHostCPUDataPtr cpuData)
+virQEMUCapsHostCPUDataClearHostCPU(virQEMUCapsHostCPUDataPtr cpuData)
 {
-    qemuMonitorCPUModelInfoFree(cpuData->info);
     virCPUDefFree(cpuData->reported);
     virCPUDefFree(cpuData->migratable);
     virCPUDefFree(cpuData->full);
+}
+
+
+static void
+virQEMUCapsHostCPUDataClear(virQEMUCapsHostCPUDataPtr cpuData)
+{
+    qemuMonitorCPUModelInfoFree(cpuData->info);
+    virQEMUCapsHostCPUDataClearHostCPU(cpuData);
 
     memset(cpuData, 0, sizeof(*cpuData));
 }
@@ -2834,6 +2841,16 @@ virQEMUCapsNewHostCPUModel(void)
 }
 
 
+void
+virQEMUCapsFreeHostCPUModel(virQEMUCapsPtr qemuCaps,
+                            virDomainVirtType type)
+{
+    virQEMUCapsHostCPUDataPtr cpuData = virQEMUCapsGetHostCPUData(qemuCaps, type);
+
+    virQEMUCapsHostCPUDataClearHostCPU(cpuData);
+}
+
+
 void
 virQEMUCapsInitHostCPUModel(virQEMUCapsPtr qemuCaps,
                             virArch hostArch,
diff --git a/src/qemu/qemu_capspriv.h b/src/qemu/qemu_capspriv.h
index 0199501c93..fea039ef3a 100644
--- a/src/qemu/qemu_capspriv.h
+++ b/src/qemu/qemu_capspriv.h
@@ -56,6 +56,10 @@ void
 virQEMUCapsSetArch(virQEMUCapsPtr qemuCaps,
                    virArch arch);
 
+void
+virQEMUCapsFreeHostCPUModel(virQEMUCapsPtr qemuCaps,
+                            virDomainVirtType type);
+
 void
 virQEMUCapsInitHostCPUModel(virQEMUCapsPtr qemuCaps,
                             virArch hostArch,
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index a6a40e273e..61c7ae59aa 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -388,6 +388,9 @@ testUpdateQEMUCaps(const struct testInfo *info,
     if (testAddCPUModels(info->qemuCaps, info->skipLegacyCPUs) < 0)
         goto cleanup;
 
+    virQEMUCapsFreeHostCPUModel(info->qemuCaps, VIR_DOMAIN_VIRT_KVM);
+    virQEMUCapsFreeHostCPUModel(info->qemuCaps, VIR_DOMAIN_VIRT_QEMU);
+
     virQEMUCapsInitHostCPUModel(info->qemuCaps, caps->host.arch,
                                 VIR_DOMAIN_VIRT_KVM);
     virQEMUCapsInitHostCPUModel(info->qemuCaps, caps->host.arch,
-- 
2.16.4

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

On Thu, May 31, 2018 at 12:11:36 +0200, Michal Privoznik wrote:
> When preparing qemuCaps for test cases the following is
> happening:
> 
> qemuTestParseCapabilitiesArch() is called, which calls
> virQEMUCapsLoadCache() which in turn calls
> virQEMUCapsInitHostCPUModel() which sets qemuCaps->kvmCPU and
> qemuCaps->tcgCPU.
> 
> But then the code tries to update the capabilities:
> 
> testCompareXMLToArgv() calls testUpdateQEMUCaps() which calls
> virQEMUCapsInitHostCPUModel() again overwriting previously
> allocated memory. The solution is to free host cpuData in
> testUpdateQEMUCaps().
> 
> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
> ---
> 
> Technically this is v2 of [1] but since it implements completely
> different approach I'm sending it as v1.
> 
> 1: https://www.redhat.com/archives/libvir-list/2018-May/msg02260.html
> 
>  src/qemu/qemu_capabilities.c | 21 +++++++++++++++++++--
>  src/qemu/qemu_capspriv.h     |  4 ++++
>  tests/qemuxml2argvtest.c     |  3 +++
>  3 files changed, 26 insertions(+), 2 deletions(-)
> 
> diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
> index e2e76e4dd8..ea1ff520d8 100644
> --- a/src/qemu/qemu_capabilities.c
> +++ b/src/qemu/qemu_capabilities.c
> @@ -1516,12 +1516,19 @@ virQEMUCapsHostCPUDataCopy(virQEMUCapsHostCPUDataPtr dst,
>  
>  
>  static void
> -virQEMUCapsHostCPUDataClear(virQEMUCapsHostCPUDataPtr cpuData)
> +virQEMUCapsHostCPUDataClearHostCPU(virQEMUCapsHostCPUDataPtr cpuData)

The name is weired with two HostCPU substrings. How about

    virQEMUCapsHostCPUDataClearModels

as it frees the generated CPU models, or

    virQEMUCapsHostCPUDataClearGenerated

to emphasize the function clears the CPU models generated from other
data which we store in the cache?

>  {
> -    qemuMonitorCPUModelInfoFree(cpuData->info);
>      virCPUDefFree(cpuData->reported);
>      virCPUDefFree(cpuData->migratable);
>      virCPUDefFree(cpuData->full);
> +}
> +
> +
> +static void
> +virQEMUCapsHostCPUDataClear(virQEMUCapsHostCPUDataPtr cpuData)
> +{
> +    qemuMonitorCPUModelInfoFree(cpuData->info);
> +    virQEMUCapsHostCPUDataClearHostCPU(cpuData);

And this would need to be changed too, of course.

>  
>      memset(cpuData, 0, sizeof(*cpuData));
>  }

With the changed name

Reviewed-by: Jiri Denemark <jdenemar@redhat.com>

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