libnvme: Add direction parameter to get feature length#494
libnvme: Add direction parameter to get feature length#494igaw merged 2 commits intolinux-nvme:masterfrom
Conversation
Codecov Report
@@ Coverage Diff @@
## master #494 +/- ##
==========================================
- Coverage 23.56% 23.53% -0.04%
==========================================
Files 31 31
Lines 5882 5890 +8
Branches 1203 1205 +2
==========================================
Hits 1386 1386
- Misses 4040 4048 +8
Partials 456 456
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
There was a problem hiding this comment.
I think it would be better to introduce an enum for the dir argument. The bool isn't really self explanatory.
Another thing it will break the existing API. One nasty idea is we could abuse the 'len' argument for passing in the dir but this is probably a too big hack.
So I suggest to add nvme_get_feature_length2() which has this new argument and nvme_get_feature_length() just calls nvme_get_feature_length2() with dir=NVME_HOST_MEM_HOST_TO_CTRL.
There was a problem hiding this comment.
Thanks for your comments. About the suggestion mensioned to make sure let me confirm below.
If added nvme_get_feature_length2() but still the original API does not work correctly for the feature ID reported by the issue. Is this really okay?
There was a problem hiding this comment.
Yes, we need keep the nvme_get_feature_length() around, because removing or changing this function is not possible as we promised not to break the API.
There was a problem hiding this comment.
I see. Thanks for the explanation.
There was a problem hiding this comment.
Just pushed the changes as suggested.
ikegami-t
force-pushed
the
set-feature-0dh
branch
2 times, most recently
from
1d1e60a to
7ad759b
Compare
|
Almost, I wanted the functions names to be the other way round. I'll do the update quickly. |
Got confused by the web view. But there are couple of small things I'd like to cleanup. Just a sec. |
Since feature identifier 0Dh: host memory buffer length is affected by the direction as mentioned by the issue #1681. Signed-off-by: Tokunori Ikegami <ikegami.t@gmail.com> [dwagner: reordered definitions and declerations] Signed-off-by: Daniel Wagner <dwagner@suse.de>
Use consistently the fallthrough statement as we have a fallback in util.h in case the compiler doesn't support it. Signed-off-by: Daniel Wagner <dwagner@suse.de>
4 participants
Since feature identifier 0Dh: host memory buffer length is affected by the direction as mentioned by the issue #1681.
Signed-off-by: Tokunori Ikegami ikegami.t@gmail.com