diff mbox

[RFC] spapr: add ibmveth to the supported network adapters list

Message ID 1381460995-11855-1-git-send-email-aik@ozlabs.ru
State New
Headers show

Commit Message

Alexey Kardashevskiy Oct. 11, 2013, 3:09 a.m. UTC
The problem is that "-net nic,model=?" does not print "ibmveth" in
the list while it is actually supported.

Most of the QEMU emulated network devices are PCI but "ibmveth"
(a.k.a. spapr-vlan) is not. However with "-net nic,model=?", QEMU prints
only PCI devices in the list, even if it does not say that the list is
all about PCI devices.

This adds "?"/"help" handling in spapr.c and adds "ibmveth" in the beginning
of the list.

Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---

This is an RFC patch.

The other solutions could be:
1. add "ibmveth" into pci_nic_models[] in hw/pci/pci.c but this would not
be correct as "ibmveth" is not PCI and it must appear only on pseries machine.

2. implemement short version of qdev_print_category_devices() and call it
with DEVICE_CATEGORY_NETWORK but that would print more devices than
pci_nic_init_nofail() can handle (vmxnet3, usb-bt-dongle).

3. fix qemu_check_nic_model() to specifically say that this is a list of
PCI devices and there might be some other devices which "-net nic,model+"
supports but there are not PCI but that could break compatibility (some
management software may rely on this exact string).

4. Reject the patch and just say that people must stop using "-net". Ok for me :)

Since "-net" is kind of obsolete interface and does not seem to be extended ever,
the proposed patch does not look too ugly, does not it?
---
 hw/ppc/spapr.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

Comments

Alexey Kardashevskiy Oct. 18, 2013, 5:39 a.m. UTC | #1
On 10/11/2013 02:09 PM, Alexey Kardashevskiy wrote:
> The problem is that "-net nic,model=?" does not print "ibmveth" in
> the list while it is actually supported.
> 
> Most of the QEMU emulated network devices are PCI but "ibmveth"
> (a.k.a. spapr-vlan) is not. However with "-net nic,model=?", QEMU prints
> only PCI devices in the list, even if it does not say that the list is
> all about PCI devices.
> 
> This adds "?"/"help" handling in spapr.c and adds "ibmveth" in the beginning
> of the list.
> 
> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> ---
> 
> This is an RFC patch.


Ping?


> The other solutions could be:
> 1. add "ibmveth" into pci_nic_models[] in hw/pci/pci.c but this would not
> be correct as "ibmveth" is not PCI and it must appear only on pseries machine.
> 
> 2. implemement short version of qdev_print_category_devices() and call it
> with DEVICE_CATEGORY_NETWORK but that would print more devices than
> pci_nic_init_nofail() can handle (vmxnet3, usb-bt-dongle).
> 
> 3. fix qemu_check_nic_model() to specifically say that this is a list of
> PCI devices and there might be some other devices which "-net nic,model+"
> supports but there are not PCI but that could break compatibility (some
> management software may rely on this exact string).
> 
> 4. Reject the patch and just say that people must stop using "-net". Ok for me :)
> 
> Since "-net" is kind of obsolete interface and does not seem to be extended ever,
> the proposed patch does not look too ugly, does not it?
> ---
>  hw/ppc/spapr.c | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
> 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index c0613e4..45ed3da 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -1276,6 +1276,21 @@ static void ppc_spapr_init(QEMUMachineInitArgs *args)
>  
>          if (strcmp(nd->model, "ibmveth") == 0) {
>              spapr_vlan_create(spapr->vio_bus, nd);
> +        } else if (is_help_option(nd->model)) {
> +            static const char * const nic_models[] = {
> +                "ibmveth",
> +                "ne2k_pci",
> +                "i82551",
> +                "i82557b",
> +                "i82559er",
> +                "rtl8139",
> +                "e1000",
> +                "pcnet",
> +                "virtio",
> +                NULL
> +            };
> +            qemu_show_nic_models(nd->model, nic_models);
> +            exit(0);
>          } else {
>              pci_nic_init_nofail(&nd_table[i], phb->bus, nd->model, NULL);
>          }
>
Alexander Graf Oct. 27, 2013, 6:03 p.m. UTC | #2
On 10.10.2013, at 20:09, Alexey Kardashevskiy <aik@ozlabs.ru> wrote:

