On Tue, 23 Jun 2015 13:57:08 +0200
Markus Armbruster <arm...@redhat.com> wrote:
> Ting Wang <kathy.wangt...@huawei.com> writes:
>
> > Hi Luiz and Markus,
> >
> > Would you like to pick up this patch, which has
> > been reviewed by Stefan and Fam?
>
> Looks like this fell through the cracks back in March. You should've
> asked for merge much earlier. Pinging the maintainer after two weeks is
> fair.
>
> I just did a monitor pull, and I can't yet say whether I'll do another
> for 2.4.
It would be great if you could pick this one, because I'll be out for
some days. I wouldn't mind this one going through the block tree either,
as the QMP command did go through it. Worst case I can merge this patch
and post a pull request right before hard freeze deadline.
>
> Quick review inline.
>
> >
> > Thanks.
> >
> > Ting
> >
> > On 2015-3-13 16:58, Ting Wang wrote:
> >> Make "info iothreads" available on the HMP monitor.
> >>
> >> The results are as follows:
> >> id1: thread_id=thread_id1
> >> id2: thread_id=thread_id2
>
> Actually, they are like
>
> id1: thread_id=123
> id2: thread_id=456
>
> Recommend to paste actual output from your testing.
>
> >>
> >> Signed-off-by: Ting Wang <kathy.wangt...@huawei.com>
> >> ---
> >> v4: use the PRId64 format specifier macro for the int64_t thread_id
> >> v3: fix comment and the trailing whitespace
> >> v2: add braces for if
> >> ---
> >> hmp-commands.hx | 2 ++
> >> hmp.c | 22 ++++++++++++++++++++++
> >> hmp.h | 1 +
> >> monitor.c | 7 +++++++
> >> 4 files changed, 32 insertions(+)
> >>
> >> diff --git a/hmp-commands.hx b/hmp-commands.hx
> >> index d5022d8..67d76ed 100644
> >> --- a/hmp-commands.hx
> >> +++ b/hmp-commands.hx
> >> @@ -1746,6 +1746,8 @@ show roms
> >> show the TPM device
> >> @item info memory-devices
> >> show the memory devices
> >> +@item info iothreads
> >> +show iothreads
> >> @end table
> >> ETEXI
> >>
> >> diff --git a/hmp.c b/hmp.c
> >> index 71c28bc..445a8ad 100644
> >> --- a/hmp.c
> >> +++ b/hmp.c
> >> @@ -821,6 +821,28 @@ void hmp_info_tpm(Monitor *mon, const QDict *qdict)
> >> qapi_free_TPMInfoList(info_list);
> >> }
> >>
> >> +void hmp_info_iothreads(Monitor *mon, const QDict *qdict)
> >> +{
> >> + IOThreadInfoList *head = NULL, *elem = NULL;
> >> +
> >> + head = qmp_query_iothreads(NULL);
> >> + if (!head) {
> >> + monitor_printf(mon, "No iothread has been added\n");
> >> + return;
> >> + }
>
> Printing something instead of nothing when the list is empty is matter
> of taste. Consistency would be nice, though. I know several commands
> that print nothing. Can you quote commands printing something?
>
> >> +
> >> + elem = head;
> >> + while (elem) {
> >> + if (elem->value) {
> >> + monitor_printf(mon, "%s: thread_id=%" PRId64 "\n",
> >> elem->value->id,
> >> + elem->value->thread_id);
> >> + }
> >> + elem = elem->next;
> >> + }
>
> for (info = info_list; info; info = info->next) {
> monitor_printf(mon, "%s: thread_id=%" PRId64 "\n",
> elem->value->id, elem->value->thread_id);
> }
>
> 1. for is more readable than while, because it has the full loop control
> in one place.
>
> 2. List elements cannot be null, so don't bother checking for it.
>
> >> +
> >> + qapi_free_IOThreadInfoList(head);
> >> +}
> >> +
> >> void hmp_quit(Monitor *mon, const QDict *qdict)
> >> {
> >> monitor_suspend(mon);
> >> diff --git a/hmp.h b/hmp.h
> >> index 81177b2..d99090e 100644
> >> --- a/hmp.h
> >> +++ b/hmp.h
> >> @@ -38,6 +38,7 @@ void hmp_info_balloon(Monitor *mon, const QDict *qdict);
> >> void hmp_info_pci(Monitor *mon, const QDict *qdict);
> >> void hmp_info_block_jobs(Monitor *mon, const QDict *qdict);
> >> void hmp_info_tpm(Monitor *mon, const QDict *qdict);
> >> +void hmp_info_iothreads(Monitor *mon, const QDict *qdict);
> >> void hmp_quit(Monitor *mon, const QDict *qdict);
> >> void hmp_stop(Monitor *mon, const QDict *qdict);
> >> void hmp_system_reset(Monitor *mon, const QDict *qdict);
> >> diff --git a/monitor.c b/monitor.c
> >> index c86a89e..076a306 100644
> >> --- a/monitor.c
> >> +++ b/monitor.c
> >> @@ -2924,6 +2924,13 @@ static mon_cmd_t info_cmds[] = {
> >> .mhandler.cmd = hmp_info_memory_devices,
> >> },
> >> {
> >> + .name = "iothreads",
> >> + .args_type = "",
> >> + .params = "",
> >> + .help = "show iothreads",
> >> + .mhandler.cmd = hmp_info_iothreads,
> >> + },
> >> + {
> >> .name = NULL,
> >> },
> >> };
> >>
>
