[PATCH v2 00/15] qdev: Add JSON -device

Kevin Wolf posted 15 patches 2 years, 6 months ago
Failed in applying to current master (apply log)
There is a newer version of this series
qapi/qdev.json                      | 15 +++--
include/hw/qdev-core.h              | 15 +++--
include/hw/virtio/virtio-net.h      |  2 +
include/monitor/qdev.h              | 27 +++++++-
include/net/net.h                   |  2 +
hw/arm/virt.c                       |  2 +-
hw/core/qdev-properties-system.c    |  6 ++
hw/core/qdev.c                      | 11 +++-
hw/net/virtio-net.c                 | 85 ++++++++++++-------------
hw/pci-bridge/pci_expander_bridge.c |  2 +-
hw/ppc/e500.c                       |  2 +-
hw/vfio/pci.c                       |  4 +-
hw/xen/xen-legacy-backend.c         |  3 +-
net/vhost-user.c                    | 41 ++++--------
net/vhost-vdpa.c                    | 37 ++++-------
qom/object.c                        |  7 +-
qom/object_interfaces.c             | 19 ++----
softmmu/qdev-monitor.c              | 99 +++++++++++++++++++----------
softmmu/vl.c                        | 63 ++++++++++++++++--
util/qemu-option.c                  |  4 +-
tests/qemu-iotests/051              |  2 +-
tests/qemu-iotests/051.pc.out       |  4 +-
tests/qemu-iotests/245              |  4 +-
23 files changed, 278 insertions(+), 178 deletions(-)
[PATCH v2 00/15] qdev: Add JSON -device
Posted by Kevin Wolf 2 years, 6 months ago
It's still a long way until we'll have QAPIfied devices, but there are
some improvements that we can already make now to make the future switch
easier.

One important part of this is having code paths without QemuOpts, which
we want to get rid of and replace with the keyval parser in the long
run. This series adds support for JSON syntax to -device, which bypasses
QemuOpts.

While we're not using QAPI yet, devices are based on QOM, so we already
do have type checks and an implied schema. JSON syntax supported now can
be supported by QAPI later and regarding command line compatibility,
actually switching to it becomes an implementation detail this way (of
course, it will still add valuable user-visible features like
introspection and documentation).

Apart from making things more future proof, this also immediately adds
a way to do non-scalar properties on the command line. nvme could have
used list support recently, and the lack of it in -device led to some
rather unnatural solution in the first version (doing the relationship
between a device and objects backwards) and loss of features in the
following. With this series, using a list as a device property should be
possible without any weird tricks.

Unfortunately, even QMP device_add goes through QemuOpts before this
series, which destroys any type safety QOM provides and also can't
support non-scalar properties. This is a bug, but it turns out that
libvirt actually relies on it and passes only strings for everything.
So this series still leaves device_add alone until libvirt is fixed.

v2:
- Drop type safe QMP device_add because libvirt gets it wrong, too
- More network patches to eliminate dependencies on the legacy QemuOpts
  data structures which would not contain all devices any more after
  this series. Fix some bugs there as a side effect.
- Remove an unnecessary use of ERRP_GUARD()
- Replaced error handling patch for qdev_set_id() with Damien's
- Drop the deprecation patch until libvirt uses the new JSON syntax

Damien Hedde (1):
  softmmu/qdev-monitor: add error handling in qdev_set_id

Kevin Wolf (14):
  net: Introduce NetClientInfo.check_peer_type()
  net/vhost-user: Fix device compatibility check
  net/vhost-vdpa: Fix device compatibility check
  qom: Reduce use of error_propagate()
  iotests/245: Fix type for iothread property
  iotests/051: Fix typo
  qdev: Avoid using string visitor for properties
  qdev: Make DeviceState.id independent of QemuOpts
  qemu-option: Allow deleting opts during qemu_opts_foreach()
  qdev: Add Error parameter to hide_device() callbacks
  virtio-net: Store failover primary opts pointer locally
  virtio-net: Avoid QemuOpts in failover_find_primary_device()
  qdev: Base object creation on QDict rather than QemuOpts
  vl: Enable JSON syntax for -device

 qapi/qdev.json                      | 15 +++--
 include/hw/qdev-core.h              | 15 +++--
 include/hw/virtio/virtio-net.h      |  2 +
 include/monitor/qdev.h              | 27 +++++++-
 include/net/net.h                   |  2 +
 hw/arm/virt.c                       |  2 +-
 hw/core/qdev-properties-system.c    |  6 ++
 hw/core/qdev.c                      | 11 +++-
 hw/net/virtio-net.c                 | 85 ++++++++++++-------------
 hw/pci-bridge/pci_expander_bridge.c |  2 +-
 hw/ppc/e500.c                       |  2 +-
 hw/vfio/pci.c                       |  4 +-
 hw/xen/xen-legacy-backend.c         |  3 +-
 net/vhost-user.c                    | 41 ++++--------
 net/vhost-vdpa.c                    | 37 ++++-------
 qom/object.c                        |  7 +-
 qom/object_interfaces.c             | 19 ++----
 softmmu/qdev-monitor.c              | 99 +++++++++++++++++++----------
 softmmu/vl.c                        | 63 ++++++++++++++++--
 util/qemu-option.c                  |  4 +-
 tests/qemu-iotests/051              |  2 +-
 tests/qemu-iotests/051.pc.out       |  4 +-
 tests/qemu-iotests/245              |  4 +-
 23 files changed, 278 insertions(+), 178 deletions(-)

