Re: [libvirt] [PATCH RFC] Add support for QEMU vhost-user feature
On 06/05/2014 05:35 PM, Daniel P. Berrange wrote:
On Thu, Jun 05, 2014 at 05:09:02PM +0300, Laine Stump wrote:
> Daniel pointed out that qemu actually accepts any generic character
> device to connect to, and it makes sense to support this by using
> virDomainChrSourceDefPtr. However, the parse function for
> virDomainChrSourceDef requires that the type of character device be
> already parsed and set in the object, implying that it needs to be in
> the config somewhere outside of the <source> element itself.
>
> Related to that, I'm wondering if maybe instead of setting the type of
> <interface> to the very specific "vhostuser", maybe it could be set
to
> the type of character device ("socket", "tcp", etc), and the fact
that
> vhostuser is used could be specified in exactly the same way that
> vhost-net is turned on for tap devices - with <driver name="vhost"/>.
So
> for example, you would have this:
>
>
> <interface type='unix'> <!-- for a unix domain socket -->
> <model type='virtio'/>
> <driver name='vhost'/>
> <source path='xyzzy' mode='server'/>
> ...
> </interface>
The problem with this is that some of the chardev 'type' options
clash with some of the interface 'type' options, so I think we
must keep them separated.
I thought about that when writing my earlier mail, and without actually
looking at the list of accepted types for both I assumed it was the
case, but figured that the disambiguation of functionality could be
handled by the presence/absence of <driver name='vhost'/>. After
checking, I see that currently none of the the chardev types actually
clash with interface types:
virDomainChr: "null", "vc", "pty", "dev",
"file", "pipe", "stdio",
"udp", "tcp", "unix", "spicevmc",
"spiceport", "nmdm"
virDomainNet: "user", "ethernet", "server",
"client", "mcast",
"network", "bridge", "internal", "direct",
"hostdev"
But this is really just because the two implementations are inconsistent
in details with each other, if they were consistent, there *would* be
clashes (e.g. what is type='udp' in chardev is nearly identical in
function to an interface with type='client' (assuming subordinate
options are set correctly)). This means that if we tried to just add the
chardev types to the interface types, likely people would confuse one
with the other and make a lot of non-working configs.
So I think I do agree with you that it would be cleaner to have a single
toplevel type, both to avoid the need to translate the same value
between the two different enums, and to prevent confusion between nearly
identical types, as well potential *real* clashes when we add something
to one or the other.
I do agree that 'vhostuser' is a somewhat
QEMU specific name - at least with 'tap' this was a common terminology
across multiple OS and projects. This is really a variant on 'tap'
which is avoiding use of kernel devices. To perhaps we should call
this new mode 'usertap' ? Or another idea would be to call it
'tapstream'
eg
For some reason I'm more partial to "tapstream". Maybe because it
doesn't force any future uses to only be in userspace. But at some level
a name is just a name, so...
<interface type='usertap'>
<model type='virtio'/>
<driver name='vhost'/>
<source type='unix'>
...chardev source schema...
</source>
If we were to use the existing virDomainChrSourceDefFormat, the <source>
element would end up looking like this:
<source type='unix'>
<source path='xyzzy' mode='listen'/>
</source>
which is a bit redundant. What if instead, we enhanced the <source>
element to allow specifying the type as an attribute of that element
itself (at least for this case)? (We did something similar for <address
- it can include "type='pci'" in some contexts - see
virDevicePCIAddressFormat; it has an arg bool includeTypeInAddr). If we
did this (by adding a small bit of code to
virDomainChrSourceDef(Format|ParseXML), the source address could be the
simpler and easier on the eye:
<source type='unix' path='xyzzy' mode='listen'/>