On 2/17/25 10:48 AM, Laine Stump wrote:
On 2/17/25 9:13 AM, Andrea Bolognani wrote:
On Sat, Feb 15, 2025 at 12:20:17AM -0500, Laine Stump wrote:
+vhost-user connection with passt backend
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+:since:`Since 11.1.0 (QEMU and KVM only)` passt can be used as the
+other end of the vhost-user connection. This is a compelling
+alternative, because passt provides all of its network connectivity
+without requiring any elevated privileges or capabilities, and
+vhost-user uses shared memory to make this unprivileged connection
+very high performance as well. You can set a type='vhostuser'
+interface to use passt as the backend by adding ``<backend
+type='passt'/>``. When passt is the backend, only a single driver
+queue is supported,
This should be added to the validation step.
Good point. I figured it wouldn't work, so I wasn't surprised that it
failed when I tried it, but I didn't think to add validation to prevent
it. I'll make a patch for that.
Otherwise:
error: internal error: QEMU unexpectedly closed the monitor (vm='test'):
qemu-system-x86_64: -netdev
{"type":"vhost-user","chardev":"charnet0","queues":5,"id":"hostnet0"}:
Failed to read msg header. Read -1 instead of 12. Original request 1.
qemu-system-x86_64: -netdev
{"type":"vhost-user","chardev":"charnet0","queues":5,"id":"hostnet0"}:
vhost_backend_init failed: Protocol error
qemu-system-x86_64: -netdev
{"type":"vhost-user","chardev":"charnet0","queues":5,"id":"hostnet0"}:
failed to init vhost_net for queue 0
qemu-system-x86_64: -netdev
{"type":"vhost-user","chardev":"charnet0","queues":5,"id":"hostnet0"}:
Failed to connect to '/run/libvirt/qemu/passt/1-test-net0.socket':
Connection refused
qemu-system-x86_64: -netdev
{"type":"vhost-user","chardev":"charnet0","queues":5,"id":"hostnet0"}:
Device 'vhost-user' could not be initialized
and the ``<source>`` path/type/mode are all
+implied to be "matching the passt process" so **must not** be
+specified.
These seem to simply be ignored if specified, which I guess is better
than accepting values that we know aren't going to work. Explicitly
rejecting them might be better.
Yeah, you're again correct.
Now that I've had more time to drink coffee and retrace the past events
(pun not intended), I remember the reason I didn't validate that the
type & mode settings are empty:
1) The "mode" setting is just directly saved into a bool ("listen") in
the struct (virDomainChrDef) by the parser (ie. only 2 possible values
(t/f), not 3 (t/f/unspecified)), so by the time you get to any
validation function you no longer have any way of knowing if that bool
is false because mode was unspecified, or if it's false because the
input had "mode='client'". I could have used the hack of adding a
"mode_specified" bool to virDomainChrDef, but that struct has several
different users in different contexts, and a "mode_specified" would be
nonsensical and confusing for most of them (especially since the
variable in the struct is "listen", not "mode"), and the last thing I
want to do is add *even more* to the confusion.
2) The 'type' setting is an enum in the virDomainChrDef, and that enum
doesn't have an "unspecified" or "default" value as 0; it instead has
VIR_DOMAIN_CHR_TYPE_NULL. I haven't tried to see what happens if I enter
"type='null'" for some other type of chrdev, but that value is listed
for VIR_ENUM_IMPL(virDomainChr), and every place that
virDomainChrTypeFromString() is called, only a return of < 0 is
considered an error (rather than <= 0) (both the name choice and all of
the < 0 return value checks implying that there could be/is a place
where setting "type='null'" has some valid functionality). Anyway the
end of this is that it seemed "risky" to begin interpreting
VIR_DOMAIN_CHR_TYPE_NULL to mean "type not specified", and anyway that
would require that we add special handling at each place type is
examined (there are many). Alternatively I could add a *new* 0 value
VIR_DOMAIN_CHR_TYPE_UNSPECIFIED (or ..._NONE or something), but that
would still require changing all of the checks on
virDomainChrTypeFromString() to <= 0, and then changing every
switch(type) to equate ..._NONE with ..._UNIX (but then in the future,
one of the other uses for that enum could want _NONE to be equivalent to
something else rather than _UNIX). So adding a new enum value was also
increasing the fingers of change for this functionality way beyond just
the vhostuser interface code, and I preferred to not have to
regression-test every single usage of virDomainChrDef just for this one
new feature.
3) Definitely I can/should check that they haven't tried to specify the
path when backend type='passt' - that one has none of the complications
of type/mode
Incidentally, dumpxml doesn't report the actual values, which I kinda
expected to happen.
Since it's an internal implementation detail, and there's really nothing
you can do with those values if you have them, I think they should just
completely not exist in the XML output. (for type='user' passt the
socket path is never seen outside of the passt commandline (iirc libvirt
passes it to QEMU as an open fd))
When I had gotten close to the end of the implementation, I realized
that this all could probably be much cleaner and (in some ways) more
straightforward if the relevant XML attributes were stored in a simpler
struct in the virDomainNetDef (rather than a full
virDomainChrSourceDef), and we then constructed a virDomainChrSourceDef
in the NetDef's privateData object at runtime and used *that* to open
the socket.
Just wanted to re-mention the previous paragraph, because I think we
really should do that (just not today/yesterday).
But without looking at the other uses of the chardev code at
all, it seems like that could require quite a lot of changes to other
code of functionalities not related to vhostuser network interfaces,
leading to a higher likelyhood of regressions in places that wouldn't be
caught until full regression tests done at a later time. I still think
it should be done, but at a later time.
Not sure how other vhost-user device behave in
this sense.
To be truthful, the code surrounding the socket path (and ifname) in the
case of normal vhostuser was convoluted enough that I didn't try too
hard to fully understand it. I think I remember that you're required to
set <dev target='blah'/> even though that name has no functional use (in
the case that you're hooking up to an OVS switch)(although possibly I
was just witnessing odd behavior of an interim state of the code), and
may even be replaced by a value returned from a call to OVS anyway (or
something like that - I think it is the name of the port on the OVS
switch), but in the case where you have two QEMU's connected directly to
each other it is completely meaningless, yet you still have to have it).
Again a more general cleanup is in order.
