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@redhat.com> --- 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@redhat.com> John