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);
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;
--
2.31.1
Thomas Huth <thuth@redhat.com> writes:
> 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);
> 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;
> +}
Is it worth freeing as you go and playing the next/list dance when you
could just:
GPtrArray *qemu_get_nic_models(const char *device_type)
{
GPtrArray *nic_models = g_ptr_array_new();
g_autoptr(GSList) list = object_class_get_list_sorted(device_type, false);
do {
DeviceClass *dc = OBJECT_CLASS_CHECK(DeviceClass, list->data,
TYPE_DEVICE);
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);
}
} while ((list = g_slist_next(list)));
g_ptr_array_add(nic_models, NULL);
return nic_models;
}
I must admit I'm not super clear on the lifetimes
object_class_get_list_sorted but I assume the contents are static and we
only need the equivalent of g_slist_free.
> +
> int qemu_show_nic_models(const char *arg, const char *const *models)
> {
> int i;
--
Alex Bennée
On 07/11/2022 18.34, Alex Bennée wrote:
>
> Thomas Huth <thuth@redhat.com> writes:
>
>> 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);
>> 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;
>> +}
>
> Is it worth freeing as you go and playing the next/list dance when you
> could just:
>
> GPtrArray *qemu_get_nic_models(const char *device_type)
> {
> GPtrArray *nic_models = g_ptr_array_new();
> g_autoptr(GSList) list = object_class_get_list_sorted(device_type, false);
>
> do {
> DeviceClass *dc = OBJECT_CLASS_CHECK(DeviceClass, list->data,
> TYPE_DEVICE);
> 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);
> }
> } while ((list = g_slist_next(list)));
> g_ptr_array_add(nic_models, NULL);
>
> return nic_models;
> }
>
> I must admit I'm not super clear on the lifetimes
> object_class_get_list_sorted but I assume the contents are static and we
> only need the equivalent of g_slist_free.
Looks like it could work, too. I'll add a patch on top to change it.
Thomas
On 10/11/2022 11.35, Thomas Huth wrote:
> On 07/11/2022 18.34, Alex Bennée wrote:
>>
>> Thomas Huth <thuth@redhat.com> writes:
>>
>>> 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);
>>> 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;
>>> +}
>>
>> Is it worth freeing as you go and playing the next/list dance when you
>> could just:
>>
>> GPtrArray *qemu_get_nic_models(const char *device_type)
>> {
>> GPtrArray *nic_models = g_ptr_array_new();
>> g_autoptr(GSList) list = object_class_get_list_sorted(device_type,
>> false);
>>
>> do {
>> DeviceClass *dc = OBJECT_CLASS_CHECK(DeviceClass, list->data,
>> TYPE_DEVICE);
>> 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);
>> }
>> } while ((list = g_slist_next(list)));
>> g_ptr_array_add(nic_models, NULL);
>>
>> return nic_models;
>> }
>>
>> I must admit I'm not super clear on the lifetimes
>> object_class_get_list_sorted but I assume the contents are static and we
>> only need the equivalent of g_slist_free.
>
> Looks like it could work, too. I'll add a patch on top to change it.
Ok, now I've tried, and it does not work - valgrind compains about leaking
memory here. The problem is: You have to keep the pointer to the list head
around, by doing list = g_slist_next(list) you leave memory behind that
cannot be freed anymore. Thus I'll drop this change for now, since keeping
the pointer to the list head just for freeing it later is also ugly.
Thomas
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
© 2016 - 2026 Red Hat, Inc.