[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] Fix monitor 'info registers' command on multi p
From: |
Erlon Cruz |
Subject: |
Re: [Qemu-devel] [PATCH] Fix monitor 'info registers' command on multi processor guests |
Date: |
Thu, 31 Jan 2013 05:12:25 -0800 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130106 Thunderbird/17.0.2 |
On 01/31/2013 07:51 AM, Markus Armbruster wrote:
> "Erlon Cruz" <address@hidden> writes:
>
>> QEMU monitor command 'info registers' only displays information for the first
>> CPU. This fix that by show registers information for each CPU in the system
> This is incorrect. It displays information for the *current* CPU.
> Monitor command "cpu" selects the current CPU.
That is a bit confusing. May be pointing in the command documentation
that the current cpu could be changed would help.
>
> I doubt we want to change this command. But I'm reviewing it regardless
> of my doubts.
>
>> Signed-off-by: address@hidden
>> ---
>> monitor.c | 19 +++++++++++++++----
>> 1 file changed, 15 insertions(+), 4 deletions(-)
>>
>> diff --git a/monitor.c b/monitor.c
>> index c0e32d6..70e9227 100644
>> --- a/monitor.c
>> +++ b/monitor.c
>> @@ -898,8 +898,14 @@ int monitor_get_cpu_index(void)
>> static void do_info_registers(Monitor *mon)
>> {
>> CPUArchState *env;
>> - env = mon_get_cpu();
>> - cpu_dump_state(env, (FILE *)mon, monitor_fprintf, CPU_DUMP_FPU);
>> +
>> + for (env = first_cpu; env != NULL; env = env->next_cpu) {
>> + monitor_fprintf((FILE *)mon, "========================= "
>> + "Registers on CPU %d =========================\n",
>> + env->cpu_index);
>> + cpu_dump_state(env, (FILE *)mon, monitor_fprintf, CPU_DUMP_FPU);
>> + monitor_fprintf((FILE *)mon, "\n");
>> + }
>> }
>>
> You lost the cpu_synchronize_state() in mon_get_cpu(). Why is that
> okay?
I missed that. It will show inconsistent information on KVM guests if we
dont use cpu_syncronize_state().
>
>> static void do_info_jit(Monitor *mon)
>> @@ -930,8 +936,13 @@ static void do_info_cpu_stats(Monitor *mon)
>> {
>> CPUArchState *env;
>>
>> - env = mon_get_cpu();
>> - cpu_dump_statistics(env, (FILE *)mon, &monitor_fprintf, 0);
>> + for (env = first_cpu; env != NULL; env = env->next_cpu) {
>> + monitor_fprintf((FILE *)mon, "========================= "
>> + "Statistics for CPU %d =========================\n",
>> + env->cpu_index);
>> + cpu_dump_statistics(env, (FILE *)mon, &monitor_fprintf, 0);
>> + monitor_fprintf((FILE *)mon, "\n");
>> + }
>> }
>> #endif
> Same here.
>
> Furthermore, you neglected to update the command's documentation in
> hmp-commands.hx, and the reference to it in qmp-commands.hx.
> .
In this case the command syntax didn't changed right? What would need
to be changed in the documentation?
Legal Disclaimer:
The information contained in this message may be privileged and confidential.
It is intended to be read only by the individual or entity to whom it is
addressed or by their designee. If the reader of this message is not the
intended recipient, you are on notice that any distribution of this message, in
any form, is strictly prohibited. If you have received this message in error,
please immediately notify the sender and delete or destroy any copy of this
message!