-- 
2.31.1

Re: [PATCH v2 00/15] qdev: Add JSON -device
Posted by Michael S. Tsirkin 2 years, 6 months ago
On Fri, Oct 08, 2021 at 03:34:27PM +0200, Kevin Wolf wrote:
> It's still a long way until we'll have QAPIfied devices, but there are
> some improvements that we can already make now to make the future switch
> easier.
> 
> One important part of this is having code paths without QemuOpts, which
> we want to get rid of and replace with the keyval parser in the long
> run. This series adds support for JSON syntax to -device, which bypasses
> QemuOpts.
> 
> While we're not using QAPI yet, devices are based on QOM, so we already
> do have type checks and an implied schema. JSON syntax supported now can
> be supported by QAPI later and regarding command line compatibility,
> actually switching to it becomes an implementation detail this way (of
> course, it will still add valuable user-visible features like
> introspection and documentation).
> 
> Apart from making things more future proof, this also immediately adds
> a way to do non-scalar properties on the command line. nvme could have
> used list support recently, and the lack of it in -device led to some
> rather unnatural solution in the first version (doing the relationship
> between a device and objects backwards) and loss of features in the
> following. With this series, using a list as a device property should be
> possible without any weird tricks.
> 
> Unfortunately, even QMP device_add goes through QemuOpts before this
> series, which destroys any type safety QOM provides and also can't
> support non-scalar properties. This is a bug, but it turns out that
> libvirt actually relies on it and passes only strings for everything.
> So this series still leaves device_add alone until libvirt is fixed.


Reviewed-by: Michael S. Tsirkin <mst@redhat.com>

I assume you are merging this?

> v2:
> - Drop type safe QMP device_add because libvirt gets it wrong, too
> - More network patches to eliminate dependencies on the legacy QemuOpts
>   data structures which would not contain all devices any more after
>   this series. Fix some bugs there as a side effect.
> - Remove an unnecessary use of ERRP_GUARD()
> - Replaced error handling patch for qdev_set_id() with Damien's
> - Drop the deprecation patch until libvirt uses the new JSON syntax
> 
> Damien Hedde (1):
>   softmmu/qdev-monitor: add error handling in qdev_set_id
> 
> Kevin Wolf (14):
>   net: Introduce NetClientInfo.check_peer_type()
>   net/vhost-user: Fix device compatibility check
>   net/vhost-vdpa: Fix device compatibility check
>   qom: Reduce use of error_propagate()
>   iotests/245: Fix type for iothread property
>   iotests/051: Fix typo
>   qdev: Avoid using string visitor for properties
>   qdev: Make DeviceState.id independent of QemuOpts
>   qemu-option: Allow deleting opts during qemu_opts_foreach()
>   qdev: Add Error parameter to hide_device() callbacks
>   virtio-net: Store failover primary opts pointer locally
>   virtio-net: Avoid QemuOpts in failover_find_primary_device()
>   qdev: Base object creation on QDict rather than QemuOpts
>   vl: Enable JSON syntax for -device
> 
>  qapi/qdev.json                      | 15 +++--
>  include/hw/qdev-core.h              | 15 +++--
>  include/hw/virtio/virtio-net.h      |  2 +
>  include/monitor/qdev.h              | 27 +++++++-
>  include/net/net.h                   |  2 +
>  hw/arm/virt.c                       |  2 +-
>  hw/core/qdev-properties-system.c    |  6 ++
>  hw/core/qdev.c                      | 11 +++-
>  hw/net/virtio-net.c                 | 85 ++++++++++++-------------
>  hw/pci-bridge/pci_expander_bridge.c |  2 +-
>  hw/ppc/e500.c                       |  2 +-
>  hw/vfio/pci.c                       |  4 +-
>  hw/xen/xen-legacy-backend.c         |  3 +-
>  net/vhost-user.c                    | 41 ++++--------
>  net/vhost-vdpa.c                    | 37 ++++-------
>  qom/object.c                        |  7 +-
>  qom/object_interfaces.c             | 19 ++----
>  softmmu/qdev-monitor.c              | 99 +++++++++++++++++++----------
>  softmmu/vl.c                        | 63 ++++++++++++++++--
>  util/qemu-option.c                  |  4 +-
>  tests/qemu-iotests/051              |  2 +-
>  tests/qemu-iotests/051.pc.out       |  4 +-
>  tests/qemu-iotests/245              |  4 +-
>  23 files changed, 278 insertions(+), 178 deletions(-)
> 
> -- 
> 2.31.1

