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@xxxxxxxxxxxxxxxxxx>
> Signed-off-by: Pradipta Kumar Banerjee <bpradip@xxxxxxxxxx>
> ---
> 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 :|
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list