> The problem is that "-net nic,model=?" does not print "ibmveth" in
> the list while it is actually supported.
> 
> Most of the QEMU emulated network devices are PCI but "ibmveth"
> (a.k.a. spapr-vlan) is not. However with "-net nic,model=?", QEMU prints
> only PCI devices in the list, even if it does not say that the list is
> all about PCI devices.
> 
> This adds "?"/"help" handling in spapr.c and adds "ibmveth" in the beginning
> of the list.
> 
> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> ---
> 
> This is an RFC patch.
> 
> The other solutions could be:
> 1. add "ibmveth" into pci_nic_models[] in hw/pci/pci.c but this would not
> be correct as "ibmveth" is not PCI and it must appear only on pseries machine.
> 
> 2. implemement short version of qdev_print_category_devices() and call it
> with DEVICE_CATEGORY_NETWORK but that would print more devices than
> pci_nic_init_nofail() can handle (vmxnet3, usb-bt-dongle).
> 
> 3. fix qemu_check_nic_model() to specifically say that this is a list of
> PCI devices and there might be some other devices which "-net nic,model+"
> supports but there are not PCI but that could break compatibility (some
> management software may rely on this exact string).
> 
> 4. Reject the patch and just say that people must stop using "-net". Ok for me :)
> 
> Since "-net" is kind of obsolete interface and does not seem to be extended ever,
> the proposed patch does not look too ugly, does not it?
> ---
> hw/ppc/spapr.c | 15 +++++++++++++++
> 1 file changed, 15 insertions(+)
> 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index c0613e4..45ed3da 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -1276,6 +1276,21 @@ static void ppc_spapr_init(QEMUMachineInitArgs *args)
> 
>         if (strcmp(nd->model, "ibmveth") == 0) {
>             spapr_vlan_create(spapr->vio_bus, nd);
> +        } else if (is_help_option(nd->model)) {
> +            static const char * const nic_models[] = {
> +                "ibmveth",
> +                "ne2k_pci",
> +                "i82551",
> +                "i82557b",
> +                "i82559er",
> +                "rtl8139",
> +                "e1000",
> +                "pcnet",
> +                "virtio",
> +                NULL
> +            };

I don't like the idea of duplicating that list. Basically the list of supported -net models is incorrect today even on x86 where you can say -net nic,model=ne2k_isa. It really is only a list of PCI devices.

I can think of a number of convoluted ways to fix this up, but I think that ignoring fully accuracy of the output of -net model=? is the most straight forward thing to do.


Alex
Alexey Kardashevskiy Nov. 1, 2013, 10:52 a.m. UTC | #3
On 10/28/2013 05:03 AM, Alexander Graf wrote:
> 
> On 10.10.2013, at 20:09, Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
> 
>> The problem is that "-net nic,model=?" does not print "ibmveth" in
>> the list while it is actually supported.
>>
>> Most of the QEMU emulated network devices are PCI but "ibmveth"
>> (a.k.a. spapr-vlan) is not. However with "-net nic,model=?", QEMU prints
>> only PCI devices in the list, even if it does not say that the list is
>> all about PCI devices.
>>
>> This adds "?"/"help" handling in spapr.c and adds "ibmveth" in the beginning
>> of the list.
>>
>> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
>> ---
>>
>> This is an RFC patch.
>>
>> The other solutions could be:
>> 1. add "ibmveth" into pci_nic_models[] in hw/pci/pci.c but this would not
>> be correct as "ibmveth" is not PCI and it must appear only on pseries machine.
>>
>> 2. implemement short version of qdev_print_category_devices() and call it
>> with DEVICE_CATEGORY_NETWORK but that would print more devices than
>> pci_nic_init_nofail() can handle (vmxnet3, usb-bt-dongle).
>>
>> 3. fix qemu_check_nic_model() to specifically say that this is a list of
>> PCI devices and there might be some other devices which "-net nic,model+"
>> supports but there are not PCI but that could break compatibility (some
>> management software may rely on this exact string).
>>
>> 4. Reject the patch and just say that people must stop using "-net". Ok for me :)
>>
>> Since "-net" is kind of obsolete interface and does not seem to be extended ever,
>> the proposed patch does not look too ugly, does not it?
>> ---
>> hw/ppc/spapr.c | 15 +++++++++++++++
>> 1 file changed, 15 insertions(+)
>>
>> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
>> index c0613e4..45ed3da 100644
>> --- a/hw/ppc/spapr.c
>> +++ b/hw/ppc/spapr.c
>> @@ -1276,6 +1276,21 @@ static void ppc_spapr_init(QEMUMachineInitArgs *args)
>>
>>         if (strcmp(nd->model, "ibmveth") == 0) {
>>             spapr_vlan_create(spapr->vio_bus, nd);
>> +        } else if (is_help_option(nd->model)) {
>> +            static const char * const nic_models[] = {
>> +                "ibmveth",
>> +                "ne2k_pci",
>> +                "i82551",
>> +                "i82557b",
>> +                "i82559er",
>> +                "rtl8139",
>> +                "e1000",
>> +                "pcnet",
>> +                "virtio",
>> +                NULL
>> +            };
> 
> I don't like the idea of duplicating that list.

Neither do I :) But the list itself already looks quite ugly.

