Re: [PATCH 1/3] net: Move the code to collect available NIC models to a separate function

[Claudio Fontana]

On 11/4/22 13:57, Thomas Huth wrote:
> The code that collects the available NIC models is not really specific
> to PCI anymore and will be required in the next patch, too, so let's
> move this into a new separate function in net.c instead.
> 
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>  include/net/net.h |  1 +
>  hw/pci/pci.c      | 29 +----------------------------
>  net/net.c         | 36 ++++++++++++++++++++++++++++++++++++
>  3 files changed, 38 insertions(+), 28 deletions(-)
> 
> diff --git a/include/net/net.h b/include/net/net.h
> index 3db75ff841..c96cefb89a 100644
> --- a/include/net/net.h
> +++ b/include/net/net.h
> @@ -189,6 +189,7 @@ void qemu_set_vnet_hdr_len(NetClientState *nc, int len);
>  int qemu_set_vnet_le(NetClientState *nc, bool is_le);
>  int qemu_set_vnet_be(NetClientState *nc, bool is_be);
>  void qemu_macaddr_default_if_unset(MACAddr *macaddr);
> +GPtrArray *qemu_get_nic_models(const char *device_type);

I know there is no precedent in this file, but it would be useful to document 
this function,
what it does exactly and what it returns, the return value, allocation 
assumptions etc.

>  int qemu_show_nic_models(const char *arg, const char *const *models);
>  void qemu_check_nic_model(NICInfo *nd, const char *model);
>  int qemu_find_nic_model(NICInfo *nd, const char * const *models,
> diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> index 2f450f6a72..2b7b343e82 100644
> --- a/hw/pci/pci.c
> +++ b/hw/pci/pci.c
> @@ -1964,7 +1964,6 @@ PCIDevice *pci_nic_init_nofail(NICInfo *nd, PCIBus 
> *rootbus,
>                                 const char *default_devaddr)
>  {
>      const char *devaddr = nd->devaddr ? nd->devaddr : default_devaddr;
> -    GSList *list;
>      GPtrArray *pci_nic_models;
>      PCIBus *bus;
>      PCIDevice *pci_dev;
> @@ -1979,33 +1978,7 @@ PCIDevice *pci_nic_init_nofail(NICInfo *nd, PCIBus 
> *rootbus,
>          nd->model = g_strdup("virtio-net-pci");
>      }
>  
> -    list = object_class_get_list_sorted(TYPE_PCI_DEVICE, false);
> -    pci_nic_models = g_ptr_array_new();
> -    while (list) {
> -        DeviceClass *dc = OBJECT_CLASS_CHECK(DeviceClass, list->data,
> -                                             TYPE_DEVICE);
> -        GSList *next;
> -        if (test_bit(DEVICE_CATEGORY_NETWORK, dc->categories) &&
> -            dc->user_creatable) {
> -            const char *name = object_class_get_name(list->data);
> -            /*
> -             * A network device might also be something else than a NIC, see
> -             * e.g. the "rocker" device. Thus we have to look for the 
> "netdev"
> -             * property, too. Unfortunately, some devices like virtio-net 
> only
> -             * create this property during instance_init, so we have to 
> create
> -             * a temporary instance here to be able to check it.
> -             */
> -            Object *obj = object_new_with_class(OBJECT_CLASS(dc));
> -            if (object_property_find(obj, "netdev")) {
> -                g_ptr_array_add(pci_nic_models, (gpointer)name);
> -            }
> -            object_unref(obj);
> -        }
> -        next = list->next;
> -        g_slist_free_1(list);
> -        list = next;
> -    }
> -    g_ptr_array_add(pci_nic_models, NULL);
> +    pci_nic_models = qemu_get_nic_models(TYPE_PCI_DEVICE);
>  
>      if (qemu_show_nic_models(nd->model, (const char 
> **)pci_nic_models->pdata)) {
>          exit(0);
> diff --git a/net/net.c b/net/net.c
> index 840ad9dca5..c0516a8067 100644
> --- a/net/net.c
> +++ b/net/net.c
> @@ -899,6 +899,42 @@ static int nic_get_free_idx(void)
>      return -1;
>  }
>  
> +GPtrArray *qemu_get_nic_models(const char *device_type)
> +{
> +    GPtrArray *nic_models;
> +    GSList *list;
> +
> +    list = object_class_get_list_sorted(device_type, false);
> +    nic_models = g_ptr_array_new();
> +    while (list) {
> +        DeviceClass *dc = OBJECT_CLASS_CHECK(DeviceClass, list->data,
> +                                             TYPE_DEVICE);
> +        GSList *next;
> +        if (test_bit(DEVICE_CATEGORY_NETWORK, dc->categories) &&
> +            dc->user_creatable) {
> +            const char *name = object_class_get_name(list->data);
> +            /*
> +             * A network device might also be something else than a NIC, see
> +             * e.g. the "rocker" device. Thus we have to look for the 
> "netdev"
> +             * property, too. Unfortunately, some devices like virtio-net 
> only
> +             * create this property during instance_init, so we have to 
> create
> +             * a temporary instance here to be able to check it.
> +             */
> +            Object *obj = object_new_with_class(OBJECT_CLASS(dc));
> +            if (object_property_find(obj, "netdev")) {
> +                g_ptr_array_add(nic_models, (gpointer)name);
> +            }
> +            object_unref(obj);
> +        }
> +        next = list->next;
> +        g_slist_free_1(list);
> +        list = next;
> +    }
> +    g_ptr_array_add(nic_models, NULL);
> +
> +    return nic_models;
> +}
> +
>  int qemu_show_nic_models(const char *arg, const char *const *models)
>  {
>      int i;

Claudio