Re: [libvirt] [PATCH 1/2] cpu-max: Implement cpu-max attribute into capabilities XML output
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(a)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(a)redhat.com>, RHCE, Red Hat
Virtualization | libvirt-php bindings | php-virt-control.org