Skip to content

Commit f4c6550

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 f4c6550

2 files changed

Lines changed: 191 additions & 161 deletions

File tree

‎CONTRIBUTING.md‎

Lines changed: 187 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,159 @@ 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+
### Spec-mirroring definitions (`nvme_` / `nvmf_`)
2220

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

25171
The project follows the Linux kernel mailing list workflow,
26172
thus commit messages should be structured like this:
@@ -42,6 +188,39 @@ Show new contributors the project's commit guidelines
42188
Signed-off-by: John Doe <[email protected]>
43189
```
44190

45-
### Bug Reports
191+
### How to clean up your series before creating a PR
192+
193+
This example here assumes the changes are in a branch called
194+
fix-something, which branched away from master in the past. In the
195+
meantime the upstream project has changed, hence the fix-something
196+
branch is not based on the current HEAD. Before posting the PR, the
197+
branch should be rebased on the current HEAD and retest everything.
198+
199+
For example, rebasing can be done by the following steps
200+
201+
```shell
202+
# Update master branch
203+
# upstream == https://github.com/linux-nvme/nvme-cli.git
204+
$ git switch master
205+
$ git fetch --all
206+
$ git reset --hard upstream/master
207+
208+
# Make sure all dependencies are up to date and make a sanity build
209+
$ meson subprojects update
210+
$ ninja -C .build
211+
212+
# Go back to the fix-something branch
213+
$ git switch fix-something
214+
215+
# Rebase it to the current HEAD
216+
$ git rebase master
217+
[fixup all merge conflicts]
218+
[retest]
219+
220+
# Push your changes to Github and trigger a PR
221+
$ git push -u origin fix-something
222+
```
223+
224+
## Bug Reports
46225

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

‎README.md‎

Lines changed: 4 additions & 153 deletions
Original file line numberDiff line numberDiff line change
@@ -213,165 +213,16 @@ is available as part of the `meta-openembedded` layer collection.
213213
`nvme-cli` is available as a [buildroot](https://buildroot.org) package. The
214214
package is named `nvme`.
215215

216-
## Developers
217-
218-
You may wish to add a new command or possibly an entirely new plug-in
219-
for some special extension outside the spec.
220-
221-
This project provides macros that help generate the code for you. If
222-
you're interested in how that works, it is very similar to how trace
223-
events are created by Linux kernel's 'ftrace' component.
224-
225-
### Add command to existing built-in
226-
227-
The first thing to do is define a new command entry in the command
228-
list. This is declared in nvme-builtin.h. Simply append a new "ENTRY" into
229-
the list. The ENTRY normally takes three arguments: the "name" of the
230-
subcommand (this is what the user will type at the command line to invoke
231-
your command), a short help description of what your command does, and the
232-
name of the function callback that you're going to write. Additionally,
233-
you can declare an alias name of the subcommand with a fourth argument, if
234-
needed.
235-
236-
After the ENTRY is defined, you need to implement the callback. It takes
237-
four arguments: argc, argv, the command structure associated with the
238-
callback, and the plug-in structure that contains that command. The
239-
prototype looks like this:
240-
241-
```c
242-
int f(int argc, char **argv, struct command *command, struct plugin *plugin);
243-
```
244-
245-
The argc and argv are adjusted from the command line arguments to start
246-
after the sub-command. So if the command line is "nvme foo --option=bar",
247-
the argc is 1 and argv starts at "--option".
248-
249-
You can then define argument parsing for your sub-command's specific
250-
options then do some command-specific action in your callback.
251-
252-
### Add a new plugin
253-
254-
The nvme-cli provides macros to make defining a new plug-in simpler. You
255-
can certainly do all this by hand if you want, but it should be easier
256-
to get going using the macros. To start, first create a header file
257-
to define your plugin. This is where you will give your plugin a name,
258-
description, and define all the sub-commands your plugin implements.
259-
260-
The macros must appear in a specific order within the header file. The following
261-
is a basic example on how to start this:
262-
263-
File: foo-plugin.h
264-
```c
265-
#undef CMD_INC_FILE
266-
#define CMD_INC_FILE plugins/foo/foo-plugin
267-
268-
#if !defined(FOO) || defined(CMD_HEADER_MULTI_READ)
269-
#define FOO
270-
271-
#include "cmd.h"
272-
273-
PLUGIN(NAME("foo", "Foo plugin"),
274-
COMMAND_LIST(
275-
ENTRY("bar", "foo bar", bar)
276-
ENTRY("baz", "foo baz", baz)
277-
ENTRY("qux", "foo qux", qux)
278-
)
279-
);
280-
281-
#endif
282-
283-
#include "define_cmd.h"
284-
```
285-
286-
In order to have the compiler generate the plugin through the xmacro
287-
expansion, you need to include this header in your source file, with
288-
a pre-defining macro directive to create the commands.
289-
290-
To get started from the above example, we just need to define "CREATE_CMD"
291-
and include the header:
292-
293-
File: foo-plugin.c
294-
```c
295-
#include "nvme.h"
296-
297-
#define CREATE_CMD
298-
#include "foo-plugin.h"
299-
```
300-
301-
After that, you just need to implement the functions you defined in each
302-
ENTRY, then append the object file name to the meson.build "sources".
303-
304-
### Updating the libnvme accessor functions
305-
306-
libnvme exposes auto-generated getter/setter accessor functions for its
307-
ABI-stable opaque structs. Two sets of accessors are maintained:
308-
309-
| Set | Input header | Generated files |
310-
|-----|-------------|-----------------|
311-
| Common NVMe | `libnvme/src/nvme/private.h` | `src/nvme/accessors.{h,c}`, `src/accessors.ld` |
312-
| NVMe-oF | `libnvme/src/nvme/private-fabrics.h` | `src/nvme/accessors-fabrics.{h,c}`, `src/accessors-fabrics.ld` |
313-
314-
The generated `.h` and `.c` files are committed to the source tree and are
315-
**not** regenerated during a normal build. To regenerate after modifying a
316-
`/*!generate-accessors*/` struct:
317-
318-
```shell
319-
$ meson compile -C .build update-accessors
320-
```
321-
322-
See [`libnvme/README.md`](libnvme/README.md#accessor-generation) for full
323-
details, including how to maintain the `.ld` version-script files.
324-
325216
## Dependency
326217

327218
libnvme depends on the `/sys/class/nvme-subsystem` interface which was
328219
introduced in Linux kernel v4.15. nvme-cli requires kernel v4.15 or later.
329220

330-
## How to contribute
331-
332-
There are two ways to send code changes to the project. The first one
333-
is by sending the changes to [email protected]. The
334-
second one is by posting a pull request on Github. In both cases
335-
please follow the Linux contributions guidelines as documented in
336-
[Submitting patches](https://docs.kernel.org/process/submitting-patches.html).
221+
## Contributing
337222

338-
That means the changes should be a clean series (no merges should be
339-
present in a Github PR for example) and every commit should build.
340-
341-
See also [How to create a pull request on GitHub](https://opensource.com/article/19/7/create-pull-request-github).
342-
343-
### How to clean up your series before creating a PR
344-
345-
This example here assumes the changes are in a branch called
346-
fix-something, which branched away from master in the past. In the
347-
meantime the upstream project has changed, hence the fix-something
348-
branch is not based on the current HEAD. Before posting the PR, the
349-
branch should be rebased on the current HEAD and retest everything.
350-
351-
For example, rebasing can be done by the following steps
352-
353-
```shell
354-
# Update master branch
355-
# upstream == https://github.com/linux-nvme/nvme-cli.git
356-
$ git switch master
357-
$ git fetch --all
358-
$ git reset --hard upstream/master
359-
360-
# Make sure all dependencies are up to date and make a sanity build
361-
$ meson subprojects update
362-
$ ninja -C .build
363-
364-
# Go back to the fix-something branch
365-
$ git switch fix-something
366-
367-
# Rebase it to the current HEAD
368-
$ git rebase master
369-
[fixup all merge conflicts]
370-
[retest]
371-
372-
# Push your changes to Github and trigger a PR
373-
$ git push -u origin fix-something
374-
```
223+
For information on adding commands, adding plugins, API naming conventions,
224+
commit guidelines, and the pull request workflow, see
225+
[CONTRIBUTING.md](CONTRIBUTING.md).
375226

376227
## Persistent and volatile configuration
377228

0 commit comments

Comments
 (0)