qemu-devel
[Top][All Lists]
Advanced

[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!



reply via email to

[Prev in Thread] Current Thread [Next in Thread]