search

rust-vmm / vm-virtio

virtio implementation
Apache License 2.0
372 stars 89 forks source link

Split virtio-queue into multiple submodules for maintenance #120

Closed jiangliu closed 2 years ago

jiangliu commented 2 years ago

Fix #87 by split the lib.rs of virtio-queue into multiple submodules: 1) split it into several submodules for maintenance 2) use setter/getter to avoid direct access to data structure fields 3) refine unit test cases to improve code coverage, now it reaches 90% code coverage.

bonzini commented 2 years ago

The QueueStateSync API is absolutely not ready for being published.

jiangliu commented 2 years ago

The QueueStateSync API is absolutely not ready for being published.

Well formed API designs are always welcomed:) But we hope we could publish the virtio-queue crate sooner than later, because several other projects depend on it. The situation has last more than one year. We have tried several times during the past year to migrate to the upstream vm-virtio implementation but failed. Then the cloud hypervisor, virtiofsd and our projects turned to maintain their own private versions, it's really a burden for us:(

jiangliu commented 2 years ago

The QueueStateSync API is absolutely not ready for being published.

BTW, I have one more patch to convert QueueStateGuard from an enum into a trait, just as one of your previous patch did. It does help to make the API clear and extensible:)

bonzini commented 2 years ago

The problem is that Queue is not solving the problem in a way that works for the vhost-backend crate for example.

I think we don't even need Queue or QueueSync. What we need is a good guard object so that one doesn't have to use the flexible API with GuestMemory arguments, when the easier one can be used.

In addition, signal_used_queue must definitely be a trait and the guard should know about it. I'll try to come up with something later this week.

jiangliu commented 2 years ago

The problem is that Queue is not solving the problem in a way that works for the vhost-backend crate for example.

Actually we need to support several use cases: 1) support normal virtio devices, such as virtio-blk/virtio-net 2) support vhost daemons with vhost-user-backend 3) support rust async io to improve performance

I think we don't even need Queue or QueueSync. What we need is a good guard object so that one doesn't have to use the flexible API with GuestMemory arguments, when the easier one can be used.

Is this something you are thinking of? https://github.com/rust-vmm/vm-virtio/pull/121 It introduces two changes:

In addition, signal_used_queue must definitely be a trait and the guard should know about it. I'll try to come up with something later this week.