Re: [dm-devel] [RFC PATCH 13/20] libmultipath: print: convert API to generic data type

[Benjamin Marzinski]

On Tue, Feb 20, 2018 at 02:26:51PM +0100, Martin Wilck wrote:
> Convert higher level API (snprint_multipath_topology() etc) to
> using the generic multipath API. This will allow "foreign"
> multipath objects that implement the generic API to be printed
> exactly like native multipathd objects.
> 
> The previous API (using "struct multipath*" and "struct path" remains
> in place through macros mapping to the new functions. By doing this
> and testing in regular setups, it's easily verified that the new
> API works and produces the same results.
> 
> Moreover, abstract out the code to determine the output format from multipath
> properties into snprint_multipath_style(), to be able to use it as generic
> ->style() method.
> 
> Signed-off-by: Martin Wilck 
> ---
>  libmultipath/configure.c  |   1 +
>  libmultipath/dm-generic.c |   2 +-
>  libmultipath/print.c  | 116 
> +-
>  libmultipath/print.h  |  28 ---
>  multipath/main.c  |   1 +
>  multipathd/cli_handlers.c |   1 +
>  6 files changed, 100 insertions(+), 49 deletions(-)
> 
> diff --git a/libmultipath/configure.c b/libmultipath/configure.c
> index 13e14cc25fff..42b7c896ee65 100644
> --- a/libmultipath/configure.c
> +++ b/libmultipath/configure.c
> @@ -30,6 +30,7 @@
>  #include "discovery.h"
>  #include "debug.h"
>  #include "switchgroup.h"
> +#include "dm-generic.h"
>  #include "print.h"
>  #include "configure.h"
>  #include "pgpolicies.h"
> diff --git a/libmultipath/dm-generic.c b/libmultipath/dm-generic.c
> index 42a26085d087..bdc9ca0a488b 100644
> --- a/libmultipath/dm-generic.c
> +++ b/libmultipath/dm-generic.c
> @@ -56,7 +56,7 @@ const struct gen_multipath_ops dm_gen_multipath_ops = {
>   .get_pathgroups = dm_mp_get_pgs,
>   .rel_pathgroups = dm_mp_rel_pgs,
>   .snprint = snprint_multipath_attr,
> - /* .style = snprint_multipath_style, TBD */
> + .style = snprint_multipath_style,
>  };
>  
>  const struct gen_pathgroup_ops dm_gen_pathgroup_ops = {
> diff --git a/libmultipath/print.c b/libmultipath/print.c
> index e6f56381791f..8846765066ef 100644
> --- a/libmultipath/print.c
> +++ b/libmultipath/print.c
> @@ -31,7 +31,8 @@
>  #include "discovery.h"
>  #include "dm-generic.h"
>  
> -#define MAX(x,y) (x > y) ? x : y
> +#define MAX(x,y) (((x) > (y)) ? (x) : (y))
> +#define MIN(x,y) (((x) > (y)) ? (y) : (x))
>  #define TAIL (line + len - 1 - c)
>  #define NOPADs = c
>  #define PAD(x) \
> @@ -846,8 +847,8 @@ snprint_multipath_header (char * line, int len, const 
> char * format)
>  }
>  
>  int
> -snprint_multipath (char * line, int len, const char * format,
> -  const struct multipath * mpp, int pad)
> +_snprint_multipath (const struct gen_multipath * gmp,
> + char * line, int len, const char * format, int pad)
>  {
>   char * c = line;   /* line cursor */
>   char * s = line;   /* for padding */
> @@ -870,7 +871,7 @@ snprint_multipath (char * line, int len, const char * 
> format,
>   if (!(data = mpd_lookup(*f)))
>   continue;
>  
> - data->snprint(buff, MAX_FIELD_LEN, mpp);
> + gmp->ops->snprint(gmp, buff, MAX_FIELD_LEN, *f);
>   PRINT(c, TAIL, "%s", buff);
>   if (pad)
>   PAD(data->width);
> @@ -913,8 +914,8 @@ snprint_path_header (char * line, int len, const char * 
> format)
>  }
>  
>  int
> -snprint_path (char * line, int len, const char * format,
> -  const struct path * pp, int pad)
> +_snprint_path (const struct gen_path * gp, char * line, int len,
> +const char * format, int pad)
>  {
>   char * c = line;   /* line cursor */
>   char * s = line;   /* for padding */
> @@ -937,7 +938,7 @@ snprint_path (char * line, int len, const char * format,
>   if (!(data = pd_lookup(*f)))
>   continue;
>  
> - data->snprint(buff, MAX_FIELD_LEN, pp);
> + gp->ops->snprint(gp, buff, MAX_FIELD_LEN, *f);
>   PRINT(c, TAIL, "%s", buff);
>   if (pad)
>   PAD(data->width);
> @@ -948,8 +949,8 @@ snprint_path (char * line, int len, const char * format,
>  }
>  
>  int
> -snprint_pathgroup (char * line, int len, char * format,
> -const struct pathgroup * pgp)
> +_snprint_pathgroup (const struct gen_pathgroup * ggp, char * line, int len,
> + char * format)
>  {
>   char * c = line;   /* line cursor */
>   char * s = line;   /* for padding */
> @@ -972,7 +973,7 @@ snprint_pathgroup (char * line, int len, char * format,
>   if (!(data = pgd_lookup(*f)))
>   continue;
>  
> - data->snprint(buff, MAX_FIELD_LEN, pgp);
> + ggp->ops->snprint(ggp, buff, MAX_FIELD_LEN, *f);
>   PRINT(c, TAIL, "%s", buff);
>   PAD(data->width);
>   } while (*f++);
> @@ -980,8 +981,10 @@ snprint_pathgroup 

[dm-devel] [RFC PATCH 13/20] libmultipath: print: convert API to generic data type

[Martin Wilck]

Convert higher level API (snprint_multipath_topology() etc) to
using the generic multipath API. This will allow "foreign"
multipath objects that implement the generic API to be printed
exactly like native multipathd objects.

The previous API (using "struct multipath*" and "struct path" remains
in place through macros mapping to the new functions. By doing this
and testing in regular setups, it's easily verified that the new
API works and produces the same results.

Moreover, abstract out the code to determine the output format from multipath
properties into snprint_multipath_style(), to be able to use it as generic
->style() method.

Signed-off-by: Martin Wilck 
---
 libmultipath/configure.c  |   1 +
 libmultipath/dm-generic.c |   2 +-
 libmultipath/print.c  | 116 +-
 libmultipath/print.h  |  28 ---
 multipath/main.c  |   1 +
 multipathd/cli_handlers.c |   1 +
 6 files changed, 100 insertions(+), 49 deletions(-)

diff --git a/libmultipath/configure.c b/libmultipath/configure.c
index 13e14cc25fff..42b7c896ee65 100644
--- a/libmultipath/configure.c
+++ b/libmultipath/configure.c
@@ -30,6 +30,7 @@
 #include "discovery.h"
 #include "debug.h"
 #include "switchgroup.h"
+#include "dm-generic.h"
 #include "print.h"
 #include "configure.h"
 #include "pgpolicies.h"
diff --git a/libmultipath/dm-generic.c b/libmultipath/dm-generic.c
index 42a26085d087..bdc9ca0a488b 100644
--- a/libmultipath/dm-generic.c
+++ b/libmultipath/dm-generic.c
@@ -56,7 +56,7 @@ const struct gen_multipath_ops dm_gen_multipath_ops = {
.get_pathgroups = dm_mp_get_pgs,
.rel_pathgroups = dm_mp_rel_pgs,
.snprint = snprint_multipath_attr,
-   /* .style = snprint_multipath_style, TBD */
+   .style = snprint_multipath_style,
 };
 
 const struct gen_pathgroup_ops dm_gen_pathgroup_ops = {
diff --git a/libmultipath/print.c b/libmultipath/print.c
index e6f56381791f..8846765066ef 100644
--- a/libmultipath/print.c
+++ b/libmultipath/print.c
@@ -31,7 +31,8 @@
 #include "discovery.h"
 #include "dm-generic.h"
 
-#define MAX(x,y) (x > y) ? x : y
+#define MAX(x,y) (((x) > (y)) ? (x) : (y))
+#define MIN(x,y) (((x) > (y)) ? (y) : (x))
 #define TAIL (line + len - 1 - c)
 #define NOPADs = c
 #define PAD(x) \
@@ -846,8 +847,8 @@ snprint_multipath_header (char * line, int len, const char 
* format)
 }
 
 int
-snprint_multipath (char * line, int len, const char * format,
-const struct multipath * mpp, int pad)
+_snprint_multipath (const struct gen_multipath * gmp,
+   char * line, int len, const char * format, int pad)
 {
char * c = line;   /* line cursor */
char * s = line;   /* for padding */
@@ -870,7 +871,7 @@ snprint_multipath (char * line, int len, const char * 
format,
if (!(data = mpd_lookup(*f)))
continue;
 
-   data->snprint(buff, MAX_FIELD_LEN, mpp);
+   gmp->ops->snprint(gmp, buff, MAX_FIELD_LEN, *f);
PRINT(c, TAIL, "%s", buff);
if (pad)
PAD(data->width);
@@ -913,8 +914,8 @@ snprint_path_header (char * line, int len, const char * 
format)
 }
 
 int
-snprint_path (char * line, int len, const char * format,
-const struct path * pp, int pad)
+_snprint_path (const struct gen_path * gp, char * line, int len,
+  const char * format, int pad)
 {
char * c = line;   /* line cursor */
char * s = line;   /* for padding */
@@ -937,7 +938,7 @@ snprint_path (char * line, int len, const char * format,
if (!(data = pd_lookup(*f)))
continue;
 
-   data->snprint(buff, MAX_FIELD_LEN, pp);
+   gp->ops->snprint(gp, buff, MAX_FIELD_LEN, *f);
PRINT(c, TAIL, "%s", buff);
if (pad)
PAD(data->width);
@@ -948,8 +949,8 @@ snprint_path (char * line, int len, const char * format,
 }
 
 int
-snprint_pathgroup (char * line, int len, char * format,
-  const struct pathgroup * pgp)
+_snprint_pathgroup (const struct gen_pathgroup * ggp, char * line, int len,
+   char * format)
 {
char * c = line;   /* line cursor */
char * s = line;   /* for padding */
@@ -972,7 +973,7 @@ snprint_pathgroup (char * line, int len, char * format,
if (!(data = pgd_lookup(*f)))
continue;
 
-   data->snprint(buff, MAX_FIELD_LEN, pgp);
+   ggp->ops->snprint(ggp, buff, MAX_FIELD_LEN, *f);
PRINT(c, TAIL, "%s", buff);
PAD(data->width);
} while (*f++);
@@ -980,8 +981,10 @@ snprint_pathgroup (char * line, int len, char * format,
__endline(line, len, c);
return (c - line);
 }
+#define snprint_pathgroup(line, len, fmt, pgp) \
+   _snprint_pathgroup(dm_pathgroup_to_gen(pgp), line, len, fmt)
 
-void