On 08/12/2016 09:33 AM, Jiri Denemark wrote:
> Changing a host architecture or a CPU is not as easy as assigning a new
> value to the appropriate element in virCaps since there is a relation
> between the CPU and host architecture (we don't really want to test
> anything on an AArch64 host with core2duo CPU). This patch introduces
> qemuTestSetHostArch and qemuTestSetHostCPU helpers which will make sure
> the host architecture matches the host CPU.
>
> Signed-off-by: Jiri Denemark <jdenemar@xxxxxxxxxx>
> ---
> tests/testutilsqemu.c | 51 ++++++++++++++++++++++++++++++++++++++++++++++-----
> tests/testutilsqemu.h | 8 ++++++--
> 2 files changed, 52 insertions(+), 7 deletions(-)
>
> diff --git a/tests/testutilsqemu.c b/tests/testutilsqemu.c
> index 11dd20e..9b66101 100644
> --- a/tests/testutilsqemu.c
> +++ b/tests/testutilsqemu.c
> @@ -16,6 +16,7 @@
>
> virCPUDefPtr cpuDefault;
> virCPUDefPtr cpuHaswell;
> +virCPUDefPtr cpuPower8;
^^
Nothing in the commit message that indicates that we're also adding a
power8 type.
Thus it would seem that this could be two commits - I would think adding
the cpuPower8Data
[1] Also, should each of these should be initialized to NULL?
>
> static virCPUFeatureDef cpuDefaultFeatures[] = {
> { (char *) "lahf_lm", -1 },
> @@ -92,6 +93,15 @@ static virCPUDef cpuHaswellData = {
> cpuHaswellFeatures, /* features */
> };
>
> +static virCPUDef cpuPower8Data = {
> + .type = VIR_CPU_TYPE_HOST,
> + .arch = VIR_ARCH_PPC64,
> + .model = (char *) "POWER8",
> + .sockets = 1,
> + .cores = 8,
> + .threads = 8,
> +};
> +
> static virCapsGuestMachinePtr *testQemuAllocMachines(int *nmachines)
> {
> virCapsGuestMachinePtr *machines;
> @@ -331,7 +341,8 @@ virCapsPtr testQemuCapsInit(void)
> goto cleanup;
>
> if (!(cpuDefault = virCPUDefCopy(&cpuDefaultData)) ||
> - !(cpuHaswell = virCPUDefCopy(&cpuHaswellData)))
> + !(cpuHaswell = virCPUDefCopy(&cpuHaswellData)) ||
> + !(cpuPower8 = virCPUDefCopy(&cpuPower8Data)))
> goto cleanup;
>
> caps->host.cpu = cpuDefault;
Should this change to a qemuTestSetHostCPU(caps, NULL) to work properly
on power8?
> @@ -440,15 +451,45 @@ virCapsPtr testQemuCapsInit(void)
>
> cleanup:
> virCapabilitiesFreeMachines(machines, nmachines);
> - if (caps->host.cpu != cpuDefault)
> - virCPUDefFree(cpuDefault);
> - if (caps->host.cpu != cpuHaswell)
> - virCPUDefFree(cpuHaswell);
> + caps->host.cpu = NULL;
> + virCPUDefFree(cpuDefault);
> + virCPUDefFree(cpuHaswell);
> + virCPUDefFree(cpuPower8);
[1]
Since we can get to cleanup without ever initializing any of these...
ACK 1-10 - just be sure to check/consider thoughts left in 2, 3, and
here. Of course we're post freeze now, so I guess there's a bit more
waiting to do anyway.
John
> virObjectUnref(caps);
> return NULL;
> }
>
>
> +void
> +qemuTestSetHostArch(virCapsPtr caps,
> + virArch arch)
> +{
> + if (arch == VIR_ARCH_NONE)
> + arch = VIR_ARCH_X86_64;
> + caps->host.arch = arch;
> + qemuTestSetHostCPU(caps, NULL);
> +}
> +
> +
> +void
> +qemuTestSetHostCPU(virCapsPtr caps,
> + virCPUDefPtr cpu)
> +{
> + virArch arch = caps->host.arch;
> +
> + if (!cpu) {
> + if (ARCH_IS_X86(arch))
> + cpu = cpuDefault;
> + else if (ARCH_IS_PPC64(arch))
> + cpu = cpuPower8;
> + }
> +
> + if (cpu)
> + caps->host.arch = cpu->arch;
> + caps->host.cpu = cpu;
> +}
> +
> +
> virQEMUCapsPtr
> qemuTestParseCapabilities(const char *capsFile)
> {
> diff --git a/tests/testutilsqemu.h b/tests/testutilsqemu.h
> index f2b71e9..6d35116f 100644
> --- a/tests/testutilsqemu.h
> +++ b/tests/testutilsqemu.h
> @@ -19,8 +19,12 @@ virQEMUCapsPtr qemuTestParseCapabilities(const char *capsFile);
>
> extern virCPUDefPtr cpuDefault;
> extern virCPUDefPtr cpuHaswell;
> -void testQemuCapsSetCPU(virCapsPtr caps,
> - virCPUDefPtr hostCPU);
> +extern virCPUDefPtr cpuPower8;
> +
> +void qemuTestSetHostArch(virCapsPtr caps,
> + virArch arch);
> +void qemuTestSetHostCPU(virCapsPtr caps,
> + virCPUDefPtr cpu);
>
> int qemuTestDriverInit(virQEMUDriver *driver);
> void qemuTestDriverFree(virQEMUDriver *driver);
>
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list