| Message ID | 20171213181540.7949-3-danielhb@linux.vnet.ibm.com |
|---|---|
| State | New |
| Headers | show |
| Series | QMP: implementing qmp_cpu | expand |
On 12/13/2017 04:15 PM, Daniel Henrique Barboza wrote: > qmp_query_cpus always return the same information, ignoring the > monitor state. This means that information such as cpu->value->current > will always return the same value ((cpu == first_cpu) at this moment). > This function is used by hmp_info_cpus that does its own logic based on > monitor_get_cpu_index() to print the current/active CPU information, > ignoring cpu->value->current. This value is used as is in QMP > query-cpus though. > > This wasn't a problem because QMP couldn't set its current CPU via > qmp_cpu anyway, so query-cpus returning the same state all the time > (regarding current CPU for the monitor) wasn't a problem. But now > we can use qmp_cpu to set the current monitor CPU, making the return > from query-cpus wrong. > > This patch makes a simple change in qmp_query_cpus to change the > cpu->value->current logic to consider the current monitor state, > like it was being done in hmp_info_cpus. Now, cpu->value->current > will return an accurate value for both QMP and HMP calls. hmp_info_cpus > can no use this value directly instead of doing its own logic. > > Signed-off-by: Daniel Henrique Barboza <danielhb@linux.vnet.ibm.com> > --- The purpose of this patch watered away with qmp_cpu doing nothing. Although there is a point to be made that this might be "nicer" that what we have, a sort of cleanup, I am not sure if it's worth it. Apply it or leave it alone, either way works for me. Daniel > cpus.c | 2 +- > hmp.c | 6 +----- > 2 files changed, 2 insertions(+), 6 deletions(-) > > diff --git a/cpus.c b/cpus.c > index 114c29b6a0..d3b79d1c02 100644 > --- a/cpus.c > +++ b/cpus.c > @@ -1919,7 +1919,7 @@ CpuInfoList *qmp_query_cpus(Error **errp) > info = g_malloc0(sizeof(*info)); > info->value = g_malloc0(sizeof(*info->value)); > info->value->CPU = cpu->cpu_index; > - info->value->current = (cpu == first_cpu); > + info->value->current = (cpu->cpu_index == monitor_get_cpu_index()); > info->value->halted = cpu->halted; > info->value->qom_path = object_get_canonical_path(OBJECT(cpu)); > info->value->thread_id = cpu->thread_id; > diff --git a/hmp.c b/hmp.c > index 7506f105a0..1259cafc1f 100644 > --- a/hmp.c > +++ b/hmp.c > @@ -363,11 +363,7 @@ void hmp_info_cpus(Monitor *mon, const QDict *qdict) > cpu_list = qmp_query_cpus(NULL); > > for (cpu = cpu_list; cpu; cpu = cpu->next) { > - int active = ' '; > - > - if (cpu->value->CPU == monitor_get_cpu_index()) { > - active = '*'; > - } > + int active = cpu->value->current ? '*' : ' '; > > monitor_printf(mon, "%c CPU #%" PRId64 ":", active, cpu->value->CPU); >
diff --git a/cpus.c b/cpus.c index 114c29b6a0..d3b79d1c02 100644 --- a/cpus.c +++ b/cpus.c @@ -1919,7 +1919,7 @@ CpuInfoList *qmp_query_cpus(Error **errp) info = g_malloc0(sizeof(*info)); info->value = g_malloc0(sizeof(*info->value)); info->value->CPU = cpu->cpu_index; - info->value->current = (cpu == first_cpu); + info->value->current = (cpu->cpu_index == monitor_get_cpu_index()); info->value->halted = cpu->halted; info->value->qom_path = object_get_canonical_path(OBJECT(cpu)); info->value->thread_id = cpu->thread_id; diff --git a/hmp.c b/hmp.c index 7506f105a0..1259cafc1f 100644 --- a/hmp.c +++ b/hmp.c @@ -363,11 +363,7 @@ void hmp_info_cpus(Monitor *mon, const QDict *qdict) cpu_list = qmp_query_cpus(NULL); for (cpu = cpu_list; cpu; cpu = cpu->next) { - int active = ' '; - - if (cpu->value->CPU == monitor_get_cpu_index()) { - active = '*'; - } + int active = cpu->value->current ? '*' : ' '; monitor_printf(mon, "%c CPU #%" PRId64 ":", active, cpu->value->CPU);
qmp_query_cpus always return the same information, ignoring the monitor state. This means that information such as cpu->value->current will always return the same value ((cpu == first_cpu) at this moment). This function is used by hmp_info_cpus that does its own logic based on monitor_get_cpu_index() to print the current/active CPU information, ignoring cpu->value->current. This value is used as is in QMP query-cpus though. This wasn't a problem because QMP couldn't set its current CPU via qmp_cpu anyway, so query-cpus returning the same state all the time (regarding current CPU for the monitor) wasn't a problem. But now we can use qmp_cpu to set the current monitor CPU, making the return from query-cpus wrong. This patch makes a simple change in qmp_query_cpus to change the cpu->value->current logic to consider the current monitor state, like it was being done in hmp_info_cpus. Now, cpu->value->current will return an accurate value for both QMP and HMP calls. hmp_info_cpus can no use this value directly instead of doing its own logic. Signed-off-by: Daniel Henrique Barboza <danielhb@linux.vnet.ibm.com> --- cpus.c | 2 +- hmp.c | 6 +----- 2 files changed, 2 insertions(+), 6 deletions(-)