Skip to content

Conversation

hcsch
Copy link

@hcsch hcsch commented Aug 25, 2025

Add the Traditional Memory Balloon Device based on VIRTIO v1.2

@phip1611
Copy link
Member

Thanks for your contribution :)

please resolve the conflicts by rebasing your branch

@mkroening mkroening self-requested a review August 25, 2025 15:07
@mkroening mkroening self-assigned this Aug 25, 2025
Copy link
Member

@mkroening mkroening left a comment

Choose a reason for hiding this comment

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

Thanks a lot for the PR! I have two suggestions regarding docs/comments, though.

Feel free to squash any changes. :)

#[derive(VolatileFieldAccess)]
#[repr(C)]
pub struct Config {
#[access(ReadOnly)] // TODO: Not actually specified by the spec?
Copy link
Member

Choose a reason for hiding this comment

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

I think we can safely remove the TODO, right? Every time this field is mentioned in the spec, the driver reads it. It would also not make sense to make this driver-writable in the grand scheme of things.

Suggested change
#[access(ReadOnly)] // TODO: Not actually specified by the spec?
#[access(ReadOnly)]

Comment on lines +542 to +544
/// Deflate of the balloon is always? permitted on guest out of memory condition.
///
/// TODO: Spec is a bit confusing on this feature, see <https://github.com/oasis-tcs/virtio-spec/issues/228>
Copy link
Member

@mkroening mkroening Sep 3, 2025

Choose a reason for hiding this comment

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

Let's stick to the spec first and add notes afterwards:

Suggested change
/// Deflate of the balloon is always? permitted on guest out of memory condition.
///
/// TODO: Spec is a bit confusing on this feature, see <https://github.com/oasis-tcs/virtio-spec/issues/228>
/// Deflate balloon on guest out of memory condition.
///
/// <div class="warning">
///
/// The specification is a bit confusing on this feature, see [oasis-tcs/virtio-spec#228](https://github.com/oasis-tcs/virtio-spec/issues/228).
///
/// </div>

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