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

[Lai Jiangshan]

On 04/16/2011 02:13 AM, Eric Blake wrote:
> 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.
> 

The real world NMI button just sends NMI to all cpus, the qemu side code will
also be modified that hmp nmi command just sends NMI to all CPU and
the cpu-index parameter will be removed.

My original qemu side patch lefts cpu-index parameter for kernel debugging,
but the qemu guys persuade me that inject-nmi command should just send NMI
to CPUs. I accepted it. I think the libvirt will handle it at the same way.

Thanks,
Lai.