On Fri, Sep 14, 2018 at 02:50:59PM +0200, Martin Wilck wrote:
Reviewed-by: Benjamin Marzinski <bmarzins@xxxxxxxxxx>
> The path detection logic of the NVMe code relies on the "slaves"
> symlinks from NVMe subsys to controllers in sysfs, which have
> been removed in the 4.16 kernel.
>
> With this patch, we use the symlinks on the NVMe subsys level
> instead.
>
> Example: a multipathed NVMe subsystem with 2 controllers:
>
> /sys/devices/virtual/nvme-subsystem/nvme-subsys2/nvme2n1
> /sys/devices/virtual/nvme-fabrics/ctl/nvme2/nvme2c6n1
> /sys/devices/virtual/nvme-fabrics/ctl/nvme3/nvme2c8n1
>
> The controllers are found from the subsystem like this:
>
> /sys/devices/virtual/nvme-subsystem/nvme-subsys2/nvme2 ->
> ../../nvme-fabrics/ctl/nvme2
> /sys/devices/virtual/nvme-subsystem/nvme-subsys2/nvme3 ->
> ../../nvme-fabrics/ctl/nvme3
>
> Signed-off-by: Martin Wilck <mwilck@xxxxxxxx>
> ---
> libmultipath/foreign/nvme.c | 153 ++++++++++++++++++++++++++++++------
> 1 file changed, 127 insertions(+), 26 deletions(-)
>
> diff --git a/libmultipath/foreign/nvme.c b/libmultipath/foreign/nvme.c
> index 280b6bd2..fca3235f 100644
> --- a/libmultipath/foreign/nvme.c
> +++ b/libmultipath/foreign/nvme.c
> @@ -26,6 +26,7 @@
> #include <limits.h>
> #include <dirent.h>
> #include <errno.h>
> +#include <ctype.h>
> #include "vector.h"
> #include "generic.h"
> #include "foreign.h"
> @@ -442,17 +443,92 @@ _find_path_by_syspath(struct nvme_map *map, const char *syspath)
> return NULL;
> }
>
> -static int no_dotfiles(const struct dirent *di)
> +static void _udev_device_unref(void *p)
> {
> - return di->d_name[0] != '.';
> + udev_device_unref(p);
> }
>
> -static void _find_slaves(struct context *ctx, struct nvme_map *map)
> +static void _udev_enumerate_unref(void *p)
> {
> - char pathbuf[PATH_MAX];
> + udev_enumerate_unref(p);
> +}
> +
> +static int _dirent_controller(const struct dirent *di)
> +{
> + static const char nvme_prefix[] = "nvme";
> + const char *p;
> +
> +#ifdef _DIRENT_HAVE_D_TYPE
> + if (di->d_type != DT_LNK)
> + return 0;
> +#endif
> + if (strncmp(di->d_name, nvme_prefix, sizeof(nvme_prefix) - 1))
> + return 0;
> + p = di->d_name + sizeof(nvme_prefix) - 1;
> + if (*p == '\0' || !isdigit(*p))
> + return 0;
> + for (++p; *p != '\0'; ++p)
> + if (!isdigit(*p))
> + return 0;
> + return 1;
> +}
> +
> +/* Find the block device for a given nvme controller */
> +struct udev_device *get_ctrl_blkdev(const struct context *ctx,
> + struct udev_device *ctrl)
> +{
> + struct udev_list_entry *item;
> + struct udev_device *blkdev = NULL;
> + struct udev_enumerate *enm = udev_enumerate_new(ctx->udev);
> +
> + if (enm == NULL)
> + return NULL;
> +
> + pthread_cleanup_push(_udev_enumerate_unref, enm);
> + if (udev_enumerate_add_match_parent(enm, ctrl) < 0)
> + goto out;
> + if (udev_enumerate_add_match_subsystem(enm, "block"))
> + goto out;
> +
> + if (udev_enumerate_scan_devices(enm) < 0) {
> + condlog(1, "%s: %s: error enumerating devices", __func__, THIS);
> + goto out;
> + }
> +
> + for (item = udev_enumerate_get_list_entry(enm);
> + item != NULL;
> + item = udev_list_entry_get_next(item)) {
> + struct udev_device *tmp;
> +
> + tmp = udev_device_new_from_syspath(ctx->udev,
> + udev_list_entry_get_name(item));
> + if (tmp == NULL)
> + continue;
> + if (!strcmp(udev_device_get_devtype(tmp), "disk")) {
> + blkdev = tmp;
> + break;
> + } else
> + udev_device_unref(tmp);
> + }
> +
> + if (blkdev == NULL)
> + condlog(1, "%s: %s: failed to get blockdev for %s",
> + __func__, THIS, udev_device_get_sysname(ctrl));
> + else
> + condlog(5, "%s: %s: got %s", __func__, THIS,
> + udev_device_get_sysname(blkdev));
> +out:
> + pthread_cleanup_pop(1);
> + return blkdev;
> +}
> +
> +static void _find_controllers(struct context *ctx, struct nvme_map *map)
> +{
> + char pathbuf[PATH_MAX], realbuf[PATH_MAX];
> struct dirent **di = NULL;
> + struct udev_device *subsys;
> struct nvme_path *path;
> - int r, i;
> + int r, i, n;
>
> if (map == NULL || map->udev == NULL)
> return;
> @@ -460,33 +536,65 @@ static void _find_slaves(struct context *ctx, struct nvme_map *map)
> vector_foreach_slot(map->pathvec, path, i)
> path->seen = false;
>
> - snprintf(pathbuf, sizeof(pathbuf),
> - "%s/slaves",
> - udev_device_get_syspath(map->udev));
> + subsys = udev_device_get_parent_with_subsystem_devtype(map->udev,
> + "nvme-subsystem",
> + NULL);
> + if (subsys == NULL) {
> + condlog(1, "%s: %s: BUG: no NVME subsys for %s", __func__, THIS,
> + udev_device_get_sysname(map->udev));
> + return;
> + }
>
> - r = scandir(pathbuf, &di, no_dotfiles, alphasort);
> + n = snprintf(pathbuf, sizeof(pathbuf), "%s",
> + udev_device_get_syspath(subsys));
> + r = scandir(pathbuf, &di, _dirent_controller, alphasort);
>
> if (r == 0) {
> - condlog(3, "%s: %s: no paths for %s", __func__, THIS,
> + condlog(3, "%s: %s: no controllers for %s", __func__, THIS,
> udev_device_get_sysname(map->udev));
> return;
> } else if (r < 0) {
> - condlog(1, "%s: %s: error %d scanning paths of %s", __func__,
> - THIS, errno, udev_device_get_sysname(map->udev));
> + condlog(1, "%s: %s: error %d scanning controllers of %s",
> + __func__, THIS, errno,
> + udev_device_get_sysname(map->udev));
> return;
> }
>
> pthread_cleanup_push(free, di);
> for (i = 0; i < r; i++) {
> char *fn = di[i]->d_name;
> - struct udev_device *udev;
> + struct udev_device *ctrl, *udev;
> +
> + if (snprintf(pathbuf + n, sizeof(pathbuf) - n, "/%s", fn)
> + >= sizeof(pathbuf) - n)
> + continue;
> + if (realpath(pathbuf, realbuf) == NULL) {
> + condlog(3, "%s: %s: realpath: %s", __func__, THIS,
> + strerror(errno));
> + continue;
> + }
> + condlog(4, "%s: %s: found %s", __func__, THIS, realbuf);
>
> - if (snprintf(pathbuf, sizeof(pathbuf), "%s/slaves/%s",
> - udev_device_get_syspath(map->udev), fn)
> - >= sizeof(pathbuf))
> + ctrl = udev_device_new_from_syspath(ctx->udev, realbuf);
> + if (ctrl == NULL) {
> + condlog(1, "%s: %s: failed to get udev device for %s",
> + __func__, THIS, realbuf);
> + continue;
> + }
> +
> + pthread_cleanup_push(_udev_device_unref, ctrl);
> + udev = get_ctrl_blkdev(ctx, ctrl);
> + /*
> + * We give up the reference to the nvme device here and get
> + * it back from the child below.
> + * This way we don't need to worry about unreffing it.
> + */
> + pthread_cleanup_pop(1);
> +
> + if (udev == NULL)
> continue;
>
> - path = _find_path_by_syspath(map, pathbuf);
> + path = _find_path_by_syspath(map, udev_device_get_syspath(udev));
> if (path != NULL) {
> path->seen = true;
> condlog(4, "%s: %s already known",
> @@ -494,13 +602,6 @@ static void _find_slaves(struct context *ctx, struct nvme_map *map)
> continue;
> }
>
> - udev = udev_device_new_from_syspath(ctx->udev, pathbuf);
> - if (udev == NULL) {
> - condlog(1, "%s: %s: failed to get udev device for %s",
> - __func__, THIS, fn);
> - continue;
> - }
> -
> path = calloc(1, sizeof(*path));
> if (path == NULL)
> continue;
> @@ -591,7 +692,7 @@ static int _add_map(struct context *ctx, struct udev_device *ud,
> return FOREIGN_ERR;
> }
> vector_set_slot(ctx->mpvec, map);
> - _find_slaves(ctx, map);
> + _find_controllers(ctx, map);
>
> return FOREIGN_CLAIMED;
> }
> @@ -688,7 +789,7 @@ void _check(struct context *ctx)
> vector_foreach_slot(ctx->mpvec, gm, i) {
> struct nvme_map *map = gen_mp_to_nvme(gm);
>
> - _find_slaves(ctx, map);
> + _find_controllers(ctx, map);
> }
> }
>
> --
> 2.18.0
--
dm-devel mailing list
dm-devel@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/dm-devel
