Emit history_events on zpool create #6712

behlendorf · 2017-10-03T17:28:26Z

Description

Alternate implementation for #6688, to address issue #6679. It removes the suppression of command history events during pool creation.

This change results in a significant number of additional history log entries being added to the log as part of pool creation. However, this is mitigated for the most part since they are marked as internal events and this won't show up in zpool history unless the -i flag is provided. They will appear in the events stream.

$ zpool create -f -o multihost=on tank vdb

$ zpool history
History for 'tank':
2017-10-03.13:03:49 lt-zpool create -f -o multihost=on tank vdb

$ zpool history -i
History for 'tank':
2017-10-03.13:03:49 [txg:4] create pool version 5000; software version 5000/5; uts localhost.localdomain 3.10.0-514.26.2.el7.x86_64 #1 SMP Tue Jul 4 15:04:05 UTC 2017 x86_64
2017-10-03.13:03:49 [txg:4] set multihost=1
2017-10-03.13:03:49 [txg:4] set feature@async_destroy=enabled
...
2017-10-03.13:03:49 lt-zpool create -f -o multihost=on tank vdb

$ pool events -v | grep history_internal_str
  history_internal_str = "pool version 5000; software version...
  history_internal_str = "multihost=1"
  ....
  history_internal_str = "feature@encryption=enabled"

Motivation and Context

#6679, this is useful to support interfaces like IML which need to be notified of modifications to the pool.

How Has This Been Tested?

Locally, needs review.

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)
Documentation (a change to man pages or other documentation)

Checklist:

My code follows the ZFS on Linux code style requirements.
I have updated the documentation accordingly.
I have read the CONTRIBUTING document.
I have added tests to cover my changes.
All new and existing tests passed.
All commit messages are properly formatted and contain Signed-off-by.
Change has been approved by a ZFS on Linux member.

utopiabound · 2017-10-04T14:13:24Z

This looks good to me. I'll close pulll request #6688

codecov · 2017-10-19T01:07:40Z

Codecov Report

Merging #6712 into master will decrease coverage by 38.14%.
The diff coverage is 100%.

@@             Coverage Diff             @@
##           master    #6712       +/-   ##
===========================================
- Coverage   74.36%   36.22%   -38.15%     
===========================================
  Files         297      261       -36     
  Lines       94371    83093    -11278     
===========================================
- Hits        70181    30100    -40081     
- Misses      24190    52993    +28803

Flag	Coverage Δ
#kernel	`41.6% <100%> (-32.69%)`	⬇️
#user	`24.68% <ø> (-41.73%)`	⬇️

Impacted Files	Coverage Δ
module/zfs/spa.c	`53.98% <100%> (-29.42%)`	⬇️
module/zfs/spa_history.c	`68.83% <100%> (-25.29%)`	⬇️
module/zfs/zle.c	`0% <0%> (-100%)`	⬇️
lib/libspl/include/sys/byteorder.h	`0% <0%> (-100%)`	⬇️
lib/libspl/include/sys/mnttab.h	`0% <0%> (-100%)`	⬇️
lib/libspl/strnlen.c	`0% <0%> (-100%)`	⬇️
include/sys/xvattr.h	`0% <0%> (-100%)`	⬇️
include/linux/xattr_compat.h	`0% <0%> (-100%)`	⬇️
include/linux/dcache_compat.h	`0% <0%> (-100%)`	⬇️
lib/libspl/include/sys/uio.h	`0% <0%> (-100%)`	⬇️
... and 237 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6044cf5...855fc85. Read the comment docs.

History commands and events were being suppressed for the 'zpool create' command since the history object did not yet exist. Create the object earlier so this history doesn't get lost. Split the pool_destroy event in to pool_destroy and pool_export so they may be distinguished. Updated events_001_pos and events_002_pos test cases. They now check for the expected history events and were reworked to be more reliable. NOTE: Custom event runfile added to verify reliability. This will be removed in the final merge. Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov> Issue openzfs#6486

dinatale2

LGTM. Couple nits that don't impact anything and could be fixed, but nothing critical.

dinatale2 · 2017-10-20T22:26:25Z

tests/zfs-tests/tests/functional/events/events_002_pos.ksh

@@ -51,7 +51,6 @@ function cleanup
 	done

 	log_must rm -f $TMP_EVENTS_ZED $TMP_EVENTS_ZED


nit: you probably don't need the log_must here. But, it's really no big deal.

dinatale2 · 2017-10-20T22:32:58Z

tests/zfs-tests/tests/functional/events/events_common.kshlib

+	log_must zpool sync -f
+	log_must file_wait $zedlog $delay
+	log_must cp $zedlog $TMP_EVENTS_ZED
+	log_must eval "zpool events >$TMP_EVENTS 2>/dev/null"


nit: missing space zpool events > $TMP_EVENTS 2>/dev/null

behlendorf · 2017-10-20T22:52:20Z

@dinatale2 thanks for the additional review. Let's go with it as-is since those are both minor style issues.

History commands and events were being suppressed for the 'zpool create' command since the history object did not yet exist. Create the object earlier so this history doesn't get lost. Split the pool_destroy event in to pool_destroy and pool_export so they may be distinguished. Updated events_001_pos and events_002_pos test cases. They now check for the expected history events and were reworked to be more reliable. Reviewed-by: Nathaniel Clark <nathaniel.l.clark@intel.com> Reviewed-by: Giuseppe Di Natale <dinatale2@llnl.gov> Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov> Closes openzfs#6712 Closes openzfs#6486

adilger · 2017-11-23T19:09:04Z

@behlendorf will this fix be included into 0.7.x, or only 0.8.x?

