Skip to content

feat: add virtio-blk definitions - #33

Open
mkroening wants to merge 1 commit into
mainfrom
virtio-blk
Open

feat: add virtio-blk definitions#33
mkroening wants to merge 1 commit into
mainfrom
virtio-blk

Conversation

@mkroening

Copy link
Copy Markdown
Member

This PR adds the virtio-blk definitions. This is a combination of Kreb216@3f41c1e and stlankes@94d7608.

I have reworked the code to adhere to the style and naming conventions of this project. I have also added more definitions. I have tested this with the driver of @Kreb216 and the driver of @stlankes and both compile after adjusting a few names.

I have removed RequestHeader from @stlankes. Ideally, we would have a virtio::blk::Req struct corresponding to the definition in the spec:

struct virtio_blk_req { 
        le32 type; 
        le32 reserved; 
        le64 sector; 
        u8 data[]; 
        u8 status; 
};

I did model this, and it works well from a Rust type perspective, but the first three fields and the last one are written by the driver, and the data is written by the device, though. This requires having three descriptors for one type, which does not work in the Hermit driver at the moment. Since it should be possible in principle, though, I would like to keep any implementation-specific helper types such as a header out of this project. It works fine to define the type in the Hermit driver, though.

@Kreb216, I cannot request a review from you through GitHub, but you are welcome to give this a try and leave comments if you have any. :)

This PR does not conflict with, but depends on, the following PRs:

Closes #28.

Co-authored-by: Stefan Lankes <slankes@eonerc.rwth-aachen.de>
Co-authored-by: Martin Kröning <martin.kroening@eonerc.rwth-aachen.de>
@mkroening
mkroening requested review from phip1611 and stlankes August 21, 2026 07:44
@mkroening mkroening self-assigned this Aug 21, 2026

@phip1611 phip1611 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM but please consider adding context (why you performed certain changes) to the commit messages.

Comment thread src/blk.rs
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

Successfully merging this pull request may close these issues.

3 participants