> Basically the list of supported -net models is incorrect today even on
> x86 where you can say -net nic,model=ne2k_isa. It really is only a list
> of PCI devices.


> I can think of a number of convoluted ways to fix this up, but I think
> that ignoring fully accuracy of the output of -net model=? is the most
> straight forward thing to do.

Does any of your "convoluted" ways include adding a new category
(DEVICE_CATEGORY_NETWORK_LEGACY?) into enum DeviceCategory, adding devices
from the list above and fixing qemu_show_nic_models() to show what is in
the category?

Or "-net" interface is "deprecated" and we do not want even touch it?
Alexander Graf Nov. 1, 2013, 12:47 p.m. UTC | #4
Am 01.11.2013 um 03:52 schrieb Alexey Kardashevskiy <aik@ozlabs.ru>:

> On 10/28/2013 05:03 AM, Alexander Graf wrote:
>> 
>> On 10.10.2013, at 20:09, Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
>> 
>>> The problem is that "-net nic,model=?" does not print "ibmveth" in
>>> the list while it is actually supported.
>>> 
>>> Most of the QEMU emulated network devices are PCI but "ibmveth"
>>> (a.k.a. spapr-vlan) is not. However with "-net nic,model=?", QEMU prints
>>> only PCI devices in the list, even if it does not say that the list is
>>> all about PCI devices.
>>> 
>>> This adds "?"/"help" handling in spapr.c and adds "ibmveth" in the beginning
>>> of the list.
>>> 
>>> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
>>> ---
>>> 
>>> This is an RFC patch.
>>> 
>>> The other solutions could be:
>>> 1. add "ibmveth" into pci_nic_models[] in hw/pci/pci.c but this would not
>>> be correct as "ibmveth" is not PCI and it must appear only on pseries machine.
>>> 
>>> 2. implemement short version of qdev_print_category_devices() and call it
>>> with DEVICE_CATEGORY_NETWORK but that would print more devices than
>>> pci_nic_init_nofail() can handle (vmxnet3, usb-bt-dongle).
>>> 
>>> 3. fix qemu_check_nic_model() to specifically say that this is a list of
>>> PCI devices and there might be some other devices which "-net nic,model+"
>>> supports but there are not PCI but that could break compatibility (some
>>> management software may rely on this exact string).
>>> 
>>> 4. Reject the patch and just say that people must stop using "-net". Ok for me :)
>>> 
>>> Since "-net" is kind of obsolete interface and does not seem to be extended ever,
>>> the proposed patch does not look too ugly, does not it?
>>> ---
>>> hw/ppc/spapr.c | 15 +++++++++++++++
>>> 1 file changed, 15 insertions(+)
>>> 
>>> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
>>> index c0613e4..45ed3da 100644
>>> --- a/hw/ppc/spapr.c
>>> +++ b/hw/ppc/spapr.c
>>> @@ -1276,6 +1276,21 @@ static void ppc_spapr_init(QEMUMachineInitArgs *args)
>>> 
>>>        if (strcmp(nd->model, "ibmveth") == 0) {
>>>            spapr_vlan_create(spapr->vio_bus, nd);
>>> +        } else if (is_help_option(nd->model)) {
>>> +            static const char * const nic_models[] = {
>>> +                "ibmveth",
>>> +                "ne2k_pci",
>>> +                "i82551",
>>> +                "i82557b",
>>> +                "i82559er",
>>> +                "rtl8139",
>>> +                "e1000",
>>> +                "pcnet",
>>> +                "virtio",
>>> +                NULL
>>> +            };
>> 
>> I don't like the idea of duplicating that list.
> 
> Neither do I :) But the list itself already looks quite ugly.
> 
>> Basically the list of supported -net models is incorrect today even on
>> x86 where you can say -net nic,model=ne2k_isa. It really is only a list
>> of PCI devices.
> 
> 
>> I can think of a number of convoluted ways to fix this up, but I think
>> that ignoring fully accuracy of the output of -net model=? is the most
>> straight forward thing to do.
> 
> Does any of your "convoluted" ways include adding a new category
> (DEVICE_CATEGORY_NETWORK_LEGACY?) into enum DeviceCategory, adding devices
> from the list above and fixing qemu_show_nic_models() to show what is in
> the category?