History commands and events were being suppressed for the 'zpool create' command since the history object did not yet exist. Create the object earlier so this history doesn't get lost. Split the pool_destroy event in to pool_destroy and pool_export so they may be distinguished. Updated events_001_pos and events_002_pos test cases. They now check for the expected history events and were reworked to be more reliable. Reviewed-by: Nathaniel Clark <nathaniel.l.clark@intel.com> Reviewed-by: Giuseppe Di Natale <dinatale2@llnl.gov> Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov> Closes openzfs#6712 Closes openzfs#6486

behlendorf · 2017-11-28T17:54:54Z

@adilger I've added this to the stack for 0.7.4.

History commands and events were being suppressed for the 'zpool create' command since the history object did not yet exist. Create the object earlier so this history doesn't get lost. Split the pool_destroy event in to pool_destroy and pool_export so they may be distinguished. Updated events_001_pos and events_002_pos test cases. They now check for the expected history events and were reworked to be more reliable. Reviewed-by: Nathaniel Clark <nathaniel.l.clark@intel.com> Reviewed-by: Giuseppe Di Natale <dinatale2@llnl.gov> Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov> Closes openzfs#6712 Closes openzfs#6486

History commands and events were being suppressed for the 'zpool create' command since the history object did not yet exist. Create the object earlier so this history doesn't get lost. Split the pool_destroy event in to pool_destroy and pool_export so they may be distinguished. Updated events_001_pos and events_002_pos test cases. They now check for the expected history events and were reworked to be more reliable. Reviewed-by: Nathaniel Clark <nathaniel.l.clark@intel.com> Reviewed-by: Giuseppe Di Natale <dinatale2@llnl.gov> Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov> Closes #6712 Closes #6486 Conflicts: tests/zfs-tests/tests/functional/events/events_002_pos.ksh

History commands and events were being suppressed for the 'zpool create' command since the history object did not yet exist. Create the object earlier so this history doesn't get lost. Split the pool_destroy event in to pool_destroy and pool_export so they may be distinguished. Updated events_001_pos and events_002_pos test cases. They now check for the expected history events and were reworked to be more reliable. Reviewed-by: Nathaniel Clark <nathaniel.l.clark@intel.com> Reviewed-by: Giuseppe Di Natale <dinatale2@llnl.gov> Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov> Closes openzfs#6712 Closes openzfs#6486 Conflicts: tests/zfs-tests/tests/functional/events/events_002_pos.ksh

History commands and events were being suppressed for the 'zpool create' command since the history object did not yet exist. Create the object earlier so this history doesn't get lost. Split the pool_destroy event in to pool_destroy and pool_export so they may be distinguished. Updated events_001_pos and events_002_pos test cases. They now check for the expected history events and were reworked to be more reliable. Reviewed-by: Nathaniel Clark <nathaniel.l.clark@intel.com> Reviewed-by: Giuseppe Di Natale <dinatale2@llnl.gov> Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov> Closes openzfs#6712 Closes openzfs#6486

behlendorf mentioned this pull request Oct 3, 2017

Emit history_events on zpool create #6688

Closed

13 tasks

behlendorf added the Status: Work in Progress Not yet ready for general review label Oct 3, 2017

behlendorf force-pushed the issue-6679 branch from 0595d67 to 802bd2d Compare October 19, 2017 01:07

behlendorf force-pushed the issue-6679 branch 2 times, most recently from da50f23 to bd54e0d Compare October 19, 2017 20:27

behlendorf added Ready for Testing and removed Status: Work in Progress Not yet ready for general review labels Oct 19, 2017

behlendorf force-pushed the issue-6679 branch 4 times, most recently from b568627 to 8077ea8 Compare October 20, 2017 16:22

behlendorf force-pushed the issue-6679 branch from 8077ea8 to 855fc85 Compare October 20, 2017 18:09

dinatale2 approved these changes Oct 20, 2017

View reviewed changes

behlendorf merged commit d5e024c into openzfs:master Oct 23, 2017

behlendorf added this to PR Needed for 0.7.4 in 0.7.4 Nov 28, 2017

behlendorf moved this from PR Needed for 0.7.4 to Merged to 0.7.4 in 0.7.4 Dec 12, 2017

behlendorf deleted the issue-6679 branch April 19, 2021 19:31

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Emit history_events on zpool create #6712

Emit history_events on zpool create #6712

behlendorf commented Oct 3, 2017

utopiabound commented Oct 4, 2017

codecov bot commented Oct 19, 2017 •

edited

dinatale2 left a comment

dinatale2 Oct 20, 2017

dinatale2 Oct 20, 2017

behlendorf commented Oct 20, 2017

adilger commented Nov 23, 2017

behlendorf commented Nov 28, 2017

		@@ -51,7 +51,6 @@ function cleanup
		done

		log_must rm -f $TMP_EVENTS_ZED $TMP_EVENTS_ZED

Emit history_events on zpool create #6712

Emit history_events on zpool create #6712

Conversation

behlendorf commented Oct 3, 2017

Description

Motivation and Context

How Has This Been Tested?

Types of changes

Checklist:

utopiabound commented Oct 4, 2017

codecov bot commented Oct 19, 2017 • edited

Codecov Report

dinatale2 left a comment

Choose a reason for hiding this comment

dinatale2 Oct 20, 2017

Choose a reason for hiding this comment

dinatale2 Oct 20, 2017

Choose a reason for hiding this comment

behlendorf commented Oct 20, 2017

adilger commented Nov 23, 2017

behlendorf commented Nov 28, 2017

codecov bot commented Oct 19, 2017 •

edited