Re: [libvirt] [PATCH] qemu: fix unsuitable error report when get memory stats

On 2015/5/29 19:16, Peter Krempa wrote: > On Fri, May 29, 2015 at 17:17:07 +0800, Wang Yufei wrote: >> From: Zhang Bo <oscar.zhangbo(a)huawei.com&gt; >> >> when we run the command 'virsh dommemstat xxx', >> althrough memballoon's model is set 'none' in vm's XML, >> it still reports an error in libvirtd.log. >> error : qemuMonitorFindBalloonObjectPath:1042 : internal error: Cannot determine balloon device path >> Apparently, if we don't set memballoon, we don't need to >> set balloon device path.

>> >> Signed-off-by: Wang Yufei <james.wangyufei(a)huawei.com&gt; >> Signed-off-by: Zhang Bo <oscar.zhangbo(a)huawei.com&gt; >> --- >> .gnulib | 2 +- >> src/qemu/qemu_monitor.c | 4 +++- >> 2 files changed, 4 insertions(+), 2 deletions(-) >> >> diff --git a/.gnulib b/.gnulib >> index 875ec93..106a386 160000 >> --- a/.gnulib >> +++ b/.gnulib >> @@ -1 +1 @@ >> -Subproject commit 875ec93e1501d2d2a8bab1b64fa66b8ceb51dc67 >> +Subproject commit 106a3866d01f9dd57ab4f10dbeb0d5a8db73a9f7 > > Regular patches should not change the gnulib commit id. Also it is > probably decreasing version of gnulib. Please make sure to update the > submodule before commiting code. > OK, I'll check it, thanks. >> diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c >> index f959b74..c72702d 100644 >> --- a/src/qemu/qemu_monitor.c >> +++ b/src/qemu/qemu_monitor.c >> @@ -1711,7 +1711,9 @@ qemuMonitorGetMemoryStats(qemuMonitorPtr mon, >> QEMU_CHECK_MONITOR(mon); >> >> if (mon->json) { >> - ignore_value(qemuMonitorFindBalloonObjectPath(mon, "/")); >> + if (mon->vm->def->memballoon && >> + mon->vm->def->memballoon->model != VIR_DOMAIN_MEMBALLOON_MODEL_NONE) >> + ignore_value(qemuMonitorFindBalloonObjectPath(mon, "/")); > > The qemu monitor code is not the right place to do this check. The API > that is calling the function should make sure that it makes sense to do > this call. > In my opinion, no matter whether we set up memballoon, we both can call qemuMonitorGetMemoryStats to get mem stats. There're two situation to get mem stats in qemuMonitorGetMemoryStats: one with memballoon, the other without memballoon. Right? If we judge the memballoon in the caller 'qemuDomainMemoryStats', qemuDomainObjEnterMonitor(driver, vm); ret = qemuMonitorGetMemoryStats(priv->mon, stats, nr_stats); if (qemuDomainObjExitMonitor(driver, vm) < 0) what should we do? Supply two different functions to get mem stats?