Most of them consist of a full redesign of the way -net works :).

> 
> Or "-net" interface is "deprecated" and we do not want even touch it?

I don't think we should deprecate it. It's easier to use than anything else. Ahci adoption heavily suffered from not being enabled in -drive - I don't want that again here.

Alex

> 
> 
> 
> -- 
> Alexey
Paolo Bonzini Nov. 2, 2013, 10:56 a.m. UTC | #5
Il 01/11/2013 11:52, Alexey Kardashevskiy ha scritto:
> On 10/28/2013 05:03 AM, Alexander Graf wrote:
>>
>> On 10.10.2013, at 20:09, Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
>>
>>> The problem is that "-net nic,model=?" does not print "ibmveth" in
>>> the list while it is actually supported.
>>>
>>> Most of the QEMU emulated network devices are PCI but "ibmveth"
>>> (a.k.a. spapr-vlan) is not. However with "-net nic,model=?", QEMU prints
>>> only PCI devices in the list, even if it does not say that the list is
>>> all about PCI devices.
>>>
>>> This adds "?"/"help" handling in spapr.c and adds "ibmveth" in the beginning
>>> of the list.
>>>
>>> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
>>> ---
>>>
>>> This is an RFC patch.
>>>
>>> The other solutions could be:
>>> 1. add "ibmveth" into pci_nic_models[] in hw/pci/pci.c but this would not
>>> be correct as "ibmveth" is not PCI and it must appear only on pseries machine.
>>>
>>> 2. implemement short version of qdev_print_category_devices() and call it
>>> with DEVICE_CATEGORY_NETWORK but that would print more devices than
>>> pci_nic_init_nofail() can handle (vmxnet3, usb-bt-dongle).
>>>
>>> 3. fix qemu_check_nic_model() to specifically say that this is a list of
>>> PCI devices and there might be some other devices which "-net nic,model+"
>>> supports but there are not PCI but that could break compatibility (some
>>> management software may rely on this exact string).
>>>
>>> 4. Reject the patch and just say that people must stop using "-net". Ok for me :)
>>>
>>> Since "-net" is kind of obsolete interface and does not seem to be extended ever,
>>> the proposed patch does not look too ugly, does not it?
>>> ---
>>> hw/ppc/spapr.c | 15 +++++++++++++++
>>> 1 file changed, 15 insertions(+)
>>>
>>> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
>>> index c0613e4..45ed3da 100644
>>> --- a/hw/ppc/spapr.c
>>> +++ b/hw/ppc/spapr.c
>>> @@ -1276,6 +1276,21 @@ static void ppc_spapr_init(QEMUMachineInitArgs *args)
>>>
>>>         if (strcmp(nd->model, "ibmveth") == 0) {
>>>             spapr_vlan_create(spapr->vio_bus, nd);
>>> +        } else if (is_help_option(nd->model)) {
>>> +            static const char * const nic_models[] = {
>>> +                "ibmveth",
>>> +                "ne2k_pci",
>>> +                "i82551",
>>> +                "i82557b",
>>> +                "i82559er",
>>> +                "rtl8139",
>>> +                "e1000",
>>> +                "pcnet",
>>> +                "virtio",
>>> +                NULL
>>> +            };
>>
>> I don't like the idea of duplicating that list.
> 
> Neither do I :) But the list itself already looks quite ugly.
> 
>> Basically the list of supported -net models is incorrect today even on
>> x86 where you can say -net nic,model=ne2k_isa. It really is only a list
>> of PCI devices.
> 
> 
>> I can think of a number of convoluted ways to fix this up, but I think
>> that ignoring fully accuracy of the output of -net model=? is the most
>> straight forward thing to do.
> 
> Does any of your "convoluted" ways include adding a new category
> (DEVICE_CATEGORY_NETWORK_LEGACY?) into enum DeviceCategory, adding devices
> from the list above and fixing qemu_show_nic_models() to show what is in
> the category?

