[PATCH 0/3] python: testing fixes

2022-12-02 Thread John Snow
A few tiny touchups needed for cutting edge 'flake8' tooling, a minor
type touchup in iotests, and extending the python tests to cover the
recently released Python 3.11.

John Snow (3):
  Python: fix flake8 config
  iotests/check: Fix typing for sys.exit() value
  python: add 3.11 to supported list

 python/setup.cfg | 6 --
 tests/qemu-iotests/check | 2 +-
 2 files changed, 5 insertions(+), 3 deletions(-)

-- 
2.38.1





[PATCH 3/3] python: add 3.11 to supported list

2022-12-02 Thread John Snow
Signed-off-by: John Snow 
---
 python/setup.cfg | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/python/setup.cfg b/python/setup.cfg
index c0d7bab168e..56418157065 100644
--- a/python/setup.cfg
+++ b/python/setup.cfg
@@ -19,6 +19,7 @@ classifiers =
 Programming Language :: Python :: 3.8
 Programming Language :: Python :: 3.9
 Programming Language :: Python :: 3.10
+Programming Language :: Python :: 3.11
 Typing :: Typed
 
 [options]
@@ -159,7 +160,7 @@ multi_line_output=3
 # of python available on your system to run this test.
 
 [tox:tox]
-envlist = py36, py37, py38, py39, py310
+envlist = py36, py37, py38, py39, py310, py311
 skip_missing_interpreters = true
 
 [testenv]
-- 
2.38.1




[PATCH 2/3] iotests/check: Fix typing for sys.exit() value

2022-12-02 Thread John Snow
Signed-off-by: John Snow 
---
 tests/qemu-iotests/check | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/qemu-iotests/check b/tests/qemu-iotests/check
index 75de1b4691e..9bdda1394e7 100755
--- a/tests/qemu-iotests/check
+++ b/tests/qemu-iotests/check
@@ -159,7 +159,7 @@ if __name__ == '__main__':
 if not tests:
 raise ValueError('No tests selected')
 except ValueError as e:
-sys.exit(e)
+sys.exit(str(e))
 
 if args.dry_run:
 print('\n'.join(tests))
-- 
2.38.1




[PATCH 1/3] Python: fix flake8 config

2022-12-02 Thread John Snow
Newer flake8 versions are a bit pickier about the config file, and my
in-line comment confuses the parser. Fix it.

Signed-off-by: John Snow 
---
 python/setup.cfg | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/python/setup.cfg b/python/setup.cfg
index c2c61c75190..c0d7bab168e 100644
--- a/python/setup.cfg
+++ b/python/setup.cfg
@@ -71,7 +71,8 @@ console_scripts =
 qmp-tui = qemu.qmp.qmp_tui:main [tui]
 
 [flake8]
-extend-ignore = E722  # Prefer pylint's bare-except checks to flake8's
+# Prefer pylint's bare-except checks to flake8's
+extend-ignore = E722
 exclude = __pycache__,
 
 [mypy]
-- 
2.38.1




Re: [RFC PATCH] test-bdrv-drain: keep graph manipulations out of coroutines

2022-12-02 Thread Paolo Bonzini

On 12/2/22 14:42, Emanuele Giuseppe Esposito wrote:



Am 02/12/2022 um 14:27 schrieb Paolo Bonzini:

Changes to the BlockDriverState graph will have to take the
corresponding lock for writing, and therefore cannot be done
inside a coroutine.  Move them outside the test body.

Signed-off-by: Paolo Bonzini 
---
  tests/unit/test-bdrv-drain.c | 63 ++--
  1 file changed, 46 insertions(+), 17 deletions(-)

diff --git a/tests/unit/test-bdrv-drain.c b/tests/unit/test-bdrv-drain.c
index 6ae44116fe79..d85083dd4f9e 100644
--- a/tests/unit/test-bdrv-drain.c
+++ b/tests/unit/test-bdrv-drain.c
@@ -199,25 +199,40 @@ static void do_drain_end_unlocked(enum drain_type 
drain_type, BlockDriverState *
  }
  }
  
+static BlockBackend *blk;

