Re: [Qemu-devel] [PATCH v2] hmp: allow cpu index for "info lapic"

[<wang.yi59@zte.com.cn>]

>On Wed, Jul 19, 2017 at 08:17:49PM +0100, Dr. David Alan Gilbert wrote:

>> * Eduardo Habkost (address@hidden) wrote:

>> > On Wed, Jul 19, 2017 at 10:17:36AM -0500, Eric Blake wrote:

>> > > On 07/19/2017 10:07 AM, Daniel P. Berrange wrote:

>> > > >> It doesn't.  Perhaps we should add that as a future libvirt-qemu.so API

>> > > >> addition, although it's probably easier to just use QMP than HMP when

>> > > >> using 'virsh qemu-monitor-command' if HMP doesn't do what you want.

>> > > > 

>> > > > Or special case the "cpu 1" command - ie notice that it is being

>> > > > requested and don't execute 'human-montor-command'. Instead just

>> > > > record the CPU index, and use that for future "human-monitor-command"

>> > > > invokations, so we get full compat with the (dubious) stateful HMP

>> > > > semantics that traditionally existed.

>> > > 

>> > > Is 'cpu' (and the followup commands affected by it) the only stateful

>> > > HMP command pairing?  Is there a way to specify multiple HMP commands in

>> > > a single human-monitor-command QMP call?

>> > > 

>> > > Indeed, tweaking qemu's human-monitor-command call to track the state

>> > > might be cleaner than having libvirt have to tweak API to work around

>> > > this wart of HMP.

>> > 

>> > The CPU index was the only state kept by the human monitor, and I

>> > think it's by design that it stopped being considered "monitor

>> > state" to be tracked, and became just an argument to

>> > human-monitor-command.

>> > 

>> > It's true that it broke compatibility of

>> >   "virsh qemu-monitor-command <domain> --hmp 'cpu <n>'",

>> > when we moved to QMP, but this happened years ago, and it looks

>> > like nobody was relying on it.  I don't see the point of trying

>> > to emulate the previous stateful interface.

>> 

>> IMHO Yi's fix (once reworked) is the right fix - it removes the

>> use of that piece of state, when the optional parameter is used.

>> (OK, so it needs rework not to change that state and to

>> come to some agreement as to what to use instead of cpu index number

>> etc).

>

>Agreed, as it helps us to keep the "virsh qemu-monitor-command"

>interface simpler.  But we have 8 commands that use

>mon_get_cpu(), we shouldn't fix only "info lapic".




Thank you all!

I will rework this patch in this way:

  - extend 'info registers' with apic id value instead of current, like this:

      CPU#1 (socket-id: a, core-id: b, thread-id: c, apic-id: d)

  - add parameter 'apic id' for 'info lapic'




As to other commands, I want to send some other patches 'cause in my opinion not

all commands need 'apic-id' as index.




---

Best wishes

Yi Wang