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

Re: [libvirt] [PATCH] Add NUMA memory information to virsh capabilities output.



On Thu, Feb 14, 2013 at 12:52:38PM -0500, Dusty Mabe wrote:
> ---
>  docs/schemas/capability.rng               | 10 ++++
>  src/conf/capabilities.c                   |  8 +++
>  src/conf/capabilities.h                   |  2 +
>  src/nodeinfo.c                            | 64 +++++++++++++++++++++-
>  src/test/test_driver.c                    |  2 +-
>  src/xen/xend_internal.c                   |  2 +-
>  tests/capabilityschemadata/caps-test3.xml | 88 +++++++++++++++++++++++++++++++
>  7 files changed, 173 insertions(+), 3 deletions(-)
>  create mode 100644 tests/capabilityschemadata/caps-test3.xml
> 
> diff --git a/docs/schemas/capability.rng b/docs/schemas/capability.rng
> index 53fb04a..e0396aa 100644
> --- a/docs/schemas/capability.rng
> +++ b/docs/schemas/capability.rng
> @@ -177,6 +177,10 @@
>        </attribute>
>  
>        <optional>
> +        <ref name='memory'/>
> +      </optional>
> +
> +      <optional>
>          <element name='cpus'>
>            <attribute name='num'>
>              <ref name='unsignedInt'/>
> @@ -189,6 +193,12 @@
>      </element>
>    </define>
>  
> +  <define name='memory'>
> +    <element name='memory'>
> +        <ref name='scaledInteger'/>
> +    </element>
> +  </define>
> +
>    <define name='cpu'>
>      <element name='cpu'>
>        <attribute name='id'>
> diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c
> index a0e597b..9824f0c 100644
> --- a/src/conf/capabilities.c
> +++ b/src/conf/capabilities.c
> @@ -273,6 +273,7 @@ int
>  virCapabilitiesAddHostNUMACell(virCapsPtr caps,
>                                 int num,
>                                 int ncpus,
> +                               unsigned long long mem,
>                                 virCapsHostNUMACellCPUPtr cpus)
>  {
>      virCapsHostNUMACellPtr cell;
> @@ -286,6 +287,7 @@ virCapabilitiesAddHostNUMACell(virCapsPtr caps,
>  
>      cell->ncpus = ncpus;
>      cell->num = num;
> +    cell->mem = mem;
>      cell->cpus = cpus;
>  
>      caps->host.numaCell[caps->host.nnumaCell++] = cell;
> @@ -712,6 +714,12 @@ virCapabilitiesFormatNUMATopology(virBufferPtr xml,
>      virBufferAsprintf(xml, "      <cells num='%zu'>\n", ncells);
>      for (i = 0; i < ncells; i++) {
>          virBufferAsprintf(xml, "        <cell id='%d'>\n", cells[i]->num);
> +
> +        /* Print out the numacell memory total if it is available */
> +        if (cells[i]->mem)
> +        virBufferAsprintf(xml, "          <memory unit='KiB'>%llu</memory>\n",
> +                          cells[i]->mem);
> +
>          virBufferAsprintf(xml, "          <cpus num='%d'>\n", cells[i]->ncpus);
>          for (j = 0; j < cells[i]->ncpus; j++) {
>              virBufferAsprintf(xml, "            <cpu id='%d'",
> diff --git a/src/conf/capabilities.h b/src/conf/capabilities.h
> index cc01765..6c67fb3 100644
> --- a/src/conf/capabilities.h
> +++ b/src/conf/capabilities.h
> @@ -99,6 +99,7 @@ typedef virCapsHostNUMACell *virCapsHostNUMACellPtr;
>  struct _virCapsHostNUMACell {
>      int num;
>      int ncpus;
> +    unsigned long long mem; /* in kibibytes */
>      virCapsHostNUMACellCPUPtr cpus;
>  };
>  
> @@ -210,6 +211,7 @@ extern int
>  virCapabilitiesAddHostNUMACell(virCapsPtr caps,
>                                 int num,
>                                 int ncpus,
> +                               unsigned long long mem,
>                                 virCapsHostNUMACellCPUPtr cpus);
>  
>  
> diff --git a/src/nodeinfo.c b/src/nodeinfo.c
> index 1622322..f9e173b 100644
> --- a/src/nodeinfo.c
> +++ b/src/nodeinfo.c
> @@ -102,6 +102,7 @@ static int linuxNodeGetMemoryStats(FILE *meminfo,
>                                     int cellNum,
>                                     virNodeMemoryStatsPtr params,
>                                     int *nparams);
> +static unsigned long long nodeGetCellMemory(int cell);
>  
>  /* Return the positive decimal contents of the given
>   * DIR/cpu%u/FILE, or -1 on error.  If DEFAULT_VALUE is non-negative
> @@ -1531,6 +1532,7 @@ nodeCapsInitNUMA(virCapsPtr caps)
>      int n;
>      unsigned long *mask = NULL;
>      unsigned long *allonesmask = NULL;
> +    unsigned long long memory;
>      virCapsHostNUMACellCPUPtr cpus = NULL;
>      int ret = -1;
>      int max_n_cpus = NUMA_MAX_N_CPUS;
> @@ -1562,6 +1564,11 @@ nodeCapsInitNUMA(virCapsPtr caps)
>              continue;
>          }
>  
> +        /* Detect the amount of memory in the numa cell */
> +        memory = nodeGetCellMemory(n);
> +        if (memory == 0)
> +            goto cleanup;
> +
>          for (ncpus = 0, i = 0 ; i < max_n_cpus ; i++)
>              if (MASK_CPU_ISSET(mask, i))
>                  ncpus++;
> @@ -1578,7 +1585,7 @@ nodeCapsInitNUMA(virCapsPtr caps)
>              }
>          }
>  
> -        if (virCapabilitiesAddHostNUMACell(caps, n, ncpus, cpus) < 0)
> +        if (virCapabilitiesAddHostNUMACell(caps, n, ncpus, memory, cpus) < 0)
>              goto cleanup;
>      }
>  
> @@ -1665,6 +1672,54 @@ cleanup:
>      return freeMem;
>  }
>  
> +/**
> + * nodeGetCellMemory
> + * @cell: The number of the numa cell to get memory info for.
> + *
> + * Will call the numa_node_size64() function from libnuma to get
> + * the amount of total memory in bytes. It is then converted to
> + * KiB and returned.
> + *
> + * Returns 0 on failure, amount of memory in KiB on success.
> + */
> +static unsigned long long nodeGetCellMemory(int cell)
> +{
> +    long long mem;
> +    unsigned long long memKiB = 0;
> +    int maxCell;
> +
> +    if (numa_available() < 0) {
> +        virReportError(VIR_ERR_INTERNAL_ERROR,
> +                       "%s", _("NUMA not supported on this host"));
> +        goto cleanup;
> +    }

This check isn't needed since you're calling this from
nodeInitNUMA which has already done exactly this check

> +
> +    /* Make sure the provided cell number is valid. */
> +    maxCell = numa_max_node();
> +    if (cell > maxCell) {
> +        virReportError(VIR_ERR_INTERNAL_ERROR,
> +                       _("cell %d out of range (0-%d)"),
> +                       cell, maxCell);
> +        goto cleanup;
> +    }
> +
> +    /* Get the amount of memory(bytes) in the node */
> +    mem = numa_node_size64(cell, NULL);
> +    if (mem < 0) {
> +        virReportError(VIR_ERR_INTERNAL_ERROR,
> +                       _("Failed to query NUMA total memory for node: %d"),
> +                       cell);
> +        goto cleanup;
> +    }
> +
> +    /* Convert the memory from bytes to KiB */
> +    memKiB = mem >> 10;

What do people think about bytes vs kb for the capabilities XML.
I can go either way really. Bytes is more precise, but I doubt
apps will care about bytes when we're talking multiple GB of
RAM.

> +
> +cleanup:
> +    return memKiB;
> +}
> +
> +


ACK to the patch with the small change made.

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


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