Re: [PATCH 1/2] cpu-max: Implement cpu-max attribute into capabilities XML output

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxxx>
>> ---
>>  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@xxxxxxxxxx>, RHCE, Red Hat
Virtualization | libvirt-php bindings | php-virt-control.org

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]