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/docs/schemas/capability.rng b/docs/schemas/capability.rng index 106ca73..65c7c72 100644 --- a/docs/schemas/capability.rng +++ b/docs/schemas/capability.rng @@ -290,6 +290,11 @@ <text/> </attribute> </optional> + <optional> + <attribute name='maxCpus'> + <ref name='unsignedInt'/> + </attribute> + </optional> <text/> </element> </define> diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c index da92c78..5aeb2ab 100644 --- a/src/conf/capabilities.c +++ b/src/conf/capabilities.c @@ -853,6 +853,8 @@ virCapabilitiesFormatXML(virCapsPtr caps) virBufferAddLit(&xml, " <machine"); if (machine->canonical) virBufferAsprintf(&xml, " canonical='%s'", machine->canonical); + if (machine->maxCpus > 0) + virBufferAsprintf(&xml, " maxCpus='%d'", machine->maxCpus); virBufferAsprintf(&xml, ">%s</machine>\n", machine->name); } @@ -871,6 +873,8 @@ virCapabilitiesFormatXML(virCapsPtr caps) virBufferAddLit(&xml, " <machine"); if (machine->canonical) virBufferAsprintf(&xml, " canonical='%s'", machine->canonical); + if (machine->maxCpus > 0) + virBufferAsprintf(&xml, " maxCpus='%d'", machine->maxCpus); virBufferAsprintf(&xml, ">%s</machine>\n", machine->name); } virBufferAddLit(&xml, " </domain>\n"); diff --git a/src/conf/capabilities.h b/src/conf/capabilities.h index abcf6de..22b6fb6 100644 --- a/src/conf/capabilities.h +++ b/src/conf/capabilities.h @@ -46,6 +46,7 @@ typedef virCapsGuestMachine *virCapsGuestMachinePtr; struct _virCapsGuestMachine { char *name; char *canonical; + int maxCpus; }; typedef struct _virCapsGuestDomainInfo virCapsGuestDomainInfo; diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index c4e076a..89f41b8 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -256,6 +256,7 @@ struct _virQEMUCaps { size_t nmachineTypes; char **machineTypes; char **machineAliases; + int *machineMaxCpus; }; struct _virQEMUCapsCache { @@ -335,6 +336,7 @@ virQEMUCapsSetDefaultMachine(virQEMUCapsPtr qemuCaps, { char *name = qemuCaps->machineTypes[defIdx]; char *alias = qemuCaps->machineAliases[defIdx]; + int cpu_max = qemuCaps->machineMaxCpus[defIdx]; memmove(qemuCaps->machineTypes + 1, qemuCaps->machineTypes, @@ -342,8 +344,12 @@ virQEMUCapsSetDefaultMachine(virQEMUCapsPtr qemuCaps, memmove(qemuCaps->machineAliases + 1, qemuCaps->machineAliases, sizeof(qemuCaps->machineAliases[0]) * defIdx); + memmove(qemuCaps->machineMaxCpus + 1, + qemuCaps->machineMaxCpus, + sizeof(qemuCaps->machineMaxCpus[0]) * defIdx); qemuCaps->machineTypes[0] = name; qemuCaps->machineAliases[0] = alias; + qemuCaps->machineMaxCpus[0] = cpu_max; } /* Format is: @@ -390,7 +396,8 @@ virQEMUCapsParseMachineTypesStr(const char *output, } if (VIR_REALLOC_N(qemuCaps->machineTypes, qemuCaps->nmachineTypes + 1) < 0 || - VIR_REALLOC_N(qemuCaps->machineAliases, qemuCaps->nmachineTypes + 1) < 0) { + VIR_REALLOC_N(qemuCaps->machineAliases, qemuCaps->nmachineTypes + 1) < 0 || + VIR_REALLOC_N(qemuCaps->machineMaxCpus, qemuCaps->nmachineTypes + 1) < 0) { VIR_FREE(name); VIR_FREE(canonical); virReportOOMError(); @@ -404,6 +411,8 @@ virQEMUCapsParseMachineTypesStr(const char *output, qemuCaps->machineTypes[qemuCaps->nmachineTypes-1] = name; qemuCaps->machineAliases[qemuCaps->nmachineTypes-1] = NULL; } + /* Value 0 means "unknown" as it's not exposed by QEMU binary */ + qemuCaps->machineMaxCpus[qemuCaps->nmachineTypes-1] = 0; } while ((p = next)); @@ -1764,11 +1773,14 @@ virQEMUCapsPtr virQEMUCapsNewCopy(virQEMUCapsPtr qemuCaps) goto no_memory; if (VIR_ALLOC_N(ret->machineAliases, qemuCaps->nmachineTypes) < 0) goto no_memory; + if (VIR_ALLOC_N(ret->machineMaxCpus, qemuCaps->nmachineTypes) < 0) + goto no_memory; ret->nmachineTypes = qemuCaps->nmachineTypes; for (i = 0; i < qemuCaps->nmachineTypes; i++) { if (VIR_STRDUP(ret->machineTypes[i], qemuCaps->machineTypes[i]) < 0 || VIR_STRDUP(ret->machineAliases[i], qemuCaps->machineAliases[i]) < 0) goto error; + ret->machineMaxCpus[i] = qemuCaps->machineMaxCpus[i]; } return ret; @@ -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; } VIR_FREE(qemuCaps->machineTypes); VIR_FREE(qemuCaps->machineAliases); + VIR_FREE(qemuCaps->machineMaxCpus); for (i = 0; i < qemuCaps->ncpuDefinitions; i++) { VIR_FREE(qemuCaps->cpuDefinitions[i]); @@ -1932,6 +1946,7 @@ int virQEMUCapsGetMachineTypesCaps(virQEMUCapsPtr qemuCaps, if (VIR_STRDUP(mach->name, qemuCaps->machineTypes[i]) < 0) goto error; } + mach->maxCpus = qemuCaps->machineMaxCpus[i]; (*machines)[i] = mach; } @@ -1966,6 +1981,25 @@ const char *virQEMUCapsGetCanonicalMachine(virQEMUCapsPtr qemuCaps, } +int virQEMUCapsGetMachineMaxCpus(virQEMUCapsPtr qemuCaps, + const char *name) +{ + size_t i; + + if (!name) + return 0; + + for (i = 0; i < qemuCaps->nmachineTypes; i++) { + if (!qemuCaps->machineMaxCpus[i]) + continue; + if (STREQ(qemuCaps->machineTypes[i], name)) + return qemuCaps->machineMaxCpus[i]; + } + + return 0; +} + + static int virQEMUCapsProbeQMPCommands(virQEMUCapsPtr qemuCaps, qemuMonitorPtr mon) @@ -2083,6 +2117,10 @@ virQEMUCapsProbeQMPMachineTypes(virQEMUCapsPtr qemuCaps, virReportOOMError(); goto cleanup; } + if (VIR_ALLOC_N(qemuCaps->machineMaxCpus, nmachines) < 0) { + virReportOOMError(); + goto cleanup; + } for (i = 0; i < nmachines; i++) { if (VIR_STRDUP(qemuCaps->machineAliases[i], machines[i]->alias) < 0 || @@ -2090,6 +2128,7 @@ virQEMUCapsProbeQMPMachineTypes(virQEMUCapsPtr qemuCaps, goto cleanup; if (machines[i]->isDefault) defIdx = i; + qemuCaps->machineMaxCpus[i] = machines[i]->cpu_max; } qemuCaps->nmachineTypes = nmachines; 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; }; int qemuMonitorGetMachines(qemuMonitorPtr mon, diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 88a0dc9..1362acf 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -4042,6 +4042,12 @@ int qemuMonitorJSONGetMachines(qemuMonitorPtr mon, if (VIR_STRDUP(info->alias, tmp) < 0) goto cleanup; } + if (virJSONValueObjectHasKey(child, "cpu-max") && + virJSONValueObjectGetNumberInt(child, "cpu-max", &info->cpu_max) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("query-machines reply has malformed 'cpu-max' data")); + goto cleanup; + } } ret = n; 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; + } + + 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, @@ -3519,6 +3543,9 @@ int qemuProcessStart(virConnectPtr conn, vm->def->emulator))) goto cleanup; + if (!qemuValidateCpuMax(vm->def, priv->qemuCaps)) + goto cleanup; + if (qemuAssignDeviceAliases(vm->def, priv->qemuCaps) < 0) goto cleanup; diff --git a/tests/capabilityschemadata/caps-qemu-kvm.xml b/tests/capabilityschemadata/caps-qemu-kvm.xml index 36c4b49..1fbc22b 100644 --- a/tests/capabilityschemadata/caps-qemu-kvm.xml +++ b/tests/capabilityschemadata/caps-qemu-kvm.xml @@ -33,18 +33,18 @@ <arch name='i686'> <wordsize>32</wordsize> <emulator>/usr/bin/qemu</emulator> - <machine>pc-0.11</machine> - <machine canonical='pc-0.11'>pc</machine> - <machine>pc-0.10</machine> - <machine>isapc</machine> + <machine maxCpus='255'>pc-0.11</machine> + <machine canonical='pc-0.11' maxCpus='255'>pc</machine> + <machine maxCpus='255'>pc-0.10</machine> + <machine maxCpus='1'>isapc</machine> <domain type='qemu'> </domain> <domain type='kvm'> <emulator>/usr/bin/qemu-kvm</emulator> - <machine>pc-0.11</machine> - <machine canonical='pc-0.11'>pc</machine> - <machine>pc-0.10</machine> - <machine>isapc</machine> + <machine maxCpus='255'>pc-0.11</machine> + <machine canonical='pc-0.11' maxCpus='255'>pc</machine> + <machine maxCpus='255'>pc-0.10</machine> + <machine maxCpus='1'>isapc</machine> </domain> </arch> <features>

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

On 06/26/2013 04:20 PM, Michal Novotny wrote: > On 06/26/2013 04:17 PM, Martin Kletzander wrote: >> 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 > Ok Martin, would you like me to do the changes you proposed or will you > refresh the patch yourself and no need to submit v5 ? > If you'll make it with the v5 before the release, the added value will be that everyone will be offered the look to how the final patch looks like, but if you won't I'll push this one with the changes I proposed. Martin