Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

zpool: use tab to intend continuation from removal status #11674

Merged

Conversation

ThomasLamprecht
Copy link
Contributor

Bring the output of the removal status in line with the other
"fields" that zpool status outputs, and thus allows an parser to
easier detect this as continuation of the 'remove:' output.

Before:

remove: Removal of vdev 0 copied 282G in 0h9m, completed on [...]
    776K memory used for removed device mappings

Now:

remove: Removal of vdev 0 copied 282G in 0h9m, completed on [...]
	776K memory used for removed device mappings

Signed-off-by: Thomas Lamprecht [email protected]

Motivation and Context

We have a parser for the zpool status output which uses the fact that continuations of output values over multiple lines uses a tab to intend those continuations, allowing detecting them relatively easily.
We got a report of a failure in said parser and when checking the offending zpool status output we saw that the removal status is not tab intended but used four space-characters, and was not covered by our testcases.

After checking zpool status print code I figured this change would fit in and allow us to avoid special handling in the now relatively simple and general parser.

Description

Switch four spaces with a tab in two instances.

How Has This Been Tested?

Created a pool with a mirror, added an extra mirror and removed it again resulting in triggering the removal status code to print,
compared the output before and after.

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)
  • 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)

Note, this can be probably seen as bug fix only in the widest sense, so please do not take above marking as such to serious.

Checklist:

Bring the output of the removal status in line with the other
"fields" that zpool status outputs, and thus allows an parser to
easier detect this as continuation of the 'remove:' output.

Before:
remove: Removal of vdev 0 copied 282G in 0h9m, completed on [...]
    776K memory used for removed device mappings

Now:
remove: Removal of vdev 0 copied 282G in 0h9m, completed on [...]
	776K memory used for removed device mappings

Signed-off-by: Thomas Lamprecht <[email protected]>
@ThomasLamprecht ThomasLamprecht force-pushed the zpool-removal-status-use-tabs branch from d40d5fd to 170d4d1 Compare March 2, 2021 12:39
Copy link
Contributor

@behlendorf behlendorf left a comment

Choose a reason for hiding this comment

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

Thanks!

@behlendorf behlendorf added Status: Code Review Needed Ready for review and testing Status: Accepted Ready to integrate (reviewed, tested) and removed Status: Code Review Needed Ready for review and testing labels Mar 3, 2021
@behlendorf behlendorf merged commit fd1c366 into openzfs:master Mar 5, 2021
behlendorf pushed a commit that referenced this pull request Mar 8, 2021
Bring the output of the removal status in line with the other
"fields" that zpool status outputs, and thus allows an parser to
easier detect this as continuation of the 'remove:' output.

Before:
remove: Removal of vdev 0 copied 282G in 0h9m, completed on [...]
    776K memory used for removed device mappings

Now:
remove: Removal of vdev 0 copied 282G in 0h9m, completed on [...]
	776K memory used for removed device mappings

Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: Thomas Lamprecht <[email protected]>
Closes #11674
jsai20 pushed a commit to jsai20/zfs that referenced this pull request Mar 30, 2021
Bring the output of the removal status in line with the other
"fields" that zpool status outputs, and thus allows an parser to
easier detect this as continuation of the 'remove:' output.

Before:
remove: Removal of vdev 0 copied 282G in 0h9m, completed on [...]
    776K memory used for removed device mappings

Now:
remove: Removal of vdev 0 copied 282G in 0h9m, completed on [...]
	776K memory used for removed device mappings

Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: Thomas Lamprecht <[email protected]>
Closes openzfs#11674
sempervictus pushed a commit to sempervictus/zfs that referenced this pull request May 31, 2021
Bring the output of the removal status in line with the other
"fields" that zpool status outputs, and thus allows an parser to
easier detect this as continuation of the 'remove:' output.

Before:
remove: Removal of vdev 0 copied 282G in 0h9m, completed on [...]
    776K memory used for removed device mappings

Now:
remove: Removal of vdev 0 copied 282G in 0h9m, completed on [...]
	776K memory used for removed device mappings

Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: Thomas Lamprecht <[email protected]>
Closes openzfs#11674
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Accepted Ready to integrate (reviewed, tested)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants