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

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

[Ting Wang]

Make "info iothreads" available on the HMP monitor.

The results are as follows:
id1: thread_id=thread_id1
id2: thread_id=thread_id2

Signed-off-by: Ting Wang <kathy.wangting@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;
+    }
+
+    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;
+    }
+
+    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,
     },
 };
-- 
1.8.5

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

[Fam Zheng]

On Fri, 03/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
> 
> Signed-off-by: Ting Wang <kathy.wangting@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;
> +    }
> +
> +    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;
> +    }
> +
> +    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,
>      },
>  };
> -- 
> 1.8.5

Looks good, thanks!

Reviewed-by: Fam Zheng <famz@redhat.com>

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

[Patchew Tool]

This series failed Patchew automatic testing.

Find the log fragments below (grepped lines around keywords "error" and
"warning"), or open the following URL to see the full log:

http://qemu.patchew.org/testing/log/<1426237119-110112-1-git-send-email-kathy.wangting@huawei.com>

----------8<---------

  CC    qobject/qerror.o
  GEN   trace/generated-events.c
  CC    trace/control.o
  CC    trace/qmp.o
  CC    util/osdep.o
  CC    util/cutils.o
  CC    util/unicode.o
  CC    util/qemu-timer-common.o
  CC    util/oslib-posix.o
  CC    util/qemu-thread-posix.o
  CC    util/event_notifier-posix.o
  CC    util/qemu-openpty.o
  CC    util/envlist.o
  CC    util/path.o
  CC    util/module.o
  CC    util/bitmap.o
  CC    util/bitops.o
  CC    util/hbitmap.o
  CC    util/fifo8.o
  CC    util/acl.o
  CC    util/error.o
  CC    util/qemu-error.o
  CC    util/compatfd.o
  CC    util/id.o
  CC    util/iov.o
  CC    util/aes.o
  CC    util/qemu-config.o
  CC    util/qemu-sockets.o
  CC    util/uri.o
  CC    util/notify.o
  CC    util/qemu-option.o
  CC    util/qemu-progress.o
  CC    util/hexdump.o
  CC    util/crc32c.o
  CC    util/throttle.o
  CC    util/getauxval.o
  CC    util/readline.o
  CC    util/rfifolock.o
  CC    util/rcu.o
  CC    stubs/arch-query-cpu-def.o
  CC    stubs/bdrv-commit-all.o
  CC    stubs/chr-baum-init.o
  CC    stubs/chr-msmouse.o
  CC    stubs/chr-testdev.o
  CC    stubs/clock-warp.o
  CC    stubs/cpu-get-clock.o
  CC    stubs/cpu-get-icount.o
  CC    stubs/fdset-add-fd.o
  CC    stubs/fdset-find-fd.o
  CC    stubs/dump.o
  CC    stubs/fdset-get-fd.o
  CC    stubs/fdset-remove-fd.o
  CC    stubs/gdbstub.o
  CC    stubs/get-fd.o
  CC    stubs/get-next-serial.o
  CC    stubs/iothread-lock.o
  CC    stubs/get-vm-name.o
  CC    stubs/is-daemonized.o
  CC    stubs/machine-init-done.o
  CC    stubs/mon-is-qmp.o
  CC    stubs/migr-blocker.o
  CC    stubs/mon-printf.o
  CC    stubs/mon-set-error.o
  CC    stubs/monitor-init.o
  CC    stubs/notify-event.o
  CC    stubs/qtest.o
  CC    stubs/reset.o
  CC    stubs/runstate-check.o
  CC    stubs/set-fd-handler.o
  CC    stubs/slirp.o
  CC    stubs/sysbus.o
  CC    stubs/uuid.o
  CC    stubs/vc-init.o
  CC    stubs/vm-stop.o
  CC    stubs/vmstate.o
  CC    stubs/cpus.o
  CC    stubs/kvm.o
  CC    stubs/qmp_pc_dimm_device_list.o
  CC    qemu-nbd.o
  CC    async.o
  CC    thread-pool.o
  CC    nbd.o
  CC    block.o
