Re: [Qemu-devel] [RFC PATCH] spapr: add ibmveth to the supported network adapters list

[Alexey Kardashevskiy <aik@ozlabs.ru>]

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