Re: [Qemu-devel] [PATCH v4 06/16] net: Remove vlan qdev property

[Stefan Hajnoczi]

On Mon, Jun 11, 2012 at 3:24 PM, Zhi Yong Wu <address@hidden> wrote:
> On Mon, Jun 11, 2012 at 4:57 PM, Stefan Hajnoczi
> <address@hidden> wrote:
>> On Mon, Jun 11, 2012 at 08:28:57AM +0200, Paolo Bonzini wrote:
>>> Il 09/06/2012 05:04, Zhi Yong Wu ha scritto:
>>> >>>> This commit looks suspicious because it removes a user-visible qdev
>>> >>>> property but we're trying to preserve backward compatibility.  This
>>> >>>> command-line will break:
>>> >>>>
>>> >>>> x86_64-softmmu/qemu-system-x86_64 -net user,vlan=1 -device 
>>> >>>> virtio-net-pci,vlan=1
>>> >>>>
>>> >>>> Instead of dropping the qdev_prop_vlan completely the
>>> >>>> hw/qdev-properties.c code needs to call net/hub.h external functions
>>> >>>> to implement equivalent functionality:
>>> >>>>
>>> >>>> 1. Setting the vlan=<id> property looks up the hub port and assigns
>>> >>>> the NICConf->peer field.
>>> >>>> 2. Getting the vlan property looks up the hub id (i.e. vlan id) given
>>> >>>> the peer.  If the peer is not a hub port the result is -1.
>>> >>>>
>>> >>>> When I wrote this patch I missed the big picture and forgot about
>>> >>>> backwards compatibility :(.
>>> >>>>
>>> > To be honest, i am concerned if anyone uses this syntax. Since the
>>> > feature will finally be discarded, i suggest that we don't support
>>> > this now. If someone complains this later, we can fix it. If nobody
>>> > complains, that is what we hope.
>>>
>>> I think you're missing the big picture of this series, which is exactly
>>> _not_ to discard the VLAN feature, but just to rewrite it in a better way.
>>>
>>> That said, I agree that this is a somewhat fringe usage; most people
>>> will use -net nic,model=virtio,vlan=1 rather than "-device".  We may get
>>> by with dropping it.  I have no strong opinion either way.
>>
>> Either we keep backwards compatibility or we don't.  Taking a middle
>> path where we preserve only some of the "VLAN" syntax is confusing and
>> inconsistent.
> in terms of technology, i fully agree with you, but in terms of
> usefulness and our business, it will waste our effort, time and is
> meaningless if nobody or no customers use this syntax. As i said, if
> someone complain this later, we can fix it.

When users upgrade QEMU versions and find their setup is now broken
QEMU's reputation will be damaged.  You can't build critical systems
on top of software which keeps changing and breaking.  Fixing it after
a user hits the problem is not okay, users won't trust us if we do
that.

We need to be disciplined when it comes to backwards compatibility.
Either we support the "VLAN" feature or we drop it.  We already had
this discussion in another thread, here's what Anthony had to say:

"Dropping features is only something that should be approached lightly and
certainly not something that should be done just because you don't like a
particular bit of code."

http://permalink.gmane.org/gmane.comp.emulators.qemu/153600

Stefan