DeviceManagerError: Implement Error including all Variants #7061
Conversation
|
I was quite careful to ensure to not have any copy&paste errors. Fingers crossed! |
phip1611
force-pushed
the
device-error
branch
2 times, most recently
from
356ebc8 to
bbb89f2
Compare
rbradford
left a comment
rbradford
left a comment
There was a problem hiding this comment.
Thanks for tackling this - this is a biggie!
pci/src/bus.rs
Outdated
| #[derive(Error, Debug)] | ||
| pub enum PciRootError { | ||
| /// Could not allocate device address space for the device. | ||
| #[error("Could not allocate device address space for the device")] |
There was a problem hiding this comment.
I think the convention is that we put the embedded error after the message with a colon separating them - like "Could not allocate device address space for the device: {0}"
rockhopper:~/src/cloud-hypervisor (main)$ git grep "#\[error.*: {0}" | wc -l
586
There was a problem hiding this comment.
Please note that I'd like to change that treewide in/for #7066
There was a problem hiding this comment.
In which case we don't need to add that here.
There was a problem hiding this comment.
So for this commit, I follow the ": {0}" convention for the inner error for all enum variants that I touch anyway. This is likely going to be removed again in #7066, but this is another story
|
@phip1611 I'm confused - you adapted it to to the older style which means we can merge this sooner rather than holding it off? |
|
Sorry for the confusion.
Yes, adapted to the old style where each error's Display::fmt() also prints the sub error. We now have proper |
The DeviceManagerError type is among the types missing the Error trait so far. To streamline the code and to simplify usage on higher levels of this error type, this type now implements Display and Error. As not all variant values are Error yet, `#[source]` is not everywhere where it could be. This is done in the next commits. The high level goal is: Enable future work to improve the error output of cloud hypervisor. Signed-off-by: Philipp Schuster <[email protected]> On-behalf-of: SAP [email protected]
Signed-off-by: Philipp Schuster <[email protected]> On-behalf-of: SAP [email protected]
Signed-off-by: Philipp Schuster <[email protected]> On-behalf-of: SAP <[email protected]>
Signed-off-by: Philipp Schuster <[email protected]> On-behalf-of: SAP <[email protected]>
This format is also used elsewhere. Signed-off-by: Philipp Schuster <[email protected]> On-behalf-of: SAP <[email protected]>
96deca9
2 participants
EDIT: This is a prerequisite for #7066. To have smaller PRs, this is a split-out.
The DeviceManagerError type is among the types missing the
Errortrait sofar. To streamline the code and to simplify usage on higher levels of this
error type, this type now implements Display and Error.
As not all variant values are Error yet,
#[source]is not everywhere whereit could be. This is done in the next commits.
The high level goal is: Enable future work to improve the error output of
cloud hypervisor.