10:22 a.m.
When we use iothreadinfo to get iothread information, we will get
error like this:
# virsh iothreadinfo rhel7.0 --live
error: Unable to get domain IOThreads information
error: Unable to encode message payload
This is because virProcessGetAffinity() return a bitmap which map_len
is too big to send via rpc:
(gdb) p *map
$7 = {max_bit = 262144, map_len = 4096, map = 0x7f9b6c2c0a20}
To fix this issue add a new parameter &maxcpu to virProcessGetAffinity()
to let callers specify the maxcpu (also in most machine, no need loop
262144 times to check if cpu is set), if set &maxcpu to zero,
virProcessGetAffinity() will use default value (262144 or 1024) to
create bitmap. This issue was introduced in commit 825df8c.
Signed-off-by: Luyao Huang <lhuang(a)redhat.com>
---
src/qemu/qemu_driver.c | 4 ++--
src/util/virprocess.c | 7 ++++---
src/util/virprocess.h | 2 +-
3 files changed, 7 insertions(+), 6 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 2b530c8..e1ad696 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -1459,7 +1459,7 @@ qemuDomainHelperGetVcpus(virDomainObjPtr vm, virVcpuInfoPtr info,
int maxinfo,
unsigned char *cpumap = VIR_GET_CPUMAP(cpumaps, maplen, v);
virBitmapPtr map = NULL;
- if (!(map = virProcessGetAffinity(priv->vcpupids[v])))
+ if (!(map = virProcessGetAffinity(priv->vcpupids[v], hostcpus)))
return -1;
virBitmapToDataBuf(map, cpumap, maplen);
@@ -5619,7 +5619,7 @@ qemuDomainGetIOThreadsLive(virQEMUDriverPtr driver,
goto endjob;
info_ret[i]->iothread_id = iothreads[i]->iothread_id;
- if (!(map = virProcessGetAffinity(iothreads[i]->thread_id)))
+ if (!(map = virProcessGetAffinity(iothreads[i]->thread_id, hostcpus)))
goto endjob;
if (virBitmapToData(map, &info_ret[i]->cpumap,
diff --git a/src/util/virprocess.c b/src/util/virprocess.c
index 8fa7a9b..8800623 100644
--- a/src/util/virprocess.c
+++ b/src/util/virprocess.c
@@ -469,7 +469,8 @@ int virProcessSetAffinity(pid_t pid, virBitmapPtr map)
}
virBitmapPtr
-virProcessGetAffinity(pid_t pid)
+virProcessGetAffinity(pid_t pid,
+ int maxcpu)
{
size_t i;
cpu_set_t *mask;
@@ -504,10 +505,10 @@ virProcessGetAffinity(pid_t pid)
goto cleanup;
}
- if (!(ret = virBitmapNew(ncpus)))
+ if (!(ret = virBitmapNew(maxcpu ? maxcpu : ncpus)))
goto cleanup;
- for (i = 0; i < ncpus; i++) {
+ for (i = 0; i < (maxcpu ? maxcpu : ncpus); i++) {
# ifdef CPU_ALLOC
/* coverity[overrun-local] */
if (CPU_ISSET_S(i, masklen, mask))
diff --git a/src/util/virprocess.h b/src/util/virprocess.h
index 1768009..cc903ac 100644
--- a/src/util/virprocess.h
+++ b/src/util/virprocess.h
@@ -58,7 +58,7 @@ int virProcessKillPainfully(pid_t pid, bool force);
int virProcessSetAffinity(pid_t pid, virBitmapPtr map);
-virBitmapPtr virProcessGetAffinity(pid_t pid);
+virBitmapPtr virProcessGetAffinity(pid_t pid, int maxcpu);
int virProcessGetPids(pid_t pid, size_t *npids, pid_t **pids);
--
1.8.3.1
10:38 a.m.
On Mon, Jun 29, 2015 at 16:22:54 +0800, Luyao Huang wrote:
When we use iothreadinfo to get iothread information, we will get
error like this:
# virsh iothreadinfo rhel7.0 --live
error: Unable to get domain IOThreads information
error: Unable to encode message payload
This is because virProcessGetAffinity() return a bitmap which map_len
is too big to send via rpc:
(gdb) p *map
$7 = {max_bit = 262144, map_len = 4096, map = 0x7f9b6c2c0a20}
To fix this issue add a new parameter &maxcpu to virProcessGetAffinity()
to let callers specify the maxcpu (also in most machine, no need loop
262144 times to check if cpu is set), if set &maxcpu to zero,
virProcessGetAffinity() will use default value (262144 or 1024) to
create bitmap. This issue was introduced in commit 825df8c.
Signed-off-by: Luyao Huang <lhuang(a)redhat.com>
---
src/qemu/qemu_driver.c | 4 ++--
src/util/virprocess.c | 7 ++++---
src/util/virprocess.h | 2 +-
3 files changed, 7 insertions(+), 6 deletions(-)
I think a better fix will be to make virBitmapToData smarter when
formatting a bitmap by finding the last set bit rather than formatting
the full bitmap.
This will avoid re-introducing the maxcpu argument.
The diff for such change is following:
diff --git a/src/util/virbitmap.c b/src/util/virbitmap.c
index 9abc807..7234f7e 100644
--- a/src/util/virbitmap.c
+++ b/src/util/virbitmap.c
@@ -498,9 +498,12 @@ virBitmapPtr virBitmapNewData(void *data, int len)
*/
int virBitmapToData(virBitmapPtr bitmap, unsigned char **data, int *dataLen)
{
- int len;
+ ssize_t len;
- len = (bitmap->max_bit + CHAR_BIT - 1) / CHAR_BIT;
+ if ((len = virBitmapLastSetBit(bitmap)) < 0)
+ len = 1;
+ else
+ len = (len + CHAR_BIT - 1) / CHAR_BIT;
if (VIR_ALLOC_N(*data, len) < 0)
return -1;
Peter
11:12 a.m.
On 06/29/2015 04:38 PM, Peter Krempa wrote:
On Mon, Jun 29, 2015 at 16:22:54 +0800, Luyao Huang wrote:
> When we use iothreadinfo to get iothread information, we will get
> error like this:
>
> # virsh iothreadinfo rhel7.0 --live
> error: Unable to get domain IOThreads information
> error: Unable to encode message payload
>
> This is because virProcessGetAffinity() return a bitmap which map_len
> is too big to send via rpc:
>
> (gdb) p *map
> $7 = {max_bit = 262144, map_len = 4096, map = 0x7f9b6c2c0a20}
>
> To fix this issue add a new parameter &maxcpu to virProcessGetAffinity()
> to let callers specify the maxcpu (also in most machine, no need loop
> 262144 times to check if cpu is set), if set &maxcpu to zero,
> virProcessGetAffinity() will use default value (262144 or 1024) to
> create bitmap. This issue was introduced in commit 825df8c.
>
> Signed-off-by: Luyao Huang<lhuang(a)redhat.com>
> ---
> src/qemu/qemu_driver.c | 4 ++--
> src/util/virprocess.c | 7 ++++---
> src/util/virprocess.h | 2 +-
> 3 files changed, 7 insertions(+), 6 deletions(-)
>
I think a better fix will be to make virBitmapToData smarter when
formatting a bitmap by finding the last set bit rather than formatting
the full bitmap.
This will avoid re-introducing the maxcpu argument.
The diff for such change is following:
diff --git a/src/util/virbitmap.c b/src/util/virbitmap.c
index 9abc807..7234f7e 100644
--- a/src/util/virbitmap.c
+++ b/src/util/virbitmap.c
@@ -498,9 +498,12 @@ virBitmapPtr virBitmapNewData(void *data, int len)
*/
int virBitmapToData(virBitmapPtr bitmap, unsigned char **data, int *dataLen)
{
- int len;
+ ssize_t len;
- len = (bitmap->max_bit + CHAR_BIT - 1) / CHAR_BIT;
+ if ((len = virBitmapLastSetBit(bitmap)) < 0)
+ len = 1;
+ else
+ len = (len + CHAR_BIT - 1) / CHAR_BIT;
if (VIR_ALLOC_N(*data, len) < 0)
return -1;
Okay, i agree with you, and also we need remove the unused parameter in
qemuDomainHelperGetVcpus() and qemuDomainGetIOThreadsLive()
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 2b530c8..6aa4c90 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -1418,13 +1418,9 @@ static int
qemuDomainHelperGetVcpus(virDomainObjPtr vm, virVcpuInfoPtr info, int
maxinfo,
unsigned char *cpumaps, int maplen)
{
- int hostcpus;
size_t i, v;
qemuDomainObjPrivatePtr priv = vm->privateData;
- if ((hostcpus = nodeGetCPUCount()) < 0)
- return -1;
-
if (priv->vcpupids == NULL) {
virReportError(VIR_ERR_OPERATION_INVALID,
"%s", _("cpu affinity is not supported"));
@@ -5573,7 +5569,6 @@ qemuDomainGetIOThreadsLive(virQEMUDriverPtr driver,
qemuMonitorIOThreadInfoPtr *iothreads = NULL;
virDomainIOThreadInfoPtr *info_ret = NULL;
int niothreads = 0;
- int hostcpus;
size_t i;
int ret = -1;
@@ -5606,9 +5601,6 @@ qemuDomainGetIOThreadsLive(virQEMUDriverPtr driver,
goto endjob;
}
- if ((hostcpus = nodeGetCPUCount()) < 0)
- goto endjob;
-
if (VIR_ALLOC_N(info_ret, niothreads) < 0)
goto endjob;
Thanks for your quick review.
Peter
Luyao
3369
days inactive
3369
days old
2 comments
3 participants
participants (3)
-
lhuang -
Luyao Huang
-
Peter Krempa