Skip to content

feat(virtio): add generic vhost-user frontend device - #5773

Open
meAmitPatil wants to merge 9 commits into
firecracker-microvm:mainfrom
superserve-ai:feat/generic-vhost-user
Open

feat(virtio): add generic vhost-user frontend device#5773
meAmitPatil wants to merge 9 commits into
firecracker-microvm:mainfrom
superserve-ai:feat/generic-vhost-user

Conversation

@meAmitPatil

@meAmitPatil meAmitPatil commented Mar 18, 2026

Copy link
Copy Markdown

Implement a generic vhost-user frontend device that is agnostic to the specific virtio device type being emulated. Unlike per-device-type frontends, this device delegates config space ownership entirely to the backend via the mandatory CONFIG protocol feature, allowing any virtio device type (e.g. virtio-fs, virtio-scsi) to be used without a
dedicated Firecracker frontend.

A VirtioDeviceType::VhostUserGeneric sentinel (0xFF) serves as the host-side MMIO map key, while a new mmio_device_type_id() trait method returns the real virtio type ID to the guest MMIO register. Queue count
is dynamic (Vec) since different device types need different configurations. Snapshotting is stubbed out, consistent with the existing vhost-user block device.

The device is configured via PUT /vhost-user-devices/{id} and tested end-to-end with virtiofsd — guest kernel recognises the device with the correct virtio type ID.

Closes #5687

License Acceptance

By submitting this pull request, I confirm that my contribution is made under
the terms of the Apache 2.0 license. For more information on following Developer
Certificate of Origin and signing off your commits, please check
CONTRIBUTING.md.

PR Checklist

  • I have read and understand CONTRIBUTING.md.
  • I have run tools/devtool checkbuild --all to verify that the PR passes
    build checks on all supported architectures.
  • I have run tools/devtool checkstyle to verify that the PR passes the
    automated style checks.
  • I have described what is done in these changes, why they are needed, and
    how they are solving the problem in a clear and encompassing way.
  • I have updated any relevant documentation (both in code and in the docs)
    in the PR.
  • I have mentioned all user-facing changes in CHANGELOG.md.
  • If a specific issue led to this PR, this PR closes the issue.
  • When making API changes, I have followed the
    Runbook for Firecracker API changes.
  • I have tested all new and changed functionalities in unit tests and/or
    integration tests.
  • I have linked an issue to every new TODO.

  • This functionality cannot be added in rust-vmm.

Signed-off-by: Amit Patil <iamitpatil2001@gmail.com>
Signed-off-by: Amit Patil <iamitpatil2001@gmail.com>
Signed-off-by: Amit Patil <iamitpatil2001@gmail.com>
Signed-off-by: Amit Patil <iamitpatil2001@gmail.com>
Signed-off-by: Amit Patil <iamitpatil2001@gmail.com>
Signed-off-by: Amit Patil <iamitpatil2001@gmail.com>
Signed-off-by: Amit Patil <iamitpatil2001@gmail.com>
Comment thread docs/vhost-user.md Outdated
Comment on lines +97 to +99
- **Configuration space writes are not forwarded** to the backend. The
backend owns the configuration space in read-only mode from the guest
perspective.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this a Firecracker limitation?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey, This is an implementation limitation, not a protocol one. The vhost-user protocol supports forwarding writes via VHOST_USER_SET_CONFIG. I deferred it since most backends (virtiofsd, SPDK) don't rely on guest-initiated config writes, and the existing vhost-user block device in Firecracker follows the same pattern. Happy to add it in this PR if you think it should be included.

Comment thread docs/vhost-user.md Outdated
Comment on lines +100 to +102
- **The backend must be started before Firecracker.** Firecracker
connects to the socket during device configuration and will return an
error if the backend is not available.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Before Firecracker is started, or before the device is attached? I’m guessing you mean the latter.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right, before the device is attached via the PUT /vhost-user-devices/{id} API call. Will fix the wording, thanks.

Signed-off-by: Amit Patil <iamitpatil2001@gmail.com>
@ShadowCurse

Copy link
Copy Markdown
Contributor

Hi @meAmitPatil, thank you for the PR. Unfortunately currently we don't have much bandwidth to give it a proper review, but after taking a quick look, it seems fine, but there are couple things to note:

  • We will need integration tests for this like we do with vhost-user-block
  • Since this is a generic device and it is very closely based on existing vhost-user-block, I think we should remove the duplication from existing vhost-user-block and use your generic backend for it instead.

Signed-off-by: Amit Patil <iamitpatil2001@gmail.com>
@meAmitPatil

Copy link
Copy Markdown
Author

Hi @meAmitPatil, thank you for the PR. Unfortunately currently we don't have much bandwidth to give it a proper review, but after taking a quick look, it seems fine, but there are couple things to note:

  • We will need integration tests for this like we do with vhost-user-block
  • Since this is a generic device and it is very closely based on existing vhost-user-block, I think we should remove the duplication from existing vhost-user-block and use your generic backend for it instead.

@ShadowCurse Thanks for taking a look! No worries on the review timeline. I've added integration tests. For the duplication refactor (using the generic backend for vhost-user-block), would you prefer that in this PR or as a
follow-up?

@ShadowCurse

Copy link
Copy Markdown
Contributor

For the duplication refactor (using the generic backend for vhost-user-block), would you prefer that in this PR or as a follow-up?

