Re: [PATCH 10/41] testutilsqemu: Helpers for changing host CPU and arch

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




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



[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]