Re: [Qemu-devel] [qemu-s390x] [PATCH 1/3] qmp: expose s390-specific CPU info

[Eric Blake]

On 02/13/2018 06:20 AM, Viktor Mihajlovski wrote:

We're adding the same information to query-cpus-fast. Why do we
need to duplicate this into query-cpus? Do you plan to keep using
query-cpus? If yes, why?


Wonder if we could simply pass a flag to query-cpus "fast=true", that
makes it behave differently. (either not indicate the critical values or
simply provide dummy values - e.g. simply halted=false)


That was one of the ideas in the earlier stages of this discussion (and
I was inclined to go that way initially). The major drawback of this
approach that the slow call is hard to deprecate (OK, one could change
the default to fast=true over time). It's easier to deprecate the entire
query-cpus function. The other issue, maybe not as bad, is that one has
to deal with fields that are suddenly optional or obsolete in way not
confusing every one.
Bottom line is that I'm convinced it's better to have both APIs and to
deprecate the slow one over time. But I have to confess I'm not familiar
with QAPI deprecation rules, maybe Eric can shed some light on this...


You are correct that it is easier to have two commands if we plan for 
one to completely disappear (after a proper deprecation period, where it 
is well-documented for a couple of releases that we plan on removing the 
older command).  The alternative of adding a bool that controls whether 
the painful fields are omitted, is partially introspecitble (you can 
learn whether the new bool exists, in which case you use it for the new 
behavior), but later changing the default value of that bool over time 
is not (as we don't yet have a way to introspect default values - maybe 
we should add that someday, but we're not there yet).  But right now, it 
is easy to introspect the addition of a new command (if it exists, use 
it) and a later disappearance of a command.  So I'm in agreement with 
your approach of a new command.


--
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Re: [Qemu-devel] [qemu-s390x] [PATCH 1/3] qmp: expose s390-specific CPU info

[Viktor Mihajlovski]

On 13.02.2018 12:16, David Hildenbrand wrote:
> On 12.02.2018 19:03, Luiz Capitulino wrote:
>> On Mon, 12 Feb 2018 13:14:30 +0100
>> Viktor Mihajlovski  wrote:
>>
>>> Presently s390x is the only architecture not exposing specific
>>> CPU information via QMP query-cpus. Upstream discussion has shown
>>> that it could make sense to report the architecture specific CPU
>>> state, e.g. to detect that a CPU has been stopped.
>>>
>>> With this change the output of query-cpus will look like this on
>>> s390:
>>>
>>>[
>>>  {"arch": "s390", "current": true,
>>>   "props": {"core-id": 0}, "cpu-state": "operating", "CPU": 0,
>>>   "qom_path": "/machine/unattached/device[0]",
>>>   "halted": false, "thread_id": 63115},
>>>  {"arch": "s390", "current": false,
>>>   "props": {"core-id": 1}, "cpu-state": "stopped", "CPU": 1,
>>>   "qom_path": "/machine/unattached/device[1]",
>>>   "halted": true, "thread_id": 63116}
>>>]
>>
>> We're adding the same information to query-cpus-fast. Why do we
>> need to duplicate this into query-cpus? Do you plan to keep using
>> query-cpus? If yes, why?
> 
> Wonder if we could simply pass a flag to query-cpus "fast=true", that
> makes it behave differently. (either not indicate the critical values or
> simply provide dummy values - e.g. simply halted=false)
> 
That was one of the ideas in the earlier stages of this discussion (and
I was inclined to go that way initially). The major drawback of this
approach that the slow call is hard to deprecate (OK, one could change
the default to fast=true over time). It's easier to deprecate the entire
query-cpus function. The other issue, maybe not as bad, is that one has
to deal with fields that are suddenly optional or obsolete in way not
confusing every one.
Bottom line is that I'm convinced it's better to have both APIs and to
deprecate the slow one over time. But I have to confess I'm not familiar
with QAPI deprecation rules, maybe Eric can shed some light on this...
>>
>> Libvirt for one, should move away from it. We don't want to run
>> into the risk of having the same issue we had with x86 in other
>> archs.
>>
>>>
> 
> 


-- 
Regards,
 Viktor Mihajlovski

Re: [Qemu-devel] [qemu-s390x] [PATCH 1/3] qmp: expose s390-specific CPU info

[David Hildenbrand]

On 12.02.2018 19:03, Luiz Capitulino wrote:
> On Mon, 12 Feb 2018 13:14:30 +0100
> Viktor Mihajlovski  wrote:
> 
>> Presently s390x is the only architecture not exposing specific
>> CPU information via QMP query-cpus. Upstream discussion has shown
>> that it could make sense to report the architecture specific CPU
>> state, e.g. to detect that a CPU has been stopped.
>>
>> With this change the output of query-cpus will look like this on
>> s390:
>>
>>[
>>  {"arch": "s390", "current": true,
>>   "props": {"core-id": 0}, "cpu-state": "operating", "CPU": 0,
>>   "qom_path": "/machine/unattached/device[0]",
>>   "halted": false, "thread_id": 63115},
>>  {"arch": "s390", "current": false,
>>   "props": {"core-id": 1}, "cpu-state": "stopped", "CPU": 1,
>>   "qom_path": "/machine/unattached/device[1]",
>>   "halted": true, "thread_id": 63116}
>>]
> 
> We're adding the same information to query-cpus-fast. Why do we
> need to duplicate this into query-cpus? Do you plan to keep using
> query-cpus? If yes, why?

Wonder if we could simply pass a flag to query-cpus "fast=true", that
makes it behave differently. (either not indicate the critical values or
simply provide dummy values - e.g. simply halted=false)

> 
> Libvirt for one, should move away from it. We don't want to run
> into the risk of having the same issue we had with x86 in other
> archs.
> 
>>


-- 

Thanks,

David / dhildenb