Re: [libvirt] [PATCH 6/6] qemu, inject-nmi: Implement the driver methods

On 04/12/2011 11:01 PM, Lai Jiangshan wrote: > +++ b/src/qemu/qemu_driver.c > @@ -1701,6 +1701,48 @@ static int qemudDomainSetMaxMemory(virDomainPtr dom, unsigned long memory) > return qemudDomainSetMemoryFlags(dom, memory, VIR_DOMAIN_MEM_MAXIMUM); > } > > +static int qemuDomainInjectNMI(virDomainPtr domain, unsigned int flags) > +{ > + struct qemud_driver *driver = domain->conn->privateData; > + virDomainObjPtr vm = NULL; > + int ret = -1; > + qemuDomainObjPrivatePtr priv; > + Right here, you should have a virCheckFlags(0, -1) to enforce that we don't honor any flags for now. At which point, you don't need to pass flags on down to the monitor calls. > +++ b/src/qemu/qemu_monitor_json.c > @@ -2513,3 +2513,32 @@ cleanup: > > return ret; > } > + > +int qemuMonitorJSONInjectNMI(qemuMonitorPtr mon, unsigned int flags ATTRIBUTE_UNUSED) > +{ Since neither monitor needed flags this low, you don't have to propogate it any further than qemu_driver.c's virCheckFlags(). > + int ret; > + virJSONValuePtr cmd; > + virJSONValuePtr reply = NULL; > + > + /* > + * FIXME: qmp nmi is not supported until qemu-0.16.0, > + * use human-monitor-command instead temporary. > + * > + * FIXME: qemu's nmi command just injects NMI to a specified CPU, > + * use "nmi 0" instead temporary. > + */ > + cmd = qemuMonitorJSONMakeCommand("human-monitor-command", > + "s:command-line", "nmi 0", > + NULL); We've already got a preferred form for issuing HMP commands from JSON. Rather than building up human-monitor-command manually, you should instead be using qemuMonitorTextInjectNMI; for example, see how qemuMonitorJSONDriveDel falls back to hmp. This also covers the case of a qemu binary that has JSON but not hmp giving a more useful error message. > +++ b/src/qemu/qemu_monitor_text.c > @@ -2628,3 +2628,23 @@ int qemuMonitorTextArbitraryCommand(qemuMonitorPtr mon, const char *cmd, > > return ret; > } > + > +int qemuMonitorTextInjectNMI(qemuMonitorPtr mon, unsigned int flags ATTRIBUTE_UNUSED) > +{ > + const char *cmd = "nmi 0"; > + char *reply = NULL; > + > + /* > + * FIXME: qemu's nmi command just injects NMI to a specified CPU, > + * use "nmi 0" instead temporary. > + */ This bothers me. Is it possible to inject NMI to a particular CPU in bare-metal hardware? If so, then we ought to support that in the API.