[libvirt] [PATCH v2 2/2] virCaps: Expose distance between host NUMA nodes

[Daniel P. Berrange]

On Tue, Jun 03, 2014 at 03:26:59PM +0200, Michal Privoznik wrote:
> If user or management application wants to create a guest,
> it may be useful to know the cost of internode latencies
> before the guest resources are pinned. For example:
> 
> <capabilities>
> 
>   <host>
>     ...
>     <topology>
>       <cells num='2'>
>         <cell id='0'>
>           <memory unit='KiB'>4004132</memory>
>           <distances>
>             <sibling id='0' value='10'/>
>             <sibling id='1' value='20'/>
>           </distances>
>           <cpus num='2'>
>             <cpu id='0' socket_id='0' core_id='0' siblings='0'/>
>             <cpu id='2' socket_id='0' core_id='2' siblings='2'/>
>           </cpus>
>         </cell>
>         <cell id='1'>
>           <memory unit='KiB'>4030064</memory>
>           <distances>
>             <sibling id='0' value='20'/>
>             <sibling id='1' value='10'/>
>           </distances>
>           <cpus num='2'>
>             <cpu id='1' socket_id='0' core_id='0' siblings='1'/>
>             <cpu id='3' socket_id='0' core_id='2' siblings='3'/>
>           </cpus>
>         </cell>
>       </cells>
>     </topology>
>     ...
>   </host>
>   ...
> </capabilities>
> 
> we can see the distance from node1 to node0 is 20 and within nodes 10.
> 
> Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
> ---
>  docs/schemas/capability.rng | 15 +++++++++++
>  src/conf/capabilities.c     | 19 +++++++++++++-
>  src/conf/capabilities.h     | 13 +++++++++-
>  src/libxl/libxl_conf.c      |  4 +--
>  src/nodeinfo.c              | 61 ++++++++++++++++++++++++++++++++++++++++++---
>  src/test/test_driver.c      |  2 +-
>  src/xen/xend_internal.c     |  3 ++-
>  tests/vircapstest.c         |  4 +--
>  8 files changed, 110 insertions(+), 11 deletions(-)

I notice our docs for the capabilities XML are non-existant. Separately
it'd be nice to start fixing this. 

> @@ -766,6 +771,18 @@ virCapabilitiesFormatNUMATopology(virBufferPtr buf,
>              virBufferAsprintf(buf, "<memory unit='KiB'>%llu</memory>\n",
>                                cells[i]->mem);
>  
> +        if (cells[i]->nsiblings) {
> +          virBufferAddLit(buf, "<distances>\n");
> +          virBufferAdjustIndent(buf, 2);
> +          for (j = 0; j < cells[i]->nsiblings; j++) {
> +            virBufferAsprintf(buf, "<sibling id='%d' value='%d'/>\n",
> +                              cells[i]->siblings[j].node,
> +                              cells[i]->siblings[j].distance);
> +          }
> +          virBufferAdjustIndent(buf, -2);
> +          virBufferAddLit(buf, "</distances>\n");
> +        }

Indentation is a bit off - should be 4 space indent rather than 2


> @@ -194,8 +203,10 @@ extern int
>  virCapabilitiesAddHostNUMACell(virCapsPtr caps,
>                                 int num,
>                                 int ncpus,
> +                               int nsiblings,

I'd have a slight prefence for 'nsiblings' to be immediately
before the 'siblings' parameter, to make it more obvious that
ther're  related. While you're changing it, I'd probably also
shuffle 'mem' before 'ncpus' so thjat 'ncpus' and 'cpus' are
adjacent.

>                                 unsigned long long mem,
> -                               virCapsHostNUMACellCPUPtr cpus);
> +                               virCapsHostNUMACellCPUPtr cpus,
> +                               virCapsHostNUMACellSiblingInfoPtr siblings);
>  
>  
>  extern int


ACK with those nits fixed

Regards,
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 :|