[Qemu-devel] [PATCH v2 0/3] change q35 default NIC to e1000e

[Qemu-devel] [PATCH v2 0/3] change q35 default NIC to e1000e

[Paolo Bonzini]

The Intel 82574 NIC has better performance and more features than
the aging e1000 (aka 82540), for example MSI-X.  This patch chooses it
by default for the Q35 machine type.  As suggested by Thomas, instead
of special casing e1000e, all PCI NIC device names become valid models
for "-net nic,model=..." or "-nic model=..." (patches 1-2).

Paolo

v1->v2: fix compilation

Paolo Bonzini (3):
  qom: introduce object_class_get_list_sorted
  net: allow using any PCI NICs in -net or -nic
  q35: change default NIC to e1000e

 hw/i386/pc.c            |  7 +++---
 hw/i386/pc_piix.c       |  6 ++++-
 hw/i386/pc_q35.c        |  8 ++++++-
 hw/pci/pci.c            | 61 ++++++++++++++++++++++++-------------------------
 include/hw/i386/pc.h    |  3 ++-
 include/qom/object.h    | 11 +++++++++
 qdev-monitor.c          |  9 +-------
 qom/object.c            | 13 +++++++++++
 target/alpha/cpu.c      | 15 +-----------
 target/hppa/cpu.c       | 15 +-----------
 target/lm32/cpu.c       | 15 +-----------
 target/sh4/cpu.c        | 15 +-----------
 target/tricore/helper.c |  2 +-
 13 files changed, 78 insertions(+), 102 deletions(-)

-- 
2.14.3

[Qemu-devel] [PATCH 1/3] qom: introduce object_class_get_list_sorted

[Paolo Bonzini]

Unify half a dozen copies of very similar code (the only difference being
whether comparisons were case-sensitive) and use it also in Tricore,
which did not do any sorting of CPU model names.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 include/qom/object.h    | 11 +++++++++++
 qdev-monitor.c          |  9 +--------
 qom/object.c            | 13 +++++++++++++
 target/alpha/cpu.c      | 15 +--------------
 target/hppa/cpu.c       | 15 +--------------
 target/lm32/cpu.c       | 15 +--------------
 target/sh4/cpu.c        | 15 +--------------
 target/tricore/helper.c |  2 +-
 8 files changed, 30 insertions(+), 65 deletions(-)

diff --git a/include/qom/object.h b/include/qom/object.h
index 4f07090db0..96ce81bc5e 100644
--- a/include/qom/object.h
+++ b/include/qom/object.h
@@ -913,6 +913,17 @@ void object_class_foreach(void (*fn)(ObjectClass *klass, void *opaque),
 GSList *object_class_get_list(const char *implements_type,
                               bool include_abstract);
 
+/**
+ * object_class_get_list_sorted:
+ * @implements_type: The type to filter for, including its derivatives.
+ * @include_abstract: Whether to include abstract classes.
+ *
+ * Returns: A singly-linked list of the classes in alphabetical
+ * case-insensitive order.
+ */
+GSList *object_class_get_list_sorted(const char *implements_type,
+                                     bool include_abstract);
+
 /**
  * object_ref:
  * @obj: the object
diff --git a/qdev-monitor.c b/qdev-monitor.c
index b7e3291f8b..61e0300991 100644
--- a/qdev-monitor.c
+++ b/qdev-monitor.c
@@ -122,12 +122,6 @@ static void qdev_print_devinfo(DeviceClass *dc)
     error_printf("\n");
 }
 
-static gint devinfo_cmp(gconstpointer a, gconstpointer b)
-{
-    return strcasecmp(object_class_get_name((ObjectClass *)a),
-                      object_class_get_name((ObjectClass *)b));
-}
-
 static void qdev_print_devinfos(bool show_no_user)
 {
     static const char *cat_name[DEVICE_CATEGORY_MAX + 1] = {
@@ -146,8 +140,7 @@ static void qdev_print_devinfos(bool show_no_user)
     int i;
     bool cat_printed;
 
-    list = g_slist_sort(object_class_get_list(TYPE_DEVICE, false),
-                        devinfo_cmp);
+    list = object_class_get_list_sorted(TYPE_DEVICE, false);
 
     for (i = 0; i <= DEVICE_CATEGORY_MAX; i++) {
         cat_printed = false;
diff --git a/qom/object.c b/qom/object.c
index 755ad03819..6088f55943 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -891,6 +891,19 @@ GSList *object_class_get_list(const char *implements_type,
     return list;
 }
 
+static gint object_class_cmp(gconstpointer a, gconstpointer b)
+{
+    return strcasecmp(object_class_get_name((ObjectClass *)a),
+                      object_class_get_name((ObjectClass *)b));
+}
+
+GSList *object_class_get_list_sorted(const char *implements_type,
+                                     bool include_abstract)
+{
+    return g_slist_sort(object_class_get_list(implements_type, include_abstract),
+                        object_class_cmp);
+}
+
 void object_ref(Object *obj)
 {
     if (!obj) {
diff --git a/target/alpha/cpu.c b/target/alpha/cpu.c
index 55675ce419..b08078e7fc 100644
--- a/target/alpha/cpu.c
+++ b/target/alpha/cpu.c
@@ -71,18 +71,6 @@ static void alpha_cpu_realizefn(DeviceState *dev, Error **errp)
     acc->parent_realize(dev, errp);
 }
 
-/* Sort alphabetically by type name. */
-static gint alpha_cpu_list_compare(gconstpointer a, gconstpointer b)
-{
-    ObjectClass *class_a = (ObjectClass *)a;
-    ObjectClass *class_b = (ObjectClass *)b;
-    const char *name_a, *name_b;
-
-    name_a = object_class_get_name(class_a);
-    name_b = object_class_get_name(class_b);
-    return strcmp(name_a, name_b);
-}
-
 static void alpha_cpu_list_entry(gpointer data, gpointer user_data)
 {
     ObjectClass *oc = data;
@@ -100,8 +88,7 @@ void alpha_cpu_list(FILE *f, fprintf_function cpu_fprintf)
     };
     GSList *list;
 
-    list = object_class_get_list(TYPE_ALPHA_CPU, false);
-    list = g_slist_sort(list, alpha_cpu_list_compare);
+    list = object_class_get_list_sorted(TYPE_ALPHA_CPU, false);
     (*cpu_fprintf)(f, "Available CPUs:\n");
     g_slist_foreach(list, alpha_cpu_list_entry, &s);
     g_slist_free(list);
diff --git a/target/hppa/cpu.c b/target/hppa/cpu.c
index 969f628f0a..c261b6b090 100644
--- a/target/hppa/cpu.c
+++ b/target/hppa/cpu.c
@@ -110,18 +110,6 @@ static void hppa_cpu_realizefn(DeviceState *dev, Error **errp)
 #endif
 }
 
-/* Sort hppabetically by type name. */
-static gint hppa_cpu_list_compare(gconstpointer a, gconstpointer b)
-{
-    ObjectClass *class_a = (ObjectClass *)a;
-    ObjectClass *class_b = (ObjectClass *)b;
-    const char *name_a, *name_b;
-
-    name_a = object_class_get_name(class_a);
-    name_b = object_class_get_name(class_b);
-    return strcmp(name_a, name_b);
-}
-
 static void hppa_cpu_list_entry(gpointer data, gpointer user_data)
 {
     ObjectClass *oc = data;
@@ -138,8 +126,7 @@ void hppa_cpu_list(FILE *f, fprintf_function cpu_fprintf)
     };
     GSList *list;
 
-    list = object_class_get_list(TYPE_HPPA_CPU, false);
-    list = g_slist_sort(list, hppa_cpu_list_compare);
+    list = object_class_get_list_sorted(TYPE_HPPA_CPU, false);
     (*cpu_fprintf)(f, "Available CPUs:\n");
     g_slist_foreach(list, hppa_cpu_list_entry, &s);
     g_slist_free(list);
diff --git a/target/lm32/cpu.c b/target/lm32/cpu.c
index 96c2499d0b..0003152469 100644
--- a/target/lm32/cpu.c
+++ b/target/lm32/cpu.c
@@ -32,18 +32,6 @@ static void lm32_cpu_set_pc(CPUState *cs, vaddr value)
     cpu->env.pc = value;
 }
 
-/* Sort alphabetically by type name. */
-static gint lm32_cpu_list_compare(gconstpointer a, gconstpointer b)
-{
-    ObjectClass *class_a = (ObjectClass *)a;
-    ObjectClass *class_b = (ObjectClass *)b;
-    const char *name_a, *name_b;
-
-    name_a = object_class_get_name(class_a);
-    name_b = object_class_get_name(class_b);
-    return strcmp(name_a, name_b);
-}
-
 static void lm32_cpu_list_entry(gpointer data, gpointer user_data)
 {
     ObjectClass *oc = data;
@@ -65,8 +53,7 @@ void lm32_cpu_list(FILE *f, fprintf_function cpu_fprintf)
     };
     GSList *list;
 