--
GTESTER tests/test-mul64
GTESTER tests/test-int128
GTESTER tests/rcutorture
GTESTER tests/test-rcu-list
GTESTER tests/test-bitops
GTESTER tests/check-qom-interface
GTESTER tests/test-qemu-opts
GTESTER tests/test-write-threshold
GTESTER check-qtest-x86_64
GTESTER tests/test-qmp-output-visitor
GTESTER tests/test-qmp-input-visitor
GTESTER tests/test-qmp-input-strict
GTESTER tests/test-qmp-commands
GTESTER tests/test-string-input-visitor
GTESTER tests/test-string-output-visitor
GTESTER tests/test-qmp-event
GTESTER tests/test-opts-visitor
GTESTER tests/test-visitor-serialization
socket_accept failed: Resource temporarily unavailable
**
ERROR:/var/tmp/patchew-test/git/tests/libqtest.c:192:qtest_init: assertion failed: (s->fd >= 0 && s->qmp_fd >= 0)
GTester: last random seed: R02Sa6e2a5d48f79c1916f52824e1b74ecc5
socket_accept failed: Resource temporarily unavailable
**
ERROR:/var/tmp/patchew-test/git/tests/libqtest.c:192:qtest_init: assertion failed: (s->fd >= 0 && s->qmp_fd >= 0)
GTester: last random seed: R02Sd05e2b63284437ca347a3e639d127a65
blkdebug: Suspended request 'A'
blkdebug: Resuming request 'A'
main-loop: WARNING: I/O thread spun for 1000 iterations
main-loop: WARNING: I/O thread spun for 1000 iterations
main-loop: WARNING: I/O thread spun for 1000 iterations
main-loop: WARNING: I/O thread spun for 1000 iterations
[vmxnet3][WR][vmxnet3_peer_has_vnet_hdr]: Peer has no virtio extension. Task offloads will be emulated.
make: *** [check-qtest-x86_64] Error 1

Test failed.

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

[Fam Zheng]

On Fri, 03/13 02:16, Patchew Tool wrote:
> socket_accept failed: Resource temporarily unavailable
> **
> ERROR:/var/tmp/patchew-test/git/tests/libqtest.c:192:qtest_init: assertion failed: (s->fd >= 0 && s->qmp_fd >= 0)
> GTester: last random seed: R02Sa6e2a5d48f79c1916f52824e1b74ecc5
> socket_accept failed: Resource temporarily unavailable
> **
> ERROR:/var/tmp/patchew-test/git/tests/libqtest.c:192:qtest_init: assertion failed: (s->fd >= 0 && s->qmp_fd >= 0)
> GTester: last random seed: R02Sd05e2b63284437ca347a3e639d127a65
> blkdebug: Suspended request 'A'
> blkdebug: Resuming request 'A'
> main-loop: WARNING: I/O thread spun for 1000 iterations
> main-loop: WARNING: I/O thread spun for 1000 iterations
> main-loop: WARNING: I/O thread spun for 1000 iterations
> main-loop: WARNING: I/O thread spun for 1000 iterations
> [vmxnet3][WR][vmxnet3_peer_has_vnet_hdr]: Peer has no virtio extension. Task offloads will be emulated.
> make: *** [check-qtest-x86_64] Error 1

Hmm.. looks like a false alarm, the socket initialization failed but that's
definitely not related to this patch.

Fam

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

[Markus Armbruster]

Fam Zheng <famz@redhat.com> writes:

> On Fri, 03/13 02:16, Patchew Tool wrote:
>> socket_accept failed: Resource temporarily unavailable
>> **
>> ERROR:/var/tmp/patchew-test/git/tests/libqtest.c:192:qtest_init:
>> assertion failed: (s->fd >= 0 && s->qmp_fd >= 0)
>> GTester: last random seed: R02Sa6e2a5d48f79c1916f52824e1b74ecc5
>> socket_accept failed: Resource temporarily unavailable
>> **
>> ERROR:/var/tmp/patchew-test/git/tests/libqtest.c:192:qtest_init:
>> assertion failed: (s->fd >= 0 && s->qmp_fd >= 0)
>> GTester: last random seed: R02Sd05e2b63284437ca347a3e639d127a65
>> blkdebug: Suspended request 'A'
>> blkdebug: Resuming request 'A'
>> main-loop: WARNING: I/O thread spun for 1000 iterations
>> main-loop: WARNING: I/O thread spun for 1000 iterations
>> main-loop: WARNING: I/O thread spun for 1000 iterations
>> main-loop: WARNING: I/O thread spun for 1000 iterations
>> [vmxnet3][WR][vmxnet3_peer_has_vnet_hdr]: Peer has no virtio
>> extension. Task offloads will be emulated.
>> make: *** [check-qtest-x86_64] Error 1
>
> Hmm.. looks like a false alarm, the socket initialization failed but that's
> definitely not related to this patch.