+static BlockDriverState *bs, *backing;
+
+static void test_drv_cb_init(void)
+{
+blk = blk_new(qemu_get_aio_context(), BLK_PERM_ALL, BLK_PERM_ALL);
+bs = bdrv_new_open_driver(&bdrv_test, "test-node", BDRV_O_RDWR,
+  &error_abort);
+blk_insert_bs(blk, bs, &error_abort);
+
+backing = bdrv_new_open_driver(&bdrv_test, "backing", 0, &error_abort);
+bdrv_set_backing_hd(bs, backing, &error_abort);
+}
+
+static void test_drv_cb_fini(void)


fini stands for "finito"? :)


No, for finish :) 
http://ftp.math.utah.edu/u/ma/hohn/linux/misc/elf/node3.html



Anyways, an alternative solution for this is also here (probably coming
from you too):
https://lists.nongnu.org/archive/html/qemu-devel/2022-03/msg03517.html


Much better.  At least patches 7-8 from that series have to be salvaged, 
possibly 10 as well.


Paolo




Re: [PATCH v15 1/6] qmp: add QMP command x-query-virtio

2022-12-02 Thread Markus Armbruster
Philippe Mathieu-Daudé  writes:

> On 2/12/22 13:23, Jonah Palmer wrote:
>> On 11/30/22 11:16, Philippe Mathieu-Daudé wrote:
>>> Hi,
>>>
>>> On 11/8/22 14:24, Jonah Palmer wrote:
 From: Laurent Vivier 

 This new command lists all the instances of VirtIODevices with
 their canonical QOM path and name.

 [Jonah: @virtio_list duplicates information that already exists in
   the QOM composition tree. However, extracting necessary information
   from this tree seems to be a bit convoluted.

   Instead, we still create our own list of realized virtio devices
   but use @qmp_qom_get with the device's canonical QOM path to confirm
   that the device exists and is realized. If the device exists but
   is actually not realized, then we remove it from our list (for
   synchronicity to the QOM composition tree).

How could this happen?


   Also, the QMP command @x-query-virtio is redundant as @qom-list
   and @qom-get are sufficient to search '/machine/' for realized
   virtio devices. However, @x-query-virtio is much more convenient
   in listing realized virtio devices.]

 Signed-off-by: Laurent Vivier 
 Signed-off-by: Jonah Palmer 
 ---
   hw/virtio/meson.build  |  2 ++
   hw/virtio/virtio-stub.c    | 14 
   hw/virtio/virtio.c | 44 
   include/hw/virtio/virtio.h |  1 +
   qapi/meson.build   |  1 +
   qapi/qapi-schema.json  |  1 +
   qapi/virtio.json   | 68 ++
   tests/qtest/qmp-cmd-test.c |  1 +
   8 files changed, 132 insertions(+)
   create mode 100644 hw/virtio/virtio-stub.c
   create mode 100644 qapi/virtio.json
