qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH v4] hmp: add info iothreads command


From: Markus Armbruster
Subject: Re: [Qemu-devel] [PATCH v4] hmp: add info iothreads command
Date: Tue, 23 Jun 2015 13:57:08 +0200
User-agent: Gnus/5.13 (Gnus v5.13) Emacs/24.3 (gnu/linux)

Ting Wang <address@hidden> 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.

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 <address@hidden>
>> ---
>> 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
>> address@hidden 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,
>>      },
>>  };
>> 



reply via email to

[Prev in Thread] Current Thread [Next in Thread]