-    list = object_class_get_list(TYPE_LM32_CPU, false);
-    list = g_slist_sort(list, lm32_cpu_list_compare);
+    list = object_class_get_list_sorted(TYPE_LM32_CPU, false);
     (*cpu_fprintf)(f, "Available CPUs:\n");
     g_slist_foreach(list, lm32_cpu_list_entry, &s);
     g_slist_free(list);
diff --git a/target/sh4/cpu.c b/target/sh4/cpu.c
index 6302cfda3a..541ffc2d97 100644
--- a/target/sh4/cpu.c
+++ b/target/sh4/cpu.c
@@ -85,18 +85,6 @@ typedef struct SuperHCPUListState {
     FILE *file;
 } SuperHCPUListState;
 
-/* Sort alphabetically by type name. */
-static gint superh_cpu_list_compare(gconstpointer a, gconstpointer b)
-{
-    ObjectClass *class_a = (ObjectClass *)a;
-    ObjectClass *class_b = (ObjectClass *)b;
-    const char *name_a, *name_b;
-
-    name_a = object_class_get_name(class_a);
-    name_b = object_class_get_name(class_b);
-    return strcmp(name_a, name_b);
-}
-
 static void superh_cpu_list_entry(gpointer data, gpointer user_data)
 {
     SuperHCPUListState *s = user_data;
@@ -114,8 +102,7 @@ void sh4_cpu_list(FILE *f, fprintf_function cpu_fprintf)
     };
     GSList *list;
 
-    list = object_class_get_list(TYPE_SUPERH_CPU, false);
-    list = g_slist_sort(list, superh_cpu_list_compare);
+    list = object_class_get_list_sorted(TYPE_SUPERH_CPU, false);
     g_slist_foreach(list, superh_cpu_list_entry, &s);
     g_slist_free(list);
 }
diff --git a/target/tricore/helper.c b/target/tricore/helper.c
index 45276d3782..dad7eea085 100644
--- a/target/tricore/helper.c
+++ b/target/tricore/helper.c
@@ -101,7 +101,7 @@ void tricore_cpu_list(FILE *f, fprintf_function cpu_fprintf)
     };
     GSList *list;
 
-    list = object_class_get_list(TYPE_TRICORE_CPU, false);
+    list = object_class_get_list_sorted(TYPE_TRICORE_CPU, false);
     (*cpu_fprintf)(f, "Available CPUs:\n");
     g_slist_foreach(list, tricore_cpu_list_entry, &s);
     g_slist_free(list);
-- 
2.14.3

[Qemu-devel] [PATCH 2/3] net: allow using any PCI NICs in -net or -nic

[Paolo Bonzini]

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

[Qemu-devel] [PATCH 3/3] q35: change default NIC to e1000e

[Paolo Bonzini]

The e1000 NIC is getting old and is not a very good default for a
PCIe machine type.  Change it to e1000e, which should be supported
by a good number of guests.

In particular, drivers for 82574 were added first to Linux 2.6.27 (2008)
and Windows 2008 R2.  This does mean that Windows 2008 will not work
anymore with Q35 machine types and a default "-net nic -net xxx" network
configuration; it did work before because it does have an AHCI driver.
However, Windows 2008 has been declared out of main stream support
in 2015.  It will get out of extended support in 2020.  Windows 2008
R2 has the same end of support dates and, since the two are basically
Vista vs. Windows 7, R2 probably is more popular.

Cc: Jason Wang <jasowang@redhat.com>
Cc: Thomas Huth <thuth@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/i386/pc.c         | 7 ++++---
 hw/i386/pc_piix.c    | 6 +++++-
 hw/i386/pc_q35.c     | 8 +++++++-
 hw/pci/pci.c         | 2 +-
 include/hw/i386/pc.h | 3 ++-
 5 files changed, 19 insertions(+), 7 deletions(-)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 35fcb6efdf..d3e1d50b59 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1619,18 +1619,19 @@ void pc_basic_device_init(ISABus *isa_bus, qemu_irq *gsi,
     }
 }
 