Why do you even need a new category?  Just take everything in the
network category and put it in the help.

Paolo
Markus Armbruster Nov. 2, 2013, 11:51 a.m. UTC | #6
Alexander Graf <agraf@suse.de> writes:

> Am 01.11.2013 um 03:52 schrieb Alexey Kardashevskiy <aik@ozlabs.ru>:
>
[...]
>> Or "-net" interface is "deprecated" and we do not want even touch it?
>
> I don't think we should deprecate it. It's easier to use than anything
> else. Ahci adoption heavily suffered from not being enabled in -drive
> - I don't want that again here.

How exactly is

    -net nic,netdev=NET-ID,macaddr=MACADDR,model=MODEL,...

easier to use than

    -device DEVNAME,netdev=NET-ID,macaddr=MACADDR,...

?

Especially now that -device help shows valid DEVNAMEs under the heading
"Network devices".
Markus Armbruster Nov. 2, 2013, 12:07 p.m. UTC | #7
Paolo Bonzini <pbonzini@redhat.com> writes:

> Il 01/11/2013 11:52, Alexey Kardashevskiy ha scritto:
>> On 10/28/2013 05:03 AM, Alexander Graf wrote:
>>>
>>> On 10.10.2013, at 20:09, Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
>>>
>>>> The problem is that "-net nic,model=?" does not print "ibmveth" in
>>>> the list while it is actually supported.
>>>>
>>>> Most of the QEMU emulated network devices are PCI but "ibmveth"
>>>> (a.k.a. spapr-vlan) is not. However with "-net nic,model=?", QEMU prints
>>>> only PCI devices in the list, even if it does not say that the list is
>>>> all about PCI devices.
>>>>
>>>> This adds "?"/"help" handling in spapr.c and adds "ibmveth" in the beginning
>>>> of the list.
>>>>
>>>> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
>>>> ---
>>>>
>>>> This is an RFC patch.
>>>>
>>>> The other solutions could be:
>>>> 1. add "ibmveth" into pci_nic_models[] in hw/pci/pci.c but this would not
>>>> be correct as "ibmveth" is not PCI and it must appear only on pseries machine.
>>>>
>>>> 2. implemement short version of qdev_print_category_devices() and call it
>>>> with DEVICE_CATEGORY_NETWORK but that would print more devices than
>>>> pci_nic_init_nofail() can handle (vmxnet3, usb-bt-dongle).
>>>>
>>>> 3. fix qemu_check_nic_model() to specifically say that this is a list of
>>>> PCI devices and there might be some other devices which "-net nic,model+"
>>>> supports but there are not PCI but that could break compatibility (some
>>>> management software may rely on this exact string).
>>>>
>>>> 4. Reject the patch and just say that people must stop using "-net". Ok for me :)
>>>>
>>>> Since "-net" is kind of obsolete interface and does not seem to be extended ever,
>>>> the proposed patch does not look too ugly, does not it?
>>>> ---
>>>> hw/ppc/spapr.c | 15 +++++++++++++++
>>>> 1 file changed, 15 insertions(+)
>>>>
>>>> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
>>>> index c0613e4..45ed3da 100644
>>>> --- a/hw/ppc/spapr.c
>>>> +++ b/hw/ppc/spapr.c
>>>> @@ -1276,6 +1276,21 @@ static void ppc_spapr_init(QEMUMachineInitArgs *args)
>>>>
>>>>         if (strcmp(nd->model, "ibmveth") == 0) {
>>>>             spapr_vlan_create(spapr->vio_bus, nd);
>>>> +        } else if (is_help_option(nd->model)) {
>>>> +            static const char * const nic_models[] = {
>>>> +                "ibmveth",
>>>> +                "ne2k_pci",
>>>> +                "i82551",
>>>> +                "i82557b",
>>>> +                "i82559er",
>>>> +                "rtl8139",
>>>> +                "e1000",
>>>> +                "pcnet",
>>>> +                "virtio",
>>>> +                NULL
>>>> +            };
>>>
>>> I don't like the idea of duplicating that list.
>> 
>> Neither do I :) But the list itself already looks quite ugly.
>> 
>>> Basically the list of supported -net models is incorrect today even on
>>> x86 where you can say -net nic,model=ne2k_isa. It really is only a list
>>> of PCI devices.
>> 
>> 
>>> I can think of a number of convoluted ways to fix this up, but I think
>>> that ignoring fully accuracy of the output of -net model=? is the most
>>> straight forward thing to do.
>> 
>> Does any of your "convoluted" ways include adding a new category
>> (DEVICE_CATEGORY_NETWORK_LEGACY?) into enum DeviceCategory, adding devices
>> from the list above and fixing qemu_show_nic_models() to show what is in
>> the category?
>
> Why do you even need a new category?  Just take everything in the
> network category and put it in the help.