>>>
 diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
 index 5d607aeaa0..bdfa82e9c0 100644
 --- a/hw/virtio/virtio.c
 +++ b/hw/virtio/virtio.c
 @@ -13,12 +13,18 @@
     #include "qemu/osdep.h"
   #include "qapi/error.h"
 +#include "qapi/qmp/qdict.h"
 +#include "qapi/qapi-commands-virtio.h"
 +#include "qapi/qapi-commands-qom.h"
 +#include "qapi/qapi-visit-virtio.h"
 +#include "qapi/qmp/qjson.h"
   #include "cpu.h"
   #include "trace.h"
   #include "qemu/error-report.h"
   #include "qemu/log.h"
   #include "qemu/main-loop.h"
   #include "qemu/module.h"
 +#include "qom/object_interfaces.h"
   #include "hw/virtio/virtio.h"
   #include "migration/qemu-file-types.h"
   #include "qemu/atomic.h"
 @@ -29,6 +35,9 @@
   #include "sysemu/runstate.h"
   #include "standard-headers/linux/virtio_ids.h"
   +/* QAPI list of realized VirtIODevices */
 +static QTAILQ_HEAD(, VirtIODevice) virtio_list;
 +
   /*
    * The alignment to use between consumer and producer parts of vring.
    * x86 pagesize again. This is the default, used by transports like PCI
 @@ -3698,6 +3707,7 @@ static void virtio_device_realize(DeviceState *dev, 
 Error **errp)
   vdev->listener.commit = virtio_memory_listener_commit;
   vdev->listener.name = "virtio";
   memory_listener_register(&vdev->listener, vdev->dma_as);
 +    QTAILQ_INSERT_TAIL(&virtio_list, vdev, next);
   }
     static void virtio_device_unrealize(DeviceState *dev)
 @@ -3712,6 +3722,7 @@ static void virtio_device_unrealize(DeviceState *dev)
   vdc->unrealize(dev);
   }
   +    QTAILQ_REMOVE(&virtio_list, vdev, next);
   g_free(vdev->bus_name);
   vdev->bus_name = NULL;
   }
 @@ -3885,6 +3896,8 @@ static void virtio_device_class_init(ObjectClass 
 *klass, void *data)
   vdc->stop_ioeventfd = virtio_device_stop_ioeventfd_impl;
     vdc->legacy_features |= VIRTIO_LEGACY_FEATURES;
 +
 +    QTAILQ_INIT(&virtio_list);
   }
     bool virtio_device_ioeventfd_enabled(VirtIODevice *vdev)
 @@ -3895,6 +3908,37 @@ bool virtio_device_ioeventfd_enabled(VirtIODevice 
 *vdev)
   return virtio_bus_ioeventfd_enabled(vbus);
   }
   +VirtioInfoList *qmp_x_query_virtio(Error **errp)
 +{
 +    VirtioInfoList *list = NULL;
 +    VirtioInfoList *node;
 +    VirtIODevice *vdev;
 +
 +    QTAILQ_FOREACH(vdev, &virtio_list, next) {
 +    DeviceState *dev = DEVICE(vdev);
 +    Error *err = NULL;
 +    QObject *obj = qmp_qom_get(dev->canonical_path, "realized", &err);
 +
 +    if (err == NULL) {
 +    GString *is_realized = qobject_to_json_pretty(obj, true);
 +    /* virtio device is NOT realized, remove it from list */
>>>
>>> Why not check dev->realized instead of calling qmp_qom_get() & 
>>> qobject_to_json_pretty()?
>>
>> This check queries the QOM composition tree to check that the device 
>> actually exists and is
>> realized. In other words, we just want to confirm with the QOM composition 
>> tree for the dev

Re: [PATCH v15 1/6] qmp: add QMP command x-query-virtio

2022-12-02 Thread Philippe Mathieu-Daudé

On 2/12/22 13:23, Jonah Palmer wrote:


On 11/30/22 11:16, Philippe Mathieu-Daudé wrote:

Hi,

On 11/8/22 14:24, Jonah Palmer wrote:

From: Laurent Vivier 

This new command lists all the instances of VirtIODevices with
their canonical QOM path and name.

[Jonah: @virtio_list duplicates information that already exists in
  the QOM composition tree. However, extracting necessary information
  from this tree seems to be a bit convoluted.

  Instead, we still create our own list of realized virtio devices
  but use @qmp_qom_get with the device's canonical QOM path to confirm
  that the device exists and is realized. If the device exists but
  is actually not realized, then we remove it from our list (for
  synchronicity to the QOM composition tree).

  Also, the QMP command @x-query-virtio is redundant as @qom-list
  and @qom-get are sufficient to search '/machine/' for realized
  virtio devices. However, @x-query-virtio is much more convenient
  in listing realized virtio devices.]

Signed-off-by: Laurent Vivier 
Signed-off-by: Jonah Palmer 
---
  hw/virtio/meson.build  |  2 ++
  hw/virtio/virtio-stub.c    | 14 
  hw/virtio/virtio.c | 44 
  include/hw/virtio/virtio.h |  1 +
  qapi/meson.build   |  1 +
  qapi/qapi-schema.json  |  1 +
  qapi/virtio.json   | 68 ++
  tests/qtest/qmp-cmd-test.c |  1 +
  8 files changed, 132 insertions(+)
  create mode 100644 hw/virtio/virtio-stub.c
  create mode 100644 qapi/virtio.json



diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index 5d607aeaa0..bdfa82e9c0 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -13,12 +13,18 @@
    #include "qemu/osdep.h"
  #include "qapi/error.h"
+#include "qapi/qmp/qdict.h"
+#include "qapi/qapi-commands-virtio.h"
+#include "qapi/qapi-commands-qom.h"
+#include "qapi/qapi-visit-virtio.h"
+#include "qapi/qmp/qjson.h"
  #include "cpu.h"
  #include "trace.h"
  #include "qemu/error-report.h"
  #include "qemu/log.h"
  #include "qemu/main-loop.h"
  #include "qemu/module.h"
+#include "qom/object_interfaces.h"
  #include "hw/virtio/virtio.h"
  #include "migration/qemu-file-types.h"
  #include "qemu/atomic.h"
@@ -29,6 +35,9 @@
  #include "sysemu/runstate.h"
  #include "standard-headers/linux/virtio_ids.h"
  +/* QAPI list of realized VirtIODevices */
+static QTAILQ_HEAD(, VirtIODevice) virtio_list;
+
  /*
   * The alignment to use between consumer and producer parts of vring.
   * x86 pagesize again. This is the default, used by transports like 
PCI
@@ -3698,6 +3707,7 @@ static void virtio_device_realize(DeviceState 
*dev, Error **errp)

  vdev->listener.commit = virtio_memory_listener_commit;
  vdev->listener.name = "virtio";
  memory_listener_register(&vdev->listener, vdev->dma_as);
+    QTAILQ_INSERT_TAIL(&virtio_list, vdev, next);
  }
    static void virtio_device_unrealize(DeviceState *dev)
@@ -3712,6 +3722,7 @@ static void virtio_device_unrealize(DeviceState 
*dev)

  vdc->unrealize(dev);
  }
  +    QTAILQ_REMOVE(&virtio_list, vdev, next);
  g_free(vdev->bus_name);
  vdev->bus_name = NULL;
  }
@@ -3885,6 +3896,8 @@ static void 
virtio_device_class_init(ObjectClass *klass, void *data)

  vdc->stop_ioeventfd = virtio_device_stop_ioeventfd_impl;
    vdc->legacy_features |= VIRTIO_LEGACY_FEATURES;
+
+    QTAILQ_INIT(&virtio_list);
  }
    bool virtio_device_ioeventfd_enabled(VirtIODevice *vdev)
@@ -3895,6 +3908,37 @@ bool 
virtio_device_ioeventfd_enabled(VirtIODevice *vdev)

  return virtio_bus_ioeventfd_enabled(vbus);
  }
  +VirtioInfoList *qmp_x_query_virtio(Error **errp)
+{
+    VirtioInfoList *list = NULL;
+    VirtioInfoList *node;
+    VirtIODevice *vdev;
+
+    QTAILQ_FOREACH(vdev, &virtio_list, next) {
+    DeviceState *dev = DEVICE(vdev);
+    Error *err = NULL;
+    QObject *obj = qmp_qom_get(dev->canonical_path, "realized", 
&err);

+
+    if (err == NULL) {
+    GString *is_realized = qobject_to_json_pretty(obj, true);
+    /* virtio device is NOT realized, remove it from list */


Why not check dev->realized instead of calling qmp_qom_get() & 
qobject_to_json_pretty()?


This check queries the QOM composition tree to check that the device actually 
exists and is
realized. In other words, we just want to confirm with the QOM composition tree 
for the device.

This was done to have some kind of synchronicity between @virtio_list and the 
QOM composition
tree, since the list duplicates information that already exists in the tree.

This check was recommended in v10 and added in v11 of this patch series.


Thanks, I found Markus comments:

v10:
https://lore.kernel.org/qemu-devel/87ee6ogbiw@dusky.pond.sub.org/
v11:
https://lore.kernel.org/qemu-devel/87ee4abtdu@pond.sub.org/

Having the justification from v11 in the code rather than the commit
description could help.




