[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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



On Wed, Jul 19, 2017 at 08:17:49PM +0100, Dr. David Alan Gilbert wrote:
> * Eduardo Habkost (ehabkost redhat com) 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".

-- 
Eduardo


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]