qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [RFC PATCH] spapr: add ibmveth to the supported network adapters list
@ 2013-10-11  3:09 Alexey Kardashevskiy
  2013-10-18  5:39 ` Alexey Kardashevskiy
  2013-10-27 18:03 ` Alexander Graf
  0 siblings, 2 replies; 11+ messages in thread
From: Alexey Kardashevskiy @ 2013-10-11  3:09 UTC (permalink / raw)
  To: qemu-devel
  Cc: Alexey Kardashevskiy, qemu-ppc, Alexander Graf, Anthony Liguori

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
+            };
+            qemu_show_nic_models(nd->model, nic_models);
+            exit(0);
         } else {
             pci_nic_init_nofail(&nd_table[i], phb->bus, nd->model, NULL);
         }
-- 
1.8.4.rc4

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [Qemu-devel] [RFC PATCH] spapr: add ibmveth to the supported network adapters list
  2013-10-11  3:09 [Qemu-devel] [RFC PATCH] spapr: add ibmveth to the supported network adapters list Alexey Kardashevskiy
@ 2013-10-18  5:39 ` Alexey Kardashevskiy
  2013-10-27 18:03 ` Alexander Graf
  1 sibling, 0 replies; 11+ messages in thread
From: Alexey Kardashevskiy @ 2013-10-18  5:39 UTC (permalink / raw)
  To: Alexey Kardashevskiy, qemu-devel
  Cc: qemu-ppc, Alexander Graf, Anthony Liguori

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


-- 
Alexey

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Qemu-devel] [RFC PATCH] spapr: add ibmveth to the supported network adapters list
  2013-10-11  3:09 [Qemu-devel] [RFC PATCH] spapr: add ibmveth to the supported network adapters list Alexey Kardashevskiy
  2013-10-18  5:39 ` Alexey Kardashevskiy
@ 2013-10-27 18:03 ` Alexander Graf
  2013-11-01 10:52   ` Alexey Kardashevskiy
  1 sibling, 1 reply; 11+ messages in thread
From: Alexander Graf @ 2013-10-27 18:03 UTC (permalink / raw)
  To: Alexey Kardashevskiy; +Cc: open list:PReP, QEMU Developers, Anthony Liguori


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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Qemu-devel] [RFC PATCH] spapr: add ibmveth to the supported network adapters list
  2013-10-27 18:03 ` Alexander Graf
@ 2013-11-01 10:52   ` Alexey Kardashevskiy
  2013-11-01 12:47     ` Alexander Graf
  2013-11-02 10:56     ` Paolo Bonzini
  0 siblings, 2 replies; 11+ messages in thread
From: Alexey Kardashevskiy @ 2013-11-01 10:52 UTC (permalink / raw)
  To: Alexander Graf; +Cc: PReP, QEMU Developers, Anthony Liguori

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?



-- 
Alexey

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Qemu-devel] [RFC PATCH] spapr: add ibmveth to the supported network adapters list
  2013-11-01 10:52   ` Alexey Kardashevskiy
@ 2013-11-01 12:47     ` Alexander Graf
  2013-11-02 11:51       ` Markus Armbruster
  2013-11-02 10:56     ` Paolo Bonzini
  1 sibling, 1 reply; 11+ messages in thread
From: Alexander Graf @ 2013-11-01 12:47 UTC (permalink / raw)
  To: Alexey Kardashevskiy
  Cc: open@suse.de, list@suse.de:PReP, QEMU Developers, Anthony Liguori



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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Qemu-devel] [RFC PATCH] spapr: add ibmveth to the supported network adapters list
  2013-11-01 10:52   ` Alexey Kardashevskiy
  2013-11-01 12:47     ` Alexander Graf
@ 2013-11-02 10:56     ` Paolo Bonzini
  2013-11-02 12:07       ` Markus Armbruster
  2013-11-02 12:26       ` Alexey Kardashevskiy
  1 sibling, 2 replies; 11+ messages in thread
From: Paolo Bonzini @ 2013-11-02 10:56 UTC (permalink / raw)
  To: Alexey Kardashevskiy
  Cc: PReP, Alexander Graf, Anthony Liguori, QEMU Developers

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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Qemu-devel] [RFC PATCH] spapr: add ibmveth to the supported network adapters list
  2013-11-01 12:47     ` Alexander Graf
@ 2013-11-02 11:51       ` Markus Armbruster
  2013-11-02 22:52         ` Alexander Graf
  0 siblings, 1 reply; 11+ messages in thread
From: Markus Armbruster @ 2013-11-02 11:51 UTC (permalink / raw)
  To: Alexander Graf
  Cc: Alexey Kardashevskiy, open@suse.de, list@suse.de:PReP,
	QEMU Developers, Anthony Liguori

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".

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Qemu-devel] [RFC PATCH] spapr: add ibmveth to the supported network adapters list
  2013-11-02 10:56     ` Paolo Bonzini
@ 2013-11-02 12:07       ` Markus Armbruster
  2013-11-04 10:22         ` Paolo Bonzini
  2013-11-02 12:26       ` Alexey Kardashevskiy
  1 sibling, 1 reply; 11+ messages in thread
From: Markus Armbruster @ 2013-11-02 12:07 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: Alexey Kardashevskiy, PReP, Alexander Graf, Anthony Liguori,
	QEMU Developers

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.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Qemu-devel] [RFC PATCH] spapr: add ibmveth to the supported network adapters list
  2013-11-02 10:56     ` Paolo Bonzini
  2013-11-02 12:07       ` Markus Armbruster
@ 2013-11-02 12:26       ` Alexey Kardashevskiy
  1 sibling, 0 replies; 11+ messages in thread
From: Alexey Kardashevskiy @ 2013-11-02 12:26 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: PReP, Alexander Graf, Anthony Liguori, QEMU Developers

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.



-- 
Alexey

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Qemu-devel] [RFC PATCH] spapr: add ibmveth to the supported network adapters list
  2013-11-02 11:51       ` Markus Armbruster
@ 2013-11-02 22:52         ` Alexander Graf
  0 siblings, 0 replies; 11+ messages in thread
From: Alexander Graf @ 2013-11-02 22:52 UTC (permalink / raw)
  To: Markus Armbruster
  Cc: Alexey Kardashevskiy, open@suse.de, list@suse.de:PReP,
	QEMU Developers, Anthony Liguori



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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Qemu-devel] [RFC PATCH] spapr: add ibmveth to the supported network adapters list
  2013-11-02 12:07       ` Markus Armbruster
@ 2013-11-04 10:22         ` Paolo Bonzini
  0 siblings, 0 replies; 11+ messages in thread
From: Paolo Bonzini @ 2013-11-04 10:22 UTC (permalink / raw)
  To: Markus Armbruster
  Cc: Alexey Kardashevskiy, PReP, Alexander Graf, Anthony Liguori,
	QEMU Developers

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

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2013-11-04 10:22 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-10-11  3:09 [Qemu-devel] [RFC PATCH] spapr: add ibmveth to the supported network adapters list Alexey Kardashevskiy
2013-10-18  5:39 ` Alexey Kardashevskiy
2013-10-27 18:03 ` Alexander Graf
2013-11-01 10:52   ` Alexey Kardashevskiy
2013-11-01 12:47     ` Alexander Graf
2013-11-02 11:51       ` Markus Armbruster
2013-11-02 22:52         ` Alexander Graf
2013-11-02 10:56     ` Paolo Bonzini
2013-11-02 12:07       ` Markus Armbruster
2013-11-04 10:22         ` Paolo Bonzini
2013-11-02 12:26       ` Alexey Kardashevskiy

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).