Skip to content

Commit baa2c2a

Browse files
author
Martin Belanger
committed
doc: consolidate contributor guidance into CONTRIBUTING.md
README.md was mixing user-facing content (build instructions, configuration, distro support) with developer-facing content (how to add commands, how to add plugins, API naming conventions, PR workflow). Readers looking to contribute had no obvious starting point. Move the three developer-facing sections — "Developers", "API naming conventions", and "How to contribute" — into CONTRIBUTING.md, where they join and supersede the existing commit-conventions and bug-report stubs. Replace all three sections in README.md with a short "Contributing" paragraph that links to CONTRIBUTING.md. While reorganizing CONTRIBUTING.md, document the new API naming convention (libnvme_/libnvmf_ for libnvme, nvme_/nvmf_ for nvme-cli) so contributors know which prefix to use when adding new functions. Signed-off-by: Martin Belanger <[email protected]>
1 parent da2f1b1 commit baa2c2a

2 files changed

Lines changed: 210 additions & 161 deletions

File tree

CONTRIBUTING.md

Lines changed: 206 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
<!-- SPDX-License-Identifier: GPL-2.0-only -->
2-
# Contributing to the NVM-e CLI
2+
# Contributing to nvme-cli and libnvme
33

44
Here you will find instructions on how to contribute to the NVM-Express command
5-
line interface.
5+
line interface and the libnvme library.
66

77
Contributions and new ideas are most welcome!
88

