qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Paolo Bonzini <pbonzini@redhat.com>
To: Alexey Kardashevskiy <aik@ozlabs.ru>
Cc: PReP <qemu-ppc@nongnu.org>, Alexander Graf <agraf@suse.de>,
	Anthony Liguori <anthony@codemonkey.ws>,
	QEMU Developers <qemu-devel@nongnu.org>
Subject: Re: [Qemu-devel] [RFC PATCH] spapr: add ibmveth to the supported network adapters list
Date: Sat, 02 Nov 2013 11:56:35 +0100	[thread overview]
Message-ID: <5274DA63.8090903@redhat.com> (raw)
In-Reply-To: <527387F4.4090407@ozlabs.ru>

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

  parent reply	other threads:[~2013-11-02 10:57 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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 [this message]
2013-11-02 12:07       ` Markus Armbruster
2013-11-04 10:22         ` Paolo Bonzini
2013-11-02 12:26       ` Alexey Kardashevskiy

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5274DA63.8090903@redhat.com \
    --to=pbonzini@redhat.com \
    --cc=agraf@suse.de \
    --cc=aik@ozlabs.ru \
    --cc=anthony@codemonkey.ws \
    --cc=qemu-devel@nongnu.org \
    --cc=qemu-ppc@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).