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.

I think even caring about -net tap,fd=1,fd=2 is a bit silly.  If this
resulted in fd=2 by mistake, I don't think it was ever intentionally
legal.
As Jason points out we have list support and for better or worse
it is currently using repeated options, e.g. with dnssearch, hostfwd and
guestfwd.
Isn't it better to be consistent?

> 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

This introduces : as a special character. However fds can
be fd names passed in with getfd, where : is a legal character.

-- 
MST