Skip to content

build: Bump zerocopy to 0.8.23#40

Merged
rbradford merged 1 commit intorust-vmm:mainfrom
RuoqingHe:update-zerocopy
Mar 25, 2025
Merged

build: Bump zerocopy to 0.8.23#40
rbradford merged 1 commit intorust-vmm:mainfrom
RuoqingHe:update-zerocopy

Conversation

@RuoqingHe
Copy link
Member

Summary of the PR

Manually bump zerocopy to 0.8.23 to unblock our downstream to upgrade zerocopy.

Requirements

Before submitting your PR, please make sure you addressed the following
requirements:

  • All commits in this PR have Signed-Off-By trailers (with
    git commit -s), and the commit message has max 60 characters for the
    summary and max 75 characters for each description line.
  • All added/changed functionality has a corresponding unit/integration
    test.
  • All added/changed public-facing functionality has entries in the "Upcoming
    Release" section of CHANGELOG.md (if no such section exists, please create one).
  • Any newly added unsafe code is properly documented.

@rbradford
Copy link
Collaborator

@RuoqingHe Can you please rebase this and update now that clippy issue is resolved

@RuoqingHe RuoqingHe force-pushed the update-zerocopy branch 3 times, most recently from bfdf3a5 to 43243cc Compare March 21, 2025 09:27
@RuoqingHe
Copy link
Member Author

@rbradford I'm not sure this is the correct way of doing this, please take a look 👀

Copy link
Collaborator

@rbradford rbradford left a comment

Choose a reason for hiding this comment

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

This seems to align with the upgrade guide in google/zerocopy#1680

src/sdt.rs Outdated
/// Write a value at the given offset
pub fn write<T: AsBytes>(&mut self, offset: usize, value: T) {
self.write_bytes(offset, value.as_bytes())
pub fn write<T: IntoBytes + FromBytes>(&mut self, offset: usize, mut value: T) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

What's with this mut change? We don't change the contents of value do we?

Copy link
Member Author

Choose a reason for hiding this comment

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

You are right, I was scratching my head while trying to understand the trait changes and made this mistake. I will fix this 👍

rbradford
rbradford previously approved these changes Mar 21, 2025
Copy link
Collaborator

@rbradford rbradford left a comment

Choose a reason for hiding this comment

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

Looks good to me. @RuoqingHe Have you mocked up the CH changes, if any that are necessary for this?

@RuoqingHe
Copy link
Member Author

Looks good to me. @RuoqingHe Have you mocked up the CH changes, if any that are necessary for this?

That's what get me started to do this, I think we still need to align rust-vmm/kvm crate to 0.8.23 before we eventually bump CH's zerocopy

Manually bump zerocopy to 0.8.23 to unblock our downstream to upgrade
zerocopy.

Signed-off-by: Ruoqing He <heruoqing@iscas.ac.cn>
Copy link
Collaborator

@rbradford rbradford left a comment

Choose a reason for hiding this comment

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

@timw-rivos Is this okay with you?

@timw-rivos
Copy link
Collaborator

@timw-rivos Is this okay with you?

Probably fine, maybe it's time to update this dependency in our downstream as well

@timw-rivos timw-rivos self-requested a review March 24, 2025 14:27
@rbradford rbradford merged commit e08a3f0 into rust-vmm:main Mar 25, 2025
14 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