rust-vmm / community

rust-vmm community content
499 stars 28 forks source link

Crate Addition Request: vhost-user-backend #99

Closed slp closed 4 years ago

slp commented 4 years ago

Crate Name

vhost-user-backend

Short Description

vhost-user-backend extends vhost implementing the common infrastructure required by vhost-user devices. It provides the VhostUserBackend trait, that must be implemented by crates providing a device personality, and a multi-thread VringWorker to abstract low-level queue operations from the upper layers.

This would be based on Cloud Hypervisor's vhost-user-backend. It requires some refactoring as rust-vmm's vm-virtio::Queue lifetime is linked to a GuestMemory instance, so we need to decouple the VringWorker from the VhostUserBackend, but I already have a set of commits doing that. I think we can start by importing the repository as is, and then improving on it.

Why is this crate relevant to the rust-vmm project?

This is the final piece needed (with the others being vhost, vm-virtio and vm-memory) in rust-vmm to provide a common framework for writing vhost-user device in Rust.

andreeaflorescu commented 4 years ago

+1 on creating the repository.

This would be based on Cloud Hypervisor's vhost-user-backend. It requires some refactoring as rust-vmm's vm-virtio::Queue lifetime is linked to a GuestMemory instance, so we need to decouple the VringWorker from the VhostUserBackend, but I already have a set of commits doing that. I think we can start by importing the repository as is, and then improving on it.

It looks like the repository in Cloud Hypervisor has a git referenced dependency on a vm-virtio fork from CH. Maybe we can add the required changes in rust-vmm/vm-virtio, so that the dependency is not needed anymore. We can discuss this on the PR that introduces the code though :D

slp commented 4 years ago

+1 on creating the repository.

This would be based on Cloud Hypervisor's vhost-user-backend. It requires some refactoring as rust-vmm's vm-virtio::Queue lifetime is linked to a GuestMemory instance, so we need to decouple the VringWorker from the VhostUserBackend, but I already have a set of commits doing that. I think we can start by importing the repository as is, and then improving on it.

It looks like the repository in Cloud Hypervisor has a git referenced dependency on a vm-virtio fork from CH. Maybe we can add the required changes in rust-vmm/vm-virtio, so that the dependency is not needed anymore. We can discuss this on the PR that introduces the code though :D

Yes, I have some commits here to make vhost-user-backend use rust-vmm/vm-virtio instead.

BTW, how many approvals are needed for greenlighting the creation of the repository?

andreeaflorescu commented 4 years ago

We were typically waiting for 2. @sameo are you okay with creating this as well?

jiangliu commented 4 years ago

+1

andreeaflorescu commented 4 years ago

@slp we are now creating all new repositories using the crate-template: https://github.com/rust-vmm/crate-template

If I also create this repository from the template, I am not sure if you'll then be able to add a PR with the commits from the existing code in Cloud Hypervisor. There might be some conflicts that you'll need to fix manually. Are you okay with that? Or should I just create an empty repository?

rbradford commented 4 years ago

I would like to see the version of the code in cloud-hypervisor ported to rust-vmm vm-virtio before it is split out. Otherwise we will end up with another divergent repository.

slp commented 4 years ago

@slp we are now creating all new repositories using the crate-template: https://github.com/rust-vmm/crate-template

If I also create this repository from the template, I am not sure if you'll then be able to add a PR with the commits from the existing code in Cloud Hypervisor. There might be some conflicts that you'll need to fix manually. Are you okay with that? Or should I just create an empty repository?

Yes, let's go with the template and I'll try to rebase the commit history without generating too much noise. Worst case scenario, we can always remove the repo and recreate it empty.

slp commented 4 years ago

I would like to see the version of the code in cloud-hypervisor ported to rust-vmm vm-virtio before it is split out. Otherwise we will end up with another divergent repository.

I can create a draft (we're still missing https://github.com/rust-vmm/vm-virtio/pull/21 and deriving Clone for DescriptorChain) PR in cloud-hypervisor/vhost-user-backend to discuss the changes over there.

andreeaflorescu commented 4 years ago

Ok, sounds good to me.

@rbradford are you okay with these next steps?

sameo commented 4 years ago

We were typically waiting for 2. @sameo are you okay with creating this as well?

Sure, but I agree with @rbradford that we need to have the cloud-hypervisor crate point to rust-vmm/vm-virtio before sending any PR to it.

andreeaflorescu commented 4 years ago

@sameo RFC should be fine though, cause they're not mergable, right? The RFC are good for having people take a look at them.

andreeaflorescu commented 4 years ago

Created repo: https://github.com/rust-vmm/vhost-user-backend

andreeaflorescu commented 4 years ago

Closing this. Discussions should be moved to the repository.