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

Re: [libvirt] [PATCH] The numa node numbers can be non-sequential.



On Tue, Feb 04, 2014 at 05:57:22AM -0500, Shivaprasad G Bhat wrote:
> On PowerNV platform the numa node numbers can be non-sequential,
> #numactl --hardware
> node distances:
> node   0   1  16  17
>  0:  10  40  40  40
>  1:  40  10  40  40
> 16:  40  40  10  40
> 17:  40  40  40  10
> 
> The numa node number is used as index and vice versa which can cause
> the libvirt to crash.
> 
> Signed-off-by: Shivaprasad G Bhat <sbhat linux vnet ibm com>
> Signed-off-by: Pradipta Kumar Banerjee <bpradip in ibm com>
> ---
>  src/conf/capabilities.c |   12 ++++++++++--
>  src/qemu/qemu_driver.c  |    5 +++--
>  src/qemu/qemu_process.c |    2 +-
>  3 files changed, 14 insertions(+), 5 deletions(-)
> 
> diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c
> index 726ec3f..78f65cb 100644
> --- a/src/conf/capabilities.c
> +++ b/src/conf/capabilities.c
> @@ -1000,10 +1000,18 @@ virCapabilitiesGetCpusForNode(virCapsPtr caps,
>                                size_t node,
>                                virBitmapPtr cpumask)
>  {
> -    virCapsHostNUMACellPtr cell = caps->host.numaCell[node];
> +    virCapsHostNUMACellPtr cell = NULL;
>      size_t cpu;
> +    size_t index;
> +    /* The numa node numbers can be non-contiguous. Ex: 0,1,16,17. */
> +    for (index = 0; index < caps->host.nnumaCell; index++) {
> +        if (caps->host.numaCell[index]->num == node) {
> +            cell = caps->host.numaCell[index];
> +            break;
> +        }
> +    }
>  
> -    for (cpu = 0; cpu < cell->ncpus; cpu++) {
> +    for (cpu = 0; cell && cpu < cell->ncpus; cpu++) {
>          if (virBitmapSetBit(cpumask, cell->cpus[cpu].id) < 0) {
>              virReportError(VIR_ERR_INTERNAL_ERROR,
>                             _("Cpu '%u' in node '%zu' is out of range "


At least for this code, I think I'd like to see a test case
written. We don't have any existing test case which is targetting
the  src/conf/capabilities.c APIs directly, so just start a new
test case  tests/vircapstest.c and test this one API in it. Ideally
the test should crash, unless this code fix is applied, so we can
demonstrate the fix is working.

> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 6b3825a..ae36fbe 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -8532,12 +8532,13 @@ qemuDomainSetNumaParamsLive(virDomainObjPtr vm,
>  
>      for (i = 0; i < caps->host.nnumaCell; i++) {
>          bool result;
> -        if (virBitmapGetBit(nodeset, i, &result) < 0) {
> +        virCapsHostNUMACellPtr cell = caps->host.numaCell[i];
> +        if (virBitmapGetBit(nodeset, cell->num, &result) < 0) {
>              virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>                             _("Failed to get cpuset bit values"));
>              goto cleanup;
>          }
> -        if (result && (virBitmapSetBit(temp_nodeset, i) < 0)) {
> +        if (result && (virBitmapSetBit(temp_nodeset, cell->num) < 0)) {
>              virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>                             _("Failed to set temporary cpuset bit values"));
>              goto cleanup;
> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
> index 8bcd98e..c28f84d 100644
> --- a/src/qemu/qemu_process.c
> +++ b/src/qemu/qemu_process.c
> @@ -2002,7 +2002,7 @@ qemuPrepareCpumap(virQEMUDriverPtr driver,
>              size_t j;
>              int cur_ncpus = caps->host.numaCell[i]->ncpus;
>              bool result;
> -            if (virBitmapGetBit(nodemask, i, &result) < 0) {
> +            if (virBitmapGetBit(nodemask, caps->host.numaCell[i]->num, &result) < 0) {
>                  virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>                                 _("Failed to convert nodeset to cpuset"));
>                  virBitmapFree(cpumap);


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


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