-
Notifications
You must be signed in to change notification settings - Fork 76
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
block/033 fix #138
block/033 fix #138
Conversation
tests/block/033
Outdated
@@ -7,7 +7,7 @@ | |||
# can be covered | |||
|
|||
. tests/block/rc | |||
. common/ublk | |||
. tests/ublk/rc |
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.
This creates dependency from tests/block to tests/ublk, which is not good. Instead, I suggest to take two actions:
- move the UBLK_PROG declarations below from tests/ublk/rc to common/ublk
if which rublk > /dev/null 2>&1; then
export UBLK_PROG="src/rublk_wrapper.sh"
else
export UBLK_PROG="src/miniublk"
fi
- Replace ublk_prog in tests/block/033 with UBLK_PROG.
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.
Thanks, updated as suggested, will open another PR for the misc fix.
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.
@ming1 Please help review this change, thanks.
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 change is fine for me, thanks!
Hi @yizhanglinux , thanks for this PR. |
UBLK_PROG was not defined when calling _have_ublk, move the defination to common/ublk. Replace ublk_prog with UBLK_PROG in block/033. common/fio already included in common/rc, so remove the dup inclusion. $ ./check block/033 block/033 (add & delete ublk device and test if gendisk is leaked) [not run] driver ublk_drv is not available is not available Signed-off-by: Yi Zhang <[email protected]>
00eb843
to
3618665
Compare
@yizhanglinux Thanks! This PR has got merged. FYI, I made a follow-up commit for additional fixes. |
No description provided.