Skip to content

Conversation

@jakecorrenti
Copy link
Member

@jakecorrenti jakecorrenti commented Oct 29, 2025

Add the krun_add_disk3 API that allows users to explicitly configure the sync and cache mode for a virtio-blk device.

This is a followup to #428

@jakecorrenti jakecorrenti force-pushed the relaxed-sync-api branch 2 times, most recently from 8357725 to b514cfc Compare October 30, 2025 13:35
@jakecorrenti jakecorrenti marked this pull request as ready for review October 30, 2025 15:46
Copy link
Member

@tylerfanelli tylerfanelli left a comment

Choose a reason for hiding this comment

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

On non macOS targets, should we define the API to just return EINVAL?

@jakecorrenti
Copy link
Member Author

On non macOS targets, should we define the API to just return EINVAL?

On a non-macOS target it will be a compile-time error. IMO this better than a runtime error so it's clear it's not even an option, but if you think it should be a runtime error I'd love to hear your reasoning

@slp
Copy link
Collaborator

slp commented Oct 31, 2025

Hm... I think I would favor a more generic approach, implementing a krun_add_disk3 instead. This new API should have two additional arguments:

  • cache_mode: to choose between "CACHED" and "UNCACHED".
    • CACHED: default behavior, how libkrun works right now.
    • UNCACHED: imago configured with direct_io.
  • sync_mode: to choose between "FULL", "RELAXED" and "NONE".
    • FULL: default behavior, how libkrun worked until the relaxed mode on macOS was introduced.
    • RELAXED: on macOS, enable imago relaxed mode. On Linux, we may consider doing a partial fsync.
    • NONE: ignore VIRTIO_BLK_F_FLUSH.

@jakecorrenti
Copy link
Member Author

Hm... I think I would favor a more generic approach, implementing a krun_add_disk3 instead. This new API should have two additional arguments:

* cache_mode: to choose between "CACHED" and "UNCACHED".
  
  * CACHED: default behavior, how libkrun works right now.
  * UNCACHED: imago configured with direct_io.

* sync_mode: to choose between "FULL", "RELAXED" and "NONE".
  
  * FULL: default behavior, how libkrun worked until the relaxed mode on macOS was introduced.
  * RELAXED: on macOS, enable imago relaxed mode. On Linux, we may consider doing a partial fsync.
  * NONE: ignore VIRTIO_BLK_F_FLUSH.

Ah, ok I see now. I'll do that instead. Thanks

@jakecorrenti jakecorrenti marked this pull request as draft November 24, 2025 18:41
Signed-off-by: Jake Correnti <[email protected]>
@jakecorrenti jakecorrenti changed the title include, block: add krun_enable_relaxed_sync_for_disk API include, block: add krun_add_disk3 API Nov 24, 2025
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.

4 participants