+    if (!strncmp(is_realized->str, "false", 4)) {
+    QTAI

Re: [RFC PATCH] test-bdrv-drain: keep graph manipulations out of coroutines

2022-12-02 Thread Emanuele Giuseppe Esposito



Am 02/12/2022 um 14:27 schrieb Paolo Bonzini:
> Changes to the BlockDriverState graph will have to take the
> corresponding lock for writing, and therefore cannot be done
> inside a coroutine.  Move them outside the test body.
> 
> Signed-off-by: Paolo Bonzini 
> ---
>  tests/unit/test-bdrv-drain.c | 63 ++--
>  1 file changed, 46 insertions(+), 17 deletions(-)
> 
> diff --git a/tests/unit/test-bdrv-drain.c b/tests/unit/test-bdrv-drain.c
> index 6ae44116fe79..d85083dd4f9e 100644
> --- a/tests/unit/test-bdrv-drain.c
> +++ b/tests/unit/test-bdrv-drain.c
> @@ -199,25 +199,40 @@ static void do_drain_end_unlocked(enum drain_type 
> drain_type, BlockDriverState *
>  }
>  }
>  
> +static BlockBackend *blk;
> +static BlockDriverState *bs, *backing;
> +
> +static void test_drv_cb_init(void)
> +{
> +blk = blk_new(qemu_get_aio_context(), BLK_PERM_ALL, BLK_PERM_ALL);
> +bs = bdrv_new_open_driver(&bdrv_test, "test-node", BDRV_O_RDWR,
> +  &error_abort);
> +blk_insert_bs(blk, bs, &error_abort);
> +
> +backing = bdrv_new_open_driver(&bdrv_test, "backing", 0, &error_abort);
> +bdrv_set_backing_hd(bs, backing, &error_abort);
> +}
> +
> +static void test_drv_cb_fini(void)

fini stands for "finito"? :)

Anyways, an alternative solution for this is also here (probably coming
from you too):
https://lists.nongnu.org/archive/html/qemu-devel/2022-03/msg03517.html

Thank you,
Emanuele

> +{
> +bdrv_unref(backing);
> +bdrv_unref(bs);
> +blk_unref(blk);
> +backing = NULL;
> +bs = NULL;
> +blk = NULL;
> +}
> +
>  static void test_drv_cb_common(enum drain_type drain_type, bool recursive)
>  {
> -BlockBackend *blk;
> -BlockDriverState *bs, *backing;
>  BDRVTestState *s, *backing_s;
>  BlockAIOCB *acb;
>  int aio_ret;
>  
>  QEMUIOVector qiov = QEMU_IOVEC_INIT_BUF(qiov, NULL, 0);
>  
> -blk = blk_new(qemu_get_aio_context(), BLK_PERM_ALL, BLK_PERM_ALL);
> -bs = bdrv_new_open_driver(&bdrv_test, "test-node", BDRV_O_RDWR,
> -  &error_abort);
>  s = bs->opaque;
> -blk_insert_bs(blk, bs, &error_abort);
> -
> -backing = bdrv_new_open_driver(&bdrv_test, "backing", 0, &error_abort);
>  backing_s = backing->opaque;
> -bdrv_set_backing_hd(bs, backing, &error_abort);
>  
>  /* Simple bdrv_drain_all_begin/end pair, check that CBs are called */
>  g_assert_cmpint(s->drain_count, ==, 0);
> @@ -252,30 +267,44 @@ static void test_drv_cb_common(enum drain_type 
> drain_type, bool recursive)
>  
>  g_assert_cmpint(s->drain_count, ==, 0);
>  g_assert_cmpint(backing_s->drain_count, ==, 0);
> -
> -bdrv_unref(backing);
> -bdrv_unref(bs);
> -blk_unref(blk);
>  }
>  
> -static void test_drv_cb_drain_all(void)
> +static void test_drv_cb_do_drain_all(void)
>  {
>  test_drv_cb_common(BDRV_DRAIN_ALL, true);
>  }
>  
> -static void test_drv_cb_drain(void)
> +static void test_drv_cb_do_drain(void)
>  {
>  test_drv_cb_common(BDRV_DRAIN, false);
>  }
>  
> +static void test_drv_cb_drain_all(void)
> +{
> +test_drv_cb_init();
> +test_drv_cb_do_drain_all();
> +test_drv_cb_fini();
> +}
> +
> +static void test_drv_cb_drain(void)
> +{
> +test_drv_cb_init();
> +test_drv_cb_do_drain();
> +test_drv_cb_fini();
> +}
> +
>  static void test_drv_cb_co_drain_all(void)
>  {
> -call_in_coroutine(test_drv_cb_drain_all);
> +test_drv_cb_init();
> +call_in_coroutine(test_drv_cb_do_drain_all);
> +test_drv_cb_fini();
>  }
>  
>  static void test_drv_cb_co_drain(void)
>  {
> -call_in_coroutine(test_drv_cb_drain);
> +test_drv_cb_init();
> +call_in_coroutine(test_drv_cb_do_drain);
> +test_drv_cb_fini();
>  }
>  
>  static void test_quiesce_common(enum drain_type drain_type, bool recursive)
> 




