On 03/05/2018 06:44 AM, Viktor Mihajlovski wrote:
In order to not affect running VMs, refreshing the halted state is only performed if QEMU supports the query-cpus-fast QAPI.
Signed-off-by: Viktor Mihajlovski <mihajlov@linux.vnet.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> Reviewed-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com> --- src/qemu/qemu_domain.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-)
BTW: You'll need a patch to docs/news.xml as this is something newsworthy I would think. Although calling it "-fast" there may make people wonder about the "slowness" aspect of the current algorithm.
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 4079fb3..a5fcf22 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -8801,8 +8801,13 @@ qemuDomainRefreshVcpuHalted(virQEMUDriverPtr driver, return 0;
/* The halted state is interresting only on s390(x). On other platforms
"interesting"
- * the data would be stale at the time when it would be used. */ - if (!ARCH_IS_S390(vm->def->os.arch)) + * the data would be stale at the time when it would be used. + * Calling qemuMonitorGetCpuHalted() can adversely affect the running + * VM's performance unless QEMU supports query-cpus-fast. + */ + if (!ARCH_IS_S390(vm->def->os.arch) || + !virQEMUCapsGet(QEMU_DOMAIN_PRIVATE(vm)->qemuCaps, + QEMU_CAPS_QUERY_CPUS_FAST))
So IOW systems that reported this previously that don't get the updated QEMU bits (e.g. > 2.12) would now stop reporting halted only because of the adverse performance impact. But that is a data change and "policy" we'd be inflicting upon them which maybe they don't care about. Perhaps in that case we should have a "flag" that handles whether the caller cares or not. I understand the motivation and reasoning; however, based on how such things have been historically acked or nacked, I'm leaning towards saying this patch shouldn't go in. John
return 0;
if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0)