-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
OpenZFS 9164 - assert: newds == os->os_dsl_dataset #7336
Conversation
This should fix #6112. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@loli10K thanks for point out #6112.
@dinatale2 please update this PR to re-enable cli_root/zpool_upgrade/zpool_upgrade_007_pos.ksh
and we'll see if all is well.
e335625
to
e27ace4
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The updated fix looks good, but we should run down this warning from the newly enable test case.
20:08:07.76 /usr/share/zfs/zfs-tests/tests/functional/cli_root/zpool_upgrade/zpool_upgrade_007_pos.ksh[64]: default_check_zfs_upgrade: line 155: -: more tokens expected
"-" is not a valid number and we can't assign it to an integer (
Tested on ksh version "93u+20120801-1" (Debian8). Somehow this doesn't seem to affect Illumos: i am going to try and do some more testing on my OpenIndiana development box later today. |
This does affect Illumos: my test box has "/" on ZFS in which case the following block of code gets skipped ( |
@loli10K That explains it... I wasn't concerned about the assignment. I clearly should have been. It might look messy, but I could use a temp variable and check for |
@dinatale2 i think that should work. Also we should probably wrap those "log_must (zfs|zpool) upgrade >/dev/null" with |
e27ace4
to
74f8918
Compare
Codecov Report
@@ Coverage Diff @@
## master #7336 +/- ##
==========================================
- Coverage 76.35% 76.31% -0.05%
==========================================
Files 329 329
Lines 104191 104188 -3
==========================================
- Hits 79560 79508 -52
- Misses 24631 24680 +49
Continue to review full report at Codecov.
|
74f8918
to
e8d9cb1
Compare
Authored by: Andriy Gapon <[email protected]> Reviewed by: Matt Ahrens <[email protected]> Reviewed by: Don Brady <[email protected]> Approved by: Richard Lowe <[email protected]> Ported-by: Giuseppe Di Natale <[email protected]> Patch Notes: Tweaked the zpool_upgrade_007_pos test case to successfully run under 5 minutes. OpenZFS-issue: https://www.illumos.org/issues/9164 OpenZFS-commit: openzfs/openzfs@0e776dc06a
e8d9cb1
to
66083e8
Compare
ver=$(get_pool_prop version $1) | ||
|
||
if [ "$ver" = "-" ]; then | ||
ver="5000" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm sorry for derailing this, i was wrongfully thinking of filesystem version, not pool version.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@loli10K You didn't derail anything. Thank you for your assistance! :)
Authored by: Andriy Gapon [email protected]
Reviewed by: Matt Ahrens [email protected]
Reviewed by: Don Brady [email protected]
Approved by: Richard Lowe [email protected]
Ported-by: Giuseppe Di Natale [email protected]
OpenZFS-issue: https://www.illumos.org/issues/9164
OpenZFS-commit: openzfs/openzfs@0e776dc06a
Checklist:
Signed-off-by
.