Skip to content

devices: set CacheType::Unsafe for macOS raw disks #297

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Apr 3, 2025

Conversation

nohajc
Copy link
Contributor

@nohajc nohajc commented Apr 2, 2025

See #293

@nohajc nohajc force-pushed the disable-cache-for-raw-disks branch 2 times, most recently from 5738c9d to 1ad4667 Compare April 2, 2025 20:07
tylerfanelli
tylerfanelli previously approved these changes Apr 3, 2025
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.

LGTM. You mentioned in the issue that this problem persists with /dev/disk* as well. Do you plan to address that in this PR as well?

@nohajc
Copy link
Contributor Author

nohajc commented Apr 3, 2025

@slp said

I'm not sure if it would be safe doing it with /dev/disk* since it's buffered

Besides, there's no practical reason to use /dev/disk* when you have the raw variant which provides more efficient access anyway.

@nohajc
Copy link
Contributor Author

nohajc commented Apr 3, 2025

Don't know what's with the failing checks now... they seem unrelated.

@nohajc nohajc force-pushed the disable-cache-for-raw-disks branch from a456e6a to 9f3a706 Compare April 3, 2025 10:42
@slp
Copy link
Collaborator

slp commented Apr 3, 2025

Don't know what's with the failing checks now... they seem unrelated.

Ugg... we've got hit again by another clippy update with new restrictions. I'll fix it in a different PR, and the we can rebase this one.

@nohajc
Copy link
Contributor Author

nohajc commented Apr 3, 2025

I also noticed a crash in unit tests...

@slp
Copy link
Collaborator

slp commented Apr 3, 2025

I also noticed a crash in unit tests...

Yes, and I can reproduce the failure here after updating to 1.86.0.

@slp
Copy link
Collaborator

slp commented Apr 3, 2025

@nohajc Tests should be fixed with #298, please rebase this PR.

@nohajc nohajc force-pushed the disable-cache-for-raw-disks branch from 9f3a706 to 4a45522 Compare April 3, 2025 17:32
@nohajc
Copy link
Contributor Author

nohajc commented Apr 3, 2025

Done

Copy link
Collaborator

@slp slp left a comment

Choose a reason for hiding this comment

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

Code looks good and checks are now green, let's get it merged. Thanks @nohajc !

@slp slp merged commit 5b68d16 into containers:main Apr 3, 2025
6 checks passed
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