>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
