Re: [dm-devel] [RFC PATCH 20/20] libmultipath: foreign/nvme: implement path display

2018-02-28 Thread Benjamin Marzinski
On Tue, Feb 20, 2018 at 02:26:58PM +0100, Martin Wilck wrote:
> implement display of path information for NVMe foreign paths and maps.
> With this patch, I get output like this for Linux NVMe soft targets:
> 
> multipathd show topology
> sys0:NQN:subsysname (uuid.96926ba3-b207-437c-902c-4a4df6538c3f) [nvme] 
> nvme0n1 NVMe,Linux,4.15.0-r
> size=2097152 features='n/a' hwhandler='n/a' wp=rw
> `-+- policy='n/a' prio=n/a status=n/a
>   |- 0:1:1 nvme0c1n1 0:0 n/a n/a live
>   |- 0:2:1 nvme0c2n1 0:0 n/a n/a live
>   |- 0:3:1 nvme0c3n1 0:0 n/a n/a live
>   `- 0:4:1 nvme0c4n1 0:0 n/a n/a live
> 
> multipathd show paths format '%G %d %i %o %z %m %N'
> foreign dev   hcil  dev_st serial   multipath host WWNN
> [nvme]  nvme0c1n1 0:1:1 live   1c2c86659503a02f nvme0n1   
> rdma:traddr=192.168.201.101,trsvcid=4420
> [nvme]  nvme0c2n1 0:2:1 live   1c2c86659503a02f nvme0n1   
> rdma:traddr=192.168.202.101,trsvcid=4420
> [nvme]  nvme0c3n1 0:3:1 live   1c2c86659503a02f nvme0n1   
> rdma:traddr=192.168.203.101,trsvcid=4420
> [nvme]  nvme0c4n1 0:4:1 live   1c2c86659503a02f nvme0n1   
> rdma:traddr=192.168.204.101,trsvcid=4420
> 
> (admittedly, I abused the 'WWNN' wildcard here a bit to display information
> which is helpful for NVMe over RDMA).

ACK with one small nit.
 