[RFC PATCH] test-bdrv-drain: keep graph manipulations out of coroutines

2022-12-02 Thread Paolo Bonzini
Changes to the BlockDriverState graph will have to take the
corresponding lock for writing, and therefore cannot be done
inside a coroutine.  Move them outside the test body.

Signed-off-by: Paolo Bonzini 
---
 tests/unit/test-bdrv-drain.c | 63 ++--
 1 file changed, 46 insertions(+), 17 deletions(-)

diff --git a/tests/unit/test-bdrv-drain.c b/tests/unit/test-bdrv-drain.c
index 6ae44116fe79..d85083dd4f9e 100644
--- a/tests/unit/test-bdrv-drain.c
+++ b/tests/unit/test-bdrv-drain.c
@@ -199,25 +199,40 @@ static void do_drain_end_unlocked(enum drain_type 
drain_type, BlockDriverState *
 }
 }
 
+static BlockBackend *blk;
+static BlockDriverState *bs, *backing;
+
+static void test_drv_cb_init(void)
+{
+blk = blk_new(qemu_get_aio_context(), BLK_PERM_ALL, BLK_PERM_ALL);
+bs = bdrv_new_open_driver(&bdrv_test, "test-node", BDRV_O_RDWR,
+  &error_abort);
+blk_insert_bs(blk, bs, &error_abort);
+
+backing = bdrv_new_open_driver(&bdrv_test, "backing", 0, &error_abort);
+bdrv_set_backing_hd(bs, backing, &error_abort);
+}
+
+static void test_drv_cb_fini(void)
+{
+bdrv_unref(backing);
+bdrv_unref(bs);
+blk_unref(blk);
+backing = NULL;
+bs = NULL;
+blk = NULL;
+}
+
 static void test_drv_cb_common(enum drain_type drain_type, bool recursive)
 {
-BlockBackend *blk;
-BlockDriverState *bs, *backing;
 BDRVTestState *s, *backing_s;
 BlockAIOCB *acb;
 int aio_ret;
 
 QEMUIOVector qiov = QEMU_IOVEC_INIT_BUF(qiov, NULL, 0);
 
-blk = blk_new(qemu_get_aio_context(), BLK_PERM_ALL, BLK_PERM_ALL);
-bs = bdrv_new_open_driver(&bdrv_test, "test-node", BDRV_O_RDWR,
-  &error_abort);
 s = bs->opaque;
-blk_insert_bs(blk, bs, &error_abort);
-
-backing = bdrv_new_open_driver(&bdrv_test, "backing", 0, &error_abort);
 backing_s = backing->opaque;
-bdrv_set_backing_hd(bs, backing, &error_abort);
 
 /* Simple bdrv_drain_all_begin/end pair, check that CBs are called */
 g_assert_cmpint(s->drain_count, ==, 0);
@@ -252,30 +267,44 @@ static void test_drv_cb_common(enum drain_type 
drain_type, bool recursive)
 
 g_assert_cmpint(s->drain_count, ==, 0);
 g_assert_cmpint(backing_s->drain_count, ==, 0);
-
-bdrv_unref(backing);
-bdrv_unref(bs);
-blk_unref(blk);
 }
 
