Thomas Weißschuh [Fri, 29 Apr 2022 15:06:27 +0000 (17:06 +0200)]
util: Add feature length for host memory buffer
Without this the command "nvme get-feature /dev/nvme0 -H -f 0x0d"
segfaults because it retrieves no data but tries to use it in
nvme_show_host_mem_buffer()
Hannes Reinecke [Fri, 22 Apr 2022 09:42:47 +0000 (11:42 +0200)]
tree: coverity fixes
Coverity found some resource leaks and a possible out-of-bounds access
in nvme_ctrl_lookup_subsystem_name() where we could overflow the
'path' variable.
Hannes Reinecke [Fri, 22 Apr 2022 09:37:19 +0000 (11:37 +0200)]
fabrics: coverity fixes
Coverity reported a possible out-of-bounds access in inet6_pton
and we never checked the return value of nvme_ctrl_identify() in
nvme_fetch_cntrltype_dctype_from_id(), leading to a possible
NULL pointer access in nvmf_is_registration_supported().
Eli Schwartz [Fri, 15 Apr 2022 05:06:56 +0000 (01:06 -0400)]
reduce the exposed interface for libnvme_dep
It does not publicly expose openssl. It does publicly expose uuid.h and
json.h, but only in terms of header types, not symbols used... so use a
partial dependency that doesn't expose the link libraries.
Eli Schwartz [Fri, 15 Apr 2022 00:23:40 +0000 (20:23 -0400)]
fix broken includes for the uuid.h header
Per `man 3 uuid`, the correct include is `<uuid.h>`, and correspondingly
the util-linux build system installs the uuid.h file to
`$(includedir)/uuid/` and uuid.pc adds `-I${includedir}/uuid` to the
dependency Cflags.
Including it as `<uuid/uuid.h>` simply fails altogether. The exception
is if, by coincidence, the libuuid includedir happens to be the same as
one of the compiler's own internal include directories... which is
pretty common if that is `/usr/include`. In this case, `<uuid/uuid.h>`
fails to be found in the uuid dependency's export directories, but the
compiler picks up a copy in its builtin search path.
In other cases, such as when util-linux is compiled into a custom prefix
and added to the PKG_CONFIG_PATH, it either:
- fails with missing header errors, if libuuid-dev is not installed
- seems to succeed, but compiles against a different version of the
headers than the library that is linked to, resulting in potentially
various problems from bizarre linker errors, to subtly broken
incompatible symbols that crash, or corrupt the results
The problem is even worse if you try to use util-linux as a subproject
dependency fallback. In this case, `<uuid.h>` is correctly exported, but
there is no disk location for `<uuid/uuid.h>` and absolutely no way to
generate such a location in the builddir. This completely prevents the
use of subproject fallbacks.
Once the header include is renamed, it is revealed that the example and
test executables don't correctly build as they depend on the full public
interface of libnvme, which includes the public compile interface of
libuuid due to `libnvme.h` including `uuid.h`. Update all executables to
depend on the full public interface of libnvme_dep instead of simply
linking to the library name on its own.
Daniel Wagner [Thu, 14 Apr 2022 10:18:53 +0000 (12:18 +0200)]
fabrics: Remove double connection error logging
nvmf_connect_disc_entry() is calling nvmf_add_ctrl() to do the
connecting attempt. We have in nvmf_add_ctrl() all error paths logging
enabled, so another logging in nvmf_connect_disc_entry() is not
necessary and introduces a problem such as nmve-cli users interpreting
this as hard error.
For example the kernel reports via < 0 return values state information
such as the connection is already there if the user runs 'nvme
connect-all' twice:
$ .build/nvme connect-all -t tcp --traddr=10.161.8.24 --trsvcid=4421
Failed to write to /dev/nvme-fabrics: Operation already in progress
failed to connect controller, error 1006
Daniel Wagner [Thu, 14 Apr 2022 10:01:43 +0000 (12:01 +0200)]
fabrics: Lower log level in __nvmf_add_ctrl
9df8676d2db6 ("Add logging functionality to libnvme") introduced the
logging. The log level for writing to /dev/nvme-fabrics was set on
NOTICE because the kernel reports also status information such
EALREADY which is a soft error.
866c288a456f ("fabrics: update log level for write failures")
increased the log level without taking this into account.
Drop back to NOTICE level to avoid error message when doing
$ nvme connect-all ....
Failed to write to /dev/nvme-fabrics: Operation already in progress
Daniel Wagner [Tue, 12 Apr 2022 08:52:35 +0000 (10:52 +0200)]
ci: publish only sdist on PyPi
Pre building the binaries with wheels is difficult, as we would also
need to ship the libnvme.so with the Python binding.
This is a well known limitation of this kind of setup (binding to a
shared library). Most project fallback to just ship the sdist.
Though there is a drawback as the C library is missing and the user
has to provide the library himself, with all problems which come along
with setup. But it seems common practice with other Python bindings,
so we don't want to be hostile to the ones which know what they are
doing.
Normal users should just use the distribution packages anyway.
Daniel Wagner [Tue, 12 Apr 2022 17:00:33 +0000 (19:00 +0200)]
doc: Build ReST documantion as standalone target
In order to feed readthedocs with documecation we have to build the
ReST documentation and store it the source tree.
To avoid cluttering the root dir of the documantion directory move the
generation of the *.rst docs into the rst subdir. Unfortunatly, meson
doesn't have an option to generate the newly files into subdir. So
move the rst build instruction into rst/meson.build.
While at it, split the whole documentation into sections.
We want to replace any VERSION string in the documantion. Instead just
addressing conf.py we process all files in one go for consistency reasons.
Daniel Wagner [Tue, 12 Apr 2022 11:24:27 +0000 (13:24 +0200)]
build: Add git ref to the binary
In order to be able to figure out which binary is in use (for example
in debugging situation) it's really helpful to have the 'git describe'
ref added to the binary.
The simplest way to expose the it via good old Source Code Control
System string id. No need to come up with something complicated else
as there is no agreement on how to do this. So let's add this simple
magic string to library.
Daniel Wagner [Mon, 4 Apr 2022 15:36:20 +0000 (17:36 +0200)]
tree: Remove default port setting for TCP and RDMA ports
When the controller is created, the discovery_ctrl attribute is not
set yet. The caller is supposed to set this after the creation of the
new discovery controller. Thus we can't really set the default port at
this level. This is a bit of a chicken-egg situation.
Move this work up to the caller to provide the correct defaults.
Hannes Reinecke [Fri, 1 Apr 2022 12:29:02 +0000 (14:29 +0200)]
tree: update nvme_scan_filter_t usage
Add two more arguments (nvme_ctrl_t and nvme_ns_t) to the nvme scan
filter to allow the implementation to distinguish between the call sites.
This allows for a more targeted approach when implementing filters.
Hannes Reinecke [Fri, 1 Apr 2022 11:28:11 +0000 (13:28 +0200)]
tree: always allocate config file in nvme_read_config()
Even if there was an error when reading the config file we still
should store the config file name, as it might used later on to
update/write the config file.
So parsing errors don't really matter, and we'll get notified for
I/O errors on writing anyway.
Martin Belanger [Thu, 31 Mar 2022 11:07:01 +0000 (07:07 -0400)]
fabrics: Invoke nvmf_dim() with provided tas argument
Looks like a copy-paste issue. One of the arguments to
nvmf_register_ctrl() is "tas" (i.e. the DIM Task). This argument
should be passed to the nvmf_dim() API, but instead a fixed value
of NVMF_DIM_TAS_REGISTER was passed.
The "tas" field specifies whether to perform a "registration",
"deregistration", or a "registration update".
Signed-off-by: Martin Belanger <martin.belanger@dell.com>
Hannes Reinecke [Wed, 30 Mar 2022 15:00:48 +0000 (17:00 +0200)]
tree: rework nvme_scan_subsystem()
The nvme subsystem does not have a 'hostnqn' sysfs entry, so we
cannot infer from the nvme subsystem sysfs entry to which host
it relates. And really, the subsystem should already have been
created by the previous call to nvme_scan_ctrl().
So do not call nvme_lookup_subsystem() in nvme_scan_subystem(),
but rather just validate the sysfs subsystem entries and create
any missing subsystems.
Hannes Reinecke [Wed, 30 Mar 2022 14:42:18 +0000 (16:42 +0200)]
tree: move nvme_init_subsystem() into nvme_lookup_subsystem()
We're always calling nvme_init_subsystem() when nvme_lookup_subsystem()
is called with a non-NULL 'name' parameter. So we might as well move
it into nvme_lookup_subsystem() and simplify the callers.
Daniel Wagner [Wed, 30 Mar 2022 07:38:03 +0000 (09:38 +0200)]
ioctl: Remove attribute packed and alignedof for args structs
The attribute packed is usually used to make sure the data structures
is compatible between different compilers in regards of padding rules.
As we have sorted the members of all argument structs according their
naturual size, there are no holes to pad. This makes the packed
attributed superflous as compilers are agree on the data layout in
this case.
The alignedof attribute is used to tell the outer alignmen of the data
structure because the aligment of a packed data structure is 1.
Anyway, both attributes doen't add any benefits to the layout and
pahole agrees on this.
This is an example the diff between the packed/aligneof version and
the plain version:
projects / users / sagi / libnvme.git / log