Re: [RFC v2 01/12] qdev: Allow qdev_device_add() to add specific category device

[Jonathan Cameron <Jonathan.Cameron@Huawei.com>]

On Thu, 19 Sep 2024 14:11:17 +0800
Zhao Liu <zhao1.liu@intel.com> wrote:

> Topology devices need to be created and realized before board
> initialization.
> 
> Allow qdev_device_add() to specify category to help create topology
> devices early.
> 
> Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
It's not immediately obvious what the category parameter is.
Can you use DeviceCategory rather than long?

> ---
>  hw/net/virtio-net.c    |  2 +-
>  hw/usb/xen-usb.c       |  3 ++-
>  include/monitor/qdev.h |  4 ++--
>  system/qdev-monitor.c  | 12 ++++++++----
>  system/vl.c            |  4 ++--
>  5 files changed, 15 insertions(+), 10 deletions(-)
> 
> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
> index fb84d142ee29..0d92e09e9076 100644
> --- a/hw/net/virtio-net.c
> +++ b/hw/net/virtio-net.c
> @@ -935,7 +935,7 @@ static void failover_add_primary(VirtIONet *n, Error **errp)
>          return;
>      }
>  
> -    dev = qdev_device_add_from_qdict(n->primary_opts,
> +    dev = qdev_device_add_from_qdict(n->primary_opts, NULL,
>                                       n->primary_opts_from_json,
>                                       &err);
>      if (err) {
> diff --git a/hw/usb/xen-usb.c b/hw/usb/xen-usb.c
> index 13901625c0c8..e4168b1fec7e 100644
> --- a/hw/usb/xen-usb.c
> +++ b/hw/usb/xen-usb.c
> @@ -766,7 +766,8 @@ static void usbback_portid_add(struct usbback_info *usbif, unsigned port,
>      qdict_put_str(qdict, "hostport", portname);
>      opts = qemu_opts_from_qdict(qemu_find_opts("device"), qdict,
>                                  &error_abort);
> -    usbif->ports[port - 1].dev = USB_DEVICE(qdev_device_add(opts, &local_err));
> +    usbif->ports[port - 1].dev = USB_DEVICE(
> +                                     qdev_device_add(opts, NULL, &local_err));
>      if (!usbif->ports[port - 1].dev) {
>          qobject_unref(qdict);
>          xen_pv_printf(&usbif->xendev, 0,
> diff --git a/include/monitor/qdev.h b/include/monitor/qdev.h
> index 1d57bf657794..f5fd6e6c1ffc 100644
> --- a/include/monitor/qdev.h
> +++ b/include/monitor/qdev.h
> @@ -8,8 +8,8 @@ void hmp_info_qdm(Monitor *mon, const QDict *qdict);
>  void qmp_device_add(QDict *qdict, QObject **ret_data, Error **errp);
>  
>  int qdev_device_help(QemuOpts *opts);
> -DeviceState *qdev_device_add(QemuOpts *opts, Error **errp);
> -DeviceState *qdev_device_add_from_qdict(const QDict *opts,
> +DeviceState *qdev_device_add(QemuOpts *opts, long *category, Error **errp);
> +DeviceState *qdev_device_add_from_qdict(const QDict *opts, long *category,
>                                          bool from_json, Error **errp);
>  
>  /**
> diff --git a/system/qdev-monitor.c b/system/qdev-monitor.c
> index 457dfd05115e..fe120353fedc 100644
> --- a/system/qdev-monitor.c
> +++ b/system/qdev-monitor.c
> @@ -632,7 +632,7 @@ const char *qdev_set_id(DeviceState *dev, char *id, Error **errp)
>      return prop->name;
>  }
>  
> -DeviceState *qdev_device_add_from_qdict(const QDict *opts,
> +DeviceState *qdev_device_add_from_qdict(const QDict *opts, long *category,
>                                          bool from_json, Error **errp)
>  {
>      ERRP_GUARD();
> @@ -655,6 +655,10 @@ DeviceState *qdev_device_add_from_qdict(const QDict *opts,
>          return NULL;
>      }
>  
> +    if (category && !test_bit(*category, dc->categories)) {
> +        return NULL;
> +    }
> +
>      /* find bus */
>      path = qdict_get_try_str(opts, "bus");
>      if (path != NULL) {
> @@ -767,12 +771,12 @@ err_del_dev:
>  }
>  
>  /* Takes ownership of @opts on success */
> -DeviceState *qdev_device_add(QemuOpts *opts, Error **errp)
> +DeviceState *qdev_device_add(QemuOpts *opts, long *category, Error **errp)
>  {
>      QDict *qdict = qemu_opts_to_qdict(opts, NULL);
>      DeviceState *ret;
>  
> -    ret = qdev_device_add_from_qdict(qdict, false, errp);
> +    ret = qdev_device_add_from_qdict(qdict, category, false, errp);
>      if (ret) {
>          qemu_opts_del(opts);
>      }
> @@ -897,7 +901,7 @@ void qmp_device_add(QDict *qdict, QObject **ret_data, Error **errp)
>          qemu_opts_del(opts);
>          return;
>      }
> -    dev = qdev_device_add(opts, errp);
> +    dev = qdev_device_add(opts, NULL, errp);
>      if (!dev) {
>          /*
>           * Drain all pending RCU callbacks. This is done because
> diff --git a/system/vl.c b/system/vl.c
> index 193e7049ccbe..c40364e2f091 100644
> --- a/system/vl.c
> +++ b/system/vl.c
> @@ -1212,7 +1212,7 @@ static int device_init_func(void *opaque, QemuOpts *opts, Error **errp)
>  {
>      DeviceState *dev;
>  
> -    dev = qdev_device_add(opts, errp);
> +    dev = qdev_device_add(opts, NULL, errp);
>      if (!dev && *errp) {
>          error_report_err(*errp);
>          return -1;
> @@ -2665,7 +2665,7 @@ static void qemu_create_cli_devices(void)
>           * from the start, so call qdev_device_add_from_qdict() directly for
>           * now.
>           */
> -        dev = qdev_device_add_from_qdict(opt->opts, true, &error_fatal);
> +        dev = qdev_device_add_from_qdict(opt->opts, NULL, true, &error_fatal);
>          object_unref(OBJECT(dev));
>          loc_pop(&opt->loc);
>      }