RFC: xnvme backend support #600
Conversation
Signed-off-by: Vikash kumar <[email protected]>
Signed-off-by: Vikash kumar <[email protected]>
Extend the api-types with the following device-handle types: * struct dev_handle * enum nvme_dev_type The 'dev_handle' provides a, possibly opaque, device handle instead of a "fixed" file-descriptor. This allows for non-OS managed device-types such as user-space NVMe-drivers. Additionally, the types allows for library-side dispatch. This is done in preparation for xNVMe, and thereby, support for user-space NVMe driver, io_uring_cmd etc. Signed-off-by: Vikash kumar <[email protected]>
This provides the means to use nvme-cli/libnvme via user-space NVMe drivers, io_uring command on Linux. As well as using nvme-cli natively on FreeBSD. This is done by dispatching commands to xNVMe. TODO: * This should be opt-in, e.g. use if it is available, otherwise do something else. Signed-off-by: Vikash kumar <[email protected]>
| @@ -469,7 +469,7 @@ nvme_ns_t nvme_subsystem_next_ns(nvme_subsystem_t s, nvme_ns_t n); | |||
| * | |||
| * Return: File descriptor associated with @n or -1 | |||
| */ | |||
| int nvme_ns_get_fd(nvme_ns_t n); | |||
| void* nvme_ns_get_fd(nvme_ns_t n); | |||
There was a problem hiding this comment.
The function name should also be updated alongside. Also I'd like to see a proper type returned as handle.
| @@ -38,7 +38,8 @@ static int test_ctrl(nvme_ctrl_t c) | |||
| static __u8 buf[0x1000]; | |||
|
|
|||
| enum nvme_get_features_sel sel = NVME_GET_FEATURES_SEL_CURRENT; | |||
| int ret, temp, fd = nvme_ctrl_get_fd(c); | |||
| int ret, temp; | |||
| struct dev_handle *hnd = nvme_ctrl_get_fd(c); | |||
There was a problem hiding this comment.
I prefer hdl as short hand for handle, but I don't have a strong opinion on this.
| @@ -32,6 +32,7 @@ endif | |||
| deps = [ | |||
| json_c_dep, | |||
| openssl_dep, | |||
| libxnvme_dep, | |||
There was a problem hiding this comment.
Without introducing the dependency object libxnvme_dep this will not work.
| @@ -971,9 +972,7 @@ static void __nvme_free_ctrl(nvme_ctrl_t c) | |||
| { | |||
| struct nvme_path *p, *_p; | |||
| struct nvme_ns *n, *_n; | |||
|
|
|||
There was a problem hiding this comment.
Please avoid changing white space changes here (btw, this new line here is okay)
|
Also please update the MI interfaces accordingly, see #448 |
| @@ -61,6 +61,9 @@ else | |||
| endif | |||
| conf.set('CONFIG_JSONC', json_c_dep.found(), description: 'Is json-c required?') | |||
|
|
|||
| # Check for libxnvme availability | |||
| libxnvme_dep = dependency('xnvme', version: '>=0.2.0', required: true) | |||
There was a problem hiding this comment.
Please make this dependency optional, this might pull in whole set of secondary dependencies that might not be suitable for some systems. Think of initramfs usage, different init systems (linux is not only about systemd) and constrained embedded environments.
| @@ -353,9 +353,11 @@ nvme_path_t nvme_namespace_next_path(nvme_ns_t ns, nvme_path_t p) | |||
| static void __nvme_free_ns(struct nvme_ns *n) | |||
| { | |||
| list_del_init(&n->entry); | |||
| close(n->fd); | |||
| struct dev_handle *hnd = n->hnd; | |||
There was a problem hiding this comment.
Declarations mixed inside the code.
|
Created a new PR addressing review comments. |
Extend the api-types with the following device-handle types:
The 'dev_handle' provides a, possibly opaque, device handle instead of a
"fixed" file-descriptor. This allows for non-OS managed device-types
such as user-space NVMe-drivers. Additionally, the types allows for
library-side dispatch.
This is done in preparation for xNVMe, and thereby, support for
user-space NVMe driver, io_uring_cmd etc.
This provides the means to use nvme-cli/libnvme via user-space NVMe
drivers, io_uring command on Linux. As well as using nvme-cli natively
on FreeBSD. This is done by dispatching commands to xNVMe.