-static void test_drv_cb_drain_all(void)
+static void test_drv_cb_do_drain_all(void)
 {
 test_drv_cb_common(BDRV_DRAIN_ALL, true);
 }
 
-static void test_drv_cb_drain(void)
+static void test_drv_cb_do_drain(void)
 {
 test_drv_cb_common(BDRV_DRAIN, false);
 }
 
+static void test_drv_cb_drain_all(void)
+{
+test_drv_cb_init();
+test_drv_cb_do_drain_all();
+test_drv_cb_fini();
+}
+
+static void test_drv_cb_drain(void)
+{
+test_drv_cb_init();
+test_drv_cb_do_drain();
+test_drv_cb_fini();
+}
+
 static void test_drv_cb_co_drain_all(void)
 {
-call_in_coroutine(test_drv_cb_drain_all);
+test_drv_cb_init();
+call_in_coroutine(test_drv_cb_do_drain_all);
+test_drv_cb_fini();
 }
 
 static void test_drv_cb_co_drain(void)
 {
-call_in_coroutine(test_drv_cb_drain);
+test_drv_cb_init();
+call_in_coroutine(test_drv_cb_do_drain);
+test_drv_cb_fini();
 }
 
 static void test_quiesce_common(enum drain_type drain_type, bool recursive)
-- 
2.38.1




Re: [PATCH v15 1/6] qmp: add QMP command x-query-virtio

2022-12-02 Thread Jonah Palmer


On 11/30/22 11:16, Philippe Mathieu-Daudé wrote:

Hi,

On 11/8/22 14:24, Jonah Palmer wrote:

From: Laurent Vivier 

This new command lists all the instances of VirtIODevices with
their canonical QOM path and name.

[Jonah: @virtio_list duplicates information that already exists in
  the QOM composition tree. However, extracting necessary information
  from this tree seems to be a bit convoluted.

  Instead, we still create our own list of realized virtio devices
  but use @qmp_qom_get with the device's canonical QOM path to confirm
  that the device exists and is realized. If the device exists but
  is actually not realized, then we remove it from our list (for
  synchronicity to the QOM composition tree).

  Also, the QMP command @x-query-virtio is redundant as @qom-list
  and @qom-get are sufficient to search '/machine/' for realized
  virtio devices. However, @x-query-virtio is much more convenient
  in listing realized virtio devices.]

Signed-off-by: Laurent Vivier 
Signed-off-by: Jonah Palmer 
---
  hw/virtio/meson.build  |  2 ++
  hw/virtio/virtio-stub.c    | 14 
  hw/virtio/virtio.c | 44 
  include/hw/virtio/virtio.h |  1 +
  qapi/meson.build   |  1 +
  qapi/qapi-schema.json  |  1 +
  qapi/virtio.json   | 68 ++
  tests/qtest/qmp-cmd-test.c |  1 +
  8 files changed, 132 insertions(+)
  create mode 100644 hw/virtio/virtio-stub.c
  create mode 100644 qapi/virtio.json



diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index 5d607aeaa0..bdfa82e9c0 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -13,12 +13,18 @@
    #include "qemu/osdep.h"
  #include "qapi/error.h"
+#include "qapi/qmp/qdict.h"
+#include "qapi/qapi-commands-virtio.h"
+#include "qapi/qapi-commands-qom.h"
+#include "qapi/qapi-visit-virtio.h"
+#include "qapi/qmp/qjson.h"
  #include "cpu.h"
  #include "trace.h"
  #include "qemu/error-report.h"
  #include "qemu/log.h"
  #include "qemu/main-loop.h"
  #include "qemu/module.h"
+#include "qom/object_interfaces.h"
  #include "hw/virtio/virtio.h"
  #include "migration/qemu-file-types.h"
  #include "qemu/atomic.h"
@@ -29,6 +35,9 @@
  #include "sysemu/runstate.h"
  #include "standard-headers/linux/virtio_ids.h"
  +/* QAPI list of realized VirtIODevices */