I see this occasionally in my own testing.  Never got around to
harnening the test harness against it.

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

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 653 bytes --]

On Fri, Mar 13, 2015 at 04:58:39PM +0800, 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
> 
> Signed-off-by: Ting Wang <kathy.wangting@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(+)

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

[-- Attachment #2: Type: application/pgp-signature, Size: 473 bytes --]

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

[Ting Wang]

Hi Luiz and Markus,

Would you like to pick up this patch, which has
been reviewed by Stefan and Fam?

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
> 
> Signed-off-by: Ting Wang <kathy.wangting@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;
> +    }
> +
> +    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;
> +    }
> +
> +    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,
>      },
>  };
> 

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

[Markus Armbruster]

Ting Wang <kathy.wangting@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.

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.wangting@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,
>>      },
>>  };
>> 

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

[Christian Borntraeger]

Am 23.06.2015 um 13:57 schrieb Markus Armbruster:
> Ting Wang <kathy.wangting@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.
> 
> Quick review inline.
> 

Is there some patch available that allows someone to query the disks attached to
an iothread (or check if a disk uses iothreads?). I had several testers asking why
x-dataplane is off after specifying iothread. Maybe this information could be
added here as well?



>>
>> 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.wangting@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,
>>>      },
>>>  };
>>>
> 

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

[Christian Borntraeger]

Am 23.06.2015 um 14:19 schrieb Christian Borntraeger:
> Am 23.06.2015 um 13:57 schrieb Markus Armbruster:
>> Ting Wang <kathy.wangting@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.
>>
>> Quick review inline.
>>
> 
> Is there some patch available that allows someone to query the disks attached to
> an iothread (or check if a disk uses iothreads?). I had several testers asking why
> x-dataplane is off after specifying iothread. Maybe this information could be
> added here as well?
> 

To make my last point clear. I know that x-dataplane != iothread, but people
google for "is data plane enabled" and the only answer they find is x-data-plane,
because iothreads are not visible for drives (e.g. via info qtree).

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

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 1161 bytes --]

On Tue, Jun 23, 2015 at 02:19:07PM +0200, Christian Borntraeger wrote:
> Am 23.06.2015 um 13:57 schrieb Markus Armbruster:
> > Ting Wang <kathy.wangting@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.
> > 
> > Quick review inline.
> > 
> 
> Is there some patch available that allows someone to query the disks attached to
> an iothread (or check if a disk uses iothreads?). I had several testers asking why
> x-dataplane is off after specifying iothread. Maybe this information could be
> added here as well?

There is currently no way to query that by iothread.

The virtio-blk device's "iothread" property is a QOM link property, so
it is not displayed by "info qtree".  You should be able to see it with
qom-ls/qom-get but there is not human-friendly interface.

Stefan

[-- Attachment #2: Type: application/pgp-signature, Size: 473 bytes --]

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

[Luiz Capitulino]

On Tue, 23 Jun 2015 13:57:08 +0200
Markus Armbruster <armbru@redhat.com> wrote:

> Ting Wang <kathy.wangting@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.wangting@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,
> >>      },
> >>  };
> >> 
> 

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

[Ting Wang]

On 2015-6-23 19:57, Markus Armbruster wrote:
> Ting Wang <kathy.wangting@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.
> 
> Quick review inline.
> 

Sorry, I have forgotten it.

>>
>> 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.
> 

OK

>>>
>>> Signed-off-by: Ting Wang <kathy.wangting@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.
> 

Thank you for your suggestion, I will modify it.

Ting

>>> +
>>> +    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,
>>>      },
>>>  };
>>>
> 
> .
>