Re: [Qemu-devel] [PATCH 00/12] Multiqueue virtio-net

[Michael S. Tsirkin]

On Wed, Jan 16, 2013 at 10:14:33AM -0600, Anthony Liguori wrote:
> "Michael S. Tsirkin" <address@hidden> writes:
> 
> > On Wed, Jan 16, 2013 at 09:09:49AM -0600, Anthony Liguori wrote:
> >> Jason Wang <address@hidden> writes:
> >> 
> >> > On 01/15/2013 03:44 AM, Anthony Liguori wrote:
> >> >> Jason Wang <address@hidden> writes:
> >> >>
> >> >>> Hello all:
> >> >>>
> >> >>> This seires is an update of last version of multiqueue virtio-net 
> >> >>> support.
> >> >>>
> >> >>> Recently, linux tap gets multiqueue support. This series implements 
> >> >>> basic
> >> >>> support for multiqueue tap, nic and vhost. Then use it as an 
> >> >>> infrastructure to
> >> >>> enable the multiqueue support for virtio-net.
> >> >>>
> >> >>> Both vhost and userspace multiqueue were implemented for virtio-net, 
> >> >>> but
> >> >>> userspace could be get much benefits since dataplane like parallized 
> >> >>> mechanism
> >> >>> were not implemented.
> >> >>>
> >> >>> User could start a multiqueue virtio-net card through adding a "queues"
> >> >>> parameter to tap.
> >> >>>
> >> >>> ./qemu -netdev tap,id=hn0,queues=2,vhost=on -device 
> >> >>> virtio-net-pci,netdev=hn0
> >> >>>
> >> >>> Management tools such as libvirt can pass multiple pre-created fds 
> >> >>> through
> >> >>>
> >> >>> ./qemu -netdev tap,id=hn0,queues=2,fd=X,fd=Y -device
> >> >>> virtio-net-pci,netdev=hn0
> >> >> I'm confused/frightened that this syntax works.  You shouldn't be
> >> >> allowed to have two values for the same property.  Better to have a
> >> >> syntax like fd[0]=X,fd[1]=Y or something along those lines.
> >> >
> >> > Yes, but this what current a StringList type works for command line.
> >> > Some other parameters such as dnssearch, hostfwd and guestfwd have
> >> > already worked in this way. Looks like your suggestions need some
> >> > extension on QemuOps visitor, maybe we can do this on top.
> >> 
> >> It's a silly syntax and breaks compatibility.  This is valid syntax:
> >> 
> >> -net tap,fd=3,fd=4
> >> 
> >> In this case, it means 'fd=4' because the last fd overwrites the first
> >> one.
> >> 
> >> Now you've changed it to mean something else.  Having one thing mean
> >> something in one context, but something else in another context is
> >> terrible interface design.
> >> 
> >> Regards,
> >> 
> >> Anthony Liguori
> >
> > Aha so just renaming the field 'fds' would address this issue?
> 
> No, you still have the problem of different meanings.
> 
> -netdev tap,fd=X,fd=Y
> 
> -netdev tap,fds=X,fds=Y
> 
> Would have wildly different behavior.

fd=X,fd=Y is more a bug than a feature. It could have failed
just as well.

> Just do:
> 
> -netdev tap,fds=X:Y
> 
> And then we're staying consistent wrt the interpretation of multiple
> properties of the same name.
> 
> Regards,
> 
> Anthony Liguori


Issue is ':' would only work for a list of numbers.
As Jason points out StringList is already used - do
we really want to invent yet another syntax for a list
that will work only for this case?

-- 
MST