Re: [libvirt] [Qemu-devel] [PATCH v2] hmp: allow cpu index for "info lapic"
[adding libvirt] On 07/19/2017 07:41 AM, Eduardo Habkost wrote:
virsh qemu-monitor-command --domain rhel6.8 --hmp --cmd "cpu 1" virsh qemu-monitor-command --domain rhel6.8 --hmp --cmd "info lapic" dumping local APIC state for CPU 0
Right, the "cpu" command is useless inside a 'human-monitor-command' QMP command. The 'cpu-index' argument should be used instead. should make "cpu" print an error if ran inside 'human-monitor-command' instead of silently pretend it worked.
If virsh doesn't support the 'cpu-index' argument to 'human-monitor-command',
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.
it's possible to work around that limitation by building your own QMP command. e.g.:
# virsh qemu-monitor-command f26test '{"execute":"human-monitor-command", "arguments":{"command-line":"info lapic", "cpu-index":1}}' | jq -r '.return'
Indeed, there's the use of QMP to work around the HMP deficiency. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
On Wed, Jul 19, 2017 at 10:02:04AM -0500, Eric Blake wrote:
[adding libvirt]
On 07/19/2017 07:41 AM, Eduardo Habkost wrote:
virsh qemu-monitor-command --domain rhel6.8 --hmp --cmd "cpu 1" virsh qemu-monitor-command --domain rhel6.8 --hmp --cmd "info lapic" dumping local APIC state for CPU 0
Right, the "cpu" command is useless inside a 'human-monitor-command' QMP command. The 'cpu-index' argument should be used instead. should make "cpu" print an error if ran inside 'human-monitor-command' instead of silently pretend it worked.
If virsh doesn't support the 'cpu-index' argument to 'human-monitor-command',
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.
it's possible to work around that limitation by building your own QMP command. e.g.:
# virsh qemu-monitor-command f26test '{"execute":"human-monitor-command", "arguments":{"command-line":"info lapic", "cpu-index":1}}' | jq -r '.return'
Indeed, there's the use of QMP to work around the HMP deficiency.
-- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
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. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
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. -- Eduardo
* 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). Dave
-- Eduardo -- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
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
Pk9uIFdlZCwgSnVsIDE5LCAyMDE3IGF0IDA4OjE3OjQ5UE0gKzAxMDAsIERyLiBEYXZpZCBBbGFu IEdpbGJlcnQgd3JvdGU6DQoNCj4+ICogRWR1YXJkbyBIYWJrb3N0IChhZGRyZXNzQGhpZGRlbikg d3JvdGU6DQoNCj4+ID4gT24gV2VkLCBKdWwgMTksIDIwMTcgYXQgMTA6MTc6MzZBTSAtMDUwMCwg RXJpYyBCbGFrZSB3cm90ZToNCg0KPj4gPiA+IE9uIDA3LzE5LzIwMTcgMTA6MDcgQU0sIERhbmll bCBQLiBCZXJyYW5nZSB3cm90ZToNCg0KPj4gPiA+ID4+IEl0IGRvZXNuJ3QuICBQZXJoYXBzIHdl IHNob3VsZCBhZGQgdGhhdCBhcyBhIGZ1dHVyZSBsaWJ2aXJ0LXFlbXUuc28gQVBJDQoNCj4+ID4g PiA+PiBhZGRpdGlvbiwgYWx0aG91Z2ggaXQncyBwcm9iYWJseSBlYXNpZXIgdG8ganVzdCB1c2Ug UU1QIHRoYW4gSE1QIHdoZW4NCg0KPj4gPiA+ID4+IHVzaW5nICd2aXJzaCBxZW11LW1vbml0b3It Y29tbWFuZCcgaWYgSE1QIGRvZXNuJ3QgZG8gd2hhdCB5b3Ugd2FudC4NCg0KPj4gPiA+ID4gDQoN Cj4+ID4gPiA+IE9yIHNwZWNpYWwgY2FzZSB0aGUgImNwdSAxIiBjb21tYW5kIC0gaWUgbm90aWNl IHRoYXQgaXQgaXMgYmVpbmcNCg0KPj4gPiA+ID4gcmVxdWVzdGVkIGFuZCBkb24ndCBleGVjdXRl ICdodW1hbi1tb250b3ItY29tbWFuZCcuIEluc3RlYWQganVzdA0KDQo+PiA+ID4gPiByZWNvcmQg dGhlIENQVSBpbmRleCwgYW5kIHVzZSB0aGF0IGZvciBmdXR1cmUgImh1bWFuLW1vbml0b3ItY29t bWFuZCINCg0KPj4gPiA+ID4gaW52b2thdGlvbnMsIHNvIHdlIGdldCBmdWxsIGNvbXBhdCB3aXRo IHRoZSAoZHViaW91cykgc3RhdGVmdWwgSE1QDQoNCj4+ID4gPiA+IHNlbWFudGljcyB0aGF0IHRy YWRpdGlvbmFsbHkgZXhpc3RlZC4NCg0KPj4gPiA+IA0KDQo+PiA+ID4gSXMgJ2NwdScgKGFuZCB0 aGUgZm9sbG93dXAgY29tbWFuZHMgYWZmZWN0ZWQgYnkgaXQpIHRoZSBvbmx5IHN0YXRlZnVsDQoN Cj4+ID4gPiBITVAgY29tbWFuZCBwYWlyaW5nPyAgSXMgdGhlcmUgYSB3YXkgdG8gc3BlY2lmeSBt dWx0aXBsZSBITVAgY29tbWFuZHMgaW4NCg0KPj4gPiA+IGEgc2luZ2xlIGh1bWFuLW1vbml0b3It Y29tbWFuZCBRTVAgY2FsbD8NCg0KPj4gPiA+IA0KDQo+PiA+ID4gSW5kZWVkLCB0d2Vha2luZyBx ZW11J3MgaHVtYW4tbW9uaXRvci1jb21tYW5kIGNhbGwgdG8gdHJhY2sgdGhlIHN0YXRlDQoNCj4+ ID4gPiBtaWdodCBiZSBjbGVhbmVyIHRoYW4gaGF2aW5nIGxpYnZpcnQgaGF2ZSB0byB0d2VhayBB UEkgdG8gd29yayBhcm91bmQNCg0KPj4gPiA+IHRoaXMgd2FydCBvZiBITVAuDQoNCj4+ID4gDQoN Cj4+ID4gVGhlIENQVSBpbmRleCB3YXMgdGhlIG9ubHkgc3RhdGUga2VwdCBieSB0aGUgaHVtYW4g bW9uaXRvciwgYW5kIEkNCg0KPj4gPiB0aGluayBpdCdzIGJ5IGRlc2lnbiB0aGF0IGl0IHN0b3Bw ZWQgYmVpbmcgY29uc2lkZXJlZCAibW9uaXRvcg0KDQo+PiA+IHN0YXRlIiB0byBiZSB0cmFja2Vk LCBhbmQgYmVjYW1lIGp1c3QgYW4gYXJndW1lbnQgdG8NCg0KPj4gPiBodW1hbi1tb25pdG9yLWNv bW1hbmQuDQoNCj4+ID4gDQoNCj4+ID4gSXQncyB0cnVlIHRoYXQgaXQgYnJva2UgY29tcGF0aWJp bGl0eSBvZg0KDQo+PiA+ICAgInZpcnNoIHFlbXUtbW9uaXRvci1jb21tYW5kIDxkb21haW4+IC0t aG1wICdjcHUgPG4+JyIsDQoNCj4+ID4gd2hlbiB3ZSBtb3ZlZCB0byBRTVAsIGJ1dCB0aGlzIGhh cHBlbmVkIHllYXJzIGFnbywgYW5kIGl0IGxvb2tzDQoNCj4+ID4gbGlrZSBub2JvZHkgd2FzIHJl bHlpbmcgb24gaXQuICBJIGRvbid0IHNlZSB0aGUgcG9pbnQgb2YgdHJ5aW5nDQoNCj4+ID4gdG8g ZW11bGF0ZSB0aGUgcHJldmlvdXMgc3RhdGVmdWwgaW50ZXJmYWNlLg0KDQo+PiANCg0KPj4gSU1I TyBZaSdzIGZpeCAob25jZSByZXdvcmtlZCkgaXMgdGhlIHJpZ2h0IGZpeCAtIGl0IHJlbW92ZXMg dGhlDQoNCj4+IHVzZSBvZiB0aGF0IHBpZWNlIG9mIHN0YXRlLCB3aGVuIHRoZSBvcHRpb25hbCBw YXJhbWV0ZXIgaXMgdXNlZC4NCg0KPj4gKE9LLCBzbyBpdCBuZWVkcyByZXdvcmsgbm90IHRvIGNo YW5nZSB0aGF0IHN0YXRlIGFuZCB0bw0KDQo+PiBjb21lIHRvIHNvbWUgYWdyZWVtZW50IGFzIHRv IHdoYXQgdG8gdXNlIGluc3RlYWQgb2YgY3B1IGluZGV4IG51bWJlcg0KDQo+PiBldGMpLg0KDQo+ DQoNCj5BZ3JlZWQsIGFzIGl0IGhlbHBzIHVzIHRvIGtlZXAgdGhlICJ2aXJzaCBxZW11LW1vbml0 b3ItY29tbWFuZCINCg0KPmludGVyZmFjZSBzaW1wbGVyLiAgQnV0IHdlIGhhdmUgOCBjb21tYW5k cyB0aGF0IHVzZQ0KDQo+bW9uX2dldF9jcHUoKSwgd2Ugc2hvdWxkbid0IGZpeCBvbmx5ICJpbmZv IGxhcGljIi4NCg0KDQoNCg0KVGhhbmsgeW91IGFsbCENCg0KSSB3aWxsIHJld29yayB0aGlzIHBh dGNoIGluIHRoaXMgd2F5Og0KDQogIC0gZXh0ZW5kICdpbmZvIHJlZ2lzdGVycycgd2l0aCBhcGlj IGlkIHZhbHVlIGluc3RlYWQgb2YgY3VycmVudCwgbGlrZSB0aGlzOg0KDQogICAgICBDUFUjMSAo c29ja2V0LWlkOiBhLCBjb3JlLWlkOiBiLCB0aHJlYWQtaWQ6IGMsIGFwaWMtaWQ6IGQpDQoNCiAg LSBhZGQgcGFyYW1ldGVyICdhcGljIGlkJyBmb3IgJ2luZm8gbGFwaWMnDQoNCg0KDQoNCkFzIHRv IG90aGVyIGNvbW1hbmRzLCBJIHdhbnQgdG8gc2VuZCBzb21lIG90aGVyIHBhdGNoZXMgJ2NhdXNl IGluIG15IG9waW5pb24gbm90DQoNCmFsbCBjb21tYW5kcyBuZWVkICdhcGljLWlkJyBhcyBpbmRl eC4NCg0KDQoNCg0KLS0tDQoNCkJlc3Qgd2lzaGVzDQoNCllpIFdhbmc=
participants (5)
-
Daniel P. Berrange -
Dr. David Alan Gilbert -
Eduardo Habkost -
Eric Blake -
wang.yi59＠zte.com.cn