Remove the hard-coded list of PCI NIC names; instead, fill an array
using all PCI devices listed under DEVICE_CATEGORY_NETWORK. Keep
the old shortcut "virtio" for virtio-net-pci.
Suggested-by: Thomas Huth <thuth@redhat.com>
Cc: Jason Wang <jasowang@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
hw/pci/pci.c | 61 ++++++++++++++++++++++++++++++------------------------------
1 file changed, 30 insertions(+), 31 deletions(-)
diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index 2174c254eb..aa24a26680 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -1815,49 +1815,48 @@ PciInfoList *qmp_query_pci(Error **errp)
return head;
}
-static const char * const pci_nic_models[] = {
- "ne2k_pci",
- "i82551",
- "i82557b",
- "i82559er",
- "rtl8139",
- "e1000",
- "pcnet",
- "virtio",
- "sungem",
- NULL
-};
-
-static const char * const pci_nic_names[] = {
- "ne2k_pci",
- "i82551",
- "i82557b",
- "i82559er",
- "rtl8139",
- "e1000",
- "pcnet",
- "virtio-net-pci",
- "sungem",
- NULL
-};
-
/* Initialize a PCI NIC. */
PCIDevice *pci_nic_init_nofail(NICInfo *nd, PCIBus *rootbus,
const char *default_model,
const char *default_devaddr)
{
const char *devaddr = nd->devaddr ? nd->devaddr : default_devaddr;
+ GSList *list;
+ GPtrArray *pci_nic_models;
PCIBus *bus;
PCIDevice *pci_dev;
DeviceState *dev;
int devfn;
int i;
- if (qemu_show_nic_models(nd->model, pci_nic_models)) {
+ if (!strcmp(nd->model, "virtio")) {
+ g_free(nd->model);
+ nd->model = g_strdup("virtio-net-pci");
+ }
+
+ list = object_class_get_list_sorted(TYPE_PCI_DEVICE, false);
+ pci_nic_models = g_ptr_array_new_with_free_func(g_free);
+ 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) ;
+ g_ptr_array_add(pci_nic_models, (gpointer)name);
+ }
+ next = list->next;
+ g_slist_free_1(list);
+ list = next;
+ }
+ g_ptr_array_add(pci_nic_models, NULL);
+
+ if (qemu_show_nic_models(nd->model, (const char **)pci_nic_models->pdata)) {
exit(0);
}
- i = qemu_find_nic_model(nd, pci_nic_models, default_model);
+ i = qemu_find_nic_model(nd, (const char **)pci_nic_models->pdata,
+ default_model);
if (i < 0) {
exit(1);
}
@@ -1865,15 +1864,15 @@ PCIDevice *pci_nic_init_nofail(NICInfo *nd, PCIBus *rootbus,
bus = pci_get_bus_devfn(&devfn, rootbus, devaddr);
if (!bus) {
error_report("Invalid PCI device address %s for device %s",
- devaddr, pci_nic_names[i]);
+ devaddr, nd->model);
exit(1);
}
- pci_dev = pci_create(bus, devfn, pci_nic_names[i]);
+ pci_dev = pci_create(bus, devfn, nd->model);
dev = &pci_dev->qdev;
qdev_set_nic_properties(dev, nd);
qdev_init_nofail(dev);
-
+ g_ptr_array_free(pci_nic_models, true);
return pci_dev;
}
--
2.14.3
On 06.03.2018 20:45, Paolo Bonzini wrote:
> Remove the hard-coded list of PCI NIC names; instead, fill an array
> using all PCI devices listed under DEVICE_CATEGORY_NETWORK. Keep
> the old shortcut "virtio" for virtio-net-pci.
>
> Suggested-by: Thomas Huth <thuth@redhat.com>
> Cc: Jason Wang <jasowang@redhat.com>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
> hw/pci/pci.c | 61 ++++++++++++++++++++++++++++++------------------------------
> 1 file changed, 30 insertions(+), 31 deletions(-)
>
> diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> index 2174c254eb..aa24a26680 100644
> --- a/hw/pci/pci.c
> +++ b/hw/pci/pci.c
> @@ -1815,49 +1815,48 @@ PciInfoList *qmp_query_pci(Error **errp)
> return head;
> }
>
> -static const char * const pci_nic_models[] = {
> - "ne2k_pci",
> - "i82551",
> - "i82557b",
> - "i82559er",
> - "rtl8139",
> - "e1000",
> - "pcnet",
> - "virtio",
> - "sungem",
> - NULL
> -};
> -
> -static const char * const pci_nic_names[] = {
> - "ne2k_pci",
> - "i82551",
> - "i82557b",
> - "i82559er",
> - "rtl8139",
> - "e1000",
> - "pcnet",
> - "virtio-net-pci",
> - "sungem",
> - NULL
> -};
> -
> /* Initialize a PCI NIC. */
> PCIDevice *pci_nic_init_nofail(NICInfo *nd, PCIBus *rootbus,
> const char *default_model,
> const char *default_devaddr)
> {
> const char *devaddr = nd->devaddr ? nd->devaddr : default_devaddr;
> + GSList *list;
> + GPtrArray *pci_nic_models;
> PCIBus *bus;
> PCIDevice *pci_dev;
> DeviceState *dev;
> int devfn;
> int i;
>
> - if (qemu_show_nic_models(nd->model, pci_nic_models)) {
> + if (!strcmp(nd->model, "virtio")) {
> + g_free(nd->model);
> + nd->model = g_strdup("virtio-net-pci");
Should we maybe also print out a deprecation message in this case, so
that we could get rid of this hack in a couple of releases? (i.e. only
allow "virtio-net-pci" in future releases?)
> + }
> +
> + list = object_class_get_list_sorted(TYPE_PCI_DEVICE, false);
> + pci_nic_models = g_ptr_array_new_with_free_func(g_free);
OK, so that means that the entries will be freed when the array is
destroyed, right? ...
> + 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) ;
... but object_class_get_name() returns the original name, not a
g_strdup of it, right?
(apart from that: Please remove the space before the semicolon)
> + g_ptr_array_add(pci_nic_models, (gpointer)name);
> + }
> + next = list->next;
> + g_slist_free_1(list);
> + list = next;
> + }
> + g_ptr_array_add(pci_nic_models, NULL);
> +
> + if (qemu_show_nic_models(nd->model, (const char **)pci_nic_models->pdata)) {
> exit(0);
> }
>
> - i = qemu_find_nic_model(nd, pci_nic_models, default_model);
> + i = qemu_find_nic_model(nd, (const char **)pci_nic_models->pdata,
> + default_model);
> if (i < 0) {
> exit(1);
> }
> @@ -1865,15 +1864,15 @@ PCIDevice *pci_nic_init_nofail(NICInfo *nd, PCIBus *rootbus,
> bus = pci_get_bus_devfn(&devfn, rootbus, devaddr);
> if (!bus) {
> error_report("Invalid PCI device address %s for device %s",
> - devaddr, pci_nic_names[i]);
> + devaddr, nd->model);
> exit(1);
> }
>
> - pci_dev = pci_create(bus, devfn, pci_nic_names[i]);
> + pci_dev = pci_create(bus, devfn, nd->model);
> dev = &pci_dev->qdev;
> qdev_set_nic_properties(dev, nd);
> qdev_init_nofail(dev);
> -
> + g_ptr_array_free(pci_nic_models, true);
... so this will g_free the original pointers of the device classes - I
guess this will sooner or later cause some trouble.
Apart from that, I really like this patch - that hard-coded
pci_nic_models[] was really ugly.
Thomas
On 08/03/2018 10:02, Thomas Huth wrote:
> On 06.03.2018 20:45, Paolo Bonzini wrote:
>> Remove the hard-coded list of PCI NIC names; instead, fill an array
>> using all PCI devices listed under DEVICE_CATEGORY_NETWORK. Keep
>> the old shortcut "virtio" for virtio-net-pci.
>>
>> Suggested-by: Thomas Huth <thuth@redhat.com>
>> Cc: Jason Wang <jasowang@redhat.com>
>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
>> ---
>> hw/pci/pci.c | 61 ++++++++++++++++++++++++++++++------------------------------
>> 1 file changed, 30 insertions(+), 31 deletions(-)
>>
>> diff --git a/hw/pci/pci.c b/hw/pci/pci.c
>> index 2174c254eb..aa24a26680 100644
>> --- a/hw/pci/pci.c
>> +++ b/hw/pci/pci.c
>> @@ -1815,49 +1815,48 @@ PciInfoList *qmp_query_pci(Error **errp)
>> return head;
>> }
>>
>> -static const char * const pci_nic_models[] = {
>> - "ne2k_pci",
>> - "i82551",
>> - "i82557b",
>> - "i82559er",
>> - "rtl8139",
>> - "e1000",
>> - "pcnet",
>> - "virtio",
>> - "sungem",
>> - NULL
>> -};
>> -
>> -static const char * const pci_nic_names[] = {
>> - "ne2k_pci",
>> - "i82551",
>> - "i82557b",
>> - "i82559er",
>> - "rtl8139",
>> - "e1000",
>> - "pcnet",
>> - "virtio-net-pci",
>> - "sungem",
>> - NULL
>> -};
>> -
>> /* Initialize a PCI NIC. */
>> PCIDevice *pci_nic_init_nofail(NICInfo *nd, PCIBus *rootbus,
>> const char *default_model,
>> const char *default_devaddr)
>> {
>> const char *devaddr = nd->devaddr ? nd->devaddr : default_devaddr;
>> + GSList *list;
>> + GPtrArray *pci_nic_models;
>> PCIBus *bus;
>> PCIDevice *pci_dev;
>> DeviceState *dev;
>> int devfn;
>> int i;
>>
>> - if (qemu_show_nic_models(nd->model, pci_nic_models)) {
>> + if (!strcmp(nd->model, "virtio")) {
>> + g_free(nd->model);
>> + nd->model = g_strdup("virtio-net-pci");
>
> Should we maybe also print out a deprecation message in this case, so
> that we could get rid of this hack in a couple of releases? (i.e. only
> allow "virtio-net-pci" in future releases?)
I don't know... Like -net, this is probably present in countless
tutorials/blog posts, and it would be very hard to remove it.
>> + }
>> +
>> + list = object_class_get_list_sorted(TYPE_PCI_DEVICE, false);
>> + pci_nic_models = g_ptr_array_new_with_free_func(g_free);
>
> OK, so that means that the entries will be freed when the array is
> destroyed, right? ...
Duh. valgrind isn't happy either. I'll send v3. :(
Paolo
© 2016 - 2025 Red Hat, Inc.