Return ENA when submitting an ereport#2438
Conversation
mkeeter
force-pushed
the
mkeeter/return-ereport-ena
branch
3 times, most recently
from
c2fef13 to
c9148e1
Compare
| ); | ||
| match eresult { | ||
| Ok(len) => ringbuf_entry!(Trace::EreportSent(len)), | ||
| Ok((len, _ena)) => ringbuf_entry!(Trace::EreportSent(len)), |
There was a problem hiding this comment.
@hawkw Is there a reason Sidecar's not using the helper macro?
There was a problem hiding this comment.
to be precise: i had just forgotten that the sidecar sequencer even had ereports in it now, because i didn't remember adding them. which, it turns out, did not actually mean "no one added them", it meant "cliff added them".
hawkw
left a comment
hawkw
left a comment
There was a problem hiding this comment.
nice --- i had some smallish notes
| ); | ||
| match eresult { | ||
| Ok(len) => ringbuf_entry!(Trace::EreportSent(len)), | ||
| Ok((len, _ena)) => ringbuf_entry!(Trace::EreportSent(len)), |
There was a problem hiding this comment.
to be precise: i had just forgotten that the sidecar sequencer even had ereports in it now, because i didn't remember adding them. which, it turns out, did not actually mean "no one added them", it meant "cliff added them".
hawkw
added
the
fault-management
mkeeter
force-pushed
the
mkeeter/return-ereport-ena
branch
2 times, most recently
from
73186f9 to
8b7452e
Compare
mkeeter
force-pushed
the
mkeeter/return-ereport-ena
branch
from
8b7452e to
0359c6f
Compare
mkeeter
force-pushed
the
mkeeter/return-ereport-ena
branch
from
0359c6f to
22e5df5
Compare
hawkw
left a comment
hawkw
left a comment
There was a problem hiding this comment.
I like the changes you've made here, I think the new result type makes sense. I had some nitpicks about the documentation and naming and stuff, but once those are addressed I like this change a lot.
lib/ereports/src/lib.rs
Outdated
| /// ereport failed). It elides the details of each failure mode, which are | ||
| /// preserved in the ringbuf and are not as relevant to the caller. | ||
| #[derive(Copy, Clone)] | ||
| pub enum EreportDeliverError { |
There was a problem hiding this comment.
turbo nitpick, sorry: I feel like this feels slightly more grammatically correct as
| pub enum EreportDeliverError { | |
| pub enum EreportDeliveryError { |
lib/ereports/src/lib.rs
Outdated
| }; | ||
| } | ||
|
|
||
| /// Lightweight error returned from the generated `deliver_ereport` function |
There was a problem hiding this comment.
turbo nitpick, sorry: perhaps this should link to what generates the function?
| /// Lightweight error returned from the generated `deliver_ereport` function | |
| /// Lightweight error returned from the `deliver_ereport` method | |
| /// generated by the [`declare_ereporter!`] macro. |
|
I'm done with the last round of changes and queuing up automerge! |
2 participants
This will make it easier for ereports to refer to each other!
The
ereport_messages::Enatype isn't directly encodable usingmicrocbor, but it's easy enough to extract the inneru64.The max stack size for
packratwent from 1344 to 1352, which isn't surprising (we're passing an extrau64around), so I bumped it up to the next round number.