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. I know what Dan said:
+int virDomainSendEventNMI(virDomainPtr domain, unsigned int vcpu)
Your proposal to qemu-devel to add inject-nmi for QMP does not include any CPU index parameter anymore. Instead it will automatically inject the NMI to all present CPUs. This libvirt API would appear to be incompatible with that QMP design. For Xen, it appears the API also does not allow a CPU index to be given - it just injects to the first CPU AFAICT.
So do we really need to have a 'unsigned int vcpu' parameter in the libvirt API, or can we just leave it out and always inject to CPU==0 for HMP ?
eg simplify to
int virDomainSendNMI(virDomainPtr domain)
but if there's ever any possibility that qemu might learn how to direct an NMI to a particular vcpu, I wonder if we should instead have: enum { VIR_DOMAIN_INJECT_NMI_FIRST = 1, VIR_DOMAIN_INJECT_NMI_ALL = 2, } /** * virDomainInjectNMI: * @domain: pointer to domain object, or NULL for Domain0 * @vcpu: which vcpu to send the NMI to * @flags: the flags for controlling behaviours * * Send NMI to the guest. If @flags contains * VIR_DOMAIN_INJECT_NMI_FIRST or VIR_DOMAIN_INJECT_NMI_ALL, * then @vcpu is ignored, and the NMI is sent to the first * possible vcpu or to all vcpus, respectively. Otherwise, * the NMI is sent to the specified vcpu; it is an error if * @vcpu does not correspond to a currently online processor. * * Not all hypervisors support fine-tuned control over which * vcpu(s) can be targetted, and might succeed only for a * particular value of @flags. * * Returns 0 in case of success, -1 in case of failure. */ int virDomainInjectNMI(virDomainPtr domain, unsigned int vcpu, unsigned int flags); Then xen would be hardcoded to require flags==_FIRST (and always ignore vcpu), whereas qemu can honor a particular vcpu. Or is the first vcpu always 0? That is, are there any hypervisors that let you offline vcpu 0 while leaving vcpu1 up, so that FIRST would imply 1? Maybe we don't need a flag for FIRST, but document that vcpu is ignored if ALL is passed, and then make xen error out if ALL is passed or if vcpu != 0. That said, I haven't looked at the proposed qemu side of the patches for how the monitor command for nmi will be implemented in the first place, and until that is in a formal qemu release, we may still need to be a bit flexible here on the libvirt side. Do any other hypervisors allow NMI injection, and with what semantics? -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org