Value strings: infrastructure for bitfield/enum pretty printers (with support in `zpool events`)
[Sponsors: Klara, Inc., Wasabi Technology, Inc.]
Motivation and Context
As a good C program, OpenZFS uses bitfields and enums all over. I am extremely bad at decoding these in my head, so I frequently find myself writing weird custom formatters. I got sick of that, so I made some more generic formatting facilities for bitfields and enums.
(This is the fully armed and operational version of #15945).
Description
This add zfs_valstr.h, which has some macros to help generation functions to emit string representations of bitfield and enum types. See the comments for details.
Formatters for ZIO flags (zio_flag_t), stage/pipeline (enum zio_stage) and priority (zio_priority_t) are included, and hooked up to zpool events -v for those fields:
Aug 22 2024 10:27:40.191979873 ereport.fs.zfs.io
class = "ereport.fs.zfs.io"
...
zio_err = 0x5
zio_flags = 0x1802c0 [PHYSICAL CANFAIL CONFIG_WRITER DONT_QUEUE DONT_PROPAGATE]
zio_stage = 0x2000000 [DONE]
zio_pipeline = 0x2100000 [READY DONE]
zio_delay = 0x52b2
zio_timestamp = 0xa5c8e385
zio_delta = 0x5475
zio_priority = 0x1 [SYNC_WRITE]
zio_offset = 0x22000
zio_size = 0x400
Note that nothing here uses the "bit" and "pairs" formatters, as I don't have any "production" use for them right now. I still want these facilities available though, as they're very useful during development. #15945 has examples.
I'm somewhat concerned about the string tables falling out of sync with the actual values. There's probably more we could to to make this tighter, from validating the number of items in the tables to actually generating the tables at the same place the bitfield or enum type are declared. That's more work than I wanted to do right now, which is mostly about reviving an old idea so I can use it. I don't think it has to be a huge issue but I'm happy to tackle it up front if people feel there's more danger than I do.
Requires #16469.
How Has This Been Tested?
Generating errors and eyeballing output, and then running the events suite, which has a few calls of zpool events, though nothing that cares about the formatting of those fields.
Types of changes
- [ ] Bug fix (non-breaking change which fixes an issue)
- [x] New feature (non-breaking change which adds functionality)
- [ ] Performance enhancement (non-breaking change which improves efficiency)
- [ ] Code cleanup (non-breaking change which makes code smaller or more readable)
- [ ] Breaking change (fix or feature that would cause existing functionality to change)
- [ ] Library ABI change (libzfs, libzfs_core, libnvpair, libuutil and libzfsbootenv)
- [ ] Documentation (a change to man pages or other documentation)
Checklist:
- [x] My code follows the OpenZFS code style requirements.
- [ ] I have updated the documentation accordingly.
- [x] I have read the contributing document.
- [ ] I have added tests to cover my changes.
- [x] I have run the ZFS Test Suite with this change applied.
- [x] All commit messages are properly formatted and contain
Signed-off-by.
Looks ok to me. Would you mind adding some comments so future developers know to update the bitfield strings? Something like:
diff --git a/include/sys/zio.h b/include/sys/zio.h
index 446b64ccd..3a756949a 100644
--- a/include/sys/zio.h
+++ b/include/sys/zio.h
@@ -167,6 +167,9 @@ typedef enum zio_suspend_reason {
* This was originally an enum type. However, those are 32-bit and there is no
* way to make a 64-bit enum type. Since we ran out of bits for flags, we were
* forced to upgrade it to a uint64_t.
+ *
+ * NOTE: PLEASE UPDATE THE BITFIELD STRINGS IN zfs_valstr.c IF YOU ADD ANOTHER
+ * FLAG.
*/
typedef uint64_t zio_flag_t;
/*
diff --git a/include/sys/zio_impl.h b/include/sys/zio_impl.h
index 2b026d486..ccfec856d 100644
--- a/include/sys/zio_impl.h
+++ b/include/sys/zio_impl.h
@@ -120,6 +120,9 @@ extern "C" {
/*
* zio pipeline stage definitions
+ *
+ * NOTE: PLEASE UPDATE THE BITFIELD STRINGS IN zfs_valstr.c IF YOU ADD ANOTHER
+ * STAGE.
*/
enum zio_stage {
ZIO_STAGE_OPEN = 1 << 0, /* RWFCXT */
diff --git a/include/sys/zio_priority.h b/include/sys/zio_priority.h
index 2d8e7fc36..d2375de71 100644
--- a/include/sys/zio_priority.h
+++ b/include/sys/zio_priority.h
@@ -22,6 +22,10 @@
extern "C" {
#endif
+/*
+ * NOTE: PLEASE UPDATE THE BITFIELD STRINGS IN zfs_valstr.c IF YOU ADD ANOTHER
+ * FLAG.
+ */
typedef enum zio_priority {
ZIO_PRIORITY_SYNC_READ,
ZIO_PRIORITY_SYNC_WRITE, /* ZIL */
@tonyhutter yeah, good tip, done.
(I'm hoping in the future to do something to let the compiler enforce this, but everything I've tried so far has felt awkward in other ways, so this will do for now).
Last push flips the order of bits in _bits and _pairs, so they now go low-to-high. Its more natural for pipeline bits at least, which are a sequence. I have no idea if its better in all future possible cases; nothing in this PR uses them anyway :)
Little tidbit from some work today:
[ 12.723490] NOTICE: zio_reexecute: zio=ffff99d247e25a40 type=2 flags=20000000 [REEXECUTED] stage=00000001 [OPEN] pipeline=02f300f4 <WI|IA|WC|EN|CG|DT|DA|R |VS|VD|VA|X >
Merged as 82ff9aafd687d4eebb6041c99fa822e0478a2024 17dd66dedab9f9bebc823cca3eae3405ef28c7ef