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