[libvirt] [PATCH] util:qemu: fix IOThreadinfo always get failed when get running vm status
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@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
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@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
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@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
participants (3)
-
lhuang -
Luyao Huang
-
Peter Krempa