nvmecontrol: Implement Get/Set Feature for nvmecontrol

[wanpengqian_gmail.com]

Summary

sbin/nvmecontrol didn't support Get/Set Feature yet,

this patch will add following sub commands

feature get
feature set
feature summary

Use the same implementation as logpages, support vendor modules.
Intel vendor specific features were also added.

Test Plan

execute each commands and compare the result with Linux's nvme command.

[wanpengqian_gmail.com]

sbin/nvmecontrol/nvmecontrol.c

185 ↗

(On Diff #97992)

Right, you are correct.
Actually, I found the sys/dev/nvme/nvme_qpair.c has already have these defines, and the function a not accessable outside.

static const char *
get_status_string(uint16_t sct, uint16_t sc)
{
        struct nvme_status_string *entry;

        switch (sct) {
        case NVME_SCT_GENERIC:
                entry = generic_status;
                break;
        case NVME_SCT_COMMAND_SPECIFIC:
                entry = command_specific_status;
                break;
        case NVME_SCT_MEDIA_ERROR:
                entry = media_error_status;
                break;
        case NVME_SCT_PATH_RELATED:
                entry = path_related_status;
                break;
        case NVME_SCT_VENDOR_SPECIFIC:
                return ("VENDOR SPECIFIC");
        default:
                return ("RESERVED");
        }

        while (entry->sc != 0xFFFF) {
                if (entry->sc == sc)
                        return (entry->str);
                entry++;
        }
        return (entry->str);
}

Can I make it non-static?

[wanpengqian_gmail.com]

use #define instead of numbers.

[wanpengqian_gmail.com]

copy code from sys/dev/nvme/nvme_qpair.c,
move _SHIFT _MASK difinition to sys/dev/nvme/nvme.h, seperate ticket: D32885
remove these difinitions when D32885 is accepted.

[wanpengqian_gmail.com]

update manpage

[wanpengqian_gmail.com]

fix typo.

[wanpengqian_gmail.com]

Add command examples.

[wanpengqian_gmail.com]

update manpage.

[rpokala]

sbin/nvmecontrol/nvmecontrol.8

267 ↗

(On Diff #98191)

"includes"

270 ↗

(On Diff #98191)

"summary"

271 ↗

(On Diff #98191)

"sub-commands"

317 ↗

(On Diff #98191)

"summary"

I suggest running the manpage through igor; it will identify common misspellings, and also find incorrect tags.

[wanpengqian_gmail.com]

update manpage as comments suggested.
also add print Timestamp for details.

[wanpengqian_gmail.com]

sbin/nvmecontrol/nvmecontrol.8

317 ↗

(On Diff #98191)

Thank you very much. I am happy with this tool.

[rpokala]

sbin/nvmecontrol/feature.c

443 ↗

(On Diff #98285)

"milliseconds"

[wanpengqian_gmail.com]

correct the timestamp output.

[wanpengqian_gmail.com]

Use logpage like procedure to process feature.

[wanpengqian_gmail.com]

add missing feature.c

[wanpengqian_gmail.com]

Fix varible naming issue for passing the build.

[wanpengqian_gmail.com]

sbin/nvmecontrol/feature.c

589 ↗

(On Diff #97992)

Handle features just like logpage. and implement intel p3700's vendor features. It seems to be working.

[wanpengqian_gmail.com]

Fix typo and code style.

[wanpengqian_gmail.com]

sbin/nvmecontrol/feature.c

443 ↗

(On Diff #98285)

Thanks. I always spell millisecond to milisecond. It's my fault.

[pauamma_gundo.com]

sbin/nvmecontrol/nvmecontrol.8

174 ↗

(On Diff #98191)

Same typo as elsewhere.

267 ↗

(On Diff #98191)

Not clear to me whether it should be "... command includes" or "... commands include", but yes, number agreement.

272 ↗

(On Diff #98191)

"a feature" or "features", I think.

302 ↗

(On Diff #98191)

I don't think "shall" as a future imperative is common anymore, and it will likely confuse EFL users.

[wanpengqian_gmail.com]

update as comments suggested.

[wanpengqian_gmail.com]

Fix Possible Spelling Mistake warning.
Update manpage

[pauamma_gundo.com]

Manual page LGTM now. Can't speak about the rest.

[wanpengqian_gmail.com]

seperate intel's feature patch to another review.

[pauamma_gundo.com]

%anual page change still LGTM.

[pauamma_gundo.com]

With these edits, the manual page LGTM at this point; I'll rereview it when it's no longer a WIP.++

sbin/nvmecontrol/nvmecontrol.8

274 ↗

(On Diff #101343)

275 ↗

(On Diff #101343)

276 ↗

(On Diff #101343)

279 ↗

(On Diff #101343)

303 ↗

(On Diff #101343)

[wanpengqian_gmail.com]

• combine all changes to one commit.
• build check with CURRENT source.
• correct manpages

[wanpengqian_gmail.com]

• remove a unnecessary include
• fix typo
• fix code style of switch

[bcr]

Found a line break after a sentence stop. You can run textproc/igor over your man page to find these.
Also, "mandoc -T lint" may also be helpful.

sbin/nvmecontrol/nvmecontrol.8

387 ↗

(On Diff #112317)

You need a line break after the sentence stop here.

[bcr]

Any update on this?