On Thu, 27 May 2021 08:24:40 -0300 Bruno Piazera Larsen <bruno.lar...@eldorado.org.br> wrote:
> > On 27/05/2021 05:30, Greg Kurz wrote: > > On Thu, 27 May 2021 09:09:55 +0100 > > "Dr. David Alan Gilbert" <dgilb...@redhat.com> wrote: > > > >> * Greg Kurz (gr...@kaod.org) wrote: > >>> On Wed, 26 May 2021 17:21:03 -0300 > >>> "Bruno Larsen (billionai)" <bruno.lar...@eldorado.org.br> wrote: > >>> > >>>> Since ppc was the last architecture to collect these statistics and > >>>> it is currently phasing this collection out, the command that would query > >>>> this information is being removed. > >>>> > >>> So this is removing an obviously user visible feature. This should be > >>> mentioned in docs/system/removed-features.rst... but, wait, I don't > >>> see anything for it in docs/system/deprecated.rst. This is dropping > >>> a feature without following the usual deprecation policy, i.e. > >>> marking the feature as deprecated and only remove it 2 QEMU versions > >>> later. Any justification for that ? > > The command called a function that ultimately called an empty callback > unless you changed target/ppc/translate.c and removed the comments > around #define DO_PPC_STATISTICS. And like I mentioned in the cover > letter (which may not have been CC'ed to you, sorry) ppc was the last > architecture to support this feature while they were using the legacy > decode system, but as they move to decodetree, this data wouldn't even > be collected. > > That's the justification, basically. > So to sum up, this HMP command doesn't produce any output with upstream QEMU. Add a section in docs/system/removed-features.rst and just mention that. Not worth reposting the full series just for that. This can be done in a followup patch. > >> As long as the command really isn't useful any more, I wouldn't object > >> to that from an HMP point of view. > >> > > Ok then this should be documented in docs/system/removed-features.rst at > > least. > Sure, will send a patch later today with the update > > > >> Dave > >> > >>> David, > >>> > >>> Unrelated, I saw that you already applied this to ppc-for-6.1 on gitlab > >>> but the commit title appears to be broken: > >>> > >>> '65;6401;1cmonitor: removed cpustats command > >>> > >>> https://gitlab.com/dgibson/qemu/-/commit/532be563eae6b8ae834ff7e9ebb1428f53569a69 > >>> > >>>> Suggested-by: Richard Henderson <richard.hender...@linaro.org> > >>>> Signed-off-by: Bruno Larsen (billionai) <bruno.lar...@eldorado.org.br> > >>>> --- > >>>> hmp-commands-info.hx | 13 ------------- > >>>> monitor/misc.c | 11 ----------- > >>>> 2 files changed, 24 deletions(-) > >>>> > >>>> diff --git a/hmp-commands-info.hx b/hmp-commands-info.hx > >>>> index ab0c7aa5ee..b2347a6aea 100644 > >>>> --- a/hmp-commands-info.hx > >>>> +++ b/hmp-commands-info.hx > >>>> @@ -500,19 +500,6 @@ SRST > >>>> Show the current VM UUID. > >>>> ERST > >>>> > >>>> - { > >>>> - .name = "cpustats", > >>>> - .args_type = "", > >>>> - .params = "", > >>>> - .help = "show CPU statistics", > >>>> - .cmd = hmp_info_cpustats, > >>>> - }, > >>>> - > >>>> -SRST > >>>> - ``info cpustats`` > >>>> - Show CPU statistics. > >>>> -ERST > >>>> - > >>>> #if defined(CONFIG_SLIRP) > >>>> { > >>>> .name = "usernet", > >>>> diff --git a/monitor/misc.c b/monitor/misc.c > >>>> index f3a393ea59..1539e18557 100644 > >>>> --- a/monitor/misc.c > >>>> +++ b/monitor/misc.c > >>>> @@ -369,17 +369,6 @@ static void hmp_info_history(Monitor *mon, const > >>>> QDict *qdict) > >>>> } > >>>> } > >>>> > >>>> -static void hmp_info_cpustats(Monitor *mon, const QDict *qdict) > >>>> -{ > >>>> - CPUState *cs = mon_get_cpu(mon); > >>>> - > >>>> - if (!cs) { > >>>> - monitor_printf(mon, "No CPU available\n"); > >>>> - return; > >>>> - } > >>>> - cpu_dump_statistics(cs, 0); > >>>> -} > >>>> - > >>>> static void hmp_info_trace_events(Monitor *mon, const QDict *qdict) > >>>> { > >>>> const char *name = qdict_get_try_str(qdict, "name");
