Skip to content

Consider changing the vhost-user-backend API #279

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
mtjhrc opened this issue Nov 20, 2024 · 15 comments
Open

Consider changing the vhost-user-backend API #279

mtjhrc opened this issue Nov 20, 2024 · 15 comments

Comments

@mtjhrc
Copy link
Contributor

mtjhrc commented Nov 20, 2024

Background

Hi, I have been working on a vhost-user-gpu with @dorindabassey and we ran into some complications.
I think there are some things about how VhostUserBackend is designed and generally the API model that are not great for the GPU device, and in general for other devices too - personally I am also familiar with virtio-net and virtio-console devices because I worked on them in libkrun. The goal would be to make writing new devices and mainting existing ones easier by making the API more flexible.

Current API can be clunky to use in some cases:

Currently the vhost-user-backend handles spawning worker threads for the device, and calls the handle_event callback on that thread. I don't think this model is good, because it leads to some difficulties when programming.
Here are the main issues:

1. Where to store the data of a worker thread

Consider you have a worker thread that wants to have some variables only available to this single thread, this is currently difficult to do. Basically you have a couple of options:

  • Store the data inside the Backend struct and wrap in a a Mutex/RwLock - for example virtio-sound does this. I feel like this adds unnecessary complexity, and unless I am missing something, we could get rid of some of the extra locking (by using the GuestMemoryAtomic we already have).
  • If the thread specific data is !Send - such Rutabaga/virglrenderer context in the vhost-user-gpu, your only option is to lazy initialize the struct (putting something !Send inside a Mutex doesn't help), and use thread_local! or use unsafe to work around this.

2. Adding file descriptors for polling after starting the backend

  • In vhost-user-gpu we needed to add a file descriptor to be signaled by handle_event inside handle_event this is very akward to do. If you want to do this you firt need to pass the epoll handler created by the daemon to the device backend. In handle_event you now have access to the epoll, but but you have to be careful, because you cannot use the VhostUserBackendMut trait and the convenient lock around the whole struct, because if handle_event is called while the lock is held, you would deadlock yourself when num_queues is called here:
    if data <= self.backend.num_queues() as u64 {

3. VhostUserBackend vs VhostUserBackendMut

In general the split VhostUserBackend and VhostUserBackendMut traits doesn't seem to me like a good design choice. I feel like when you specify that you want 1 worker thread and you use the VhostUserBackendMut trait it mostly works fine (except the !Send problem), but if you want 2 worker threads or something more complex, you have to change over to VhostUserBackend and use manual locking. Another problem is that if you don't switch to using the VhostUserBackend trait you would have 2 worker threads, but the handle_event could never run at the same time because it is behind a lock leading to a non-obvious performance bug.

More flexible event handling API

My main idea is to just let the device spawn it's own worker threads and use epoll directly in them instead of having the handle_event callback. This would basically change the event system from "push" to "poll". I think this would be a lot more flexible, because it would allow:

  1. any custom thread <-> vring mapping
  2. adding and removing file descriptors to watch easily
  3. having detailed control of which thread has access to what leading to more understandable and efficient code

One of the problems with the current implementation, is that since handle_event is a method and a callback the type systems basically forces self to be Send + Sync, which in practise forces you to lock everything, which could be avoided.

struct FooDevice { ... };

fn worker(mem: GuestMemoryAtomic<GuestMemoryMmap>, vring: VringRwLock, exit_event: EventFd) {
    // Epoll abstraction details are not that important for this proposal, this could change
    Poller poller;
    let vring_ready_event = vring.ready_event();
    poller.add(vring_ready_event);
    poller.add(exit_event);
    
    loop {
        let event = poller.poll();

        if event == exit_event {
            return
        }

        if event == vring_ready_event {
            // handle the vring event here
            // you can even add/remove event file descriptors to the epoll
        }
    }     
}

impl VhostUserBackend for FooDevice {
    
    fn num_queues(&self) -> usize { ... }
    fn max_queue_size(&self) -> usize { .. }
    fn features(&self) -> u64 { ... }
    fn protocol_features(&self) -> VhostUserProtocolFeatures { ... }
    // Other methods like before, except the `queues_per_thread(&self)` which would be removed


    // The entry point where we start the device, this would require the device starting at least 1 worker thread, but we do that today anyway so it shouldn't be an issue.
    // (The exact mem, and vring types should be more generic, this is just an example) 
    fn start(&mut self, vrings: Box<[VringRwLock]>, quit_event: EventFd) -> Result<()> {
        // Notice the thread doesn't have to have a reference to `self` 
// if you need to shared state between the worker threads, you can use something like Arc<Mutex<YourSharedDataStruct>>, or even a channel.
        thread::spawn(move || worker(self.mem.clone(), vrings[0], quit_event));
        Ok(())
    }
}  

Consider having devices also be usable as in-process devices by VMMs

Since I am proposing changing the API it could be beneficial to make a bit more breaking changes and also allow consuming the devices as in-process devices. Actually we aren't that far from this, but this would of course require some more thoughs about the abstractions.
This could be useful for libkrun and other VMMs (anyone else interested?)

Questions

I can create a more detailed proposals of APIs, but first I wanted to see:

  1. Is there interest in doing smaller breaking changes to the API like I proposed?
  2. Is there interest in doing bigger API changes to also allow consuming the devices as in-process devices?
@stefano-garzarella
Copy link
Member

@mtjhrc sorry for my late reply.

I think this work will be great! I was looking to support macOS/BSD and our dependency on Linux specific stuff like epoll/eventfd is the biggest thing to be solved. Your work may simplify that.

I can create a more detailed proposals of APIs, but first I wanted to see:

1. Is there interest in doing smaller breaking changes to the API like I proposed?\

I think it's fine, but we should help our user (vhost-device, virtiofsd, etc.) to adapt easily their code.

2. Is there interest in doing bigger API changes to also allow consuming the devices as in-process devices?

Sure, no objection on my side.

@Ablu @eryugey @germag @jiangliu @sboeuf @slp WDYT?

@slp
Copy link
Collaborator

slp commented Dec 19, 2024

2. Is there interest in doing bigger API changes to also allow consuming the devices as in-process devices?

Sure, no objection on my side.

@Ablu @eryugey @germag @jiangliu @sboeuf @slp WDYT?

I think being able to unify out-of-process and in-process device implementations alone is already worth the effort. 👍

@mtjhrc
Copy link
Contributor Author

mtjhrc commented Dec 19, 2024

think this work will be great! I was looking to support macOS/BSD and our dependency on Linux specific stuff like epoll/eventfd is the biggest thing to be solved. Your work may simplify that.

I don't have a macOS machine, but this is definitely something I am keeping in mind. libkrun supports macOS, so if it were to use these devices, macOS support is necessary.

@stefano-garzarella
Copy link
Member

think this work will be great! I was looking to support macOS/BSD and our dependency on Linux specific stuff like epoll/eventfd is the biggest thing to be solved. Your work may simplify that.

I don't have a macOS machine, but this is definitely something I am keeping in mind. libkrun supports macOS, so if it were to use these devices, macOS support is necessary.

FYI I'll talk about it at FOSDEM next year: https://fosdem.org/2025/schedule/event/fosdem-2025-5100-can-qemu-and-vhost-user-devices-be-used-on-macos-and-bsd-/

I can mention your work ;-)

@alyssais
Copy link
Contributor

FWIW: libepoll-shim can be used on BSD and macOS.

@stefano-garzarella
Copy link
Member

FWIW: libepoll-shim can be used on BSD and macOS.

@alyssais Thanks, I was thinking also on https://github.com/smol-rs/polling

@mtjhrc
Copy link
Contributor Author

mtjhrc commented Mar 18, 2025

I have a working proof of concept of the new API with a vhost-user console (ported from libkrun) - this uses multiple worker threads, and the gpu device from vhost-device.
https://github.com/mtjhrc/virtio-device-poc
I'm working on implementing the MMIO virtio transport for the new trait, and hooking it up in libkrun to test it.

Regarding polling, for now I am still using the epoll and EventFd from vmm-sys-util, libkrun has a a shim that implements it on macOS: https://github.com/containers/libkrun/tree/main/src/utils/src/macos.
So we could maybe upstream that? Though it's not technically feature equivalent yet, you can compare the see the different EventSet bitsets in the implementations.

Regarding smol-rs/polling - this seems to use kqueue's EVFILT_USER as an EventFd equivalent, this is fine for in-process notifications - e.g. stop a worker thread, but I don't think you can use that for communicating with QEMU process. Another not so good thing is that this doesn't expose the API for specifying a custom key for the event.

In the PoC, I am using a layer built on top vmm-sys-util Epoll called EventManager (this is different than the current EventManager in rust-vmm, which makes me think I should rename this). This is based on crosvm implementation and allows you to identify the events using a rust enum. (You can still use an int if you want though, and you can even use ints inside an enum).
This allows you to write event loops like this:

#[derive(EventToken, Debug, Copy, Clone)]
enum WorkerToken {
    ControlRxq,
    ControlTxq,
    // You can also have an event like this (specify queue index inside):
    // Queue(u32),
    Quit,
}

fn run(mut self) -> anyhow::Result<()> {
    let event_manager: EventManager<WorkerToken> = EventManager::new().unwrap();
    event_manager.add(&self.control_rxq.kick_event(), WorkerToken::ControlRxq)?;
    event_manager.add(&self.control_txq.kick_event(), WorkerToken::ControlTxq)?;
    event_manager.add(&self.stopfd, WorkerToken::Quit).unwrap();
    // You can also just add any file descriptor using add_fd instead of add, or 
    // implement the EventSource trait for your struct and use the add() method
    // like above

    loop {
        for event in events_manager.wait()? {
            match event.token() {
                WorkerToken::ControlRxq => self.process_rx(),
                WorkerToken::ControlTxq => self.process_tx(),
                WorkerToken::Quit => return Ok(()),
            }
        }
    }
}

I'm not sure how to proceed now, though. I feel like the first step, if we want to go through with this, would be to merge vm-virtio and vhost repository? I'm not sure if we want a different repository for devices (both vhost-user and in-process), or put these in the same repository too? After that I could create a draft PR?
Or could we create a new repository and gradually move things there?

@stefano-garzarella
Copy link
Member

Regarding polling, for now I am still using the epoll and EventFd from vmm-sys-util, libkrun has a a shim that implements it on macOS: https://github.com/containers/libkrun/tree/main/src/utils/src/macos.
So we could maybe upstream that? Though it's not technically feature equivalent yet, you can compare the see the different EventSet bitsets in the implementations.

FYI we are going to have a GSoC student working on that in order to support vhost related crates on macOS/BSD:
https://wiki.qemu.org/Google_Summer_of_Code_2025#vhost-user_devices_in_Rust_on_macOS_and_*BSD

So, if possible I'd wait for the student and maybe help her/him to do it.

Regarding smol-rs/polling - this seems to use kqueue's EVFILT_USER as an EventFd equivalent, this is fine for in-process notifications - e.g. stop a worker thread, but I don't think you can use that for communicating with QEMU process. Another not so good thing is that this doesn't expose the API for specifying a custom key for the event.

In the PoC, I am using a layer built on top vmm-sys-util Epoll called EventManager (this is different than the current EventManager in rust-vmm, which makes me think I should rename this). This is based on crosvm implementation and allows you to identify the events using a rust enum. (You can still use an int if you want though, and you can even use ints inside an enum).

Thanks for sharing! Is it okay for you to wait for the student to work on this or is it too late?
(Obviously your help would be more than welcome and if you want to be part of the project mentors as well).

I'm not sure how to proceed now, though. I feel like the first step, if we want to go through with this, would be to merge vm-virtio and vhost repository?

I think we've already discussed this in private, but I think it's orthogonal, right?
Perhaps we are getting confused with crates and repositories. Usually in rust-vmm a repository can be a workspace and contain multiple crates (e.g. vhost, vhost-device), but this is just a development convenience, so it shouldn't block this effort. Or am I missing something?

I'm not sure if we want a different repository for devices (both vhost-user and in-process), or put these in the same repository too?

It would be nice to have a repository with all the devices implemented as libraries so that they can be consumed both in-vmm and in vhost-user.
And maybe also have in the same repository the executables for each library that implements them as vhost-user devices.

This perhaps needs to be discussed with vhost-device maintainers as well, though.

After that I could create a draft PR? Or could we create a new repository and gradually move things there?

mmm, for the new repository, I would say no, we are already fragmented enough.

In my vision maybe we should have:

  • vm-virtio with libraries implementing all devices but without depending on vhost
  • vhost can continue to be a separate workspace or possibly included in vm-virtio (but I would say that can happen later)
  • vhost-device consuming from vhost/vhost-user-backend the thread model and from vm-virtio the devices

I might have missed something though, does that makes sense for you or were you thinking something different?

@mtjhrc
Copy link
Contributor Author

mtjhrc commented Mar 24, 2025

FYI we are going to have a GSoC student working on that in order to support vhost related crates on macOS/BSD:
https://wiki.qemu.org/Google_Summer_of_Code_2025#vhost-user_devices_in_Rust_on_macOS_and_*BSD
So, if possible I'd wait for the student and maybe help her/him to do it.

Thanks for sharing! Is it okay for you to wait for the student to work on this or is it too late?
(Obviously your help would be more than welcome and if you want to be part of the project mentors as well).

Oh, I missed that, but to clarify, I haven't really worked nor I have plans to work on *BSD/macOS support. I have only read the kqueue man pages and looked into libkrun and smoll-rs code for macOS, to try to make sure my changes won't complicate porting. The EventManager I worked on is just a utility to make event handling nicer (by being able to identify the events by enum variants in a type-safe manner). This should be a higher level utility built on a lower level epoll/kqueue/eventfd abstraction anyway.

(Obviously your help would be more than welcome and if you want to be part of the project mentors as well).
I'll think about it (I'm not sure if I'll have the time)

I think we've already discussed this in private, but I think it's orthogonal, right?
Perhaps we are getting confused with crates and repositories.

No I'm not getting confused with the crates/repositories. It's just that reviewing and discussing big PR's that depend on each other spanning 3 repositories (vhost-device, vhost and vm-virtio) doesn't seem efficient (because we want to merge at least some of the repositories anyway) - so why not try to merge the repositories first?

Let me describe how I imagine this working to give you a better idea (well the PoC already works like this):
We will have a VirtioDevice trait that is generic over the memory, queue and interrupt events, etc using traits. Devices can be implemented using this model as libraries preferably to the most generic level, but technically you could specify a fixed type for example, for the memory and implement it only for that.
Note that this trait already enforces the threading model - the device will have an activate() method were it is supposed to spawn worker thread(s). In the worker threads the device can handle events using anything capable of polling file descriptors, for convenience you could use the EventManager utility mentioned above, but you should be able to integrate this with anything for example an async executor.

Btw the activate() model instead of handle_event could also make https://wiki.qemu.org/Google_Summer_of_Code_2025#Asynchronous_request_handling_for_virtiofsd nicer (maybe even tiny bit more efficent?) - the idea is that you can make the async framework wait on the EventFd for the kick directly instead of forwarding the handle_event callback, like you may end up end up having to do currently.
crosvm and cloud-hypervisor (I think) already sort of have an event/threading model like this. (libkrun currently doesn't but I plan to make it that way).

The vhost-user-backend will now need to provide an implementation for the things the VirtioDevice is generic over - e. g. it needs to provide mechanism for receiving kicks, sending used queue notifications, providing mechanism to check if queue is enabled, etc. ... A vmm consuming the device library has to obviously have an implementation for all of these things too. The vhost-user-backend will have a VhostUserDevice trait (or we can keep the name VhostUserBackend) that is a super trait of VirtioDevice in the conceptual sense. (Technically, I think we shouldn't make it a super-trait, but provide a default implementations for the methods, for the user of the library it looks works like this: https://github.com/mtjhrc/virtio-device-poc/blob/c55f6f363561e095761a42ac81424b8659d32d0e/device/vhost-gpu/src/lib.rs#L92-L109).

The vhost crate stays totally unchanged.

It would be nice to have a repository with all the devices implemented as libraries so that they can be consumed both in-vmm and in vhost-user.

Yes, this is exactly what I want to achieve.

And maybe also have in the same repository the executables for each library that implements them as vhost-user devices.

I would prefer to put the device libraries and vhost-device executables in the same repository, because they would be quite coupled and this would simplify development and testing of new features.

vhost can continue to be a separate workspace or possibly included in vm-virtio (but I would say that can happen later)

Yeah it can happen later, but like I already mentioned, why not do it first? The vhost-user-backend would now essentially be an implementation of newly added traits defined in vm-virtio using the vhost crate.

vhost-device consuming from vhost/vhost-user-backend the thread model and from vm-virtio the devices

Kind of, the threading/event model would already be defined by the VirtioDevice trait (and other relevant helper traits) in the vm-virtio repository, the vhost-user-backend would just implement it.

Hopefully the explanation of how this would work makes some sense at least.

@stefano-garzarella
Copy link
Member

No I'm not getting confused with the crates/repositories. It's just that reviewing and discussing big PR's that depend on each other spanning 3 repositories (vhost-device, vhost and vm-virtio) doesn't seem efficient (because we want to merge at least some of the repositories anyway) - so why not try to merge the repositories first?

Okay, I was confused you mentioning (maybe in a private discussion) to replicate some trait/struct in different repo.
I'm fine to merge repositories, I'm just worried it will take time.

Also I still think vhost-device should stay separated. They are binaries and we saw we had a loooot of dependencies to keeps updated, so IMHO is better to have them in a different repo, but again we should disccuss with vhost-device maintainers.

I would prefer to put the device libraries and vhost-device executables in the same repository, because they would be quite coupled and this would simplify development and testing of new features.

For example for vhost-device-sound the biggest part is the supports for the various backends (pipewire, alsa, gstreamer one day, etc.), but these are completely separate from the virtio-sound device emulation, and they carry a lot of dependencies, so it might make sense to keep them separate.

Yeah it can happen later, but like I already mentioned, why not do it first? The vhost-user-backend would now essentially be an implementation of newly added traits defined in vm-virtio using the vhost crate.

Because it will be so time-consuming. Also, the advantage of having the separate things is that I think sometime it's better to have separate PRs, so they can be reviewed in pieces and in a more humane way. As a maintainer seeing a PR coming in, that touches 5 crates scares me to review.
Whereas, having something that touches the various dependencies in pieces is easier to review.

I understand that for a contributor it's the opposite, but in this case I'd advise you not to do a mega PR that changes everything, but more PRs that do it gradually if possible (which would be forced by separate repositories), otherwise we risk no one looking at it.

Having said that, I'm not against having one repository, in fact I think it has advantages even on the reviewer side, so if you want to try it for me that's fine, but I'm afraid it's not a quick process.
We have to be careful about preserving git history, reporting issues, open PRs, etc.

Hopefully the explanation of how this would work makes some sense at least.

Yes, thanks, now it's much clearer!

@mtjhrc
Copy link
Contributor Author

mtjhrc commented Mar 25, 2025

For example for vhost-device-sound the biggest part is the supports for the various backends (pipewire, alsa, gstreamer one day, etc.), but these are completely separate from the virtio-sound device emulation, and they carry a lot of dependencies, so it might make sense to keep them separate.

I feel like you still don't fully understand what I mean:
Yes these libraries are separate from a virtio-sound emulation. But what I am proposing is for the library to use those dependencies. The library would be a full implementation of the device - so for example you would have the library implementing virtio-sound in terms of alsa/pipewire/etc. but the library wouldn't enforce any particular "transport layer". And by "transport layer" I mean PCI or MMIO in-vmm or the vhost-user IPC protocol to another process (QEMU) which ends up using PCI to expose it to the guest etc...

Of course we could also have libraries for implementing only generic parts of the devices, but not full devices (like we already have with vsock) and the full device implementation would use the vsock library.
So you would have [vhost-vsock executable] -> [vsock device as a library] -> [the existing vsock library implementing some generic utilities].

I understand that for a contributor it's the opposite, but in this case I'd advise you not to do a mega PR that changes everything, but more PRs that do it gradually if possible (which would be forced by separate repositories), otherwise we risk no one looking at it.

Yes I get that, and I am also worried about doing a mega PR for this exact reason, but here's the problem though:
So suppose I create a new VirtioDevice trait and it gets merged into vm-virtio without much trouble. (because likely no one depends on the current VirtioDevice trait - I don't think it's published in crates.io). Then I want to change how the vhost-user-backend crate works, but we have bigger discussion on how we want to change it, but changing it means changing the VirtioDevice trait again (which still isn't used by anyone)... You see my problem?
Of course I could do some of the changes to the vhost-user-backend first - refactoring the event loop. I think for the sake of reviewing, this is a good idea, but this means we will have 2 stages of breaking changes. (The other change is splitting each device into an vhost executable and transport-agnostic device library).

So to summarize, I can split it like this:

  1. PR changing the event loop organization in vhost-user-backend (this will reduce the amount of changes in 3.)
  2. PR(s) porting some device(s) to the new API
  3. PR that introduces a new VirtioDevice trait in a new virtio-device crate and VhostUserDevice in vhost-user-backend which is based on it. I guess this could also be 2 PR's if we don't merge the repositories (but will require more coordination)
  4. PR(s) porting more devices to both breaking changes

@stefano-garzarella
Copy link
Member

For example for vhost-device-sound the biggest part is the supports for the various backends (pipewire, alsa, gstreamer one day, etc.), but these are completely separate from the virtio-sound device emulation, and they carry a lot of dependencies, so it might make sense to keep them separate.

I feel like you still don't fully understand what I mean: Yes these libraries are separate from a virtio-sound emulation. But what I am proposing is for the library to use those dependencies. The library would be a full implementation of the device - so for example you would have the library implementing virtio-sound in terms of alsa/pipewire/etc. but the library wouldn't enforce any particular "transport layer". And by "transport layer" I mean PCI or MMIO in-vmm or the vhost-user IPC protocol to another process (QEMU) which ends up using PCI to expose it to the guest etc...

Yes, I didn't understand, but I don't think this is the right path.
Let's use virtio-vsock as an example. Is libkrun interested in the hybrid-vsock that we have in vhost-device-vsock?
IIUC it's only interested in the virtio-vsock device implementation, because the connections end up directly in the VMM.

Of course we could also have libraries for implementing only generic parts of the devices, but not full devices (like we already have with vsock) and the full device implementation would use the vsock library. So you would have [vhost-vsock executable] -> [vsock device as a library] -> [the existing vsock library implementing some generic utilities].

I think this is the right way, because a VMM could be interested only in the device and not in the backends (here with backends I mean for example UDS/hybrid-vsock for virtio-vsock, or pipewire for virtio-sound, or tcp for virtio-console, etc).

I understand that for a contributor it's the opposite, but in this case I'd advise you not to do a mega PR that changes everything, but more PRs that do it gradually if possible (which would be forced by separate repositories), otherwise we risk no one looking at it.

Yes I get that, and I am also worried about doing a mega PR for this exact reason, but here's the problem though: So suppose I create a new VirtioDevice trait and it gets merged into vm-virtio without much trouble. (because likely no one depends on the current VirtioDevice trait - I don't think it's published in crates.io). Then I want to change how the vhost-user-backend crate works, but we have bigger discussion on how we want to change it, but changing it means changing the VirtioDevice trait again (which still isn't used by anyone)... You see my problem? Of course I could do some of the changes to the vhost-user-backend first - refactoring the event loop. I think for the sake of reviewing, this is a good idea, but this means we will have 2 stages of breaking changes. (The other change is splitting each device into an vhost executable and transport-agnostic device library).

We've done things like this before, yes I understand the problem, but opening a draft PR on another repo to show how the changes will be used is not that dramatic.

That said, again, I'm not against having one repo, I'm just saying it's not going to be a quick thing.

So to summarize, I can split it like this:

1. PR changing the event loop organization in `vhost-user-backend` (this will reduce the amount of changes in 3.)

2. PR(s) porting some device(s) to the new API

3. PR that introduces a new `VirtioDevice trait` in a new `virtio-device` crate and `VhostUserDevice` in `vhost-user-backend` which is based on it. I guess this could also be 2 PR's if we don't merge the repositories (but will require more coordination)

4. PR(s) porting more devices to both breaking changes

Ack

@mtjhrc
Copy link
Contributor Author

mtjhrc commented Mar 25, 2025

Let's use virtio-vsock as an example. Is libkrun interested in the hybrid-vsock that we have in vhost-device-vsock?

I'm not sure what you mean by "hybrid" here, I'm not very familiar with the vsock in rust-vmm. libkrun has a vsock device that can expose host UDS to the guest using vsock, it also supports transparently forwarding TCP and UDP connections over vsock (using patched guest kernel with TSI patches).

pipewire for virtio-sound

Actually this was relevant 😃 , though possibly it's not needed now, since libkrun has a better way to forward pipewire over virtio-gpu/cross-domain. (But libkrun currently still contains a forked sound device from rust-vmm/vhost-device)

I think this is the right way, because a vmm could be interested only in the device and not in the backends (here with backends I mean for example UDS/hybrid-vsock for virtio-vsock, or pipewire for virtio-sound, or tcp for virtio-console, etc).

Sure but this is orthogonal to what I am proposing. In general how generic the devices-as-libraries are over their specific back-ends would depend on the device maintainer, and what consumers of the device need (like libkrun) and in general how practical it is to do on case-by-case basis. I don't currently plan to do changes here.

In general, I'm not doing this to gain a specific device in libkrun currently (I could just port the device, if we needed something now), but to be able to use devices in the future easily, possibly still vendor them or use them as crates. But even if we need to vendor them for some reason, this would greatly decrease divergence. Currently I'm also working on 2 diverging GPU devices one in libkrun and other in vhost-device both diverging from the original in crosvm, which is not ideal.
(Another change is of course the more flexible event handling mechanism, I talked about in the start of this issue.)

@stefano-garzarella
Copy link
Member

Let's use virtio-vsock as an example. Is libkrun interested in the hybrid-vsock that we have in vhost-device-vsock?

I'm not sure what you mean by "hybrid" here, I'm not very familiar with the vsock in rust-vmm. libkrun has a vsock device that can expose host UDS to the guest using vsock, it also supports transparently forwarding TCP and UDP connections over vsock (using patched guest kernel with TSI patches).

host UDS to the guest using vsock is the hybrid-vsock introduced by firecracker:
And also implemented by vhost-user-vsock.

pipewire for virtio-sound

Actually this was relevant 😃 , though possibly it's not needed now, since libkrun has a better way to forward pipewire over virtio-gpu/cross-domain. (But libkrun currently still contains a forked sound device from rust-vmm/vhost-device)

I think this is the right way, because a vmm could be interested only in the device and not in the backends (here with backends I mean for example UDS/hybrid-vsock for virtio-vsock, or pipewire for virtio-sound, or tcp for virtio-console, etc).

Sure but this is orthogonal to what I am proposing. In general how generic the devices-as-libraries are over their specific back-ends would depend on the device maintainer, and what consumers of the device need (like libkrun) and in general how practical it is to do on case-by-case basis. I don't currently plan to do changes here.

I don't think it's orthogonal, but simply an extra step on the library direction, however it's fine, as a first step there's no problem. I still don't understand how you're going to consume the device implemented in vhost-device-vsock in libkrun to implement TSI, but maybe I'll figure it out later.

@mtjhrc
Copy link
Contributor Author

mtjhrc commented Mar 25, 2025

I don't think it's orthogonal, but simply an extra step on the library direction,

Yeah, that's what I meant.

I still don't understand how you're going to consume the device implemented in vhost-device-vsock in libkrun to implement TSI, but maybe I'll figure it out later.

I don't plan on doing that for now. My current goal isn't to use everything from rust-vmm upstream. But in the future, who knows...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants