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 ehabk...@redhat.com 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 m...@linux.vnet.ibm.com Acked-by: Christian Borntraeger borntrae...@de.ibm.com 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 Wed, May 06, 2015 at 09:32:58AM +0200, Michael Mueller wrote: On Tue, 5 May 2015 10:14:32 -0300 Eduardo Habkost ehabk...@redhat.com 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 m...@linux.vnet.ibm.com Acked-by: Christian Borntraeger borntrae...@de.ibm.com 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 Wed, May 06, 2015 at 08:59:56AM -0400, Luiz Capitulino wrote: On Wed, 6 May 2015 07:38:53 -0300 Eduardo Habkost ehabk...@redhat.com 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 ehabk...@redhat.com 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 m...@linux.vnet.ibm.com Acked-by: Christian Borntraeger borntrae...@de.ibm.com 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 10:33:55 -0300 Eduardo Habkost ehabk...@redhat.com 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 ehabk...@redhat.com 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 ehabk...@redhat.com 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 m...@linux.vnet.ibm.com Acked-by: Christian Borntraeger borntrae...@de.ibm.com 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, 6 May 2015 07:38:53 -0300 Eduardo Habkost ehabk...@redhat.com 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 ehabk...@redhat.com 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 m...@linux.vnet.ibm.com Acked-by: Christian Borntraeger borntrae...@de.ibm.com 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 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 m...@linux.vnet.ibm.com Acked-by: Christian Borntraeger borntrae...@de.ibm.com 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