Re: [PATCH v2 00/15] qdev: Add JSON -device
Posted by Kevin Wolf 2 years, 6 months ago
Am 13.10.2021 um 17:30 hat Michael S. Tsirkin geschrieben:
> On Fri, Oct 08, 2021 at 03:34:27PM +0200, Kevin Wolf wrote:
> > It's still a long way until we'll have QAPIfied devices, but there are
> > some improvements that we can already make now to make the future switch
> > easier.
> > 
> > One important part of this is having code paths without QemuOpts, which
> > we want to get rid of and replace with the keyval parser in the long
> > run. This series adds support for JSON syntax to -device, which bypasses
> > QemuOpts.
> > 
> > While we're not using QAPI yet, devices are based on QOM, so we already
> > do have type checks and an implied schema. JSON syntax supported now can
> > be supported by QAPI later and regarding command line compatibility,
> > actually switching to it becomes an implementation detail this way (of
> > course, it will still add valuable user-visible features like
> > introspection and documentation).
> > 
> > Apart from making things more future proof, this also immediately adds
> > a way to do non-scalar properties on the command line. nvme could have
> > used list support recently, and the lack of it in -device led to some
> > rather unnatural solution in the first version (doing the relationship
> > between a device and objects backwards) and loss of features in the
> > following. With this series, using a list as a device property should be
> > possible without any weird tricks.
> > 
> > Unfortunately, even QMP device_add goes through QemuOpts before this
> > series, which destroys any type safety QOM provides and also can't
> > support non-scalar properties. This is a bug, but it turns out that
> > libvirt actually relies on it and passes only strings for everything.
> > So this series still leaves device_add alone until libvirt is fixed.
> 
> 
> Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
> 
> I assume you are merging this?

Yes, I can merge it through my tree. Thanks for the review!

Kevin

Re: [PATCH v2 00/15] qdev: Add JSON -device
Posted by Peter Krempa 2 years, 6 months ago
On Fri, Oct 08, 2021 at 15:34:27 +0200, Kevin Wolf wrote:
> It's still a long way until we'll have QAPIfied devices, but there are
> some improvements that we can already make now to make the future switch
> easier.
> 
> One important part of this is having code paths without QemuOpts, which
> we want to get rid of and replace with the keyval parser in the long
> run. This series adds support for JSON syntax to -device, which bypasses
> QemuOpts.
> 
> While we're not using QAPI yet, devices are based on QOM, so we already
> do have type checks and an implied schema. JSON syntax supported now can
> be supported by QAPI later and regarding command line compatibility,
> actually switching to it becomes an implementation detail this way (of
> course, it will still add valuable user-visible features like
> introspection and documentation).
> 
> Apart from making things more future proof, this also immediately adds
> a way to do non-scalar properties on the command line. nvme could have
> used list support recently, and the lack of it in -device led to some
> rather unnatural solution in the first version (doing the relationship
> between a device and objects backwards) and loss of features in the
> following. With this series, using a list as a device property should be
> possible without any weird tricks.
> 
> Unfortunately, even QMP device_add goes through QemuOpts before this
> series, which destroys any type safety QOM provides and also can't
> support non-scalar properties. This is a bug, but it turns out that
> libvirt actually relies on it and passes only strings for everything.
> So this series still leaves device_add alone until libvirt is fixed.

I've tested hotplug and standard -device with many (not all) devices
libvirt uses and seems that both '-device JSON' and monitor work
properly.

I'll enable '-device JSON' in libvirt once this hits upstream.

> 
> v2:
> - Drop type safe QMP device_add because libvirt gets it wrong, too

I've actually applied this patch too to make sure that libvirt is
working properly in both modes. It works now well with this too.

I'm not sure about the compatibility promise here, but libvirt is now
prepared even for teh strict version of 'device_add'. Libvirt's
requirements state that new libvirt needs to be used with new qemu and
thus from our view it's okay to do it even now.

> - More network patches to eliminate dependencies on the legacy QemuOpts
>   data structures which would not contain all devices any more after
>   this series. Fix some bugs there as a side effect.
> - Remove an unnecessary use of ERRP_GUARD()
> - Replaced error handling patch for qdev_set_id() with Damien's
> - Drop the deprecation patch until libvirt uses the new JSON syntax

Series:

Tested-by: Peter Krempa <pkrempa@redhat.com>