Skip to content
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

Remove remaining Arc<Volume> instances #1500

Merged
merged 1 commit into from
Oct 10, 2024
Merged

Conversation

mkeeter
Copy link
Contributor

@mkeeter mkeeter commented Oct 9, 2024

Nested volumes use an Arc<dyn BlockIO + Send + Sync> to represent the inner BlockIO device.

Previously, we implemented the BlockIO trait on the struct Volume itself, so we needed to make an Arc<Volume> to create nested volumes.

This PR moves the BlockIO implementation to the VolumeInner. Because the VolumeInner is already stored in an Arc, we can now cast from Arc<VolumeInner> to Arc<dyn BlockIO + Send + Sync>, removing the extra level of indirection.

@mkeeter mkeeter requested review from jmpesp and leftwo October 9, 2024 19:32
Copy link
Contributor

@leftwo leftwo left a comment

Choose a reason for hiding this comment

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

Approved assuming there was no change to volume_write_op(), it just moved locations.

@mkeeter
Copy link
Contributor Author

mkeeter commented Oct 10, 2024

Approved assuming there was no change to volume_write_op(), it just moved locations.

That's correct!

@mkeeter mkeeter merged commit c203991 into main Oct 10, 2024
19 checks passed
@mkeeter mkeeter deleted the mkeeter/remove-arc-volume branch October 10, 2024 12:20
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.

2 participants