Modernize fw_cfg emulation #705
Conversation
|
Commit message should be updated to reflect that it fixes #667 prior to integration |
|
The PHD failures are indicating that this revision is no longer migration-compatible with the current |
It is expected. The migration payload is substantially different |
|
Thanks, that's what I had figured but I thought it was worth double-checking. I kind of wonder if we want to run just the migration-compatibility tests in a separate buildomat job from the other PHD tests, so that we can more clearly distinguish between test failures that indicate a commit broke something, and commits which introduce incompatible migration payload changes... |
There was a problem hiding this comment.
Overall, this looks good to me --- I wasn't very familiar with the previous code, but I've tried to review the new code to the best of my ability. I had a few small nitpicks and questions, but no major issues; the question about enforcing ASCII-only strings is the only thing I'm actually concerned about and it's not a problem with the present code.
Being one of the early parts of the prototypical Propolis, the fw_cfg emulation was rather unpolished, and lacked the ability for entries to be added or removed after instance initialization. This change should improve it on several fronts: - Entries can be added and/or removed during runtime - Association of any RamFB device is more flexible and can also be modified during runtime - fw_cfg entries are properly migrated, instead of requiring identical population on the destination instance. - A modicum of test coverage is present
Being one of the early parts of the prototypical Propolis, the fw_cfg emulation was rather unpolished, and lacked the ability for entries to be added or removed after instance initialization.
This change should improve it on several fronts: