Skip to content

Commit 198224d

Browse files
Martin Belangerigaw
authored andcommitted
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 198224d

4 files changed

Lines changed: 473 additions & 566 deletions

File tree

CONTRIBUTING.md

Lines changed: 258 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,26 +1,237 @@
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

9-
**NOTE: If you do decide to implement code changes and contribute them,
10-
please make sure you agree your contribution can be made available
11-
under the [GPLv2-style License used for the NVMe CLI](https://github.com/linux-nvme/nvme-cli/blob/master/LICENSE).
12-
(SPDX-License-Identifier: GPL-2.0-or-later)**
9+
This repository contains two components with different licenses:
1310

14-
Because there are a few files licensed under GPL-2.0-only, the whole
15-
project is tagged as GPL-2.0-only and not as GPL-2.0-or-later.
11+
| Component | License | SPDX identifier |
12+
|-----------|---------|-----------------|
13+
| nvme-cli (CLI and plugins) | GNU General Public License v2 or later | `GPL-2.0-or-later` |
14+
| libnvme (library) | GNU Lesser General Public License v2.1 or later | `LGPL-2.1-or-later` |
1615

17-
### Code Contributions
16+
When contributing, use the appropriate SPDX identifier for the component you
17+
are modifying. New files under `libnvme/` should carry `LGPL-2.1-or-later`;
18+
new files in the CLI or plugins should carry `GPL-2.0-or-later`.
1819

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.
20+
## API naming conventions
2221

23-
#### Commit conventions
22+
### Spec-mirroring definitions (`nvme_` / `nvmf_`)
23+
24+
Types, structs, and enums that directly mirror the NVMe specifications use the
25+
short `nvme_` (base spec) and `nvmf_` (NVMe-oF spec) prefixes. These live in
26+
`libnvme/src/nvme/types.h` and `libnvme/src/nvme/cmds.h` and reflect the
27+
specification naming — they are data-layout definitions, not library API.
28+
29+
### libnvme public API (`libnvme_` / `libnvmf_`)
30+
31+
This is where the naming convention is enforced. libnvme is a shared library
32+
with a stable public ABI, so every public symbol must carry the correct prefix
33+
so that callers can immediately tell what they are working with.
34+
35+
| Prefix | Scope | Examples |
36+
|--------|-------|---------|
37+
| `libnvme_` | Common NVMe (PCIe and NVMe-oF) | `libnvme_open()`, `libnvme_create_global_ctx()`, `libnvme_first_host()`, `libnvme_ctrl_identify()` |
38+
| `libnvmf_` | NVMe-oF only | `libnvmf_connect_ctrl()`, `libnvmf_add_ctrl()`, `libnvmf_get_discovery_log()`, `libnvmf_trtype_str()` |
39+
40+
The split is enforced by two separate linker version scripts:
41+
`libnvme/src/libnvme.ld` exports all `libnvme_*` symbols and
42+
`libnvme/src/libnvmf.ld` exports all `libnvmf_*` symbols. Both are passed to
43+
the linker when building `libnvme.so`.
44+
45+
When contributing new functions to libnvme, choose the prefix based on scope:
46+
- Use `libnvme_` if the function applies to both PCIe and NVMe-oF controllers.
47+
- Use `libnvmf_` if the function is specific to NVMe-oF (fabrics transport,
48+
discovery, connect/disconnect).
49+
50+
## Adding commands and plugins
51+
52+
You may wish to add a new command or possibly an entirely new plug-in
53+
for some special extension outside the spec.
54+
55+
This project provides macros that help generate the code for you. If
56+
you're interested in how that works, it is very similar to how trace
57+
events are created by Linux kernel's 'ftrace' component.
58+
59+
### Add a command to the existing built-in
60+
61+
The first thing to do is define a new command entry in the command
62+
list. This is declared in nvme-builtin.h. Simply append a new "ENTRY" into
63+
the list. The ENTRY normally takes three arguments: the "name" of the
64+
subcommand (this is what the user will type at the command line to invoke
65+
your command), a short help description of what your command does, and the
66+
name of the function callback that you're going to write. Additionally,
67+
you can declare an alias name of the subcommand with a fourth argument, if
68+
needed.
69+
70+
After the ENTRY is defined, you need to implement the callback. It takes
71+
four arguments: argc, argv, the command structure associated with the
72+
callback, and the plug-in structure that contains that command. The
73+
prototype looks like this:
74+
75+
```c
76+
int f(int argc, char **argv, struct command *command, struct plugin *plugin);
77+
```
78+
79+
The argc and argv are adjusted from the command line arguments to start
80+
after the sub-command. So if the command line is "nvme foo --option=bar",
81+
the argc is 1 and argv starts at "--option".
82+
83+
You can then define argument parsing for your sub-command's specific
84+
options then do some command-specific action in your callback.
85+
86+
### Add a new plugin
87+
88+
The nvme-cli provides macros to make defining a new plug-in simpler. You
89+
can certainly do all this by hand if you want, but it should be easier
90+
to get going using the macros. To start, first create a header file
91+
to define your plugin. This is where you will give your plugin a name,
92+
description, and define all the sub-commands your plugin implements.
93+
94+
The macros must appear in a specific order within the header file. The following
95+
is a basic example on how to start this:
96+
97+
File: foo-plugin.h
98+
```c
99+
#undef CMD_INC_FILE
100+
#define CMD_INC_FILE plugins/foo/foo-plugin
101+
102+
#if !defined(FOO) || defined(CMD_HEADER_MULTI_READ)
103+
#define FOO
104+
105+
#include "cmd.h"
106+
107+
PLUGIN(NAME("foo", "Foo plugin"),
108+
COMMAND_LIST(
109+
ENTRY("bar", "foo bar", bar)
110+
ENTRY("baz", "foo baz", baz)
111+
ENTRY("qux", "foo qux", qux)
112+
)
113+
);
114+
115+
#endif
116+
117+
#include "define_cmd.h"
118+
```
119+
120+
In order to have the compiler generate the plugin through the xmacro
121+
expansion, you need to include this header in your source file, with
122+
a pre-defining macro directive to create the commands.
123+
124+
To get started from the above example, we just need to define "CREATE_CMD"
125+
and include the header:
126+
127+
File: foo-plugin.c
128+
```c
129+
#include "nvme.h"
130+
131+
#define CREATE_CMD
132+
#include "foo-plugin.h"
133+
```
134+
135+
After that, you just need to implement the functions you defined in each
136+
ENTRY, then append the object file name to the meson.build "sources".
137+
138+
### Updating the libnvme accessor functions
139+
140+
libnvme exposes auto-generated getter/setter accessor functions for its
141+
ABI-stable opaque structs. Two sets of accessors are maintained — one for
142+
common NVMe structs and one for NVMe-oF-specific structs. The split exists so
143+
that non-fabrics (e.g. embedded or PCIe-only) builds can exclude all fabrics
144+
code entirely.
145+
146+
| Meson target | Input header | Generated files |
147+
|---|---|---|
148+
| `update-common-accessors` | `libnvme/src/nvme/private.h` | `libnvme/src/nvme/accessors.{h,c}`, `libnvme/src/accessors.ld` |
149+
| `update-fabrics-accessors` | `libnvme/src/nvme/private-fabrics.h` | `libnvme/src/nvme/accessors-fabrics.{h,c}`, `libnvme/src/accessors-fabrics.ld` |
150+
151+
The generated `.h` and `.c` files are committed to the source tree and are
152+
**not** regenerated during a normal build.
153+
154+
#### When to regenerate
155+
156+
Regeneration is needed whenever a `/*!generate-accessors*/` struct in
157+
`private.h` or `private-fabrics.h` has a member added, removed, or renamed.
158+
159+
#### How to regenerate
160+
161+
To regenerate both sets at once:
162+
163+
```shell
164+
$ meson compile -C .build update-accessors
165+
```
166+
167+
Or regenerate only one set:
168+
169+
```shell
170+
$ meson compile -C .build update-common-accessors
171+
$ meson compile -C .build update-fabrics-accessors
172+
```
173+
174+
The script atomically updates the `.h` and `.c` files when their content
175+
changes. Commit the updated files afterward:
176+
177+
```shell
178+
$ git add libnvme/src/nvme/accessors.h libnvme/src/nvme/accessors.c
179+
$ git add libnvme/src/nvme/accessors-fabrics.h libnvme/src/nvme/accessors-fabrics.c
180+
$ git commit -m "libnvme: regenerate accessors following <struct> changes"
181+
```
182+
183+
#### Maintaining the .ld version-script files
184+
185+
The `.ld` files (`libnvme/src/accessors.ld` and
186+
`libnvme/src/accessors-fabrics.ld`) are GNU linker version scripts that
187+
control which accessor symbols are exported from the shared library and under
188+
which ABI version label they were introduced (e.g. `LIBNVME_ACCESSORS_3`,
189+
`LIBNVMF_ACCESSORS_3`).
190+
191+
These files are **not** updated automatically, because each new symbol must be
192+
placed in the correct version section by the maintainer. Adding a symbol to an
193+
already-published version section would break binary compatibility for
194+
existing users of the library.
195+
196+
When the generator detects that the symbol list has drifted, it prints a
197+
report like the following:
198+
199+
```
200+
WARNING: accessors.ld needs manual attention.
201+
202+
Symbols to ADD (new version section, e.g. LIBNVME_ACCESSORS_X_Y):
203+
libnvme_ctrl_get_new_field
204+
libnvme_ctrl_set_new_field
205+
```
206+
207+
New symbols must be added to a **new** version section that chains the
208+
previous one. For example, if the current latest section is
209+
`LIBNVME_ACCESSORS_3`, add:
210+
211+
```
212+
LIBNVME_ACCESSORS_4 {
213+
global:
214+
libnvme_ctrl_get_new_field;
215+
libnvme_ctrl_set_new_field;
216+
} LIBNVME_ACCESSORS_3;
217+
```
218+
219+
Then commit the updated `.ld` file together with the regenerated source files.
220+
221+
## Submitting changes
222+
223+
There are two ways to send code changes to the project. The first one
224+
is by sending the changes to [email protected]. The
225+
second one is by posting a pull request on Github. In both cases
226+
please follow the Linux contributions guidelines as documented in
227+
[Submitting patches](https://docs.kernel.org/process/submitting-patches.html).
228+
229+
That means the changes should be a clean series (no merges should be
230+
present in a Github PR for example) and every commit should build.
231+
232+
See also [How to create a pull request on GitHub](https://opensource.com/article/19/7/create-pull-request-github).
233+
234+
### Commit conventions
24235

25236
The project follows the Linux kernel mailing list workflow,
26237
thus commit messages should be structured like this:
@@ -42,6 +253,39 @@ Show new contributors the project's commit guidelines
42253
Signed-off-by: John Doe <[email protected]>
43254
```
44255

45-
### Bug Reports
256+
### How to clean up your series before creating a PR
257+
258+
This example here assumes the changes are in a branch called
259+
fix-something, which branched away from master in the past. In the
260+
meantime the upstream project has changed, hence the fix-something
261+
branch is not based on the current HEAD. Before posting the PR, the
262+
branch should be rebased on the current HEAD and retest everything.
263+
264+
For example, rebasing can be done by the following steps
265+
266+
```shell
267+
# Update master branch
268+
# upstream == https://github.com/linux-nvme/nvme-cli.git
269+
$ git switch master
270+
$ git fetch --all
271+
$ git reset --hard upstream/master
272+
273+
# Make sure all dependencies are up to date and make a sanity build
274+
$ meson subprojects update
275+
$ ninja -C .build
276+
277+
# Go back to the fix-something branch
278+
$ git switch fix-something
279+
280+
# Rebase it to the current HEAD
281+
$ git rebase master
282+
[fixup all merge conflicts]
283+
[retest]
284+
285+
# Push your changes to Github and trigger a PR
286+
$ git push -u origin fix-something
287+
```
288+
289+
## Bug Reports
46290

47291
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)