I see 2 ways here:

  1. Create generic impl first, use it for already existing vhost-user-block later (so the follow up PR strategy)
  2. Modify existing vhost-user-block to use generic code under the hood first, then expose generic path as a new device

I think option 2 is better since it removes a point in time when there is a big duplication of the vhost related code. It will also force generic code to be instantly compatible with the existing block device (in 1. case there can be a redundant refactoring of new generic code if some assumptions in it do not hold for block device).

I think both of these steps can be done in a single PR. Or at least both changes can start in a single PR, and if needed, it should be easy enough to split them.

Comment thread docs/vhost-user.md
[virtio specification](https://docs.oasis-open.org/virtio/virtio/v1.3/csd01/virtio-v1.3-csd01.html#x1-1930005).
For example: `26` for virtio-fs, `8` for virtio-scsi.
- `socket` - path to the vhost-user backend Unix domain socket.
- `num_queues` - number of virtqueues to configure for this device.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Testing this PR, I found that a value of "num_queues": 1 did not work, but instead would fail with --

[    0.415314] virtiofs virtio2: discovered new tag: myfs
[    0.416059] virtiofs virtio2: probe with driver virtiofs failed with error -2

And a value of "num_queues": 3 did not work either, but would allow mount -t virtiofs myfs /mnt but ls /mnt would hang.

Only "num_queues": 2 worked correctly for me, where I was able to successfully use the shared directory to read/write files.

@ShadowCurse ShadowCurse added the Status: Awaiting author Indicates that an issue or pull request requires author action label Jun 12, 2026
@ShadowCurse

Copy link
Copy Markdown
Contributor

Hey @meAmitPatil, are you still planning to continue working on this?

@1stvamp

1stvamp commented Jul 9, 2026

Copy link
Copy Markdown

Picking this up: I've rebased and continued this on a fork at triggerdotdev/firecracker, branch feat/generic-vhost-user (diff against upstream main), since it went quiet and we'd like the generic vhost-user device feature. The original implementation is @meAmitPatil's; I've kept his commits as-is and put mine on top.

What's changed since this PR last moved.

Fixed:

  • Rebased onto current main (it was ~375 commits behind, hence the conflicts). Builds on x86_64 and aarch64, checkstyle and clippy clean.
  • The num_queues bug @adamjaso hit. activate() set up every allocated queue, including ones the guest never marked ready. The guest works out how many queues to use from the backend-owned config space, not from num_queues, so a surplus queue stays unready and Queue::initialize returns NotReady, which aborts activation and hangs the guest. num_queues: 1 with virtio-fs is a different thing: virtio-fs wants a hiprio queue plus at least one request queue, so 2 or more. Fix: only set up the queues the guest marked ready, and reject a zero count. Added a multi-queue regression test.
  • PCI. The device only worked over MMIO. The PCI transport builds the device ID from device_type(), which here is the VhostUserGeneric sentinel (0xFF), so the guest got an ID it couldn't bind a driver to. PCI virtio landed after this PR was opened, so the device was never wired for it. I renamed the transport hook mmio_device_type_id to virtio_device_type_id and used it for the PCI device ID too. Both transports work now.
  • A metrics schema gap: the PR added vhost_user_count and vhost_user_fails to the PUT request metrics but didn't add them to the integration test metrics schema, so metrics validation failed across the whole suite (the existing vhost-user-block tests included).
  • @DemiMarie's docs points: reworded the config-write note (the protocol supports it, we just haven't wired it up), and documented the queue-count requirement.

Deferred, deliberately:

  • The vhost-user-block de-duplication @ShadowCurse wanted. I'd rather get the generic device right first and do the de-dup as a follow-up (your "option 1"), and I'll pick it up straight after.
  • Device-specific feature passthrough. The frontend only advertises the transport features it manages, so backend-owned bits like VIRTIO_BLK_F_RO get masked out and a read-only backend mounts read-write (the read-only test is xfailed for now). I'd rather agree the approach with you before implementing this one: advertising all the device bits naively lets the guest negotiate features that assume a queue/config layout the frontend's fixed num_queues doesn't match (e.g. block multi-queue), and that breaks activation. What masking policy makes sense here?
  • Config-space write forwarding (VHOST_USER_SET_CONFIG), same as the existing vhost-user-block device.

Integration tests pass over MMIO and PCI (60 passed, 6 xfail for the deferred read-only case). I ran those locally on x86_64; aarch64 is build-only my end, so the arm64 runtime is down to CI.

So: do you want a fresh PR off the fork, or to carry on here? Either's fine, and we'll keep it maintained.

@1stvamp

1stvamp commented Jul 23, 2026

Copy link
Copy Markdown

@ShadowCurse a gentle nudge on this. The fork branch is rebased onto current main and green: the num_queues bug, PCI transport, and the metrics schema gap are all fixed, and tests pass over MMIO and PCI (diff against main).

No rush, I know review bandwidth is tight. Whenever you get a moment: would you rather I open a fresh PR from the fork, or carry on here? Happy to open it whenever suits.

@1stvamp

1stvamp commented Jul 31, 2026

Copy link
Copy Markdown

Opened #6072 for this, rebased onto current main. @meAmitPatil's commits are kept as-is with mine on top, and the num_queues bug, the PCI transport and the metrics schema gap are all fixed there.

Probably easiest to pick up review over on #6072, but I'm happy to close it and carry on here instead if you'd rather keep it in one thread.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Status: Awaiting author Indicates that an issue or pull request requires author action

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Feature Request] Generic vhost-user

5 participants