Skip to content
This repository has been archived by the owner on Oct 24, 2022. It is now read-only.

Questions on usage, perhaps to expand the documentation? #3

Open
stsquad opened this issue Feb 16, 2021 · 10 comments
Open

Questions on usage, perhaps to expand the documentation? #3

stsquad opened this issue Feb 16, 2021 · 10 comments

Comments

@stsquad
Copy link

stsquad commented Feb 16, 2021

Hi,

A lot of this is almost certainly down to my lack of experience with Rust but it can't hurt to improve the documentation on usage.

Can vhost-user-backend be used in a single-threaded daemon?

As I understand it the Send/Sync traits are all about how to deal with concurrency for the struct. Does this mean you have to spawn additional threads for each request being serviced or can you have a simple single-threaded daemon?

Which method do incoming vhost-user messages arrive at?

I thought it was handle_event but the comments imply this is for specific FD's and not the standard vhost-user FD. A rough outline of the main message loop would be helpful here.

Can you point to any canonical use cases of vhost-user-backend?

I'm familiar with the virtiofsd daemon but it has a lot going on. A list of other known users of the crate would help in seeing example use and how to plumb things together.

Thanks.

@glitzflitz
Copy link

Which method do incoming vhost-user messages arrive at?
I thought it was handle_event but the comments imply this is for specific FD's and not the standard vhost-user FD. A rough outline of the main message loop would be helpful here.

According to my understanding I guess vhost-user messages are hadled by start which calls handle_request function

Can you point to any canonical use cases of vhost-user-backend?
I'm familiar with the virtiofsd daemon but it has a lot going on. A list of other known users of the crate would help in seeing example use and how to plumb things together.

cloud-hypervisor has vhost-user-blk and vhost-user-net for example

@stsquad
Copy link
Author

stsquad commented Feb 17, 2021

Ok that means that path must be alive. I can see at least some functions getting called:

impl VhostUserBackend for VhostUserRpmb {
    fn features(&self) -> u64 {
        dbg!("features!");
        1 << VIRTIO_F_VERSION_1
            | 1 << VIRTIO_RING_F_INDIRECT_DESC
            | 1 << VIRTIO_RING_F_EVENT_IDX
            | VhostUserVirtioFeatures::PROTOCOL_FEATURES.bits()
    }
}

Generates some debug:

src/main.rs:63] &vuh_rpmb = RwLock {
    data: VhostUserRpmb {
        backend: RpmbBackend {
            image: File {
                fd: 3,
                path: "/home/alex/lsrc/virtio.rust/vhost-user-rpmb.git/flash.img",
                read: true,
                write: true,
            },
            mmap: MmapMut {
                ptr: 0x00007f1afa4f1000,
                len: 33554432,
            },
            write_count: 0,
            read_count: 0,
        },
        event_idx: false,
        mem: None,
    },
}
[src/main.rs:73] "Daemon started" = "Daemon started"
[src/vhu_rpmb.rs:86] "features!" = "features!"
[src/vhu_rpmb.rs:86] "features!" = "features!"

which implies at least the handle_request() function is processing a message over the socket and then passing it to my backends concrete implementation. I guess I can stick it in a debugger and see where the additional startup messages are going and figure out what else I need to implement.

@glitzflitz
Copy link

Basically rough outline for adding new device is that you implement a worker(VhostUserBlkThread for example) which processes the requests and a backend/threadpool(VhostUserBlkBackend for example) along with VhostUserBackend trait methods for dealing with virtio queues

@stsquad
Copy link
Author

stsquad commented Feb 17, 2021

Do you need yet another thread? You already have a thread handling the incoming messages. For simple devices it seems excesive to pass that off to yet another thread.

@glitzflitz
Copy link

glitzflitz commented Feb 17, 2021

I think you can use a thread per queue. If you have a single queue you could just go with single thread

@jiangliu
Copy link
Member

Do you need yet another thread? You already have a thread handling the incoming messages. For simple devices it seems excesive to pass that off to yet another thread.

sadly, virtio-fs is not a "simple" device, we have run into performance issues and started to extend vm-virtio to support multi-threading for a single virtio queue.

@stsquad
Copy link
Author

stsquad commented Feb 19, 2021

I did ideally ponder if it was worth proposing a virtio-ping device (or maybe virtio-watchdog) - something so simple it could be used as a reference implementation in multiple frameworks because it could just concentrate on solving the virtio bits and not get complicated by the domain specific implementation concerns.

Currently I'm debugging my use of this framework by comparing the vhost negotiation sequence with my previously written C daemon using libvhost-user. I'm sure I'll get there in the end.

@jiangliu
Copy link
Member

I did ideally ponder if it was worth proposing a virtio-ping device (or maybe virtio-watchdog) - something so simple it could be used as a reference implementation in multiple frameworks because it could just concentrate on solving the virtio bits and not get complicated by the domain specific implementation concerns.

Currently I'm debugging my use of this framework by comparing the vhost negotiation sequence with my previously written C daemon using libvhost-user. I'm sure I'll get there in the end.

That's a great idea, I'm struggling with vhost unit tests and improve code coverage. With a virito-ping, we could improve code coverage and add integration tests.

@slp
Copy link
Collaborator

slp commented Feb 25, 2021

We definitely need to improve this crate on the documentation and testing fronts to bring it to rust-vmm standards. In fact, that's a requirement for publishing it to crates.io, which is something I'd like to see happening in 2021Q2.

@jiangliu
Copy link
Member

I'm refactoring the crate, and maybe send out PR within coming week.

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

No branches or pull requests

4 participants