-void pc_nic_init(ISABus *isa_bus, PCIBus *pci_bus)
+void pc_nic_init(PCMachineClass *pcmc, ISABus *isa_bus, PCIBus *pci_bus)
 {
     int i;
 
     rom_set_order_override(FW_CFG_ORDER_OVERRIDE_NIC);
     for (i = 0; i < nb_nics; i++) {
         NICInfo *nd = &nd_table[i];
+        const char *model = nd->model ? nd->model : pcmc->default_nic_model;
 
-        if (!pci_bus || (nd->model && strcmp(nd->model, "ne2k_isa") == 0)) {
+        if (!strcmp(model, "ne2k_isa")) {
             pc_init_ne2k_isa(isa_bus, nd);
         } else {
-            pci_nic_init_nofail(nd, pci_bus, "e1000", NULL);
+            pci_nic_init_nofail(nd, pci_bus, model, NULL);
         }
     }
     rom_reset_order_override();
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 8658bcba63..0f1966d547 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -240,7 +240,7 @@ static void pc_init1(MachineState *machine,
     pc_basic_device_init(isa_bus, pcms->gsi, &rtc_state, true,
                          (pcms->vmport != ON_OFF_AUTO_ON), pcms->pit, 0x4);
 
-    pc_nic_init(isa_bus, pci_bus);
+    pc_nic_init(pcmc, isa_bus, pci_bus);
 
     ide_drive_get(hd, ARRAY_SIZE(hd));
     if (pcmc->pci_enabled) {
@@ -417,6 +417,9 @@ static void pc_xen_hvm_init(MachineState *machine)
 
 static void pc_i440fx_machine_options(MachineClass *m)
 {
+    PCMachineClass *pcmc = PC_MACHINE_CLASS(m);
+    pcmc->default_nic_model = "e1000";
+
     m->family = "pc_piix";
     m->desc = "Standard PC (i440FX + PIIX, 1996)";
     m->default_machine_opts = "firmware=bios-256k.bin";
@@ -1114,6 +1117,7 @@ static void isapc_machine_options(MachineClass *m)
     pcmc->gigabyte_align = false;
     pcmc->smbios_legacy_mode = true;
     pcmc->has_reserved_memory = false;
+    pcmc->default_nic_model = "ne2k_isa";
     m->default_cpu_type = X86_CPU_TYPE_NAME("486");
 }
 
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index 0c0bc48137..9ae916327e 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -272,7 +272,7 @@ static void pc_q35_init(MachineState *machine)
 
     /* the rest devices to which pci devfn is automatically assigned */
     pc_vga_init(isa_bus, host_bus);
-    pc_nic_init(isa_bus, host_bus);
+    pc_nic_init(pcmc, isa_bus, host_bus);
 
     if (pcms->acpi_nvdimm_state.is_enabled) {
         nvdimm_init_acpi_state(&pcms->acpi_nvdimm_state, system_io,
@@ -294,6 +294,9 @@ static void pc_q35_init(MachineState *machine)
 
 static void pc_q35_machine_options(MachineClass *m)
 {
+    PCMachineClass *pcmc = PC_MACHINE_CLASS(m);
+    pcmc->default_nic_model = "e1000e";
+
     m->family = "pc_q35";
     m->desc = "Standard PC (Q35 + ICH9, 2009)";
     m->units_per_default_bus = 1;
@@ -316,7 +319,10 @@ DEFINE_Q35_MACHINE(v2_12, "pc-q35-2.12", NULL,
 
 static void pc_q35_2_11_machine_options(MachineClass *m)
 {
+    PCMachineClass *pcmc = PC_MACHINE_CLASS(m);
+
     pc_q35_2_12_machine_options(m);
+    pcmc->default_nic_model = "e1000";
     m->alias = NULL;
     SET_MACHINE_COMPAT(m, PC_COMPAT_2_11);
 }
diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index aa24a26680..3b0df51983 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -1829,7 +1829,7 @@ PCIDevice *pci_nic_init_nofail(NICInfo *nd, PCIBus *rootbus,
     int devfn;
     int i;
 
-    if (!strcmp(nd->model, "virtio")) {
+    if (nd->model && !strcmp(nd->model, "virtio")) {
         g_free(nd->model);
         nd->model = g_strdup("virtio-net-pci");
     }
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index bb49165fe0..e81654eb7f 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -114,6 +114,7 @@ struct PCMachineClass {
     /* Device configuration: */
     bool pci_enabled;
     bool kvmclock_enabled;
+    const char *default_nic_model;
 
     /* Compat options: */
 
@@ -248,7 +249,7 @@ void pc_init_ne2k_isa(ISABus *bus, NICInfo *nd);
 void pc_cmos_init(PCMachineState *pcms,
                   BusState *ide0, BusState *ide1,
                   ISADevice *s);
-void pc_nic_init(ISABus *isa_bus, PCIBus *pci_bus);
+void pc_nic_init(PCMachineClass *pcmc, ISABus *isa_bus, PCIBus *pci_bus);
 void pc_pci_device_init(PCIBus *pci_bus);
 
 typedef void (*cpu_set_smm_t)(int smm, void *arg);
-- 
2.14.3

Re: [Qemu-devel] [PATCH 1/3] qom: introduce object_class_get_list_sorted

[Thomas Huth]

On 06.03.2018 20:45, Paolo Bonzini wrote:
> Unify half a dozen copies of very similar code (the only difference being
> whether comparisons were case-sensitive) and use it also in Tricore,
> which did not do any sorting of CPU model names.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
[...]
> diff --git a/target/hppa/cpu.c b/target/hppa/cpu.c
> index 969f628f0a..c261b6b090 100644
> --- a/target/hppa/cpu.c
> +++ b/target/hppa/cpu.c
> @@ -110,18 +110,6 @@ static void hppa_cpu_realizefn(DeviceState *dev, Error **errp)
>  #endif
>  }
>  
> -/* Sort hppabetically by type name. */

Oh, what a pity to see that "hppabetically" go away, that looked like a
really neat search-and-replace bug :-)

Anyway, patch looks fine to me:

Reviewed-by: Thomas Huth <thuth@redhat.com>

Re: [Qemu-devel] [PATCH 2/3] net: allow using any PCI NICs in -net or -nic

[Thomas Huth]

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

Re: [Qemu-devel] [PATCH 3/3] q35: change default NIC to e1000e

[Thomas Huth]

On 06.03.2018 20:46, Paolo Bonzini wrote:
> The e1000 NIC is getting old and is not a very good default for a
> PCIe machine type.  Change it to e1000e, which should be supported
> by a good number of guests.
> 
> In particular, drivers for 82574 were added first to Linux 2.6.27 (2008)
> and Windows 2008 R2.  This does mean that Windows 2008 will not work
> anymore with Q35 machine types and a default "-net nic -net xxx" network
> configuration; it did work before because it does have an AHCI driver.
> However, Windows 2008 has been declared out of main stream support
> in 2015.  It will get out of extended support in 2020.  Windows 2008
> R2 has the same end of support dates and, since the two are basically
> Vista vs. Windows 7, R2 probably is more popular.
> 
> Cc: Jason Wang <jasowang@redhat.com>
> Cc: Thomas Huth <thuth@redhat.com>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  hw/i386/pc.c         | 7 ++++---
>  hw/i386/pc_piix.c    | 6 +++++-
>  hw/i386/pc_q35.c     | 8 +++++++-
>  hw/pci/pci.c         | 2 +-
>  include/hw/i386/pc.h | 3 ++-
>  5 files changed, 19 insertions(+), 7 deletions(-)
> 
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index 35fcb6efdf..d3e1d50b59 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -1619,18 +1619,19 @@ void pc_basic_device_init(ISABus *isa_bus, qemu_irq *gsi,
>      }
>  }
>  
> -void pc_nic_init(ISABus *isa_bus, PCIBus *pci_bus)
> +void pc_nic_init(PCMachineClass *pcmc, ISABus *isa_bus, PCIBus *pci_bus)
>  {
>      int i;
>  
>      rom_set_order_override(FW_CFG_ORDER_OVERRIDE_NIC);
>      for (i = 0; i < nb_nics; i++) {
>          NICInfo *nd = &nd_table[i];
> +        const char *model = nd->model ? nd->model : pcmc->default_nic_model;
>  
> -        if (!pci_bus || (nd->model && strcmp(nd->model, "ne2k_isa") == 0)) {
> +        if (!strcmp(model, "ne2k_isa")) {

<bikeshedpainting>
I'd prefer g_str_equal() these days ...
</bikeshedpainting>

>              pc_init_ne2k_isa(isa_bus, nd);
>          } else {
> -            pci_nic_init_nofail(nd, pci_bus, "e1000", NULL);
> +            pci_nic_init_nofail(nd, pci_bus, model, NULL);
>          }
>      }
>      rom_reset_order_override();
> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> index 8658bcba63..0f1966d547 100644
> --- a/hw/i386/pc_piix.c
> +++ b/hw/i386/pc_piix.c
> @@ -240,7 +240,7 @@ static void pc_init1(MachineState *machine,
>      pc_basic_device_init(isa_bus, pcms->gsi, &rtc_state, true,
>                           (pcms->vmport != ON_OFF_AUTO_ON), pcms->pit, 0x4);
>  
> -    pc_nic_init(isa_bus, pci_bus);
> +    pc_nic_init(pcmc, isa_bus, pci_bus);
>  
>      ide_drive_get(hd, ARRAY_SIZE(hd));
>      if (pcmc->pci_enabled) {
> @@ -417,6 +417,9 @@ static void pc_xen_hvm_init(MachineState *machine)
>  
>  static void pc_i440fx_machine_options(MachineClass *m)
>  {
> +    PCMachineClass *pcmc = PC_MACHINE_CLASS(m);
> +    pcmc->default_nic_model = "e1000";
> +
>      m->family = "pc_piix";
>      m->desc = "Standard PC (i440FX + PIIX, 1996)";
>      m->default_machine_opts = "firmware=bios-256k.bin";
> @@ -1114,6 +1117,7 @@ static void isapc_machine_options(MachineClass *m)
>      pcmc->gigabyte_align = false;
>      pcmc->smbios_legacy_mode = true;
>      pcmc->has_reserved_memory = false;
> +    pcmc->default_nic_model = "ne2k_isa";
>      m->default_cpu_type = X86_CPU_TYPE_NAME("486");
>  }
>  
> diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
> index 0c0bc48137..9ae916327e 100644
> --- a/hw/i386/pc_q35.c
> +++ b/hw/i386/pc_q35.c
> @@ -272,7 +272,7 @@ static void pc_q35_init(MachineState *machine)
>  
>      /* the rest devices to which pci devfn is automatically assigned */
>      pc_vga_init(isa_bus, host_bus);
> -    pc_nic_init(isa_bus, host_bus);
> +    pc_nic_init(pcmc, isa_bus, host_bus);
>  
>      if (pcms->acpi_nvdimm_state.is_enabled) {
>          nvdimm_init_acpi_state(&pcms->acpi_nvdimm_state, system_io,
> @@ -294,6 +294,9 @@ static void pc_q35_init(MachineState *machine)
>  
>  static void pc_q35_machine_options(MachineClass *m)
>  {
> +    PCMachineClass *pcmc = PC_MACHINE_CLASS(m);
> +    pcmc->default_nic_model = "e1000e";
> +
>      m->family = "pc_q35";
>      m->desc = "Standard PC (Q35 + ICH9, 2009)";
>      m->units_per_default_bus = 1;
> @@ -316,7 +319,10 @@ DEFINE_Q35_MACHINE(v2_12, "pc-q35-2.12", NULL,
>  
>  static void pc_q35_2_11_machine_options(MachineClass *m)
>  {
> +    PCMachineClass *pcmc = PC_MACHINE_CLASS(m);
> +
>      pc_q35_2_12_machine_options(m);
> +    pcmc->default_nic_model = "e1000";
>      m->alias = NULL;
>      SET_MACHINE_COMPAT(m, PC_COMPAT_2_11);
>  }
> diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> index aa24a26680..3b0df51983 100644
> --- a/hw/pci/pci.c
> +++ b/hw/pci/pci.c
> @@ -1829,7 +1829,7 @@ PCIDevice *pci_nic_init_nofail(NICInfo *nd, PCIBus *rootbus,
>      int devfn;
>      int i;
>  
> -    if (!strcmp(nd->model, "virtio")) {
> +    if (nd->model && !strcmp(nd->model, "virtio")) {

Please squash that into the previous patch instead.

>          g_free(nd->model);
>          nd->model = g_strdup("virtio-net-pci");
>      }
> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> index bb49165fe0..e81654eb7f 100644
> --- a/include/hw/i386/pc.h
> +++ b/include/hw/i386/pc.h
> @@ -114,6 +114,7 @@ struct PCMachineClass {
>      /* Device configuration: */
>      bool pci_enabled;
>      bool kvmclock_enabled;
> +    const char *default_nic_model;
>  
>      /* Compat options: */
>  
> @@ -248,7 +249,7 @@ void pc_init_ne2k_isa(ISABus *bus, NICInfo *nd);
>  void pc_cmos_init(PCMachineState *pcms,
>                    BusState *ide0, BusState *ide1,
>                    ISADevice *s);
> -void pc_nic_init(ISABus *isa_bus, PCIBus *pci_bus);
> +void pc_nic_init(PCMachineClass *pcmc, ISABus *isa_bus, PCIBus *pci_bus);
>  void pc_pci_device_init(PCIBus *pci_bus);
>  
>  typedef void (*cpu_set_smm_t)(int smm, void *arg);
> 

Apart from the nits, this looks fine to me.

 Thomas

Re: [Qemu-devel] [PATCH 2/3] net: allow using any PCI NICs in -net or -nic

[Paolo Bonzini]

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

Re: [Qemu-devel] [PATCH 3/3] q35: change default NIC to e1000e

[Paolo Bonzini]

> > -        if (!pci_bus || (nd->model && strcmp(nd->model, "ne2k_isa") == 0))
> > {
> > +        if (!strcmp(model, "ne2k_isa")) {
> 
> <bikeshedpainting>
> I'd prefer g_str_equal() these days ...
> </bikeshedpainting>

I guess I owe you a lot of bikeshedding.  Changed.

> > -    if (!strcmp(nd->model, "virtio")) {
> > +    if (nd->model && !strcmp(nd->model, "virtio")) {
> 
> Please squash that into the previous patch instead.

Pro tip: don't write patches at 5 AM after the son woke up, even if
you cannot fall asleep again.  Sorry for the mess.

Paolo

> >          g_free(nd->model);
> >          nd->model = g_strdup("virtio-net-pci");
> >      }
> > diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> > index bb49165fe0..e81654eb7f 100644
> > --- a/include/hw/i386/pc.h
> > +++ b/include/hw/i386/pc.h
> > @@ -114,6 +114,7 @@ struct PCMachineClass {
> >      /* Device configuration: */
> >      bool pci_enabled;
> >      bool kvmclock_enabled;
> > +    const char *default_nic_model;
> >  
> >      /* Compat options: */
> >  
> > @@ -248,7 +249,7 @@ void pc_init_ne2k_isa(ISABus *bus, NICInfo *nd);
> >  void pc_cmos_init(PCMachineState *pcms,
> >                    BusState *ide0, BusState *ide1,
> >                    ISADevice *s);
> > -void pc_nic_init(ISABus *isa_bus, PCIBus *pci_bus);
> > +void pc_nic_init(PCMachineClass *pcmc, ISABus *isa_bus, PCIBus *pci_bus);
> >  void pc_pci_device_init(PCIBus *pci_bus);
> >  
> >  typedef void (*cpu_set_smm_t)(int smm, void *arg);
> > 
> 
> Apart from the nits, this looks fine to me.
> 
>  Thomas
> 

Re: [Qemu-devel] [PATCH 2/3] net: allow using any PCI NICs in -net or -nic

[Jason Wang]

On 2018年03月09日 01:28, 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..67a3f72bd6 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 (nd->model && !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();
> +    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;
>   }
>   

Reviewed-by: Jason Wang <jasowang@redhat.com>

Re: [Qemu-devel] [PATCH 2/3] net: allow using any PCI NICs in -net or -nic

[Thomas Huth]

On 08.03.2018 18:28, 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..67a3f72bd6 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 (nd->model && !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();
> +    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;
>  }
>  
> 

Reviewed-by: Thomas Huth <thuth@redhat.com>

Re: [Qemu-devel] [PATCH 2/3] net: allow using any PCI NICs in -net or -nic

[Philippe Mathieu-Daudé]

On 03/08/2018 02:28 PM, 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>

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Tested-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

> ---
>  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..67a3f72bd6 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 (nd->model && !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();
> +    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;
>  }
>  
> 

[Qemu-devel] [PATCH 2/3] net: allow using any PCI NICs in -net or -nic

[Paolo Bonzini]

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..67a3f72bd6 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 (nd->model && !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();
+    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

Re: [Qemu-devel] [PATCH 2/3] net: allow using any PCI NICs in -net or -nic

[Philippe Mathieu-Daudé]

On 03/06/2018 01:49 PM, 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>

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

> ---
>  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;
>  }
>  
> 

[Qemu-devel] [PATCH 2/3] net: allow using any PCI NICs in -net or -nic

[Paolo Bonzini]

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