> Signed-off-by: Martin Wilck 
> ---
>  libmultipath/foreign/nvme.c | 342 
> ++--
>  1 file changed, 327 insertions(+), 15 deletions(-)
> 
> diff --git a/libmultipath/foreign/nvme.c b/libmultipath/foreign/nvme.c
> index 4e9c3a52d03c..5546a8eb178a 100644
> --- a/libmultipath/foreign/nvme.c
> +++ b/libmultipath/foreign/nvme.c
> @@ -25,42 +25,97 @@
>  #include 
>  #include 
>  #include 
> +#include 
> +#include 
>  #include "vector.h"
>  #include "generic.h"
>  #include "foreign.h"
>  #include "debug.h"
> +#include "structs.h"
> +#include "sysfs.h"
>  
> +static const char nvme_vendor[] = "NVMe";
> +static const char N_A[] = "n/a";
>  const char *THIS;
>  
> +struct nvme_map;
> +struct nvme_path {
> + struct gen_path gen;
> + struct udev_device *udev;
> + struct udev_device *ctl;
> + struct nvme_map *map;
> + bool seen;
> +};
> +
> +struct nvme_pathgroup {
> + struct gen_pathgroup gen;
> + vector pathvec;
> +};
> +
>  struct nvme_map {
>   struct gen_multipath gen;
>   struct udev_device *udev;
>   struct udev_device *subsys;
>   dev_t devt;
> + /* Just one static pathgroup for NVMe for now */
> + struct nvme_pathgroup pg;
> + struct gen_pathgroup *gpg;
> + struct _vector pgvec;
> + vector pathvec;
> + int nr_live;
>  };
>  
> -#define NAME_LEN 64 /* buffer length temp model name */
> +#define NAME_LEN 64 /* buffer length for temp attributes */
>  #define const_gen_mp_to_nvme(g) ((const struct nvme_map*)(g))
>  #define gen_mp_to_nvme(g) ((struct nvme_map*)(g))
>  #define nvme_mp_to_gen(n) &((n)->gen)
> +#define const_gen_pg_to_nvme(g) ((const struct nvme_pathgroup*)(g))
> +#define gen_pg_to_nvme(g) ((struct nvme_pathgroup*)(g))
> +#define nvme_pg_to_gen(n) &((n)->gen)
> +#define const_gen_path_to_nvme(g) ((const struct nvme_path*)(g))
> +#define gen_path_to_nvme(g) ((struct nvme_path*)(g))
> +#define nvme_path_to_gen(n) &((n)->gen)
> +
> +static void cleanup_nvme_path(struct nvme_path *path)
> +{
> + condlog(5, "%s: %p %p", __func__, path, path->udev);
> + if (path->udev)
> + udev_device_unref(path->udev);
> + /* ctl is implicitly referenced by udev, no need to unref */
> + free(path);
> +}
>  
>  static void cleanup_nvme_map(struct nvme_map *map)
>  {
> + if (map->pathvec) {
> + struct nvme_path *path;
> + int i;
> +
> + vector_foreach_slot_backwards(map->pathvec, path, i) {
> + condlog(5, "%s: %d %p", __func__, i, path);
> + cleanup_nvme_path(path);
> + vector_del_slot(map->pathvec, i);
> + }
> + }
> + vector_free(map->pathvec);
>   if (map->udev)
>   udev_device_unref(map->udev);
> - if (map->subsys)
> - udev_device_unref(map->subsys);
> + /* subsys is implicitly referenced by udev, no need to unref */
>   free(map);
>  }
>  
>  static const struct _vector*
>  nvme_mp_get_pgs(const struct gen_multipath *gmp) {
> - return NULL;
> + const struct nvme_map *nvme = const_gen_mp_to_nvme(gmp);
> +
> + /* This is all used under the lock, no need to copy */
> + return &nvme->pgvec;
>  }
>  
>  static void
>  nvme_mp_rel_pgs(const struct gen_multipath *gmp, const struct _vector *v)
>  {
> + /* empty */
>  }
>  
>  static void rstrip(char *str)
> @@ -75,7 +130,6 @@ static int snprint_nvme_map(const struct gen_multipath 
> *gmp,
>   char *buff, int len, char wildcard)
>  {
>   const struct nvme_map *nvm = const_gen_mp_to_nvme(gmp);
> - static const char nvme_vendor[] = "NVMe";
>   char fld[NAME_LEN];
>   const 

[dm-devel] [RFC PATCH 20/20] libmultipath: foreign/nvme: implement path display

2018-02-20 Thread Martin Wilck
implement display of path information for NVMe foreign paths and maps.
With this patch, I get output like this for Linux NVMe soft targets:

multipathd show topology
sys0:NQN:subsysname (uuid.96926ba3-b207-437c-902c-4a4df6538c3f) [nvme] nvme0n1 
NVMe,Linux,4.15.0-r
size=2097152 features='n/a' hwhandler='n/a' wp=rw
`-+- policy='n/a' prio=n/a status=n/a
  |- 0:1:1 nvme0c1n1 0:0 n/a n/a live
  |- 0:2:1 nvme0c2n1 0:0 n/a n/a live
  |- 0:3:1 nvme0c3n1 0:0 n/a n/a live
  `- 0:4:1 nvme0c4n1 0:0 n/a n/a live

multipathd show paths format '%G %d %i %o %z %m %N'
foreign dev   hcil  dev_st serial   multipath host WWNN
[nvme]  nvme0c1n1 0:1:1 live   1c2c86659503a02f nvme0n1   
rdma:traddr=192.168.201.101,trsvcid=4420
[nvme]  nvme0c2n1 0:2:1 live   1c2c86659503a02f nvme0n1   
rdma:traddr=192.168.202.101,trsvcid=4420
[nvme]  nvme0c3n1 0:3:1 live   1c2c86659503a02f nvme0n1   
rdma:traddr=192.168.203.101,trsvcid=4420
[nvme]  nvme0c4n1 0:4:1 live   1c2c86659503a02f nvme0n1   
rdma:traddr=192.168.204.101,trsvcid=4420

(admittedly, I abused the 'WWNN' wildcard here a bit to display information
which is helpful for NVMe over RDMA).

Signed-off-by: Martin Wilck 
---
 libmultipath/foreign/nvme.c | 342 ++--
 1 file changed, 327 insertions(+), 15 deletions(-)

diff --git a/libmultipath/foreign/nvme.c b/libmultipath/foreign/nvme.c
index 4e9c3a52d03c..5546a8eb178a 100644
--- a/libmultipath/foreign/nvme.c
+++ b/libmultipath/foreign/nvme.c
@@ -25,42 +25,97 @@
 #include 
 #include 
 #include 
+#include 
+#include 
 #include "vector.h"
 #include "generic.h"
 #include "foreign.h"
 #include "debug.h"
+#include "structs.h"
+#include "sysfs.h"
 
+static const char nvme_vendor[] = "NVMe";
+static const char N_A[] = "n/a";
 const char *THIS;
 
+struct nvme_map;
+struct nvme_path {
+   struct gen_path gen;
+   struct udev_device *udev;
+   struct udev_device *ctl;
+   struct nvme_map *map;
+   bool seen;
+};
+
+struct nvme_pathgroup {
+   struct gen_pathgroup gen;
+   vector pathvec;
+};
+
 struct nvme_map {
struct gen_multipath gen;
struct udev_device *udev;
struct udev_device *subsys;
dev_t devt;
+   /* Just one static pathgroup for NVMe for now */
+   struct nvme_pathgroup pg;
+   struct gen_pathgroup *gpg;
+   struct _vector pgvec;
+   vector pathvec;
+   int nr_live;
 };
 
-#define NAME_LEN 64 /* buffer length temp model name */
+#define NAME_LEN 64 /* buffer length for temp attributes */
 #define const_gen_mp_to_nvme(g) ((const struct nvme_map*)(g))
 #define gen_mp_to_nvme(g) ((struct nvme_map*)(g))
 #define nvme_mp_to_gen(n) &((n)->gen)
+#define const_gen_pg_to_nvme(g) ((const struct nvme_pathgroup*)(g))
+#define gen_pg_to_nvme(g) ((struct nvme_pathgroup*)(g))
+#define nvme_pg_to_gen(n) &((n)->gen)
+#define const_gen_path_to_nvme(g) ((const struct nvme_path*)(g))
+#define gen_path_to_nvme(g) ((struct nvme_path*)(g))
+#define nvme_path_to_gen(n) &((n)->gen)
+
+static void cleanup_nvme_path(struct nvme_path *path)
+{
+   condlog(5, "%s: %p %p", __func__, path, path->udev);
+   if (path->udev)
+   udev_device_unref(path->udev);
+   /* ctl is implicitly referenced by udev, no need to unref */
+   free(path);
+}
 
 static void cleanup_nvme_map(struct nvme_map *map)
 {
+   if (map->pathvec) {
+   struct nvme_path *path;
+   int i;
+
+   vector_foreach_slot_backwards(map->pathvec, path, i) {
+   condlog(5, "%s: %d %p", __func__, i, path);
+   cleanup_nvme_path(path);
+   vector_del_slot(map->pathvec, i);
+   }
+   }
+   vector_free(map->pathvec);
if (map->udev)
udev_device_unref(map->udev);
-   if (map->subsys)
-   udev_device_unref(map->subsys);
+   /* subsys is implicitly referenced by udev, no need to unref */
free(map);
 }
 
 static const struct _vector*
 nvme_mp_get_pgs(const struct gen_multipath *gmp) {
-   return NULL;
+   const struct nvme_map *nvme = const_gen_mp_to_nvme(gmp);
+
+   /* This is all used under the lock, no need to copy */
+   return &nvme->pgvec;
 }
 
 static void
 nvme_mp_rel_pgs(const struct gen_multipath *gmp, const struct _vector *v)
 {
+   /* empty */
 }
 
 static void rstrip(char *str)
@@ -75,7 +130,6 @@ static int snprint_nvme_map(const struct gen_multipath *gmp,
char *buff, int len, char wildcard)
 {
const struct nvme_map *nvm = const_gen_mp_to_nvme(gmp);
-   static const char nvme_vendor[] = "NVMe";
char fld[NAME_LEN];
const char *val;
 
@@ -92,6 +146,8 @@ static int snprint_nvme_map(const struct gen_multipath *gmp,
return snprintf(buff, len, "%s",
udev_device_get_sysattr_value(nvm->udev,
  "wwid"));