Re: [Qemu-devel] [PATCH v2] qmp: add query-cpus-fast

[Viktor Mihajlovski]

On 09.02.2018 14:49, Luiz Capitulino wrote:
> On Fri, 9 Feb 2018 08:56:19 +0100
> Viktor Mihajlovski <address@hidden> wrote:
> 
>> On 08.02.2018 21:33, Eduardo Habkost wrote:
>>> On Thu, Feb 08, 2018 at 11:17:32AM -0500, Luiz Capitulino wrote:
>>> [...]  
>>>> The "halted" field is somewhat controversial. On the one hand,
>>>> it offers a convenient way to know if a guest CPU is idle or
>>>> running. On the other hand, it's a field that can change many
>>>> times a second. In fact, the halted state can change even
>>>> before query-cpus-fast has returned. This makes one wonder if
>>>> this field should be dropped all together. Having the "halted"
>>>> field as optional gives a better option for dropping it in
>>>> the future, since we can just stop returning it.  
>>>
>>> I'd just drop it, unless we find a use case where it's really
>>> useful.
> 
> I don't think there's any, unless for debugging purposes.
> 
> I'm keeping it mainly for s390. Viktor, libvirt is still using
> this field in s390, no?
see below
> 
> Dropping halted and having management software still using query-cpus
> because of halted would be a total failure of query-cpus-fast.
> 
>>> Also, the code that sets/clears cpu->halted is target-specific,
>>> so I wouldn't be so sure that simply checking for
>>> !kvm_irqchip_in_kernel() is enough on all targets.
> 
> I checked the code and had the impression it was enough, but
> I don't have experience with other archs. So, would be nice
> if other archs maintainers could review this. I'll try to ping them.
> 
>> Right, the present patch effectively disables halted anyway (including
>> s390). 
> 
> No, it doesn't. It only disables halted for archs that require going
> to the kernel to get it.>
>> So it may be cleaner to just drop it right now.
>> Assuming the presence of architecure-specific data, libvirt can derive a
>> halted state (or an equivalent thereof) from query-cpus-fast returned
>> information.
> 
> This is a different proposal. You're proposing moving the halted state
> to a CPU-specific field. This is doable if that's what we want.
> 
In order to use query-cpus-fast, libvirt has to change code anyway.
Since libvirt is only providing cpu.n.halted for s390, and this value
can be derived from the cpu-state, there's no need for QEMU to return
halted in query-cpus-fast any more.


-- 
Regards,
 Viktor Mihajlovski