Skip to content

Conversation

@Adi-Goll
Copy link
Contributor

@Adi-Goll Adi-Goll commented Nov 11, 2025

Check if -p flag is enabled, and if so print dedup table with raw bytes. Restructure the logic in zutil_pool to check if -p flag is enabled before making calls to zfs_nicenum_format(). Other files changed update the call to/header declaration of dump_ddt_stat() to include a new variable: boolean_t parsable, indicating whether or not -p was used.

Some minor changes were made to the printing of the DDT, to ensure that when the raw bytes were printed and not truncated, there was enough space to make the output legible. This is not dynamic though, and is probably a temporary solution, but it is preferable to the previous state.

This PR also includes updates to the ABI files because the function header for zpool_dump_ddt() was modified to include a new parameter which indicates if the -p flag was used.

Fixes #11626

How Has This Been Tested?

Before this PR, when -p wasn't supported (equivalent of running the command with just -D):

$ sudo zpool status -D tank
  pool: tank
 state: ONLINE
config:

	NAME             STATE     READ WRITE CKSUM
	tank             ONLINE       0     0     0
	  /tmp/disk.img  ONLINE       0     0     0

errors: No known data errors

 dedup: DDT entries 1, size 3K on disk, 8K in core

bucket              allocated                       referenced
______   ______________________________   ______________________________
refcnt   blocks   LSIZE   PSIZE   DSIZE   blocks   LSIZE   PSIZE   DSIZE
------   ------   -----   -----   -----   ------   -----   -----   -----
    2K        1    128K    128K    128K    3.12K    400M    400M    400M
 Total        1    128K    128K    128K    3.12K    400M    400M    400M

After this PR:

$ sudo zpool status -D -p tank
  pool: tank
 state: ONLINE
config:

	NAME             STATE     READ WRITE CKSUM
	tank             ONLINE       0     0     0
	  /tmp/disk.img  ONLINE       0     0     0

errors: No known data errors

 dedup: DDT entries 1, size 3072 on disk, 8192 in core

bucket              allocated                            referenced
______   ______________________________        ______________________________
refcnt   blocks   LSIZE    PSIZE    DSIZE      blocks   LSIZE     PSIZE        DSIZE
------   ------   -----    -----    -----      ------   -----     -----        -----
  2048        1   131072   131072   131072     3200   419430400   419430400   419430400
 Total        1   131072   131072   131072     3200   419430400   419430400   419430400

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • 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)
  • Quality assurance (non-breaking change which makes the code more robust against bugs)
  • 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:

@Adi-Goll Adi-Goll marked this pull request as draft November 13, 2025 18:10
@github-actions github-actions bot added the Status: Work in Progress Not yet ready for general review label Nov 13, 2025
@Adi-Goll Adi-Goll force-pushed the adding-pflag-for-ddt-dump branch 2 times, most recently from 470a20d to e05bd74 Compare November 16, 2025 08:22
@Adi-Goll Adi-Goll marked this pull request as ready for review November 16, 2025 08:25
@github-actions github-actions bot added Status: Code Review Needed Ready for review and testing and removed Status: Work in Progress Not yet ready for general review labels Nov 16, 2025
@Adi-Goll Adi-Goll marked this pull request as draft November 16, 2025 08:27
@github-actions github-actions bot added Status: Work in Progress Not yet ready for general review and removed Status: Code Review Needed Ready for review and testing labels Nov 16, 2025
@Adi-Goll Adi-Goll force-pushed the adding-pflag-for-ddt-dump branch from e05bd74 to 1d3713c Compare November 16, 2025 08:44
@Adi-Goll Adi-Goll marked this pull request as ready for review November 16, 2025 08:45
@github-actions github-actions bot added Status: Code Review Needed Ready for review and testing and removed Status: Work in Progress Not yet ready for general review labels Nov 16, 2025
@Adi-Goll Adi-Goll force-pushed the adding-pflag-for-ddt-dump branch 3 times, most recently from 7ddd5b0 to 99578b7 Compare November 18, 2025 02:59
@Adi-Goll Adi-Goll marked this pull request as draft November 19, 2025 07:12
@github-actions github-actions bot added Status: Work in Progress Not yet ready for general review and removed Status: Code Review Needed Ready for review and testing labels Nov 19, 2025
@Adi-Goll Adi-Goll marked this pull request as ready for review November 19, 2025 09:41
@github-actions github-actions bot added Status: Code Review Needed Ready for review and testing and removed Status: Work in Progress Not yet ready for general review labels Nov 19, 2025
@Adi-Goll
Copy link
Contributor Author

Adi-Goll commented Dec 3, 2025

@amotin @behlendorf No worries if there are more pressing reviews going on right now, but I just wanted to follow up on this.

verify(nvlist_lookup_uint64_array(config, ZPOOL_CONFIG_DDT_HISTOGRAM,
(uint64_t **)&ddh, &c) == 0);
zpool_dump_ddt(dds, ddh);
zpool_dump_ddt(dds, ddh, literal);
Copy link
Contributor Author

@Adi-Goll Adi-Goll Dec 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it may still be preferrable to do dump_opt['P'] > 0 here but I'm not sure. The value for literal seems to correspond to cb.cb_literal, which is set to B_TRUE when checking for a p option being enabled, on line 11250 of this file, so this seems to be correct from what I see but let me know if you have thoughts!

@Adi-Goll Adi-Goll force-pushed the adding-pflag-for-ddt-dump branch from e175962 to cc1aa0a Compare December 21, 2025 04:58
Check if -p flag is enabled, and if so print dedup table with raw
bytes. Restructure the logic in zutil_pool to check if -p flag is
enabled before printing either the bytes or raw numbers.

Calls to print the data for DDT now all use
zfs_nicenum_format(). Increased DDT histogram column buffers
to 32 bytes to prevent truncation when -p is enabled.
Conditional printing with space modifications made to ensure raw
output is still legible.

Signed-off-by: Adi Gollamudi <adigollamudi@gmail.com>
Fixes openzfs#11626
@Adi-Goll Adi-Goll force-pushed the adding-pflag-for-ddt-dump branch 2 times, most recently from 863ea65 to 7a18ad3 Compare December 21, 2025 05:44
Boolean_t paramater "parsable" was added to zpool_dump_ddt(). This
updates ABI files accordingly.

Signed-off-by: Adi Gollamudi <adigollamudi@gmail.com>
@Adi-Goll Adi-Goll force-pushed the adding-pflag-for-ddt-dump branch from 7a18ad3 to 68efee8 Compare December 21, 2025 05:51
@Adi-Goll Adi-Goll requested a review from behlendorf December 21, 2025 06:36
zfs_nicenum_format(dds->dds_ref_psize, ref_psize,
sizeof (ref_psize), ZFS_NICENUM_BYTES);
zfs_nicenum_format(dds->dds_ref_dsize, ref_dsize,
sizeof (ref_dsize), ZFS_NICENUM_BYTES);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would prefer to not bloat the code, but introduce two variables with formats.


(void) printf("%6s %6s %5s %5s %5s %6s %5s %5s"
" %5s\n", "------", "------", "-----", "-----", "-----",
"------", "-----", "-----", "-----");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I already dislike it...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Status: Code Review Needed Ready for review and testing

Projects

None yet

Development

Successfully merging this pull request may close these issues.

zpool status -D dedup table does not support raw bytes -p output

3 participants