On Tue, Feb 06, 2018 at 11:05:06 +0000, Daniel Berrange wrote: > On Tue, Feb 06, 2018 at 12:00:31PM +0100, Peter Krempa wrote: > > On Tue, Feb 06, 2018 at 11:55:07 +0100, Peter Krempa wrote: > > > On Tue, Feb 06, 2018 at 11:18:56 +0100, Viktor Mihajlovski wrote: > > > > Refreshing the halted state can cause VM performance issues. Since > > > > s390 is currently the only architecture with a known interest in > > > > the halted state, we're avoiding to call QEMU on other platforms. > > > > > > > > Signed-off-by: Viktor Mihajlovski <mihajlov@xxxxxxxxxxxxxxxxxx> > > > > --- > > > > src/qemu/qemu_domain.c | 4 ++++ > > > > 1 file changed, 4 insertions(+) > > > > > > > > diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c > > > > index df433c2..d2c833f 100644 > > > > --- a/src/qemu/qemu_domain.c > > > > +++ b/src/qemu/qemu_domain.c > > > > @@ -8634,6 +8634,10 @@ qemuDomainRefreshVcpuHalted(virQEMUDriverPtr driver, > > > > if (vm->def->virtType == VIR_DOMAIN_VIRT_QEMU) > > > > return 0; > > > > > > > > + /* Only supported on s390(x) */ > > > > This would also benefit from a more thorough comment. > > > > > > + if (!ARCH_IS_S390(vm->def->os.arch)) > > > > + return 0; > > > > > > I think we also should remove the 'halted' field from the stats output > > > if the information was not gathered at all, since it would falsely reply > > > that the cpu is not halted all the time. > > > > Also to do so you'll probably need to convert the 'halted' bool to a > > tristate, so that the logic determining whether the field should be used > > or not is not present in the stats code but it's in the refresh code as > > you've added it. > > Or the lazy way out is to just add ARCH_IS_S390() check in the stats > method too No. That's exactly why I've sent this second mail. I don't want to litter this stuff into the stats method. The stats method should not care what weirdness is necessary for a given arch.
Attachment:
signature.asc
Description: PGP signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
