nvme: Add APIs for managing nvme_ctrl instances #19
Conversation
minwooim
force-pushed
the
add-ctrl-handles
branch
4 times, most recently
from
4df38d0 to
be8bd90
Compare
src/nvme/core.c
Outdated
|
|
||
| if (vfio_pci_open(&ctrl->pci, bdf)) | ||
| return -1; | ||
|
|
||
| ctrl->pci.bdf = strdup(bdf); |
There was a problem hiding this comment.
Is this a bug fix on its own?
There was a problem hiding this comment.
Oh, my bad. It should have not been added. Will remove it. Nice catch!
src/nvme/core.c
Outdated
| return -1; | ||
| } | ||
|
|
||
| handle = calloc(1, sizeof(struct nvme_ctrl_handle)); |
There was a problem hiding this comment.
Prefer zmalloc(), or in this case maybe znew_t(struct nvme_ctrl_handle, 1).
| struct nvme_ctrl_handle *handle = nvme_get_ctrl_handle(bdf); | ||
|
|
||
| if (!handle) | ||
| return NULL; |
There was a problem hiding this comment.
Should we set errno to ENODEV here?
There was a problem hiding this comment.
I don't think so. It might be used to check whether the library has the controller or not which means NULL itself does not mean failure.
src/nvme/core.c
Outdated
| struct nvme_ctrl_handle *handle; | ||
|
|
||
| if (nvme_get_ctrl(ctrl->pci.bdf)) { | ||
| errno = EALREADY; |
There was a problem hiding this comment.
EALREADY is usually used for network, no? Not that EEXIST is that much better (referring to files), but we already use that in other places.
There was a problem hiding this comment.
I think EINVAL would be fine since the given bdf is invalid which has already been added.
There was a problem hiding this comment.
EEXIST is also fine to me.
One might want for libvfn to manage nvme_ctrl instances which are initialized by the application inside of the library. This patch introduces nvme_get_ctrl, nvme_add_ctrl and nvme_del_ctrl APIs for application to request controller intance management. Rather than adding list_node to struct nvme_ctrl, it uses struct nvme_ctrl_handle inside of the core.c and open APIs based on the struct nvme_ctrl. Signed-off-by: Minwoo Im <[email protected]>
minwooim
force-pushed
the
add-ctrl-handles
branch
from
be8bd90 to
f9fbb44
Compare
|
@birkelund , Updated commit. |
One might want for libvfn to manage nvme_ctrl instances which are initialized by the application inside of the library. This patch introduces nvme_get_ctrl, nvme_add_ctrl and nvme_del_ctrl APIs for application to request controller intance management. Rather than adding list_node to struct nvme_ctrl, it uses struct nvme_ctrl_handle inside of the core.c and open APIs based on the struct nvme_ctrl.