You *can't* currently construct -net nic,model=help output from
registered qdev NICs, because:

* -net nic generally doesn't accept all qdev NICs, and

* the model names accepted by -net nic need *not* match the qdev device
  model names (example: virtio != virtio-net), and

* -net nic may accept model names that map to a non-qdevified NIC (not
   sure such NICs still exist).

How model names map to device models is entirely up to board code.
Boards supporting PCI use the common helper pci_nic_init_nofail() for
PCI models.

Due to the way pci_nic_init_nofail() captures model=help, the help
output lists *only* the models supported via pci_nic_init_nofail(), not
the others.  That's simply a bug.

My advice would be to let -net nic rot in peace.
Alexey Kardashevskiy Nov. 2, 2013, 12:26 p.m. UTC | #8
On 11/02/2013 09:56 PM, Paolo Bonzini wrote:
> Il 01/11/2013 11:52, Alexey Kardashevskiy ha scritto:
>> On 10/28/2013 05:03 AM, Alexander Graf wrote:
>>>
>>> On 10.10.2013, at 20:09, Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
>>>
>>>> The problem is that "-net nic,model=?" does not print "ibmveth" in
>>>> the list while it is actually supported.
>>>>
>>>> Most of the QEMU emulated network devices are PCI but "ibmveth"
>>>> (a.k.a. spapr-vlan) is not. However with "-net nic,model=?", QEMU prints
>>>> only PCI devices in the list, even if it does not say that the list is
>>>> all about PCI devices.
>>>>
>>>> This adds "?"/"help" handling in spapr.c and adds "ibmveth" in the beginning
>>>> of the list.
>>>>
>>>> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
>>>> ---
>>>>
>>>> This is an RFC patch.
>>>>
>>>> The other solutions could be:
>>>> 1. add "ibmveth" into pci_nic_models[] in hw/pci/pci.c but this would not
>>>> be correct as "ibmveth" is not PCI and it must appear only on pseries machine.
>>>>
>>>> 2. implemement short version of qdev_print_category_devices() and call it
>>>> with DEVICE_CATEGORY_NETWORK but that would print more devices than
>>>> pci_nic_init_nofail() can handle (vmxnet3, usb-bt-dongle).
>>>>
>>>> 3. fix qemu_check_nic_model() to specifically say that this is a list of
>>>> PCI devices and there might be some other devices which "-net nic,model+"
>>>> supports but there are not PCI but that could break compatibility (some
>>>> management software may rely on this exact string).
>>>>
>>>> 4. Reject the patch and just say that people must stop using "-net". Ok for me :)
>>>>
>>>> Since "-net" is kind of obsolete interface and does not seem to be extended ever,
>>>> the proposed patch does not look too ugly, does not it?
>>>> ---
>>>> hw/ppc/spapr.c | 15 +++++++++++++++
>>>> 1 file changed, 15 insertions(+)
>>>>
>>>> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
>>>> index c0613e4..45ed3da 100644
>>>> --- a/hw/ppc/spapr.c
>>>> +++ b/hw/ppc/spapr.c
>>>> @@ -1276,6 +1276,21 @@ static void ppc_spapr_init(QEMUMachineInitArgs *args)
>>>>
>>>>         if (strcmp(nd->model, "ibmveth") == 0) {
>>>>             spapr_vlan_create(spapr->vio_bus, nd);
>>>> +        } else if (is_help_option(nd->model)) {
>>>> +            static const char * const nic_models[] = {
>>>> +                "ibmveth",
>>>> +                "ne2k_pci",
>>>> +                "i82551",
>>>> +                "i82557b",
>>>> +                "i82559er",
>>>> +                "rtl8139",
>>>> +                "e1000",
>>>> +                "pcnet",
>>>> +                "virtio",
>>>> +                NULL
>>>> +            };
>>>
>>> I don't like the idea of duplicating that list.
>>
>> Neither do I :) But the list itself already looks quite ugly.
>>
>>> Basically the list of supported -net models is incorrect today even on
>>> x86 where you can say -net nic,model=ne2k_isa. It really is only a list
>>> of PCI devices.
>>
>>
>>> I can think of a number of convoluted ways to fix this up, but I think
>>> that ignoring fully accuracy of the output of -net model=? is the most
>>> straight forward thing to do.
>>
>> Does any of your "convoluted" ways include adding a new category
>> (DEVICE_CATEGORY_NETWORK_LEGACY?) into enum DeviceCategory, adding devices
>> from the list above and fixing qemu_show_nic_models() to show what is in
>> the category?
> 
> Why do you even need a new category?  Just take everything in the
> network category and put it in the help.

