Re: [Qemu-devel] [PATCH v6 04/17] Extend HMP command info cpus to display accelerator id and model name
On Wed, 6 May 2015 10:33:55 -0300 Eduardo Habkost wrote: > On Wed, May 06, 2015 at 08:59:56AM -0400, Luiz Capitulino wrote: > > On Wed, 6 May 2015 07:38:53 -0300 > > Eduardo Habkost wrote: > > > > > On Wed, May 06, 2015 at 09:32:58AM +0200, Michael Mueller wrote: > > > > On Tue, 5 May 2015 10:14:32 -0300 > > > > Eduardo Habkost wrote: > > > > > > > > > On Mon, Apr 27, 2015 at 04:53:18PM +0200, Michael Mueller wrote: > > > > > > The HMP command info cpus now displays the CPU model name and the > > > > > > backing accelerator if part of the CPUState. > > > > > > > > > > > > (qemu) info cpus > > > > > > * CPU #0: (halted) model=2827-ga2 accel=kvm thread_id=1679 > > > > > > > > > > > > Signed-off-by: Michael Mueller > > > > > > Acked-by: Christian Borntraeger > > > > > > > > > > Do we really need this? I mean: I expect the amount of CPU data we > > > > > provide to QMP clients to grow a lot in the near future, but that > > > > > doesn't mean HMP users need all that data to be printed by "info > > > > > cpus". > > > > > > > > Where do you see the limit of what is worth to be shown an what not. > > > > I personally use "info cpus" less then sporadic but already got a > > > > comment > > > > internally on that information being worthwhile to be shown. > > > > > > I really don't know, but I think we shouldn't add stuff to HMP unless we > > > have a good reason. For each new piece of data in HMP I would like to at > > > least see the description of a real use case that justifies adding it to > > > HMP and not just implementing a simple script on top of QMP. > > > > > > For accel info we already have "info kvm" that is not ideal but is > > > enough for current use cases, isn't it? CPU model name information seems > > > to be more useful, but if it is just for debugging, people can just run > > > QMP query-cpus command. > > > > > > Luiz, what do you think? > > > > I don't see a problem with that. HMP is a debugging interface anyways. > > Actually, I think it's a good test-case for QMP having a high-level > > in-tree client (vs. qmp-shell, which is too low-level). > > > > If the problem is that a command is dumping too much information to > > the point of hurting usability, we can split the command or add a '-a' > > option or something like that. > > Thanks! If HMP is seen as a debugging interface, my main objections > aren't valid. > > That said, I would prefer to keep the command output cleaner and add > only the "model" field, as people can use "info kvm" for the accel info > by now. Ok, I buy that and will kick the accel name out again. Michael >
Re: [Qemu-devel] [PATCH v6 04/17] Extend HMP command info cpus to display accelerator id and model name
On Wed, May 06, 2015 at 08:59:56AM -0400, Luiz Capitulino wrote: > On Wed, 6 May 2015 07:38:53 -0300 > Eduardo Habkost wrote: > > > On Wed, May 06, 2015 at 09:32:58AM +0200, Michael Mueller wrote: > > > On Tue, 5 May 2015 10:14:32 -0300 > > > Eduardo Habkost wrote: > > > > > > > On Mon, Apr 27, 2015 at 04:53:18PM +0200, Michael Mueller wrote: > > > > > The HMP command info cpus now displays the CPU model name and the > > > > > backing accelerator if part of the CPUState. > > > > > > > > > > (qemu) info cpus > > > > > * CPU #0: (halted) model=2827-ga2 accel=kvm thread_id=1679 > > > > > > > > > > Signed-off-by: Michael Mueller > > > > > Acked-by: Christian Borntraeger > > > > > > > > Do we really need this? I mean: I expect the amount of CPU data we > > > > provide to QMP clients to grow a lot in the near future, but that > > > > doesn't mean HMP users need all that data to be printed by "info cpus". > > > > > > Where do you see the limit of what is worth to be shown an what not. > > > I personally use "info cpus" less then sporadic but already got a comment > > > internally on that information being worthwhile to be shown. > > > > I really don't know, but I think we shouldn't add stuff to HMP unless we > > have a good reason. For each new piece of data in HMP I would like to at > > least see the description of a real use case that justifies adding it to > > HMP and not just implementing a simple script on top of QMP. > > > > For accel info we already have "info kvm" that is not ideal but is > > enough for current use cases, isn't it? CPU model name information seems > > to be more useful, but if it is just for debugging, people can just run > > QMP query-cpus command. > > > > Luiz, what do you think? > > I don't see a problem with that. HMP is a debugging interface anyways. > Actually, I think it's a good test-case for QMP having a high-level > in-tree client (vs. qmp-shell, which is too low-level). > > If the problem is that a command is dumping too much information to > the point of hurting usability, we can split the command or add a '-a' > option or something like that. Thanks! If HMP is seen as a debugging interface, my main objections aren't valid. That said, I would prefer to keep the command output cleaner and add only the "model" field, as people can use "info kvm" for the accel info by now. -- Eduardo
Re: [Qemu-devel] [PATCH v6 04/17] Extend HMP command info cpus to display accelerator id and model name
On Wed, 6 May 2015 07:38:53 -0300
Eduardo Habkost wrote:
> On Wed, May 06, 2015 at 09:32:58AM +0200, Michael Mueller wrote:
> > On Tue, 5 May 2015 10:14:32 -0300
> > Eduardo Habkost wrote:
> >
> > > On Mon, Apr 27, 2015 at 04:53:18PM +0200, Michael Mueller wrote:
> > > > The HMP command info cpus now displays the CPU model name and the
> > > > backing accelerator if part of the CPUState.
> > > >
> > > > (qemu) info cpus
> > > > * CPU #0: (halted) model=2827-ga2 accel=kvm thread_id=1679
> > > >
> > > > Signed-off-by: Michael Mueller
> > > > Acked-by: Christian Borntraeger
> > >
> > > Do we really need this? I mean: I expect the amount of CPU data we
> > > provide to QMP clients to grow a lot in the near future, but that
> > > doesn't mean HMP users need all that data to be printed by "info cpus".
> >
> > Where do you see the limit of what is worth to be shown an what not.
> > I personally use "info cpus" less then sporadic but already got a comment
> > internally on that information being worthwhile to be shown.
>
> I really don't know, but I think we shouldn't add stuff to HMP unless we
> have a good reason. For each new piece of data in HMP I would like to at
> least see the description of a real use case that justifies adding it to
> HMP and not just implementing a simple script on top of QMP.
>
> For accel info we already have "info kvm" that is not ideal but is
> enough for current use cases, isn't it? CPU model name information seems
> to be more useful, but if it is just for debugging, people can just run
> QMP query-cpus command.
>
> Luiz, what do you think?
I don't see a problem with that. HMP is a debugging interface anyways.
Actually, I think it's a good test-case for QMP having a high-level
in-tree client (vs. qmp-shell, which is too low-level).
If the problem is that a command is dumping too much information to
the point of hurting usability, we can split the command or add a '-a'
option or something like that.
>
> >
> > >
> > >
> > > > ---
> > > > hmp.c | 7 +++
> > > > 1 file changed, 7 insertions(+)
> > > >
> > > > diff --git a/hmp.c b/hmp.c
> > > > index f142d36..676d821 100644
> > > > --- a/hmp.c
> > > > +++ b/hmp.c
> > > > @@ -290,6 +290,13 @@ void hmp_info_cpus(Monitor *mon, const QDict
> > > > *qdict)
> > > > monitor_printf(mon, " (halted)");
> > > > }
> > > >
> > > > +if (cpu->value->has_model) {
> > > > +monitor_printf(mon, " model=%s", cpu->value->model);
> > > > +}
> > > > +if (cpu->value->has_accel) {
> > > > +monitor_printf(mon, " accel=%s",
> > > > AccelId_lookup[cpu->value->accel]);
> > > > +}
> > > > +
> > > > monitor_printf(mon, " thread_id=%" PRId64 "\n",
> > > > cpu->value->thread_id);
> > > > }
> > > >
> > > > --
> > > > 1.8.3.1
> > > >
> > >
> >
>
Re: [Qemu-devel] [PATCH v6 04/17] Extend HMP command info cpus to display accelerator id and model name
On Wed, May 06, 2015 at 09:32:58AM +0200, Michael Mueller wrote:
> On Tue, 5 May 2015 10:14:32 -0300
> Eduardo Habkost wrote:
>
> > On Mon, Apr 27, 2015 at 04:53:18PM +0200, Michael Mueller wrote:
> > > The HMP command info cpus now displays the CPU model name and the
> > > backing accelerator if part of the CPUState.
> > >
> > > (qemu) info cpus
> > > * CPU #0: (halted) model=2827-ga2 accel=kvm thread_id=1679
> > >
> > > Signed-off-by: Michael Mueller
> > > Acked-by: Christian Borntraeger
> >
> > Do we really need this? I mean: I expect the amount of CPU data we
> > provide to QMP clients to grow a lot in the near future, but that
> > doesn't mean HMP users need all that data to be printed by "info cpus".
>
> Where do you see the limit of what is worth to be shown an what not.
> I personally use "info cpus" less then sporadic but already got a comment
> internally on that information being worthwhile to be shown.
I really don't know, but I think we shouldn't add stuff to HMP unless we
have a good reason. For each new piece of data in HMP I would like to at
least see the description of a real use case that justifies adding it to
HMP and not just implementing a simple script on top of QMP.
For accel info we already have "info kvm" that is not ideal but is
enough for current use cases, isn't it? CPU model name information seems
to be more useful, but if it is just for debugging, people can just run
QMP query-cpus command.
Luiz, what do you think?
>
> >
> >
> > > ---
> > > hmp.c | 7 +++
> > > 1 file changed, 7 insertions(+)
> > >
> > > diff --git a/hmp.c b/hmp.c
> > > index f142d36..676d821 100644
> > > --- a/hmp.c
> > > +++ b/hmp.c
> > > @@ -290,6 +290,13 @@ void hmp_info_cpus(Monitor *mon, const QDict *qdict)
> > > monitor_printf(mon, " (halted)");
> > > }
> > >
> > > +if (cpu->value->has_model) {
> > > +monitor_printf(mon, " model=%s", cpu->value->model);
> > > +}
> > > +if (cpu->value->has_accel) {
> > > +monitor_printf(mon, " accel=%s",
> > > AccelId_lookup[cpu->value->accel]);
> > > +}
> > > +
> > > monitor_printf(mon, " thread_id=%" PRId64 "\n",
> > > cpu->value->thread_id);
> > > }
> > >
> > > --
> > > 1.8.3.1
> > >
> >
>
--
Eduardo
Re: [Qemu-devel] [PATCH v6 04/17] Extend HMP command info cpus to display accelerator id and model name
On Tue, 5 May 2015 10:14:32 -0300
Eduardo Habkost wrote:
> On Mon, Apr 27, 2015 at 04:53:18PM +0200, Michael Mueller wrote:
> > The HMP command info cpus now displays the CPU model name and the
> > backing accelerator if part of the CPUState.
> >
> > (qemu) info cpus
> > * CPU #0: (halted) model=2827-ga2 accel=kvm thread_id=1679
> >
> > Signed-off-by: Michael Mueller
> > Acked-by: Christian Borntraeger
>
> Do we really need this? I mean: I expect the amount of CPU data we
> provide to QMP clients to grow a lot in the near future, but that
> doesn't mean HMP users need all that data to be printed by "info cpus".
Where do you see the limit of what is worth to be shown an what not.
I personally use "info cpus" less then sporadic but already got a comment
internally on that information being worthwhile to be shown.
>
>
> > ---
> > hmp.c | 7 +++
> > 1 file changed, 7 insertions(+)
> >
> > diff --git a/hmp.c b/hmp.c
> > index f142d36..676d821 100644
> > --- a/hmp.c
> > +++ b/hmp.c
> > @@ -290,6 +290,13 @@ void hmp_info_cpus(Monitor *mon, const QDict *qdict)
> > monitor_printf(mon, " (halted)");
> > }
> >
> > +if (cpu->value->has_model) {
> > +monitor_printf(mon, " model=%s", cpu->value->model);
> > +}
> > +if (cpu->value->has_accel) {
> > +monitor_printf(mon, " accel=%s",
> > AccelId_lookup[cpu->value->accel]);
> > +}
> > +
> > monitor_printf(mon, " thread_id=%" PRId64 "\n",
> > cpu->value->thread_id);
> > }
> >
> > --
> > 1.8.3.1
> >
>
Re: [Qemu-devel] [PATCH v6 04/17] Extend HMP command info cpus to display accelerator id and model name
On Mon, Apr 27, 2015 at 04:53:18PM +0200, Michael Mueller wrote:
> The HMP command info cpus now displays the CPU model name and the
> backing accelerator if part of the CPUState.
>
> (qemu) info cpus
> * CPU #0: (halted) model=2827-ga2 accel=kvm thread_id=1679
>
> Signed-off-by: Michael Mueller
> Acked-by: Christian Borntraeger
Do we really need this? I mean: I expect the amount of CPU data we
provide to QMP clients to grow a lot in the near future, but that
doesn't mean HMP users need all that data to be printed by "info cpus".
> ---
> hmp.c | 7 +++
> 1 file changed, 7 insertions(+)
>
> diff --git a/hmp.c b/hmp.c
> index f142d36..676d821 100644
> --- a/hmp.c
> +++ b/hmp.c
> @@ -290,6 +290,13 @@ void hmp_info_cpus(Monitor *mon, const QDict *qdict)
> monitor_printf(mon, " (halted)");
> }
>
> +if (cpu->value->has_model) {
> +monitor_printf(mon, " model=%s", cpu->value->model);
> +}
> +if (cpu->value->has_accel) {
> +monitor_printf(mon, " accel=%s",
> AccelId_lookup[cpu->value->accel]);
> +}
> +
> monitor_printf(mon, " thread_id=%" PRId64 "\n",
> cpu->value->thread_id);
> }
>
> --
> 1.8.3.1
>
--
Eduardo
[Qemu-devel] [PATCH v6 04/17] Extend HMP command info cpus to display accelerator id and model name
The HMP command info cpus now displays the CPU model name and the
backing accelerator if part of the CPUState.
(qemu) info cpus
* CPU #0: (halted) model=2827-ga2 accel=kvm thread_id=1679
Signed-off-by: Michael Mueller
Acked-by: Christian Borntraeger
---
hmp.c | 7 +++
1 file changed, 7 insertions(+)
diff --git a/hmp.c b/hmp.c
index f142d36..676d821 100644
--- a/hmp.c
+++ b/hmp.c
@@ -290,6 +290,13 @@ void hmp_info_cpus(Monitor *mon, const QDict *qdict)
monitor_printf(mon, " (halted)");
}
+if (cpu->value->has_model) {
+monitor_printf(mon, " model=%s", cpu->value->model);
+}
+if (cpu->value->has_accel) {
+monitor_printf(mon, " accel=%s",
AccelId_lookup[cpu->value->accel]);
+}
+
monitor_printf(mon, " thread_id=%" PRId64 "\n", cpu->value->thread_id);
}
--
1.8.3.1
