[libvirt] [PATCH] qemu: Prevent crash of libvirtd when setting numa parameters
From: Alex Jia <ajia@redhat.com> It's a NULL pointer deref issue, which leads to libvirtd crash. This patch directly use 'params[i].value.s' value instead of derefing a NULL pointer on memcpy. * how to reproduce? % virsh numatune <domain> --nodeset 0 % service libvirtd status * src/qemu/qemu_driver.c (qemuDomainSetNumaParameters): avoid a NULL pointer deref. RHBZ: https://bugzilla.redhat.com/show_bug.cgi?id=771562 Signed-off-by: Alex Jia <ajia@redhat.com> --- src/qemu/qemu_driver.c | 6 ++---- 1 files changed, 2 insertions(+), 4 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 82bab67..1bd93f6 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -6721,14 +6721,12 @@ qemuDomainSetNumaParameters(virDomainPtr dom, } if (flags & VIR_DOMAIN_AFFECT_CONFIG) { - memcpy(oldnodemask, persistentDef->numatune.memory.nodemask, - VIR_DOMAIN_CPUMASK_LEN); + memcpy(oldnodemask, params[i].value.s, VIR_DOMAIN_CPUMASK_LEN); if (virDomainCpuSetParse(params[i].value.s, 0, persistentDef->numatune.memory.nodemask, VIR_DOMAIN_CPUMASK_LEN) < 0) { - memcpy(persistentDef->numatune.memory.nodemask, - oldnodemask, VIR_DOMAIN_CPUMASK_LEN); + memcpy(params[i].value.s, oldnodemask, VIR_DOMAIN_CPUMASK_LEN); ret = -1; continue; } -- 1.7.1
On Wed, Jan 04, 2012 at 03:53:19PM +0800, ajia@redhat.com wrote:
From: Alex Jia <ajia@redhat.com>
It's a NULL pointer deref issue, which leads to libvirtd crash. This patch directly use 'params[i].value.s' value instead of derefing a NULL pointer on memcpy.
* how to reproduce? % virsh numatune <domain> --nodeset 0
The domain must have no nodeset set previously (to crash in this example).
% service libvirtd status
* src/qemu/qemu_driver.c (qemuDomainSetNumaParameters): avoid a NULL pointer deref.
RHBZ: https://bugzilla.redhat.com/show_bug.cgi?id=771562
Signed-off-by: Alex Jia <ajia@redhat.com> --- src/qemu/qemu_driver.c | 6 ++---- 1 files changed, 2 insertions(+), 4 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 82bab67..1bd93f6 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -6721,14 +6721,12 @@ qemuDomainSetNumaParameters(virDomainPtr dom, }
if (flags & VIR_DOMAIN_AFFECT_CONFIG) { - memcpy(oldnodemask, persistentDef->numatune.memory.nodemask, - VIR_DOMAIN_CPUMASK_LEN); + memcpy(oldnodemask, params[i].value.s, VIR_DOMAIN_CPUMASK_LEN); if (virDomainCpuSetParse(params[i].value.s, 0, persistentDef->numatune.memory.nodemask,
Not correct. In this case persistentDef->numatune.memory.nodemask is null, and virDomainCpuSetParse will always fail, thus the nodeset will never be set.
VIR_DOMAIN_CPUMASK_LEN) < 0) { - memcpy(persistentDef->numatune.memory.nodemask, - oldnodemask, VIR_DOMAIN_CPUMASK_LEN); + memcpy(params[i].value.s, oldnodemask, VIR_DOMAIN_CPUMASK_LEN); ret = -1; continue; }
From f2fe7c7d0041779895330416dca6ac97fcbec88a Mon Sep 17 00:00:00 2001 From: Hu Tao <hutao@cn.fujitsu.com> Date: Wed, 4 Jan 2012 17:00:27 +0800 Subject: [PATCH] qemu: fix a bug in numatune
When setting numa nodeset for a domain which has no nodeset set before, libvirtd crashes by dereferring the pointer to the old nodemask which is null in the case. --- src/qemu/qemu_driver.c | 22 ++++++++++++++++++---- 1 files changed, 18 insertions(+), 4 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 82bab67..37330b4 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -6721,14 +6721,28 @@ qemuDomainSetNumaParameters(virDomainPtr dom, } if (flags & VIR_DOMAIN_AFFECT_CONFIG) { - memcpy(oldnodemask, persistentDef->numatune.memory.nodemask, - VIR_DOMAIN_CPUMASK_LEN); + bool savedmask = false; + if (!persistentDef->numatune.memory.nodemask) { + if (VIR_ALLOC_N(persistentDef->numatune.memory.nodemask, + VIR_DOMAIN_CPUMASK_LEN) < 0) { + virReportOOMError(); + ret = -1; + goto cleanup; + } + } else { + memcpy(oldnodemask, persistentDef->numatune.memory.nodemask, + VIR_DOMAIN_CPUMASK_LEN); + savedmask = true; + } if (virDomainCpuSetParse(params[i].value.s, 0, persistentDef->numatune.memory.nodemask, VIR_DOMAIN_CPUMASK_LEN) < 0) { - memcpy(persistentDef->numatune.memory.nodemask, - oldnodemask, VIR_DOMAIN_CPUMASK_LEN); + if (savedmask) + memcpy(persistentDef->numatune.memory.nodemask, + oldnodemask, VIR_DOMAIN_CPUMASK_LEN); + else + VIR_FREE(persistentDef->numatune.memory.nodemask); ret = -1; continue; } -- 1.7.4.4 -- Thanks, Hu Tao
On 01/04/2012 05:04 PM, Hu Tao wrote:
On Wed, Jan 04, 2012 at 03:53:19PM +0800, ajia@redhat.com wrote:
From: Alex Jia<ajia@redhat.com>
It's a NULL pointer deref issue, which leads to libvirtd crash. This patch directly use 'params[i].value.s' value instead of derefing a NULL pointer on memcpy.
* how to reproduce? % virsh numatune<domain> --nodeset 0 The domain must have no nodeset set previously (to crash in this example).
% service libvirtd status
* src/qemu/qemu_driver.c (qemuDomainSetNumaParameters): avoid a NULL pointer deref.
RHBZ: https://bugzilla.redhat.com/show_bug.cgi?id=771562
Signed-off-by: Alex Jia<ajia@redhat.com> --- src/qemu/qemu_driver.c | 6 ++---- 1 files changed, 2 insertions(+), 4 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 82bab67..1bd93f6 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -6721,14 +6721,12 @@ qemuDomainSetNumaParameters(virDomainPtr dom, }
if (flags& VIR_DOMAIN_AFFECT_CONFIG) { - memcpy(oldnodemask, persistentDef->numatune.memory.nodemask, - VIR_DOMAIN_CPUMASK_LEN); + memcpy(oldnodemask, params[i].value.s, VIR_DOMAIN_CPUMASK_LEN); if (virDomainCpuSetParse(params[i].value.s, 0, persistentDef->numatune.memory.nodemask, Not correct. In this case persistentDef->numatune.memory.nodemask is null, and virDomainCpuSetParse will always fail, thus the nodeset will never be set. In fact, I can successfully set nodeset value:
# virsh numatune foo --nodeset 0-1 # virsh numatune foo numa_mode : strict numa_nodeset : 0-1
VIR_DOMAIN_CPUMASK_LEN)< 0) { - memcpy(persistentDef->numatune.memory.nodemask, - oldnodemask, VIR_DOMAIN_CPUMASK_LEN); + memcpy(params[i].value.s, oldnodemask, VIR_DOMAIN_CPUMASK_LEN); ret = -1; continue; }
From f2fe7c7d0041779895330416dca6ac97fcbec88a Mon Sep 17 00:00:00 2001 From: Hu Tao<hutao@cn.fujitsu.com> Date: Wed, 4 Jan 2012 17:00:27 +0800 Subject: [PATCH] qemu: fix a bug in numatune
When setting numa nodeset for a domain which has no nodeset set before, libvirtd crashes by dereferring the pointer to the old nodemask which is null in the case. --- src/qemu/qemu_driver.c | 22 ++++++++++++++++++---- 1 files changed, 18 insertions(+), 4 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 82bab67..37330b4 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -6721,14 +6721,28 @@ qemuDomainSetNumaParameters(virDomainPtr dom, }
if (flags& VIR_DOMAIN_AFFECT_CONFIG) { - memcpy(oldnodemask, persistentDef->numatune.memory.nodemask, - VIR_DOMAIN_CPUMASK_LEN); + bool savedmask = false; + if (!persistentDef->numatune.memory.nodemask) { + if (VIR_ALLOC_N(persistentDef->numatune.memory.nodemask, + VIR_DOMAIN_CPUMASK_LEN)< 0) { + virReportOOMError(); + ret = -1; + goto cleanup; + } + } else { + memcpy(oldnodemask, persistentDef->numatune.memory.nodemask, + VIR_DOMAIN_CPUMASK_LEN); + savedmask = true; + } if (virDomainCpuSetParse(params[i].value.s, 0, persistentDef->numatune.memory.nodemask, VIR_DOMAIN_CPUMASK_LEN)< 0) { - memcpy(persistentDef->numatune.memory.nodemask, - oldnodemask, VIR_DOMAIN_CPUMASK_LEN); + if (savedmask) + memcpy(persistentDef->numatune.memory.nodemask, + oldnodemask, VIR_DOMAIN_CPUMASK_LEN); + else + VIR_FREE(persistentDef->numatune.memory.nodemask); ret = -1; continue; }
On Wed, Jan 04, 2012 at 05:15:24PM +0800, Alex Jia wrote:
On 01/04/2012 05:04 PM, Hu Tao wrote:
On Wed, Jan 04, 2012 at 03:53:19PM +0800, ajia@redhat.com wrote:
From: Alex Jia<ajia@redhat.com>
It's a NULL pointer deref issue, which leads to libvirtd crash. This patch directly use 'params[i].value.s' value instead of derefing a NULL pointer on memcpy.
* how to reproduce? % virsh numatune<domain> --nodeset 0 The domain must have no nodeset set previously (to crash in this example).
% service libvirtd status
* src/qemu/qemu_driver.c (qemuDomainSetNumaParameters): avoid a NULL pointer deref.
RHBZ: https://bugzilla.redhat.com/show_bug.cgi?id=771562
Signed-off-by: Alex Jia<ajia@redhat.com> --- src/qemu/qemu_driver.c | 6 ++---- 1 files changed, 2 insertions(+), 4 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 82bab67..1bd93f6 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -6721,14 +6721,12 @@ qemuDomainSetNumaParameters(virDomainPtr dom, }
if (flags& VIR_DOMAIN_AFFECT_CONFIG) { - memcpy(oldnodemask, persistentDef->numatune.memory.nodemask, - VIR_DOMAIN_CPUMASK_LEN); + memcpy(oldnodemask, params[i].value.s, VIR_DOMAIN_CPUMASK_LEN); if (virDomainCpuSetParse(params[i].value.s, 0, persistentDef->numatune.memory.nodemask, Not correct. In this case persistentDef->numatune.memory.nodemask is null, and virDomainCpuSetParse will always fail, thus the nodeset will never be set. In fact, I can successfully set nodeset value:
# virsh numatune foo --nodeset 0-1
# virsh numatune foo numa_mode : strict numa_nodeset : 0-1
Weird. I've never succeeded with your patch. Can you double-check again?
VIR_DOMAIN_CPUMASK_LEN)< 0) { - memcpy(persistentDef->numatune.memory.nodemask, - oldnodemask, VIR_DOMAIN_CPUMASK_LEN); + memcpy(params[i].value.s, oldnodemask, VIR_DOMAIN_CPUMASK_LEN); ret = -1; continue; }
From f2fe7c7d0041779895330416dca6ac97fcbec88a Mon Sep 17 00:00:00 2001 From: Hu Tao<hutao@cn.fujitsu.com> Date: Wed, 4 Jan 2012 17:00:27 +0800 Subject: [PATCH] qemu: fix a bug in numatune
When setting numa nodeset for a domain which has no nodeset set before, libvirtd crashes by dereferring the pointer to the old nodemask which is null in the case. --- src/qemu/qemu_driver.c | 22 ++++++++++++++++++---- 1 files changed, 18 insertions(+), 4 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 82bab67..37330b4 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -6721,14 +6721,28 @@ qemuDomainSetNumaParameters(virDomainPtr dom, }
if (flags& VIR_DOMAIN_AFFECT_CONFIG) { - memcpy(oldnodemask, persistentDef->numatune.memory.nodemask, - VIR_DOMAIN_CPUMASK_LEN); + bool savedmask = false; + if (!persistentDef->numatune.memory.nodemask) { + if (VIR_ALLOC_N(persistentDef->numatune.memory.nodemask, + VIR_DOMAIN_CPUMASK_LEN)< 0) { + virReportOOMError(); + ret = -1; + goto cleanup; + } + } else { + memcpy(oldnodemask, persistentDef->numatune.memory.nodemask, + VIR_DOMAIN_CPUMASK_LEN); + savedmask = true; + } if (virDomainCpuSetParse(params[i].value.s, 0, persistentDef->numatune.memory.nodemask, VIR_DOMAIN_CPUMASK_LEN)< 0) { - memcpy(persistentDef->numatune.memory.nodemask, - oldnodemask, VIR_DOMAIN_CPUMASK_LEN); + if (savedmask) + memcpy(persistentDef->numatune.memory.nodemask, + oldnodemask, VIR_DOMAIN_CPUMASK_LEN); + else + VIR_FREE(persistentDef->numatune.memory.nodemask);
Oops. The same should be done when --live.
ret = -1; continue; }
-- Thanks, Hu Tao
On 01/04/2012 05:28 PM, Hu Tao wrote:
On Wed, Jan 04, 2012 at 05:15:24PM +0800, Alex Jia wrote:
On 01/04/2012 05:04 PM, Hu Tao wrote:
On Wed, Jan 04, 2012 at 03:53:19PM +0800, ajia@redhat.com wrote:
From: Alex Jia<ajia@redhat.com>
It's a NULL pointer deref issue, which leads to libvirtd crash. This patch directly use 'params[i].value.s' value instead of derefing a NULL pointer on memcpy.
* how to reproduce? % virsh numatune<domain> --nodeset 0 The domain must have no nodeset set previously (to crash in this example).
% service libvirtd status
* src/qemu/qemu_driver.c (qemuDomainSetNumaParameters): avoid a NULL pointer deref.
RHBZ: https://bugzilla.redhat.com/show_bug.cgi?id=771562
Signed-off-by: Alex Jia<ajia@redhat.com> --- src/qemu/qemu_driver.c | 6 ++---- 1 files changed, 2 insertions(+), 4 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 82bab67..1bd93f6 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -6721,14 +6721,12 @@ qemuDomainSetNumaParameters(virDomainPtr dom, }
if (flags& VIR_DOMAIN_AFFECT_CONFIG) { - memcpy(oldnodemask, persistentDef->numatune.memory.nodemask, - VIR_DOMAIN_CPUMASK_LEN); + memcpy(oldnodemask, params[i].value.s, VIR_DOMAIN_CPUMASK_LEN); if (virDomainCpuSetParse(params[i].value.s, 0, persistentDef->numatune.memory.nodemask, Not correct. In this case persistentDef->numatune.memory.nodemask is null, and virDomainCpuSetParse will always fail, thus the nodeset will never be set. In fact, I can successfully set nodeset value:
# virsh numatune foo --nodeset 0-1
# virsh numatune foo numa_mode : strict numa_nodeset : 0-1 Weird. I've never succeeded with your patch. Can you double-check again? Hu Tao, Indeed, it's weird. the patch always works well for me:
# for i in $(seq 10); do virsh numatune foo --nodeset 0-$i; virsh numatune foo; done numa_mode : strict numa_nodeset : 0-1 numa_mode : strict numa_nodeset : 0-2 numa_mode : strict numa_nodeset : 0-3 numa_mode : strict numa_nodeset : 0-4 numa_mode : strict numa_nodeset : 0-5 numa_mode : strict numa_nodeset : 0-6 numa_mode : strict numa_nodeset : 0-7 numa_mode : strict numa_nodeset : 0-8 numa_mode : strict numa_nodeset : 0-9 numa_mode : strict numa_nodeset : 0-10
VIR_DOMAIN_CPUMASK_LEN)< 0) { - memcpy(persistentDef->numatune.memory.nodemask, - oldnodemask, VIR_DOMAIN_CPUMASK_LEN); + memcpy(params[i].value.s, oldnodemask, VIR_DOMAIN_CPUMASK_LEN); ret = -1; continue; }
From f2fe7c7d0041779895330416dca6ac97fcbec88a Mon Sep 17 00:00:00 2001 From: Hu Tao<hutao@cn.fujitsu.com> Date: Wed, 4 Jan 2012 17:00:27 +0800 Subject: [PATCH] qemu: fix a bug in numatune
When setting numa nodeset for a domain which has no nodeset set before, libvirtd crashes by dereferring the pointer to the old nodemask which is null in the case. --- src/qemu/qemu_driver.c | 22 ++++++++++++++++++---- 1 files changed, 18 insertions(+), 4 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 82bab67..37330b4 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -6721,14 +6721,28 @@ qemuDomainSetNumaParameters(virDomainPtr dom, }
if (flags& VIR_DOMAIN_AFFECT_CONFIG) { - memcpy(oldnodemask, persistentDef->numatune.memory.nodemask, - VIR_DOMAIN_CPUMASK_LEN); + bool savedmask = false; + if (!persistentDef->numatune.memory.nodemask) { + if (VIR_ALLOC_N(persistentDef->numatune.memory.nodemask, + VIR_DOMAIN_CPUMASK_LEN)< 0) { + virReportOOMError(); + ret = -1; + goto cleanup; + } + } else { + memcpy(oldnodemask, persistentDef->numatune.memory.nodemask, + VIR_DOMAIN_CPUMASK_LEN); + savedmask = true; + } if (virDomainCpuSetParse(params[i].value.s, 0, persistentDef->numatune.memory.nodemask, VIR_DOMAIN_CPUMASK_LEN)< 0) { - memcpy(persistentDef->numatune.memory.nodemask, - oldnodemask, VIR_DOMAIN_CPUMASK_LEN); + if (savedmask) + memcpy(persistentDef->numatune.memory.nodemask, + oldnodemask, VIR_DOMAIN_CPUMASK_LEN); + else + VIR_FREE(persistentDef->numatune.memory.nodemask); Oops. The same should be done when --live.
ret = -1; continue; }
On Wed, Jan 04, 2012 at 05:56:21PM +0800, Alex Jia wrote:
On 01/04/2012 05:28 PM, Hu Tao wrote:
On Wed, Jan 04, 2012 at 05:15:24PM +0800, Alex Jia wrote:
On 01/04/2012 05:04 PM, Hu Tao wrote:
On Wed, Jan 04, 2012 at 03:53:19PM +0800, ajia@redhat.com wrote:
From: Alex Jia<ajia@redhat.com>
It's a NULL pointer deref issue, which leads to libvirtd crash. This patch directly use 'params[i].value.s' value instead of derefing a NULL pointer on memcpy.
* how to reproduce? % virsh numatune<domain> --nodeset 0 The domain must have no nodeset set previously (to crash in this example).
% service libvirtd status
* src/qemu/qemu_driver.c (qemuDomainSetNumaParameters): avoid a NULL pointer deref.
RHBZ: https://bugzilla.redhat.com/show_bug.cgi?id=771562
Signed-off-by: Alex Jia<ajia@redhat.com> --- src/qemu/qemu_driver.c | 6 ++---- 1 files changed, 2 insertions(+), 4 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 82bab67..1bd93f6 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -6721,14 +6721,12 @@ qemuDomainSetNumaParameters(virDomainPtr dom, }
if (flags& VIR_DOMAIN_AFFECT_CONFIG) { - memcpy(oldnodemask, persistentDef->numatune.memory.nodemask, - VIR_DOMAIN_CPUMASK_LEN); + memcpy(oldnodemask, params[i].value.s, VIR_DOMAIN_CPUMASK_LEN); if (virDomainCpuSetParse(params[i].value.s, 0, persistentDef->numatune.memory.nodemask, Not correct. In this case persistentDef->numatune.memory.nodemask is null, and virDomainCpuSetParse will always fail, thus the nodeset will never be set. In fact, I can successfully set nodeset value:
# virsh numatune foo --nodeset 0-1
# virsh numatune foo numa_mode : strict numa_nodeset : 0-1 Weird. I've never succeeded with your patch. Can you double-check again? Hu Tao, Indeed, it's weird. the patch always works well for me:
# for i in $(seq 10); do virsh numatune foo --nodeset 0-$i; virsh numatune foo; done
Can you test as the following steps? 1. remove numatune element from the dom's xml. 2. restart libvirtd 3. set numa nodeset (say, virsh numatune dom --nodeset 0-2) -- Thanks, Hu Tao
On 01/05/2012 09:12 AM, Hu Tao wrote:
On Wed, Jan 04, 2012 at 05:56:21PM +0800, Alex Jia wrote:
On 01/04/2012 05:28 PM, Hu Tao wrote:
On Wed, Jan 04, 2012 at 05:15:24PM +0800, Alex Jia wrote:
On 01/04/2012 05:04 PM, Hu Tao wrote:
On Wed, Jan 04, 2012 at 03:53:19PM +0800, ajia@redhat.com wrote:
From: Alex Jia<ajia@redhat.com>
It's a NULL pointer deref issue, which leads to libvirtd crash. This patch directly use 'params[i].value.s' value instead of derefing a NULL pointer on memcpy.
* how to reproduce? % virsh numatune<domain> --nodeset 0 The domain must have no nodeset set previously (to crash in this example).
% service libvirtd status
* src/qemu/qemu_driver.c (qemuDomainSetNumaParameters): avoid a NULL pointer deref.
RHBZ: https://bugzilla.redhat.com/show_bug.cgi?id=771562
Signed-off-by: Alex Jia<ajia@redhat.com> --- src/qemu/qemu_driver.c | 6 ++---- 1 files changed, 2 insertions(+), 4 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 82bab67..1bd93f6 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -6721,14 +6721,12 @@ qemuDomainSetNumaParameters(virDomainPtr dom, }
if (flags& VIR_DOMAIN_AFFECT_CONFIG) { - memcpy(oldnodemask, persistentDef->numatune.memory.nodemask, - VIR_DOMAIN_CPUMASK_LEN); + memcpy(oldnodemask, params[i].value.s, VIR_DOMAIN_CPUMASK_LEN); if (virDomainCpuSetParse(params[i].value.s, 0, persistentDef->numatune.memory.nodemask, Not correct. In this case persistentDef->numatune.memory.nodemask is null, and virDomainCpuSetParse will always fail, thus the nodeset will never be set. In fact, I can successfully set nodeset value:
# virsh numatune foo --nodeset 0-1
# virsh numatune foo numa_mode : strict numa_nodeset : 0-1 Weird. I've never succeeded with your patch. Can you double-check again? Hu Tao, Indeed, it's weird. the patch always works well for me:
# for i in $(seq 10); do virsh numatune foo --nodeset 0-$i; virsh numatune foo; done Can you test as the following steps?
1. remove numatune element from the dom's xml. 2. restart libvirtd 3. set numa nodeset (say, virsh numatune dom --nodeset 0-2) Yeah, you're right, I need to cleanup previous <numatune> element block from guest, and your patch '[PATCHv2] qemu: fix a bug in numatune' works well for me.
Thanks for your comment, Alex
When setting numa nodeset for a domain which has no nodeset set before, libvirtd crashes by dereferring the pointer to the old nodemask which is null in the case. --- src/qemu/qemu_driver.c | 45 +++++++++++++++++++++++++++++++++++++-------- 1 files changed, 37 insertions(+), 8 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 82bab67..036d252 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -6679,6 +6679,7 @@ qemuDomainSetNumaParameters(virDomainPtr dom, } } else if (STREQ(param->field, VIR_DOMAIN_NUMA_NODESET)) { int rc; + bool savedmask; char oldnodemask[VIR_DOMAIN_CPUMASK_LEN]; if (param->type != VIR_TYPED_PARAM_STRING) { qemuReportError(VIR_ERR_INVALID_ARG, "%s", @@ -6707,28 +6708,56 @@ qemuDomainSetNumaParameters(virDomainPtr dom, /* update vm->def here so that dumpxml can read the new * values from vm->def. */ - memcpy(oldnodemask, vm->def->numatune.memory.nodemask, - VIR_DOMAIN_CPUMASK_LEN); + savedmask = false; + if (!vm->def->numatune.memory.nodemask) { + if (VIR_ALLOC_N(vm->def->numatune.memory.nodemask, + VIR_DOMAIN_CPUMASK_LEN) < 0) { + virReportOOMError(); + ret = -1; + goto cleanup; + } + } else { + memcpy(oldnodemask, vm->def->numatune.memory.nodemask, + VIR_DOMAIN_CPUMASK_LEN); + savedmask = true; + } if (virDomainCpuSetParse(params[i].value.s, 0, vm->def->numatune.memory.nodemask, VIR_DOMAIN_CPUMASK_LEN) < 0) { - memcpy(vm->def->numatune.memory.nodemask, - oldnodemask, VIR_DOMAIN_CPUMASK_LEN); + if (savedmask) + memcpy(vm->def->numatune.memory.nodemask, + oldnodemask, VIR_DOMAIN_CPUMASK_LEN); + else + VIR_FREE(vm->def->numatune.memory.nodemask); ret = -1; continue; } } if (flags & VIR_DOMAIN_AFFECT_CONFIG) { - memcpy(oldnodemask, persistentDef->numatune.memory.nodemask, - VIR_DOMAIN_CPUMASK_LEN); + savedmask = false; + if (!persistentDef->numatune.memory.nodemask) { + if (VIR_ALLOC_N(persistentDef->numatune.memory.nodemask, + VIR_DOMAIN_CPUMASK_LEN) < 0) { + virReportOOMError(); + ret = -1; + goto cleanup; + } + } else { + memcpy(oldnodemask, persistentDef->numatune.memory.nodemask, + VIR_DOMAIN_CPUMASK_LEN); + savedmask = true; + } if (virDomainCpuSetParse(params[i].value.s, 0, persistentDef->numatune.memory.nodemask, VIR_DOMAIN_CPUMASK_LEN) < 0) { - memcpy(persistentDef->numatune.memory.nodemask, - oldnodemask, VIR_DOMAIN_CPUMASK_LEN); + if (savedmask) + memcpy(persistentDef->numatune.memory.nodemask, + oldnodemask, VIR_DOMAIN_CPUMASK_LEN); + else + VIR_FREE(persistentDef->numatune.memory.nodemask); ret = -1; continue; } -- 1.7.4.4
On 01/04/2012 02:41 AM, Hu Tao wrote:
When setting numa nodeset for a domain which has no nodeset set before, libvirtd crashes by dereferring the pointer to the old
s/dereferring/dereferencing/
nodemask which is null in the case. --- src/qemu/qemu_driver.c | 45 +++++++++++++++++++++++++++++++++++++-------- 1 files changed, 37 insertions(+), 8 deletions(-)
ACK and pushed. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
participants (4)
-
ajia＠redhat.com -
Alex Jia
-
Eric Blake -
Hu Tao