[libvirt] [PATCH 0/2] Add cpu-max to capabilities and implement checks
Add cpu-max to capabilities XML output for newer QEMU and implement checks whether the number of vCPUs/maximum vCPUS doesn't exceed machine limit. For older versions of QEMU cpu-max field is not present so the cpu-max is set to 0 to disable checking and act the same as without this patch. Signed-off-by: Michal Novotny <minovotn@redhat.com> Michal Novotny (2): cpu-max: Implement cpu-max attribute into capabilities XML output qemu: Implement validation for max-cpu in XML docs/schemas/capability.rng | 5 +++++ src/conf/capabilities.c | 4 ++++ src/conf/capabilities.h | 1 + src/qemu/qemu_capabilities.c | 40 +++++++++++++++++++++++++++++++++++++++- src/qemu/qemu_capabilities.h | 3 ++- src/qemu/qemu_driver.c | 30 ++++++++++++++++++++++++++++++ src/qemu/qemu_monitor.h | 1 + src/qemu/qemu_monitor_json.c | 7 +++++++ 8 files changed, 89 insertions(+), 2 deletions(-) -- 1.7.11.7
Implement the cpu-max attribute into virConnectGetCapabilities() API output to allow caller to check maximum number of CPUs to be set for specified machine type. Signed-off-by: Michal Novotny <minovotn@redhat.com> --- docs/schemas/capability.rng | 5 +++++ src/conf/capabilities.c | 4 ++++ src/conf/capabilities.h | 1 + src/qemu/qemu_capabilities.c | 40 +++++++++++++++++++++++++++++++++++++++- src/qemu/qemu_capabilities.h | 3 ++- src/qemu/qemu_monitor.h | 1 + src/qemu/qemu_monitor_json.c | 7 +++++++ 7 files changed, 59 insertions(+), 2 deletions(-) diff --git a/docs/schemas/capability.rng b/docs/schemas/capability.rng index 106ca73..4f87397 100644 --- a/docs/schemas/capability.rng +++ b/docs/schemas/capability.rng @@ -290,6 +290,11 @@ <text/> </attribute> </optional> + <optional> + <attribute name='cpu-max'> + <ref name='unsignedInt'/> + </attribute> + </optional> <text/> </element> </define> diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c index c7ec92f..ab1bdd3 100644 --- a/src/conf/capabilities.c +++ b/src/conf/capabilities.c @@ -860,6 +860,8 @@ virCapabilitiesFormatXML(virCapsPtr caps) for (j = 0 ; j < caps->guests[i]->arch.defaultInfo.nmachines ; j++) { virCapsGuestMachinePtr machine = caps->guests[i]->arch.defaultInfo.machines[j]; virBufferAddLit(&xml, " <machine"); + if (machine->cpu_max > 0) + virBufferAsprintf(&xml, " cpu-max='%d'", machine->cpu_max); if (machine->canonical) virBufferAsprintf(&xml, " canonical='%s'", machine->canonical); virBufferAsprintf(&xml, ">%s</machine>\n", machine->name); @@ -878,6 +880,8 @@ virCapabilitiesFormatXML(virCapsPtr caps) for (k = 0 ; k < caps->guests[i]->arch.domains[j]->info.nmachines ; k++) { virCapsGuestMachinePtr machine = caps->guests[i]->arch.domains[j]->info.machines[k]; virBufferAddLit(&xml, " <machine"); + if (machine->cpu_max > 0) + virBufferAsprintf(&xml, " cpu-max='%d'", machine->cpu_max); if (machine->canonical) virBufferAsprintf(&xml, " canonical='%s'", machine->canonical); virBufferAsprintf(&xml, ">%s</machine>\n", machine->name); diff --git a/src/conf/capabilities.h b/src/conf/capabilities.h index abcf6de..16bf3de 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 cpu_max; }; typedef struct _virCapsGuestDomainInfo virCapsGuestDomainInfo; diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index d10c8aa..a03a643 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -243,6 +243,7 @@ struct _virQEMUCaps { size_t nmachineTypes; char **machineTypes; char **machineAliases; + int *machineMaxCpus; }; struct _virQEMUCapsCache { @@ -322,6 +323,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, @@ -329,8 +331,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: @@ -377,7 +383,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); goto no_memory; @@ -390,6 +397,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)); @@ -1718,6 +1727,8 @@ 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 (!(ret->machineTypes[i] = strdup(qemuCaps->machineTypes[i]))) @@ -1725,6 +1736,7 @@ virQEMUCapsPtr virQEMUCapsNewCopy(virQEMUCapsPtr qemuCaps) if (qemuCaps->machineAliases[i] && !(ret->machineAliases[i] = strdup(qemuCaps->machineAliases[i]))) goto no_memory; + ret->machineMaxCpus[i] = qemuCaps->machineMaxCpus[i]; } return ret; @@ -1885,9 +1897,11 @@ int virQEMUCapsGetMachineTypesCaps(virQEMUCapsPtr qemuCaps, goto no_memory; if (!(mach->canonical = strdup(qemuCaps->machineTypes[i]))) goto no_memory; + mach->cpu_max = qemuCaps->machineMaxCpus[i]; } else { if (!(mach->name = strdup(qemuCaps->machineTypes[i]))) goto no_memory; + mach->cpu_max = qemuCaps->machineMaxCpus[i]; } (*machines)[i] = mach; } @@ -1923,6 +1937,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) @@ -2073,6 +2106,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 (machines[i]->alias) { @@ -2087,6 +2124,7 @@ virQEMUCapsProbeQMPMachineTypes(virQEMUCapsPtr qemuCaps, } 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 728add5..8205ff7 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -224,7 +224,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 f39f009..c097953 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -650,6 +650,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 6fdd650..adbd865 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -4024,6 +4024,13 @@ int qemuMonitorJSONGetMachines(qemuMonitorPtr mon, 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; -- 1.7.11.7
On Wed, Apr 17, 2013 at 03:21:04AM +0200, Michal Novotny wrote:
Implement the cpu-max attribute into virConnectGetCapabilities() API output to allow caller to check maximum number of CPUs to be set for specified machine type.
Signed-off-by: Michal Novotny <minovotn@redhat.com> --- docs/schemas/capability.rng | 5 +++++ src/conf/capabilities.c | 4 ++++ src/conf/capabilities.h | 1 + src/qemu/qemu_capabilities.c | 40 +++++++++++++++++++++++++++++++++++++++- src/qemu/qemu_capabilities.h | 3 ++- src/qemu/qemu_monitor.h | 1 + src/qemu/qemu_monitor_json.c | 7 +++++++ 7 files changed, 59 insertions(+), 2 deletions(-)
We've previously said that we're not going to expand the information shown against individual machines in the capabilities, because this is not really scalable to cover all the capabilities you'd wish to show per machine. We'd end up with capabilties XML many MB in size. So I'm not really in favour of this patch. Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
On 04/17/2013 11:25 AM, Daniel P. Berrange wrote:
On Wed, Apr 17, 2013 at 03:21:04AM +0200, Michal Novotny wrote:
Implement the cpu-max attribute into virConnectGetCapabilities() API output to allow caller to check maximum number of CPUs to be set for specified machine type.
Signed-off-by: Michal Novotny <minovotn@redhat.com> --- docs/schemas/capability.rng | 5 +++++ src/conf/capabilities.c | 4 ++++ src/conf/capabilities.h | 1 + src/qemu/qemu_capabilities.c | 40 +++++++++++++++++++++++++++++++++++++++- src/qemu/qemu_capabilities.h | 3 ++- src/qemu/qemu_monitor.h | 1 + src/qemu/qemu_monitor_json.c | 7 +++++++ 7 files changed, 59 insertions(+), 2 deletions(-) We've previously said that we're not going to expand the information shown against individual machines in the capabilities, because this is not really scalable to cover all the capabilities you'd wish to show per machine. We'd end up with capabilties XML many MB in size. So I'm not really in favour of this patch.
Daniel
I don't know where you would like to put it however the check should remain there so if I guess just dropping XML/RNG related hunk would be enough, i.e. dropping: diff --git a/docs/schemas/capability.rng b/docs/schemas/capability.rng index 106ca73..4f87397 100644 --- a/docs/schemas/capability.rng +++ b/docs/schemas/capability.rng @@ -290,6 +290,11 @@ <text/> </attribute> </optional> + <optional> + <attribute name='cpu-max'> + <ref name='unsignedInt'/> + </attribute> + </optional> <text/> </element> </define> diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c index c7ec92f..ab1bdd3 100644 --- a/src/conf/capabilities.c +++ b/src/conf/capabilities.c @@ -860,6 +860,8 @@ virCapabilitiesFormatXML(virCapsPtr caps) for (j = 0 ; j < caps->guests[i]->arch.defaultInfo.nmachines ; j++) { virCapsGuestMachinePtr machine = caps->guests[i]->arch.defaultInfo.machines[j]; virBufferAddLit(&xml, " <machine"); + if (machine->cpu_max > 0) + virBufferAsprintf(&xml, " cpu-max='%d'", machine->cpu_max); if (machine->canonical) virBufferAsprintf(&xml, " canonical='%s'", machine->canonical); virBufferAsprintf(&xml, ">%s</machine>\n", machine->name); @@ -878,6 +880,8 @@ virCapabilitiesFormatXML(virCapsPtr caps) for (k = 0 ; k < caps->guests[i]->arch.domains[j]->info.nmachines ; k++) { virCapsGuestMachinePtr machine = caps->guests[i]->arch.domains[j]->info.machines[k]; virBufferAddLit(&xml, " <machine"); + if (machine->cpu_max > 0) + virBufferAsprintf(&xml, " cpu-max='%d'", machine->cpu_max); if (machine->canonical) virBufferAsprintf(&xml, " canonical='%s'", machine->canonical); virBufferAsprintf(&xml, ">%s</machine>\n", machine->name); This won't add anything to the domain XML file but it will be checking the number of (maximum) virtual CPUs doesn't exceed the machine type limit. However, for libvirt-based tools (i.e. virt-manager and virt-install) it would be nice to have option to expose it. Possibly by writing a new API, like: int virConnectGetMaxCPUsForMachineType(virConnectPtr conn, char *name). This would simply return number of vCPUs for specified machine type (identifier by name variable). Or do you think just having the check but not exposing to any XML file will be fine? Thanks, Michal -- Michal Novotny <minovotn@redhat.com>, RHCE, Red Hat Virtualization | libvirt-php bindings | php-virt-control.org
Check whether vcpu setting or maxvcpu settings exceeds number of maximum cpus for selected machine type on virDomainDefineXML() and virDomainCreateXML() API calls or not. Signed-off-by: Michal Novotny <minovotn@redhat.com> --- src/qemu/qemu_driver.c | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 7c167b7..abe4f8b 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -1454,6 +1454,30 @@ qemuCanonicalizeMachine(virDomainDefPtr def, virQEMUCapsPtr qemuCaps) } +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_XML_DETAIL, + "%s", _("CPUs greater than machine limit")); + return false; + } + + if (def->maxvcpus > cpu_max) { + virReportError(VIR_ERR_XML_DETAIL, + "%s", _("Maximum CPUs greater than machine limit")); + return false; + } + + return true; +} + static virDomainPtr qemuDomainCreate(virConnectPtr conn, const char *xml, unsigned int flags) { virQEMUDriverPtr driver = conn->privateData; @@ -1491,6 +1515,9 @@ static virDomainPtr qemuDomainCreate(virConnectPtr conn, const char *xml, if (qemuCanonicalizeMachine(def, qemuCaps) < 0) goto cleanup; + if (!qemuValidateCpuMax(def, qemuCaps)) + goto cleanup; + if (qemuDomainAssignAddresses(def, qemuCaps, NULL) < 0) goto cleanup; @@ -5461,6 +5488,9 @@ static virDomainPtr qemuDomainDefine(virConnectPtr conn, const char *xml) { if (qemuCanonicalizeMachine(def, qemuCaps) < 0) goto cleanup; + if (!qemuValidateCpuMax(def, qemuCaps)) + goto cleanup; + if (qemuDomainAssignAddresses(def, qemuCaps, NULL) < 0) goto cleanup; -- 1.7.11.7
participants (2)
-
Daniel P. Berrange -
Michal Novotny