Re: [libvirt] [PATCH v4] qemu: Implement CPUs check against machine type's cpu-max

On 06/25/2013 05:44 PM, Michal Novotny wrote: > Implement check whether (maximum) vCPUs doesn't exceed machine > type's cpu-max settings. > > Differences between v3 and v4 (this one): > - Rebased to latest libvirt version > - Capability XML output extended by maxCpus field > - Extended caps-qemu-kvm.xml test by maxCpus for one of test emulators > > On older versions of QEMU the check is disabled. > > Signed-off-by: Michal Novotny <minovotn(a)redhat.com&gt; > --- > docs/schemas/capability.rng | 5 ++++ > src/conf/capabilities.c | 4 +++ > src/conf/capabilities.h | 1 + > src/qemu/qemu_capabilities.c | 41 +++++++++++++++++++++++++++- > src/qemu/qemu_capabilities.h | 3 +- > src/qemu/qemu_monitor.h | 1 + > src/qemu/qemu_monitor_json.c | 6 ++++ > src/qemu/qemu_process.c | 27 ++++++++++++++++++ > tests/capabilityschemadata/caps-qemu-kvm.xml | 16 +++++------ > 9 files changed, 94 insertions(+), 10 deletions(-) > [...] > diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c [...] > @@ -1789,9 +1801,11 @@ void virQEMUCapsDispose(void *obj) > for (i = 0; i < qemuCaps->nmachineTypes; i++) { > VIR_FREE(qemuCaps->machineTypes[i]); > VIR_FREE(qemuCaps->machineAliases[i]); > + qemuCaps->machineMaxCpus[i] = -1; Pointless line. > } > VIR_FREE(qemuCaps->machineTypes); > VIR_FREE(qemuCaps->machineAliases); > + VIR_FREE(qemuCaps->machineMaxCpus); > > for (i = 0; i < qemuCaps->ncpuDefinitions; i++) { > VIR_FREE(qemuCaps->cpuDefinitions[i]); [...] > diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h > index 64a4b1d..7088747 100644 > --- a/src/qemu/qemu_capabilities.h > +++ b/src/qemu/qemu_capabilities.h > @@ -234,7 +234,8 @@ size_t virQEMUCapsGetMachineTypes(virQEMUCapsPtr qemuCaps, > char ***names); > const char *virQEMUCapsGetCanonicalMachine(virQEMUCapsPtr qemuCaps, > const char *name); > - > +int virQEMUCapsGetMachineMaxCpus(virQEMUCapsPtr qemuCaps, > + const char *name); > int virQEMUCapsGetMachineTypesCaps(virQEMUCapsPtr qemuCaps, > size_t *nmachines, > virCapsGuestMachinePtr **machines); > diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h > index 3d9afa3..06ae4c5 100644 > --- a/src/qemu/qemu_monitor.h > +++ b/src/qemu/qemu_monitor.h > @@ -654,6 +654,7 @@ struct _qemuMonitorMachineInfo { > char *name; > bool isDefault; > char *alias; > + int cpu_max; This parameter should be unified to match the previous naming (maxCpus) as cpu_max might be misunderstood as a maximum cpu number, not the maximum number of cpus. > }; > > int qemuMonitorGetMachines(qemuMonitorPtr mon, [...] > diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c > index 5a0f18b..3146ce2 100644 > --- a/src/qemu/qemu_process.c > +++ b/src/qemu/qemu_process.c > @@ -3330,6 +3330,30 @@ error: > } > > > +static bool > +qemuValidateCpuMax(virDomainDefPtr def, virQEMUCapsPtr qemuCaps) > +{ > + int cpu_max; > + > + cpu_max = virQEMUCapsGetMachineMaxCpus(qemuCaps, def->os.machine); > + if (!cpu_max) > + return true; > + > + if (def->vcpus > cpu_max) { > + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, > + "%s", _("CPUs greater than specified machine type limit")); > + return false; > + } > + This check is pointless since it's guaranteed that vcpus <= maxvcpus. > + if (def->maxvcpus > cpu_max) { > + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, > + "%s", _("Maximum CPUs greater than specified machine type limit")); > + return false; > + } > + > + return true; > +} > + > int qemuProcessStart(virConnectPtr conn, > virQEMUDriverPtr driver, > virDomainObjPtr vm, Other that that the patch looks fine, but I'd wait with the push after release. If nobody is against that (and against the changes I proposed), I'll push this after the release. Martin