| Message ID | AB3B4A2BF00C7341A4ADB28108160A9E02A26C2E@AMSACEX4.americas.ad.flextronics.com |
|---|---|
| State | New |
| Headers | show |
"Erlon Cruz" <Erlon.Cruz@fit-tecnologia.org.br> 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. I doubt we want to change this command. But I'm reviewing it regardless of my doubts. > Signed-off-by: erlon.cruz@fit-tecnologia.org.br > --- > 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? > 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.
On Thu, 31 Jan 2013 10:51:10 +0100 Markus Armbruster <armbru@redhat.com> wrote: > "Erlon Cruz" <Erlon.Cruz@fit-tecnologia.org.br> 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. > > I doubt we want to change this command. But I'm reviewing it regardless > of my doubts. I think a better way of doing this would be to add two new optional arguments to 'info registers': -i (cpu index) and -a (print all CPUs). By default the command would do what it does today.
Luiz Capitulino <lcapitulino@redhat.com> writes: > On Thu, 31 Jan 2013 10:51:10 +0100 > Markus Armbruster <armbru@redhat.com> wrote: > >> "Erlon Cruz" <Erlon.Cruz@fit-tecnologia.org.br> 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. >> >> I doubt we want to change this command. But I'm reviewing it regardless >> of my doubts. > > I think a better way of doing this would be to add two new optional > arguments to 'info registers': -i (cpu index) and -a (print all CPUs). By > default the command would do what it does today. Sounds good to me.
On 01/31/2013 07:51 AM, Markus Armbruster wrote: > "Erlon Cruz" <Erlon.Cruz@fit-tecnologia.org.br> 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: erlon.cruz@fit-tecnologia.org.br >> --- >> 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!
On 01/31/2013 09:33 AM, Luiz Capitulino wrote: > On Thu, 31 Jan 2013 10:51:10 +0100 > Markus Armbruster <armbru@redhat.com> wrote: > >> "Erlon Cruz" <Erlon.Cruz@fit-tecnologia.org.br> 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. >> >> I doubt we want to change this command. But I'm reviewing it regardless >> of my doubts. > I think a better way of doing this would be to add two new optional > arguments to 'info registers': -i (cpu index) and -a (print all CPUs). By > default the command would do what it does today. > . How about also printing the CPU index before the infos? 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!
On Thu, 31 Jan 2013 05:12:25 -0800 "Erlon Cruz" <Erlon.Cruz@fit-tecnologia.org.br> wrote: > 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! This is not appropriate for a public ML (or for sending patches), please drop this. PS: I won't reply to messages containing such disclaimer nor take patch from you if they have it.
On Thu, Jan 31, 2013 at 1:12 PM, Luiz Capitulino <lcapitulino@redhat.com>wrote: > On Thu, 31 Jan 2013 05:12:25 -0800 > "Erlon Cruz" <Erlon.Cruz@fit-tecnologia.org.br> wrote: > > > 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! > > This is not appropriate for a public ML (or for sending patches), please > drop this. > > PS: I won't reply to messages containing such disclaimer nor take patch > from you if they have it. > Sorry, I haven't noticed that my mail server was appending this.
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"); + } } 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
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 Signed-off-by: erlon.cruz@fit-tecnologia.org.br --- monitor.c | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-)