Simplify read_config

[kornelski]

Changes

read_config doesn't have to use io::Write to just copy bytes in memory.

License Acceptance

By submitting this pull request, I confirm that my contribution is made under
the terms of the Apache 2.0 license. For more information on following Developer
Certificate of Origin and signing off your commits, please check
[CONTRIBUTING.md][3].

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 82.08%. Comparing base (e3893d3) to head (21e5ab5).

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4458      +/-   ##
==========================================
- Coverage   82.10%   82.08%   -0.02%     
==========================================
  Files         255      255              
  Lines       31280    31257      -23     
==========================================
- Hits        25681    25656      -25     
- Misses       5599     5601       +2     

Flag	Coverage Δ
4.14-c5n.metal	79.57% <100.00%> (-0.03%)	⬇️
4.14-c7g.metal	?
4.14-m5n.metal	79.55% <100.00%> (-0.04%)	⬇️
4.14-m6a.metal	78.78% <100.00%> (-0.03%)	⬇️
4.14-m6g.metal	76.60% <100.00%> (-0.03%)	⬇️
4.14-m6i.metal	79.55% <100.00%> (-0.04%)	⬇️
4.14-m7g.metal	76.60% <100.00%> (-0.03%)	⬇️
5.10-c5n.metal	82.09% <100.00%> (-0.03%)	⬇️
5.10-c7g.metal	?
5.10-m5n.metal	82.07% <100.00%> (-0.03%)	⬇️
5.10-m6a.metal	81.38% <100.00%> (-0.03%)	⬇️
5.10-m6g.metal	79.37% <100.00%> (-0.03%)	⬇️
5.10-m6i.metal	82.06% <100.00%> (-0.03%)	⬇️
5.10-m7g.metal	79.37% <100.00%> (-0.03%)	⬇️
6.1-c5n.metal	82.09% <100.00%> (-0.03%)	⬇️
6.1-c7g.metal	?
6.1-m5n.metal	82.07% <100.00%> (-0.03%)	⬇️
6.1-m6a.metal	81.38% <100.00%> (-0.03%)	⬇️
6.1-m6g.metal	79.37% <100.00%> (-0.03%)	⬇️
6.1-m6i.metal	82.06% <100.00%> (-0.03%)	⬇️
6.1-m7g.metal	79.37% <100.00%> (-0.03%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[ShadowCurse]

Overall everything looks good, but please update commit messages (otherwise CI will be angry). Also can you update the write_config as well while you are at it?

[kornelski]

write_config already uses copy_from_slice, so changes to it would be just a code-golf.

However, I'm wondering if these functions should be reporting errors instead of just not reading/writing. Isn't there a risk from leaving data unchanged?

[ShadowCurse]

Don't think returning errors makes a lot of sense here. There are 2 paths: panic and abort the process, or log the error and continue execution. In case of read_config I assume driver will simply not initialize the device. WDYT @bchalios

[bchalios]

Sorry for the delay @kornelski @ShadowCurse,

I don't think there's any danger from failed reads/writes. At least, not on the host side. If the guest kernel driver is not working properly it might fail, but that's the guest's problem. We do the right thing.

LGTM, apart from the commit messages. Would you mind adding a bit of more info on what each commit does and why? Also, really a nit, but we typically have commit titles like:

virtio: simplify read_config for VirtIO devices

[ShadowCurse]

@kornelski Also please fix compilations/stylistics errors from here: https://buildkite.com/firecracker/firecracker-pr/builds/9248#_

[bchalios]

Hi @kornelski, could you fix the style issues in the PR so we can merge this?

[roypat]

Hi @kornelski,
I've gone ahead and fixed up the style issues so that we can go ahead with merging this improvement. Thanks again for your contribution!