@@ -14,13 +14,178 @@ under the [GPLv2-style License used for the NVMe CLI](https://github.com/linux-n
1414
Because there are a few files licensed under GPL-2.0-only, the whole
1515
project is tagged as GPL-2.0-only and not as GPL-2.0-or-later.
1616

17-
### Code Contributions
17+
## API naming conventions
1818

19-
Please feel free to use the github forums to ask for comments & questions on
20-
your code before submitting a pull request. The NVMe CLI project uses the
21-
common *fork and merge* workflow used by most GitHub-hosted projects.
19+
Both components in this repository follow a strict prefix convention so that
20+
the origin and scope of every symbol is immediately clear to a reader.
2221

23-
#### Commit conventions
22+
### libnvme
23+
24+
The libnvme library exposes two disjoint sets of public symbols, distinguished
25+
by their prefix:
26+
27+
| Prefix | Scope | Examples |
28+
|--------|-------|---------|
29+
| `libnvme_` | Common NVMe (PCIe and NVMe-oF) | `libnvme_open()`, `libnvme_create_global_ctx()`, `libnvme_first_host()`, `libnvme_ctrl_identify()` |
30+
| `libnvmf_` | NVMe-oF only | `libnvmf_connect_ctrl()`, `libnvmf_add_ctrl()`, `libnvmf_get_discovery_log()`, `libnvmf_trtype_str()` |
31+
32+
The split is enforced by two separate version scripts: `libnvme/src/libnvme.ld`
33+
exports all `libnvme_*` symbols, and `libnvme/src/libnvmf.ld` exports all
34+
`libnvmf_*` symbols. Both scripts are passed to the linker when building
35+
`libnvme.so`.
36+
37+
> **Note:** NVMe-spec-defined type names (structs and enums that mirror the
38+
> specification) use the shorter `nvmf_` prefix without the `lib` part (e.g.,
39+
> `nvmf_disc_log_entry`, `nvmf_discovery_log`). These are data-layout types,
40+
> not library API functions, and follow the spec naming rather than the library
41+
> naming convention.
42+
43+
### nvme-cli (the `nvme` executable)
44+
45+
Functions internal to the `nvme` executable follow the same two-prefix rule:
46+
47+
| Prefix | Scope | Examples |
48+
|--------|-------|---------|
49+
| `nvme_` | Common NVMe (PCIe and NVMe-oF) | `nvme_show_id_ctrl()`, `nvme_get_log_smart()`, `nvme_identify_ctrl()`, `nvme_show_status()` |
50+
| `nvmf_` | NVMe-oF only | `nvmf_connect()`, `nvmf_disconnect()`, `nvmf_discover()` |
51+
52+
### Summary
53+
54+
| Component | Common NVMe prefix | NVMe-oF prefix |
55+
|-----------|--------------------|----------------|
56+
| libnvme | `libnvme_` | `libnvmf_` |
57+
| nvme-cli | `nvme_` | `nvmf_` |
58+
59+
When contributing new functions, choose the prefix that matches both the
60+
component you are modifying and the scope of the functionality:
61+
- If the function works for both PCIe and NVMe-oF controllers, use the common
62+
prefix (`libnvme_` or `nvme_`).
63+
- If the function is specific to NVMe-oF (fabrics transport, discovery,
64+
connect/disconnect), use the NVMe-oF prefix (`libnvmf_` or `nvmf_`).
65+
66+
## Adding commands and plugins
67+
68+
You may wish to add a new command or possibly an entirely new plug-in
69+
for some special extension outside the spec.
70+
71+
This project provides macros that help generate the code for you. If
72+
you're interested in how that works, it is very similar to how trace
73+
events are created by Linux kernel's 'ftrace' component.
74+
75+
### Add a command to the existing built-in
76+
77+
The first thing to do is define a new command entry in the command
78+
list. This is declared in nvme-builtin.h. Simply append a new "ENTRY" into
79+
the list. The ENTRY normally takes three arguments: the "name" of the
80+
subcommand (this is what the user will type at the command line to invoke
81+
your command), a short help description of what your command does, and the
82+
name of the function callback that you're going to write. Additionally,
83+
you can declare an alias name of the subcommand with a fourth argument, if
84+
needed.
85+
86+
After the ENTRY is defined, you need to implement the callback. It takes
87+
four arguments: argc, argv, the command structure associated with the
88+
callback, and the plug-in structure that contains that command. The
89+
prototype looks like this:
90+
91+
```c
92+
int f(int argc, char **argv, struct command *command, struct plugin *plugin);
93+
```
94+
95+
The argc and argv are adjusted from the command line arguments to start
96+
after the sub-command. So if the command line is "nvme foo --option=bar",
97+
the argc is 1 and argv starts at "--option".
98+
99+
You can then define argument parsing for your sub-command's specific
100+
options then do some command-specific action in your callback.
101+
102+
### Add a new plugin
103+
104+
The nvme-cli provides macros to make defining a new plug-in simpler. You
105+
can certainly do all this by hand if you want, but it should be easier
106+
to get going using the macros. To start, first create a header file
107+
to define your plugin. This is where you will give your plugin a name,
108+
description, and define all the sub-commands your plugin implements.
109+
110+
The macros must appear in a specific order within the header file. The following
111+
is a basic example on how to start this:
112+
113+
File: foo-plugin.h
114+
```c
115+
#undef CMD_INC_FILE
116+
#define CMD_INC_FILE plugins/foo/foo-plugin
117+
118+
#if !defined(FOO) || defined(CMD_HEADER_MULTI_READ)
119+
#define FOO
120+
121+
#include "cmd.h"
122+
123+
PLUGIN(NAME("foo", "Foo plugin"),
124+
COMMAND_LIST(
125+
ENTRY("bar", "foo bar", bar)
126+
ENTRY("baz", "foo baz", baz)
127+
ENTRY("qux", "foo qux", qux)
128+
)
129+
);
130+
131+
#endif
132+
133+
#include "define_cmd.h"
134+
```
135+
136+
In order to have the compiler generate the plugin through the xmacro
137+
expansion, you need to include this header in your source file, with
138+
a pre-defining macro directive to create the commands.
139+
140+
To get started from the above example, we just need to define "CREATE_CMD"
141+
and include the header:
142+
143+
File: foo-plugin.c
144+
```c
145+
#include "nvme.h"
146+
147+
#define CREATE_CMD
148+
#include "foo-plugin.h"
149+
```
150+
151+
After that, you just need to implement the functions you defined in each
152+
ENTRY, then append the object file name to the meson.build "sources".
153+
154+
### Updating the libnvme accessor functions
155+
156+
libnvme exposes auto-generated getter/setter accessor functions for its
157+
ABI-stable opaque structs. Two sets of accessors are maintained:
158+
159+
| Set | Input header | Generated files |
160+
|-----|-------------|-----------------|
161+
| Common NVMe | `libnvme/src/nvme/private.h` | `src/nvme/accessors.{h,c}`, `src/accessors.ld` |
162+
| NVMe-oF | `libnvme/src/nvme/private-fabrics.h` | `src/nvme/accessors-fabrics.{h,c}`, `src/accessors-fabrics.ld` |
163+
164+
The generated `.h` and `.c` files are committed to the source tree and are
165+
**not** regenerated during a normal build. To regenerate after modifying a
166+
`/*!generate-accessors*/` struct:
167+
168+
```shell
169+
$ meson compile -C .build update-accessors
170+
```
171+
172+
See [`libnvme/README.md`](libnvme/README.md#accessor-generation) for full
173+
details, including how to maintain the `.ld` version-script files.
174+
175+
## Submitting changes
176+
177+
There are two ways to send code changes to the project. The first one
178+
is by sending the changes to [email protected]. The
179+
second one is by posting a pull request on Github. In both cases
180+
please follow the Linux contributions guidelines as documented in
181+
[Submitting patches](https://docs.kernel.org/process/submitting-patches.html).
182+
183+
That means the changes should be a clean series (no merges should be
184+
present in a Github PR for example) and every commit should build.
185+
186+
See also [How to create a pull request on GitHub](https://opensource.com/article/19/7/create-pull-request-github).
187+
188+
### Commit conventions
24189

25190
The project follows the Linux kernel mailing list workflow,
26191
thus commit messages should be structured like this:
@@ -42,6 +207,39 @@ Show new contributors the project's commit guidelines
42207
Signed-off-by: John Doe <[email protected]>
43208
```
44209

45-
### Bug Reports
210+
### How to clean up your series before creating a PR
211+
212+
This example here assumes the changes are in a branch called
213+
fix-something, which branched away from master in the past. In the
214+
meantime the upstream project has changed, hence the fix-something
215+
branch is not based on the current HEAD. Before posting the PR, the
216+
branch should be rebased on the current HEAD and retest everything.
217+
218+
For example, rebasing can be done by the following steps
219+
220+
```shell
221+
# Update master branch
222+
# upstream == https://github.com/linux-nvme/nvme-cli.git
223+
$ git switch master
224+
$ git fetch --all
225+
$ git reset --hard upstream/master
226+
227+
# Make sure all dependencies are up to date and make a sanity build
228+
$ meson subprojects update
229+
$ ninja -C .build
230+
231+
# Go back to the fix-something branch
232+
$ git switch fix-something
233+
234+
# Rebase it to the current HEAD
235+
$ git rebase master
236+
[fixup all merge conflicts]
237+
[retest]
238+
239+
# Push your changes to Github and trigger a PR
240+
$ git push -u origin fix-something
241+
```
242+
243+
## Bug Reports
46244

47245
Bugs for the NVM Library project are tracked in our [GitHub Issues Database](https://github.com/linux-nvme/nvme-cli/issues).

0 commit comments

Comments
 (0)