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

Upgrade vmm-sys-util to 0.8.0 #28

Merged

Conversation

sandreim
Copy link
Contributor

Reason for This PR

Upgrade to latest vmm-sys-util. Fixes rust-vmm/vmm-sys-util#85

Description of Changes

Provided in commit log.

License Acceptance

By submitting this pull request, I confirm that my contribution is made under
the terms of the Apache 2.0 license.

PR Checklist

[Author TODO: Meet these criteria.]
[Reviewer TODO: Verify that these criteria are met. Request changes if not]

  • All commits in this PR are signed (git commit -s).
  • The reason for this PR is clearly provided (issue no. or explanation).
  • The description of changes is clear and encompassing.
  • Any required documentation changes (code and docs) are included in this PR.
  • Any newly added unsafe code is properly documented.
  • Any user-facing changes are mentioned in CHANGELOG.md.

// Update Default T with the deserialized header.
std::mem::replace(object.as_mut_fam_struct(), header);
let _ = std::mem::replace(object.as_mut_fam_struct(), header);
Copy link
Member

Choose a reason for hiding this comment

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

Can you assign the value directly since you are not doing anything with the previous value from the object?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done.

@sandreim sandreim force-pushed the upgrade_vmm_sys_util branch from 831c3ac to 2f8eb04 Compare February 26, 2021 14:46
acatangiu
acatangiu previously approved these changes Feb 26, 2021
Copy link
Contributor

@acatangiu acatangiu left a comment

Choose a reason for hiding this comment

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

LGTM

tests/test.rs Outdated Show resolved Hide resolved
tests/test.rs Outdated Show resolved Hide resolved
Signed-off-by: Alexandru Cihodaru <[email protected]>
@sandreim
Copy link
Contributor Author

sandreim commented Mar 11, 2021

Maybe also update changelog or you can do that in release PR.

Copy link
Contributor

@acatangiu acatangiu left a comment

Choose a reason for hiding this comment

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

LGTM

@AlexandruCihodaru AlexandruCihodaru merged commit 7162844 into firecracker-microvm:master Mar 11, 2021
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.

FamStructWrapper: incorrect Clone implementation
5 participants