Because at the moment spapr creates PCI NICs (i.e. what pci_nic_init_nofail
can create) and ibmveth with "-net" interface and does not support
bluetooth-network and n2k-isa.
Alexander Graf Nov. 2, 2013, 10:52 p.m. UTC | #9
Am 02.11.2013 um 12:51 schrieb Markus Armbruster <armbru@redhat.com>:

> Alexander Graf <agraf@suse.de> writes:
> 
>> Am 01.11.2013 um 03:52 schrieb Alexey Kardashevskiy <aik@ozlabs.ru>:
> [...]
>>> Or "-net" interface is "deprecated" and we do not want even touch it?
>> 
>> I don't think we should deprecate it. It's easier to use than anything
>> else. Ahci adoption heavily suffered from not being enabled in -drive
>> - I don't want that again here.
> 
> How exactly is
> 
>    -net nic,netdev=NET-ID,macaddr=MACADDR,model=MODEL,...
> 
> easier to use than
> 
>    -device DEVNAME,netdev=NET-ID,macaddr=MACADDR,...
> 
> ?
> 
> Especially now that -device help shows valid DEVNAMEs under the heading
> "Network devices".

The same way -device ahci -drive ...,if=none -device ide-drive,... Is harder than -drive if=ahci. Users didn't even realize ahci support was in the code base...


Alex
Paolo Bonzini Nov. 4, 2013, 10:22 a.m. UTC | #10
Il 02/11/2013 13:07, Markus Armbruster ha scritto:
>> Why do you even need a new category?  Just take everything in the
>> network category and put it in the help.
> 
> You *can't* currently construct -net nic,model=help output from
> registered qdev NICs, because:
> 
> * -net nic generally doesn't accept all qdev NICs, and
> 
> * the model names accepted by -net nic need *not* match the qdev device
>   model names (example: virtio != virtio-net), and

Both could be solved by accepting legacy aliases, plus all non-no_user NICs.

> * -net nic may accept model names that map to a non-qdevified NIC (not
>    sure such NICs still exist).

I think any of those (and also SysBus NICs) would only accept the
default model.

Paolo
diff mbox

Patch

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index c0613e4..45ed3da 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -1276,6 +1276,21 @@  static void ppc_spapr_init(QEMUMachineInitArgs *args)
 
         if (strcmp(nd->model, "ibmveth") == 0) {
             spapr_vlan_create(spapr->vio_bus, nd);
+        } else if (is_help_option(nd->model)) {
+            static const char * const nic_models[] = {
+                "ibmveth",
+                "ne2k_pci",
+                "i82551",
+                "i82557b",
+                "i82559er",
+                "rtl8139",
+                "e1000",
+                "pcnet",
+                "virtio",
+                NULL
+            };
+            qemu_show_nic_models(nd->model, nic_models);
+            exit(0);
         } else {
             pci_nic_init_nofail(&nd_table[i], phb->bus, nd->model, NULL);
         }