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

[Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>]

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.

Like Paolo said, the point here is not to drop "VLAN" support.  We're
simply moving this feature into a regular net client (the hub) so that
the special case "VLAN" code in net.c can be removed.

Stefan