+static QTAILQ_HEAD(, VirtIODevice) virtio_list;
+
  /*
   * The alignment to use between consumer and producer parts of vring.
   * x86 pagesize again. This is the default, used by transports like 
PCI
@@ -3698,6 +3707,7 @@ static void virtio_device_realize(DeviceState 
*dev, Error **errp)

  vdev->listener.commit = virtio_memory_listener_commit;
  vdev->listener.name = "virtio";
  memory_listener_register(&vdev->listener, vdev->dma_as);
+    QTAILQ_INSERT_TAIL(&virtio_list, vdev, next);
  }
    static void virtio_device_unrealize(DeviceState *dev)
@@ -3712,6 +3722,7 @@ static void virtio_device_unrealize(DeviceState 
*dev)

  vdc->unrealize(dev);
  }
  +    QTAILQ_REMOVE(&virtio_list, vdev, next);
  g_free(vdev->bus_name);
  vdev->bus_name = NULL;
  }
@@ -3885,6 +3896,8 @@ static void 
virtio_device_class_init(ObjectClass *klass, void *data)

  vdc->stop_ioeventfd = virtio_device_stop_ioeventfd_impl;
    vdc->legacy_features |= VIRTIO_LEGACY_FEATURES;
+
+    QTAILQ_INIT(&virtio_list);
  }
    bool virtio_device_ioeventfd_enabled(VirtIODevice *vdev)
@@ -3895,6 +3908,37 @@ bool 
virtio_device_ioeventfd_enabled(VirtIODevice *vdev)

  return virtio_bus_ioeventfd_enabled(vbus);
  }
  +VirtioInfoList *qmp_x_query_virtio(Error **errp)
+{
+    VirtioInfoList *list = NULL;
+    VirtioInfoList *node;
+    VirtIODevice *vdev;
+
+    QTAILQ_FOREACH(vdev, &virtio_list, next) {
+    DeviceState *dev = DEVICE(vdev);
+    Error *err = NULL;
+    QObject *obj = qmp_qom_get(dev->canonical_path, "realized", 
&err);

+
+    if (err == NULL) {
+    GString *is_realized = qobject_to_json_pretty(obj, true);
+    /* virtio device is NOT realized, remove it from list */


Why not check dev->realized instead of calling qmp_qom_get() & 
qobject_to_json_pretty()?


This check queries the QOM composition tree to check that the device actually 
exists and is
realized. In other words, we just want to confirm with the QOM composition tree 
for the device.

This was done to have some kind of synchronicity between @virtio_list and the 
QOM composition
tree, since the list duplicates information that already exists in the tree.

This check was recommended in v10 and added in v11 of this patch series.




+    if (!strncmp(is_realized->str, "false", 4)) {
+    QTAILQ_REMOVE(&virtio_list, vdev, next);
+    } else {
+    node = g_new0(VirtioInfoList, 1);
+    node->value = g_new(VirtioInfo, 1);
+    node->value->path = g_strdup(dev->canonical_path);
+    node->value->name = g_strdup(vdev->name);
+    QAPI_L

Re: [PATCH] blockdev: add 'media=cdrom' argument to support usb cdrom emulated as cdrom

2022-12-02 Thread Paolo Bonzini

On 12/2/22 03:26, Zhipeng Lu wrote:

NAME  MAJ:MIN RM  SIZE RO TYPE MOUNTPOINT
sda 8:0    0  100M  1 disk
vda   252:0    0   10G  0 disk
├─vda1    252:1    0    1G  0 part /boot
└─vda2    252:2    0    9G  0 part
   ├─rhel-root 253:0    0    8G  0 lvm  /
   └─rhel-swap 253:1    0    1G  0 lvm  [SWAP]
lshw -short|grep cdrom -i
No cdrom.

My patch is to solve this problem, usb cdrom emulated as cdrom.


This is a libvirt bug, it should use usb-bot instead of usb-storage 
together with -blockdev.  Then it can add a scsi-cd device below usb-bot.


Paolo




在 2022/12/1 23:35, Markus Armbruster 写道:

luzhipeng  writes:


From: zhipeng Lu 

The drive interface supports media=cdrom so that the usb cdrom
can be emulated as cdrom in qemu, but libvirt deprived the drive
interface, so media=cdrom is added to the blockdev interface to
support usb cdrom emulated as cdrom

Signed-off-by: zhipeng Lu 


What problem are you trying to solve?