On 10/04/2017 10:58 AM, Jiri Denemark wrote:
> All APIs which expect a list of CPU models supported by hypervisors were
> switched from char **models and int models to just accept a pointer to
> virDomainCapsCPUModels object stored in domain capabilities. This avoids
> the need to transform virDomainCapsCPUModelsPtr into a NULL-terminated
> list of model names and also allows the various cpu driver APIs to
> access additional details (such as its usability) about each CPU model.
>
> Signed-off-by: Jiri Denemark <jdenemar@xxxxxxxxxx>
> ---
> src/cpu/cpu.c | 78 +++++++++++++-----------------------
> src/cpu/cpu.h | 28 +++++--------
> src/cpu/cpu_arm.c | 3 +-
> src/cpu/cpu_ppc64.c | 13 +++---
> src/cpu/cpu_x86.c | 25 +++++-------
> src/libxl/libxl_capabilities.c | 2 +-
> src/libxl/libxl_driver.c | 2 +-
> src/qemu/qemu_capabilities.c | 63 ++++++------------------------
> src/qemu/qemu_capabilities.h | 6 +--
> src/qemu/qemu_driver.c | 2 +-
> src/qemu/qemu_process.c | 9 +----
> src/test/test_driver.c | 2 +-
> tests/cputest.c | 89 ++++++++++++++++++++++++++++++------------
> 13 files changed, 138 insertions(+), 184 deletions(-)
>
[...]
> @@ -501,11 +491,10 @@ virCPUProbeHost(virArch arch)
> * @cpus: list of host CPU definitions
> * @ncpus: number of CPUs in @cpus
> * @models: list of CPU models that can be considered for the baseline CPU
> - * @nmodels: number of CPU models in @models
> * @migratable: requests non-migratable features to be removed from the result
> *
> * Computes the most feature-rich CPU which is compatible with all given
> - * host CPUs. If @models array is NULL, all models supported by libvirt will
> + * host CPUs. If @models is NULL, all models supported by libvirt will
> * be considered when computing the baseline CPU model, otherwise the baseline
> * CPU model will be one of the provided CPU @models.
> *
> @@ -514,21 +503,20 @@ virCPUProbeHost(virArch arch)
> virCPUDefPtr
> cpuBaseline(virCPUDefPtr *cpus,
> unsigned int ncpus,
> - const char **models,
> - unsigned int nmodels,
> + virDomainCapsCPUModelsPtr models,
> bool migratable)
> {
> struct cpuArchDriver *driver;
> size_t i;
>
> - VIR_DEBUG("ncpus=%u, nmodels=%u", ncpus, nmodels);
> + VIR_DEBUG("ncpus=%u", ncpus);
You could add models=%p... not that it's necessary
> if (cpus) {
> for (i = 0; i < ncpus; i++)
> VIR_DEBUG("cpus[%zu]=%p", i, cpus[i]);
> }
> if (models) {
> - for (i = 0; i < nmodels; i++)
> - VIR_DEBUG("models[%zu]=%s", i, NULLSTR(models[i]));
> + for (i = 0; i < models->nmodels; i++)
> + VIR_DEBUG("models[%zu]=%s", i, models->models[i].name);
> }
>
[...]
> diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
> index 7ddc6cafd4..225cee4ef9 100644
> --- a/src/qemu/qemu_capabilities.c
> +++ b/src/qemu/qemu_capabilities.c
[...]
> -int
> +virDomainCapsCPUModelsPtr
> virQEMUCapsGetCPUDefinitions(virQEMUCapsPtr qemuCaps,
> - virDomainVirtType type,
> - char ***names,
> - size_t *count)
> + virDomainVirtType type)
> {
> - size_t i;
> - char **models = NULL;
> - virDomainCapsCPUModelsPtr cpus;
> -
> - *count = 0;
> - if (names)
> - *names = NULL;
> -
> if (type == VIR_DOMAIN_VIRT_KVM)
> - cpus = qemuCaps->kvmCPUModels;
> + return qemuCaps->kvmCPUModels;
> else
> - cpus = qemuCaps->tcgCPUModels;
> + return qemuCaps->tcgCPUModels;
>
> - if (!cpus)
> - return 0;
> -
> - if (names && VIR_ALLOC_N(models, cpus->nmodels) < 0)
> - return -1;
> -
> - for (i = 0; i < cpus->nmodels; i++) {
> - virDomainCapsCPUModelPtr cpu = cpus->models + i;
> - if (models && VIR_STRDUP(models[i], cpu->name) < 0)
> - goto error;
> - }
> -
> - if (names)
> - *names = models;
> - *count = cpus->nmodels;
> - return 0;
> -
> - error:
> - virStringListFreeCount(models, i);
> - return -1;
> + return NULL;
Nice overall simplification, but how do we reach here? Wouldn't
tcgCPUModels either be something or NULL. Previous code just returns 0
w/ @models and @nmodels untouched which would seemingly be the same with
these changes.
That would also say to me the "else" above would be unnecessary.
> }
>
>
> @@ -3392,8 +3353,6 @@ virQEMUCapsInitCPUModelX86(virQEMUCapsPtr qemuCaps,
> virCPUDataPtr data = NULL;
> unsigned long long sigFamily = 0;
> unsigned long long sigModel = 0;
> - size_t nmodels = 0;
> - char **models = NULL;
> int ret = -1;
> size_t i;
>
Reviewed-by: John Ferlan <jferlan@xxxxxxxxxx>
John
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
