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'/> -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list