[dm-devel] [PATCH] dm: log-writes: record metadata flag for better flags record

2018-02-28 Thread Qu Wenruo
So developer could distinguish data and metadata bios easier.

Signed-off-by: Qu Wenruo 
---
 drivers/md/dm-log-writes.c | 12 
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/md/dm-log-writes.c b/drivers/md/dm-log-writes.c
index 189badbeddaf..c7f4a592d14d 100644
--- a/drivers/md/dm-log-writes.c
+++ b/drivers/md/dm-log-writes.c
@@ -52,10 +52,11 @@
  * in fact we want to do the data and the discard in the order that they
  * completed.
  */
-#define LOG_FLUSH_FLAG (1 << 0)
-#define LOG_FUA_FLAG (1 << 1)
-#define LOG_DISCARD_FLAG (1 << 2)
-#define LOG_MARK_FLAG (1 << 3)
+#define LOG_FLUSH_FLAG (1 << 0)
+#define LOG_FUA_FLAG   (1 << 1)
+#define LOG_DISCARD_FLAG   (1 << 2)
+#define LOG_MARK_FLAG  (1 << 3)
+#define LOG_METADATA_FLAG  (1 << 4)
 
 #define WRITE_LOG_VERSION 1ULL
 #define WRITE_LOG_MAGIC 0x6a736677736872ULL
@@ -699,6 +700,7 @@ static int log_writes_map(struct dm_target *ti, struct bio 
*bio)
bool flush_bio = (bio->bi_opf & REQ_PREFLUSH);
bool fua_bio = (bio->bi_opf & REQ_FUA);
bool discard_bio = (bio_op(bio) == REQ_OP_DISCARD);
+   bool meta_bio = (bio->bi_opf & REQ_META);
 
pb->block = NULL;
 
@@ -743,6 +745,8 @@ static int log_writes_map(struct dm_target *ti, struct bio 
*bio)
block->flags |= LOG_FUA_FLAG;
if (discard_bio)
block->flags |= LOG_DISCARD_FLAG;
+   if (meta_bio)
+   block->flags |= LOG_METADATA_FLAG;
 
block->sector = bio_to_dev_sectors(lc, bio->bi_iter.bi_sector);
block->nr_sectors = bio_to_dev_sectors(lc, bio_sectors(bio));
-- 
2.16.2

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel


[dm-devel] [PATCH 1/2] log-writes: Add support to output human readable flags

2018-02-28 Thread Qu Wenruo
Signed-off-by: Qu Wenruo 
---
 log-writes.c | 60 +---
 1 file changed, 57 insertions(+), 3 deletions(-)

diff --git a/log-writes.c b/log-writes.c
index fa4f3f3b42d6..5ef328656c89 100644
--- a/log-writes.c
+++ b/log-writes.c
@@ -117,6 +117,58 @@ int log_discard(struct log *log, struct log_write_entry 
*entry)
return 0;
 }
 
+#define DEFINE_LOG_FLAGS_STR_ENTRY(x)  \
+   {LOG_##x##_FLAG, #x}
+
+struct flags_to_str_entry {
+   u64 flags;
+   const char *str;
+} log_flags_table[] = {
+   DEFINE_LOG_FLAGS_STR_ENTRY(FLUSH),
+   DEFINE_LOG_FLAGS_STR_ENTRY(FUA),
+   DEFINE_LOG_FLAGS_STR_ENTRY(DISCARD),
+   DEFINE_LOG_FLAGS_STR_ENTRY(MARK)
+};
+
+#define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0]))
+#define LOG_FLAGS_BUF_SIZE 128
+/*
+ * Convert numeric flags to human readable flags.
+ * @flags: numeric flags
+ * @buf:   output buffer for human readable string.
+ * must have enough space (LOG_FLAGS_BUF_SIZE) to contain all
+ * the string
+ */
+static void entry_flags_to_str(u64 flags, char *buf)
+{
+   int empty = 1;
+   int left_len;
+   int i;
+
+   buf[0] = '\0';
+   for (i = 0; i < ARRAY_SIZE(log_flags_table); i++) {
+   if (flags & log_flags_table[i].flags) {
+   if (!empty)
+   strncat(buf, "|", LOG_FLAGS_BUF_SIZE);
+   empty = 0;
+   strncat(buf, log_flags_table[i].str, 
LOG_FLAGS_BUF_SIZE);
+   flags &= ~log_flags_table[i].flags;
+   }
+   }
+   if (flags) {
+   if (!empty)
+   strncat(buf, "|", LOG_FLAGS_BUF_SIZE);
+   empty = 0;
+   left_len = LOG_FLAGS_BUF_SIZE - strnlen(buf,
+   LOG_FLAGS_BUF_SIZE);
+   if (left_len > 0)
+   snprintf(buf + strnlen(buf, LOG_FLAGS_BUF_SIZE),
+left_len, "UNKNOWN.%llu", flags);
+   }
+   if (empty)
+   strncpy(buf, "NONE", LOG_FLAGS_BUF_SIZE);
+}
+
 /*
  * @log: the log we are replaying.
  * @entry: where we put the entry.
@@ -135,6 +187,7 @@ int log_replay_next_entry(struct log *log, struct 
log_write_entry *entry,
size_t read_size = read_data ? log->sectorsize :
sizeof(struct log_write_entry);
char *buf;
+   char flags_buf[LOG_FLAGS_BUF_SIZE];
ssize_t ret;
off_t offset;
 
@@ -158,16 +211,17 @@ int log_replay_next_entry(struct log *log, struct 
log_write_entry *entry,
}
}
 
+   flags = le64_to_cpu(entry->flags);
+   entry_flags_to_str(flags, flags_buf);
if (log_writes_verbose)
-   printf("replaying %d: sector %llu, size %llu, flags %llu\n",
+   printf("replaying %d: sector %llu, size %llu, flags %llu(%s)\n",
   (int)log->cur_entry - 1,
   (unsigned long long)le64_to_cpu(entry->sector),
   (unsigned long long)size,
-  (unsigned long long)le64_to_cpu(entry->flags));
+  (unsigned long long)flags, flags_buf);
if (!size)
return 0;
 
-   flags = le64_to_cpu(entry->flags);
if (flags & LOG_DISCARD_FLAG)
return log_discard(log, entry);
 
-- 
2.16.2

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel


[dm-devel] [PATCH 2/2] log-writes: Add support for METADATA flag

2018-02-28 Thread Qu Wenruo
Signed-off-by: Qu Wenruo 
---
 log-writes.c | 3 ++-
 log-writes.h | 9 +
 2 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/log-writes.c b/log-writes.c
index 5ef328656c89..651db43383ba 100644
--- a/log-writes.c
+++ b/log-writes.c
@@ -127,7 +127,8 @@ struct flags_to_str_entry {
DEFINE_LOG_FLAGS_STR_ENTRY(FLUSH),
DEFINE_LOG_FLAGS_STR_ENTRY(FUA),
DEFINE_LOG_FLAGS_STR_ENTRY(DISCARD),
-   DEFINE_LOG_FLAGS_STR_ENTRY(MARK)
+   DEFINE_LOG_FLAGS_STR_ENTRY(MARK),
+   DEFINE_LOG_FLAGS_STR_ENTRY(METADATA)
 };
 
 #define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0]))
diff --git a/log-writes.h b/log-writes.h
index 13f98ffe1730..ce41262f6aaa 100644
--- a/log-writes.h
+++ b/log-writes.h
@@ -12,10 +12,11 @@ extern int log_writes_verbose;
 typedef __u64 u64;
 typedef __u32 u32;
 
-#define LOG_FLUSH_FLAG (1 << 0)
-#define LOG_FUA_FLAG (1 << 1)
-#define LOG_DISCARD_FLAG (1 << 2)
-#define LOG_MARK_FLAG (1 << 3)
+#define LOG_FLUSH_FLAG (1 << 0)
+#define LOG_FUA_FLAG   (1 << 1)
+#define LOG_DISCARD_FLAG   (1 << 2)
+#define LOG_MARK_FLAG  (1 << 3)
+#define LOG_METADATA_FLAG  (1 << 4)
 
 #define WRITE_LOG_VERSION 1
 #define WRITE_LOG_MAGIC 0x6a736677736872
-- 
2.16.2

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel


Re: [dm-devel] [PATCH] dm: log-writes: record metadata flag for better flags record

2018-02-28 Thread Josef Bacik
On Wed, Feb 28, 2018 at 03:32:47PM +0800, Qu Wenruo wrote:
> So developer could distinguish data and metadata bios easier.
> 
> Signed-off-by: Qu Wenruo 

Reviewed-by: Josef Bacik 

Thanks,

Josef

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel


Re: [dm-devel] [PATCH 1/2] log-writes: Add support to output human readable flags

2018-02-28 Thread Josef Bacik
I merged the pull request for these, but you should maybe send them up for the
xfstests version of log-writes.  Thanks,

Josef

On Wed, Feb 28, 2018 at 03:33:25PM +0800, Qu Wenruo wrote:
> Signed-off-by: Qu Wenruo 
> ---
>  log-writes.c | 60 
> +---
>  1 file changed, 57 insertions(+), 3 deletions(-)
> 
> diff --git a/log-writes.c b/log-writes.c
> index fa4f3f3b42d6..5ef328656c89 100644
> --- a/log-writes.c
> +++ b/log-writes.c
> @@ -117,6 +117,58 @@ int log_discard(struct log *log, struct log_write_entry 
> *entry)
>   return 0;
>  }
>  
> +#define DEFINE_LOG_FLAGS_STR_ENTRY(x)\
> + {LOG_##x##_FLAG, #x}
> +
> +struct flags_to_str_entry {
> + u64 flags;
> + const char *str;
> +} log_flags_table[] = {
> + DEFINE_LOG_FLAGS_STR_ENTRY(FLUSH),
> + DEFINE_LOG_FLAGS_STR_ENTRY(FUA),
> + DEFINE_LOG_FLAGS_STR_ENTRY(DISCARD),
> + DEFINE_LOG_FLAGS_STR_ENTRY(MARK)
> +};
> +
> +#define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0]))
> +#define LOG_FLAGS_BUF_SIZE   128
> +/*
> + * Convert numeric flags to human readable flags.
> + * @flags:   numeric flags
> + * @buf: output buffer for human readable string.
> + *   must have enough space (LOG_FLAGS_BUF_SIZE) to contain all
> + *   the string
> + */
> +static void entry_flags_to_str(u64 flags, char *buf)
> +{
> + int empty = 1;
> + int left_len;
> + int i;
> +
> + buf[0] = '\0';
> + for (i = 0; i < ARRAY_SIZE(log_flags_table); i++) {
> + if (flags & log_flags_table[i].flags) {
> + if (!empty)
> + strncat(buf, "|", LOG_FLAGS_BUF_SIZE);
> + empty = 0;
> + strncat(buf, log_flags_table[i].str, 
> LOG_FLAGS_BUF_SIZE);
> + flags &= ~log_flags_table[i].flags;
> + }
> + }
> + if (flags) {
> + if (!empty)
> + strncat(buf, "|", LOG_FLAGS_BUF_SIZE);
> + empty = 0;
> + left_len = LOG_FLAGS_BUF_SIZE - strnlen(buf,
> + LOG_FLAGS_BUF_SIZE);
> + if (left_len > 0)
> + snprintf(buf + strnlen(buf, LOG_FLAGS_BUF_SIZE),
> +  left_len, "UNKNOWN.%llu", flags);
> + }
> + if (empty)
> + strncpy(buf, "NONE", LOG_FLAGS_BUF_SIZE);
> +}
> +
>  /*
>   * @log: the log we are replaying.
>   * @entry: where we put the entry.
> @@ -135,6 +187,7 @@ int log_replay_next_entry(struct log *log, struct 
> log_write_entry *entry,
>   size_t read_size = read_data ? log->sectorsize :
>   sizeof(struct log_write_entry);
>   char *buf;
> + char flags_buf[LOG_FLAGS_BUF_SIZE];
>   ssize_t ret;
>   off_t offset;
>  
> @@ -158,16 +211,17 @@ int log_replay_next_entry(struct log *log, struct 
> log_write_entry *entry,
>   }
>   }
>  
> + flags = le64_to_cpu(entry->flags);
> + entry_flags_to_str(flags, flags_buf);
>   if (log_writes_verbose)
> - printf("replaying %d: sector %llu, size %llu, flags %llu\n",
> + printf("replaying %d: sector %llu, size %llu, flags %llu(%s)\n",
>  (int)log->cur_entry - 1,
>  (unsigned long long)le64_to_cpu(entry->sector),
>  (unsigned long long)size,
> -(unsigned long long)le64_to_cpu(entry->flags));
> +(unsigned long long)flags, flags_buf);
>   if (!size)
>   return 0;
>  
> - flags = le64_to_cpu(entry->flags);
>   if (flags & LOG_DISCARD_FLAG)
>   return log_discard(log, entry);
>  
> -- 
> 2.16.2
> 
> --
> dm-devel mailing list
> dm-devel@redhat.com
> https://www.redhat.com/mailman/listinfo/dm-devel

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel


Re: [dm-devel] [RFC PATCH 05/20] libmultipath: don't update path groups when printing

2018-02-28 Thread Benjamin Marzinski
On Tue, Feb 20, 2018 at 02:26:43PM +0100, Martin Wilck wrote:
> Updating the prio values for printing makes no sense. The user wants to see
> the prio values multipath is actually using for path group selection, and
> updating the values here means actually lying to the user if the prio values
> have changed, but multipathd hasn't updated them internally.
> 
> If we really don't update the pathgroup prios when we need to, this should be
> fixed elsewhere. The current wrong output would just hide that if it occured.
> 
> Moreover, correctness forbids changing properties so deeply in a code path
> that's supposed to print them only. Finally, this piece of code prevents the
> print.c code to be converted to proper "const" usage.

Well, it is true that we've only been updating the path group priority
when we've needed it, and we've only need it to be uptodate when we are
picking a new pathgroup, or are printing it out. When failback is set to
"manual", we rarely are picking a new pathgroup, so we rarely update the
pathgroup prio. 

If we really want to be honest with the user, we probably want to reload
the multipath device whenever a path group's priority changes enough to
make their order change.  Otherwise, the kernel will still failover to
the wrong path group.  We currently only do this for FAILBACK_IMMEDIATE,
and we don't even do that very well. For instance, we will currently
reorder pathgroups when their priority has gone to 0 because they have
no valid paths. In this case, we should expect that when the paths
return, they will most likely have the same priority as they previously
had. Thus, we shouldn't lower that path group's priority while they are
down, since it will just cause two pointless table reloads (one when the
last path in the group goes down, and another when the first path comes
back). But I digress.

I'd be fine with simply updating the path group priority whever we change a
path's priority, if we aren't updating it when printing it. The bigger
work of actually making sure that the path group order it the table
is always uptodate needs to happen, but it doesn't need to happen in
this patchset.

-Ben
> 
> Signed-off-by: Martin Wilck 
> ---
>  libmultipath/print.c | 7 ---
>  1 file changed, 7 deletions(-)
> 
> diff --git a/libmultipath/print.c b/libmultipath/print.c
> index 8fb5c5058965..b5c00bfe69a5 100644
> --- a/libmultipath/print.c
> +++ b/libmultipath/print.c
> @@ -484,13 +484,6 @@ snprint_pg_selector (char * buff, size_t len, struct 
> pathgroup * pgp)
>  static int
>  snprint_pg_pri (char * buff, size_t len, struct pathgroup * pgp)
>  {
> - /*
> -  * path group priority is not updated for every path prio change,
> -  * but only on switch group code path.
> -  *
> -  * Printing is another reason to update.
> -  */
> - path_group_prio_update(pgp);
>   return snprint_int(buff, len, pgp->priority);
>  }
>  
> -- 
> 2.16.1

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel


Re: [dm-devel] [RFC PATCH 12/20] libmultipath: "generic multipath" interface

2018-02-28 Thread Benjamin Marzinski
On Tue, Feb 20, 2018 at 02:26:50PM +0100, Martin Wilck wrote:
> This patch adds a simplified abstract interface to the multipath data 
> structures.
> The idea is to allow "foreign" data structures to be treated by libmultipath
> if they implement the same interface. Currently, the intention is to use this
> only to provide formatted output about from this interface.
> 
> This interface assumes only that the data structure is organized in maps
> containing path groups containing paths, and that formatted printing (using
> the wildcards defined in libmultipath) is possible on each level of the data
> structure.
> 
> The patch also implements the interface for the internal dm_multipath data
> structure.
> 
> The style() method looks a bit exotic, but it's necessary because
> print_multipath_topology() uses different formats depending on the mpp
> properties. This needs to be in the generic interface, too, if we want to
> produce identical output.
>

I have one nit here. print.h now relies on dm-generic.h, since it uses
dm_multipath_to_gen() in its defines.  So shouldn't it simply include
dm-generic.h, which would allow configure.c, main.c, and cli_handlers.c
to not include dm-generic.h, since they only need it because they
include print.h?

-Ben
 
> Signed-off-by: Martin Wilck 
> ---
>  libmultipath/Makefile |   2 +-
>  libmultipath/dm-generic.c |  70 
>  libmultipath/dm-generic.h |  41 ++
>  libmultipath/generic.c|  39 +
>  libmultipath/generic.h| 136 
> ++
>  libmultipath/list.h   |   4 ++
>  libmultipath/print.c  |  34 
>  libmultipath/print.h  |   7 +++
>  libmultipath/structs.c|   4 ++
>  libmultipath/structs.h|   4 ++
>  10 files changed, 340 insertions(+), 1 deletion(-)
>  create mode 100644 libmultipath/dm-generic.c
>  create mode 100644 libmultipath/dm-generic.h
>  create mode 100644 libmultipath/generic.c
>  create mode 100644 libmultipath/generic.h
> 
> diff --git a/libmultipath/Makefile b/libmultipath/Makefile
> index 25b052729d48..0099d9d6cc39 100644
> --- a/libmultipath/Makefile
> +++ b/libmultipath/Makefile
> @@ -43,7 +43,7 @@ OBJS = memory.o parser.o vector.o devmapper.o callout.o \
>   switchgroup.o uxsock.o print.o alias.o log_pthread.o \
>   log.o configure.o structs_vec.o sysfs.o prio.o checkers.o \
>   lock.o waiter.o file.o wwids.o prioritizers/alua_rtpg.o prkey.o \
> - io_err_stat.o
> + io_err_stat.o dm-generic.o generic.o
>  
>  all: $(LIBS)
>  
> diff --git a/libmultipath/dm-generic.c b/libmultipath/dm-generic.c
> new file mode 100644
> index ..42a26085d087
> --- /dev/null
> +++ b/libmultipath/dm-generic.c
> @@ -0,0 +1,70 @@
> +/*
> +  Copyright (c) 2018 Martin Wilck, SUSE Linux GmbH
> +
> +  This program is free software; you can redistribute it and/or
> +  modify it under the terms of the GNU General Public License
> +  as published by the Free Software Foundation; either version 2
> +  of the License, or (at your option) any later version.
> +
> +  This program is distributed in the hope that it will be useful,
> +  but WITHOUT ANY WARRANTY; without even the implied warranty of
> +  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +  GNU General Public License for more details.
> +
> +  You should have received a copy of the GNU General Public License
> +  along with this program; if not, write to the Free Software
> +  Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301,
> +  USA.
> + */
> +
> +#include 
> +#include 
> +#include "generic.h"
> +#include "dm-generic.h"
> +#include "structs.h"
> +#include "structs_vec.h"
> +#include "config.h"
> +#include "print.h"
> +
> +static const struct _vector*
> +dm_mp_get_pgs(const struct gen_multipath *gmp)
> +{
> + return vector_convert(NULL, gen_multipath_to_dm(gmp)->pg,
> +   struct pathgroup, dm_pathgroup_to_gen);
> +}
> +
> +static void dm_mp_rel_pgs(const struct gen_multipath *gmp,
> +   const struct _vector* v)
> +{
> + vector_free_const(v);
> +}
> +
> +static const struct _vector*
> +dm_pg_get_paths(const struct gen_pathgroup *gpg)
> +{
> + return vector_convert(NULL, gen_pathgroup_to_dm(gpg)->paths,
> +   struct path, dm_path_to_gen);
> +}
> +
> +static void dm_mp_rel_paths(const struct gen_pathgroup *gpg,
> + const struct _vector* v)
> +{
> + vector_free_const(v);
> +}
> +
> +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 */
> +};
> +
> +const struct gen_pathgroup_ops dm_gen_pathgroup_ops = {
> + .get_paths = dm_pg_get_paths,
> + .rel_paths = dm_mp_rel_paths,
> + .snprint = snprint_pathgroup_attr,
> +};
> +
> +const struct gen_path_

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

2018-02-28 Thread 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 (char * line, int 

Re: [dm-devel] [RFC PATCH 15/20] libmultipath: API for foreign multipath handling

2018-02-28 Thread Benjamin Marzinski
On Tue, Feb 20, 2018 at 02:26:53PM +0100, Martin Wilck wrote:
> Add an API for "foreign" multipaths. Foreign libraries are loaded
> from ${multipath_dir}/libforeign-*.so, as we do for checkers.
> 
> Refer to "foreign.h" for details about the API itself. Like we do for
> checkers, high-level multipath code isn't supposed to call the API directly,
> but rather the wrapper functions declared in "foreign.h".
> 
> This API is used only for displaying information and for logging. An 
> extension to
> other functionality (such as monitoring or administration) might be feasible,
> but is not planned.
> 
> Foreign libraries communicate with libmultipath through the API defined in
> "foreign.h". The foreign library can implement multipath maps, pathgroups,
> and paths as it likes, they just need to provide the simple interfaces
> defined in "generic.h" to libmultipath. These interfaces are used in 
> libmultipath's
> "print" implementation to convey various bits of information to users. By
> using the same interfaces for printing that libmultipath uses internally,
> foreign library implementations can focus on the technical side without
> worrying about output formatting compatibility.

ACK, with one minor nit

> 
> Signed-off-by: Martin Wilck 
> ---
>  libmultipath/Makefile  |   2 +-
>  libmultipath/foreign.c | 471 
> +
>  libmultipath/foreign.h | 322 +
>  3 files changed, 794 insertions(+), 1 deletion(-)
>  create mode 100644 libmultipath/foreign.c
>  create mode 100644 libmultipath/foreign.h



> +struct foreign {
> + /**
> +  * method: init(api, name)
> +  * Initialize foreign library, and check API compatibility
> +  * return pointer to opaque internal data strucure if successful,
> +  * NULL otherwise.
> +  *
> +  * @param[in] api: API version
> +  * @param[in] name: name to use for references to self in log messages,
> +  * doesn't need to be strdup'd
> +  * @returns context pointer to use in future method calls.
> +  */
> + struct context* (*init)(unsigned int api, const char *name);
> +
> + /**
> +  * method: cleanup(context)
> +  * Free data structures used by foreign library, including
> +  * context itself.
> +  *
> +  * @param[in] context foreign library context. This shouldn't be
> +  * referenced any more after calling cleanup().
> +  */
> + void (*cleanup)(struct context *);
> +
> + /**
> +  * method: add(context, udev)
> +  * This is called during path detection, and for udev ADD events.
> +  *
> +  * @param[in] context foreign library context
> +  * @param[in] udev udev device to add
> +  * @returns status code
> +  * @retval FOREIGN_CLAIMED: device newly claimed
> +  * @retval FOREIGN_OK: device already registered, no action taken
> +  * @retval FOREIGN_IGNORED: device is ignored, no action taken
> +  * @retval FOREIGN_ERR: an error occured (e.g. out-of-memory)
> +  */
> + int (*add)(struct context *, struct udev_device *);
> +
> + /**
> +  * method: change
> +  * This is called on udev CHANGE events.
> +  *
> +  * @param[in] context foreign library context
> +  * @param[in] udev udev device that has generated the event
> +  * @returns status code
> +  * @retval FOREIGN_OK: event processed
> +  * @retval FOREIGN_IGNORED: the device is ignored
> +  * @retval FOREIGN_ERR: an error occured (e.g. out-of-memory)
> +  *
> +  * Note: theoretically it can happen that the status of a foreign device
> +  * (claimed vs. not claimed) changes in a change event.
> +  * Supporting this correctly would require big efforts. For now, we
> +  * don't support it. "multipathd reconfigure" starts foreign device
> +  * detection from scratch and should be able to handle this situation.
> +  */
> + int (*change)(struct context *, struct udev_device *);
> +
> + /**
> +  * method: delete
> +  * This is called on udev DELETE events.
> +  *
> +  * @param[in] context foreign library context
> +  * @param[in] udev udev device that has generated the event and
> +  *  should be deleted
> +  * @returns status code
> +  * @retval FOREIGN_OK: processed correctly (device deleted)
> +  * @retval FOREIGN_IGNORED: device wasn't registered internally
> +  * @retval FOREIGN_ERR: error occured.
> +  */
> + int (*delete)(struct context *, struct udev_device *);
> +
> + /**
> +  * method: delete_all
> +  * This is called if multipathd reconfigures itself.
> +  * Deletes all registered devices (maps and paths)
> +  *
> +  * @param[in] context foreign library context
> +  * @returns status code
> +  * @retval FOREIGN_OK: processed correctly
> +  * @retval FOREIGN_IGNORED: nothing to delete
> +  * @retval FOREIGN_ERR: error occured
> +  */
> + int (*delet

Re: [dm-devel] [RFC PATCH 17/20] libmultipath/foreign: nvme foreign library

2018-02-28 Thread Benjamin Marzinski
On Tue, Feb 20, 2018 at 02:26:55PM +0100, Martin Wilck wrote:
> This still contains stubs for path handling and checking, but it's functional
> for printing already.
> 
> Signed-off-by: Martin Wilck 
> ---
>  Makefile  |   1 +
>  libmultipath/foreign/Makefile |  30 +++
>  libmultipath/foreign/nvme.c   | 444 
> ++
>  3 files changed, 475 insertions(+)
>  create mode 100644 libmultipath/foreign/Makefile
>  create mode 100644 libmultipath/foreign/nvme.c
> 
> diff --git a/Makefile b/Makefile
> index 11c46eb4dbc9..4b145c593605 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -7,6 +7,7 @@ BUILDDIRS = \
>   libmultipath \
>   libmultipath/prioritizers \
>   libmultipath/checkers \
> + libmultipath/foreign \
>   libmpathpersist \
>   multipath \
>   multipathd \
> diff --git a/libmultipath/foreign/Makefile b/libmultipath/foreign/Makefile
> new file mode 100644
> index ..dfba11e86d76
> --- /dev/null
> +++ b/libmultipath/foreign/Makefile
> @@ -0,0 +1,30 @@
> +#
> +# Copyright (C) 2003 Christophe Varoqui, 
> +#
> +include ../../Makefile.inc
> +
> +CFLAGS += $(LIB_CFLAGS) -I..
> +
> +# If you add or remove a checker also update multipath/multipath.conf.5
> +LIBS= \
> + libforeign-nvme.so
> +
> +all: $(LIBS)
> +
> +libforeign-%.so: %.o
> + $(CC) $(LDFLAGS) $(SHARED_FLAGS) -o $@ $^
> +
> +install:
> + $(INSTALL_PROGRAM) -m 755 $(LIBS) $(DESTDIR)$(libdir)
> +
> +uninstall:
> + for file in $(LIBS); do $(RM) $(DESTDIR)$(libdir)/$$file; done
> +
> +clean: dep_clean
> + $(RM) core *.a *.o *.gz *.so
> +
> +OBJS := $(LIBS:libforeign-%.so=%.o)
> +include $(wildcard $(OBJS:.o=.d))
> +
> +dep_clean:
> + $(RM) $(OBJS:.o=.d)
> diff --git a/libmultipath/foreign/nvme.c b/libmultipath/foreign/nvme.c
> new file mode 100644
> index ..4e9c3a52d03c
> --- /dev/null
> +++ b/libmultipath/foreign/nvme.c
> @@ -0,0 +1,444 @@
> +/*
> +  Copyright (c) 2018 Martin Wilck, SUSE Linux GmbH
> +
> +  This program is free software; you can redistribute it and/or
> +  modify it under the terms of the GNU General Public License
> +  as published by the Free Software Foundation; either version 2
> +  of the License, or (at your option) any later version.
> +
> +  This program is distributed in the hope that it will be useful,
> +  but WITHOUT ANY WARRANTY; without even the implied warranty of
> +  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +  GNU General Public License for more details.
> +
> +  You should have received a copy of the GNU General Public License
> +  along with this program; if not, write to the Free Software
> +  Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301,
> +  USA.
> +*/
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include "vector.h"
> +#include "generic.h"
> +#include "foreign.h"
> +#include "debug.h"
> +
> +const char *THIS;
> +
> +struct nvme_map {
> + struct gen_multipath gen;
> + struct udev_device *udev;
> + struct udev_device *subsys;
> + dev_t devt;
> +};
> +
> +#define NAME_LEN 64 /* buffer length temp model name */
> +#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)
> +
> +static void cleanup_nvme_map(struct nvme_map *map)
> +{
> + if (map->udev)
> + udev_device_unref(map->udev);
> + if (map->subsys)
> + udev_device_unref(map->subsys);

According to the libudev reference manual, udev devices returned by
udev_device_get_parent_with_subsystem_devtype are attached to their
child and free along with them, so this unref shouldn't be necessary

> + free(map);
> +}
> +
> +static const struct _vector*
> +nvme_mp_get_pgs(const struct gen_multipath *gmp) {
> + return NULL;
> +}
> +
> +static void
> +nvme_mp_rel_pgs(const struct gen_multipath *gmp, const struct _vector *v)
> +{
> +}
> +
> +static void rstrip(char *str)
> +{
> + int n;
> +
> + for (n = strlen(str) - 1; n >= 0 && str[n] == ' '; n--);
> + str[n+1] = '\0';
> +}
> +
> +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;
> +
> + switch (wildcard) {
> + case 'd':
> + return snprintf(buff, len, "%s",
> + udev_device_get_sysname(nvm->udev));
> + case 'n':
> + return snprintf(buff, len, "%s:NQN:%s",
> + udev_device_get_sysname(nvm->subsys),
> + udev_device_get_sysattr_value(nvm->subsys,
> +   "subsysnqn"));
> + case 'w':
> + return snpr

Re: [dm-devel] [RFC PATCH 18/20] multipath: use foreign API

2018-02-28 Thread Benjamin Marzinski
On Tue, Feb 20, 2018 at 02:26:56PM +0100, Martin Wilck wrote:
> Use the "foreign" code to print information about multipath maps
> owned by foreign libraries in print mode (multipath -ll, -l).
> 

It's not a big deal, but I assume your is_claimed_by_foreign() call in
pathinfo() from your next patch was supposed to be in this one.  Until I
saw that, I couldn't figure out how foreign maps could ever get added
during a call to multipath.

> Signed-off-by: Martin Wilck 
> ---
>  multipath/main.c | 13 +
>  1 file changed, 13 insertions(+)
> 
> diff --git a/multipath/main.c b/multipath/main.c
> index a0c750e6f623..4fae49ee4325 100644
> --- a/multipath/main.c
> +++ b/multipath/main.c
> @@ -59,6 +59,7 @@
>  #include "wwids.h"
>  #include "uxsock.h"
>  #include "mpath_cmd.h"
> +#include "foreign.h"
>  
>  int logsink;
>  struct udev *udev;
> @@ -257,6 +258,14 @@ get_dm_mpvec (enum mpath_cmds cmd, vector curmp, vector 
> pathvec, char * refwwid)
>   if (cmd == CMD_CREATE)
>   reinstate_paths(mpp);
>   }
> +
> + if (cmd == CMD_LIST_SHORT || cmd == CMD_LIST_LONG) {
> + struct config *conf = get_multipath_config();
> +
> + print_foreign_topology(conf->verbosity);
> + put_multipath_config(conf);
> + }
> +
>   return 0;
>  }
>  
> @@ -460,6 +469,7 @@ configure (struct config *conf, enum mpath_cmds cmd,
>   print_all_paths(pathvec, 1);
>  
>   get_path_layout(pathvec, 0);
> + foreign_path_layout();
>  
>   if (get_dm_mpvec(cmd, curmp, pathvec, refwwid))
>   goto out;
> @@ -817,6 +827,8 @@ main (int argc, char *argv[])
>   condlog(0, "failed to initialize prioritizers");
>   goto out;
>   }
> + /* Failing here is non-fatal */
> + init_foreign(conf->multipath_dir);
>   if (cmd == CMD_USABLE_PATHS) {
>   r = check_usable_paths(conf, dev, dev_type);
>   goto out;
> @@ -892,6 +904,7 @@ out:
>   dm_lib_release();
>   dm_lib_exit();
>  
> + cleanup_foreign();
>   cleanup_prio();
>   cleanup_checkers();
>  
> -- 
> 2.16.1

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel


Re: [dm-devel] [RFC PATCH 19/20] multipathd: use foreign API

2018-02-28 Thread Benjamin Marzinski
On Tue, Feb 20, 2018 at 02:26:57PM +0100, Martin Wilck wrote:
> Call into the foreign library code when paths are discovered, uevents
> are received, and in the checker loop. Furthermore, use the foreign
> code to print information in the "multipathd show paths", "multipathd
> show maps", and "multipathd show topology" client commands.
> 
> We don't support foreign data in the individual "show map" and "show path"
> commands, and neither in the "json" commands. The former is a deliberate
> decision, the latter could be added if desired.
> 
> Signed-off-by: Martin Wilck 
> ---
>  libmultipath/discovery.c  |  4 +++-
>  multipathd/cli_handlers.c | 39 ++-
>  multipathd/main.c | 43 ---
>  3 files changed, 77 insertions(+), 9 deletions(-)
> 
> diff --git a/libmultipath/discovery.c b/libmultipath/discovery.c
> index 645224c1029c..45a4d8378893 100644
> --- a/libmultipath/discovery.c
> +++ b/libmultipath/discovery.c
> @@ -31,6 +31,7 @@
>  #include "prio.h"
>  #include "defaults.h"
>  #include "prioritizers/alua_rtpg.h"
> +#include "foreign.h"
>  
>  int
>  alloc_path_with_pathinfo (struct config *conf, struct udev_device *udevice,
> @@ -1909,7 +1910,8 @@ int pathinfo(struct path *pp, struct config *conf, int 
> mask)
>* limited by DI_BLACKLIST and occurs before this debug
>* message with the mask value.
>*/
> - if (pp->udev && filter_property(conf, pp->udev) > 0)
> + if (pp->udev && (is_claimed_by_foreign(pp->udev) ||
> +  filter_property(conf, pp->udev) > 0))
>   return PATHINFO_SKIPPED;
>  
>   if (filter_devnode(conf->blist_devnode,
> diff --git a/multipathd/cli_handlers.c b/multipathd/cli_handlers.c
> index 78f2a12bc2f8..c0ae54aae841 100644
> --- a/multipathd/cli_handlers.c
> +++ b/multipathd/cli_handlers.c
> @@ -27,6 +27,7 @@
>  #include "main.h"
>  #include "cli.h"
>  #include "uevent.h"
> +#include "foreign.h"
>  
>  int
>  show_paths (char ** r, int * len, struct vectors * vecs, char * style,
> @@ -35,11 +36,13 @@ show_paths (char ** r, int * len, struct vectors * vecs, 
> char * style,
>   int i;
>   struct path * pp;
>   char * c;
> - char * reply;
> + char * reply, * header;
>   unsigned int maxlen = INITIAL_REPLY_LEN;
>   int again = 1;
>  
>   get_path_layout(vecs->pathvec, 1);
> + foreign_path_layout();
> +
>   reply = MALLOC(maxlen);
>  
>   while (again) {
> @@ -48,18 +51,29 @@ show_paths (char ** r, int * len, struct vectors * vecs, 
> char * style,
>  
>   c = reply;
>  
> - if (pretty && VECTOR_SIZE(vecs->pathvec) > 0)
> + if (pretty)
>   c += snprint_path_header(c, reply + maxlen - c,
>style);
> + header = c;
>  
>   vector_foreach_slot(vecs->pathvec, pp, i)
>   c += snprint_path(c, reply + maxlen - c,
> style, pp, pretty);
>  
> + c += snprint_foreign_paths(c, reply + maxlen - c,
> +style, pretty);
> +
>   again = ((c - reply) == (maxlen - 1));
>  
>   REALLOC_REPLY(reply, again, maxlen);
>   }
> +
> + if (pretty && c == header) {
> + /* No output - clear header */
> + *reply = '\0';
> + c = reply;
> + }
> +
>   *r = reply;
>   *len = (int)(c - reply + 1);
>   return 0;
> @@ -134,6 +148,8 @@ show_maps_topology (char ** r, int * len, struct vectors 
> * vecs)
>   int again = 1;
>  
>   get_path_layout(vecs->pathvec, 0);
> + foreign_path_layout();
> +
>   reply = MALLOC(maxlen);
>  
>   while (again) {
> @@ -150,11 +166,13 @@ show_maps_topology (char ** r, int * len, struct 
> vectors * vecs)
>   c += snprint_multipath_topology(c, reply + maxlen - c,
>   mpp, 2);
>   }
> + c += snprint_foreign_topology(c, reply + maxlen - c, 2);
>  
>   again = ((c - reply) == (maxlen - 1));
>  
>   REALLOC_REPLY(reply, again, maxlen);
>   }
> +
>   *r = reply;
>   *len = (int)(c - reply + 1);
>   return 0;
> @@ -499,12 +517,14 @@ show_maps (char ** r, int *len, struct vectors * vecs, 
> char * style,
>  {
>   int i;
>   struct multipath * mpp;
> - char * c;
> + char * c, *header;
>   char * reply;
>   unsigned int maxlen = INITIAL_REPLY_LEN;
>   int again = 1;
>  
>   get_multipath_layout(vecs->mpvec, 1);
> + foreign_multipath_layout();
> +
>   reply = MALLOC(maxlen);
>  
>   while (again) {
> @@ -512,9 +532,10 @@ show_maps (char ** r, int *len, struct vectors * vecs, 
> char * style,
>   return 1;
>  
>   c = reply;
> - if (pretty && VECTOR_SIZE(vecs->mpvec) > 0

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 

Re: [dm-devel] [PATCH] libmultipath: fix wrong output of "multipath -t"

2018-02-28 Thread Benjamin Marzinski
On Mon, Feb 26, 2018 at 01:55:58PM +0100, Martin Wilck wrote:
> The default values printed by "multipath -t" or "multipathd show config"
> for "detect_prio", "detect_checker", and "retain_attached_hw_handler"
> don't match the actual compiled-in defaults. Moreover, several other
> options would also be displayed wrongly if the defaults were changed.
> 

Reviewed-by: Benjamin Marzinski 

> Signed-off-by: Martin Wilck 
> ---
>  libmultipath/dict.c | 25 -
>  1 file changed, 16 insertions(+), 9 deletions(-)
> 
> diff --git a/libmultipath/dict.c b/libmultipath/dict.c
> index 3694efb4..98dfc4b8744b 100644
> --- a/libmultipath/dict.c
> +++ b/libmultipath/dict.c
> @@ -345,7 +345,7 @@ declare_def_handler(checker_timeout, set_int)
>  declare_def_snprint(checker_timeout, print_nonzero)
>  
>  declare_def_handler(flush_on_last_del, set_yes_no_undef)
> -declare_def_snprint_defint(flush_on_last_del, print_yes_no_undef, YNU_NO)
> +declare_def_snprint_defint(flush_on_last_del, print_yes_no_undef, 
> DEFAULT_FLUSH)
>  declare_ovr_handler(flush_on_last_del, set_yes_no_undef)
>  declare_ovr_snprint(flush_on_last_del, print_yes_no_undef)
>  declare_hw_handler(flush_on_last_del, set_yes_no_undef)
> @@ -354,7 +354,8 @@ declare_mp_handler(flush_on_last_del, set_yes_no_undef)
>  declare_mp_snprint(flush_on_last_del, print_yes_no_undef)
>  
>  declare_def_handler(user_friendly_names, set_yes_no_undef)
> -declare_def_snprint_defint(user_friendly_names, print_yes_no_undef, YNU_NO)
> +declare_def_snprint_defint(user_friendly_names, print_yes_no_undef,
> +DEFAULT_USER_FRIENDLY_NAMES)
>  declare_ovr_handler(user_friendly_names, set_yes_no_undef)
>  declare_ovr_snprint(user_friendly_names, print_yes_no_undef)
>  declare_hw_handler(user_friendly_names, set_yes_no_undef)
> @@ -372,21 +373,24 @@ declare_def_handler(prkeys_file, set_str)
>  declare_def_snprint(prkeys_file, print_str)
>  
>  declare_def_handler(retain_hwhandler, set_yes_no_undef)
> -declare_def_snprint_defint(retain_hwhandler, print_yes_no_undef, YNU_NO)
> +declare_def_snprint_defint(retain_hwhandler, print_yes_no_undef,
> +DEFAULT_RETAIN_HWHANDLER)
>  declare_ovr_handler(retain_hwhandler, set_yes_no_undef)
>  declare_ovr_snprint(retain_hwhandler, print_yes_no_undef)
>  declare_hw_handler(retain_hwhandler, set_yes_no_undef)
>  declare_hw_snprint(retain_hwhandler, print_yes_no_undef)
>  
>  declare_def_handler(detect_prio, set_yes_no_undef)
> -declare_def_snprint_defint(detect_prio, print_yes_no_undef, YNU_NO)
> +declare_def_snprint_defint(detect_prio, print_yes_no_undef,
> +DEFAULT_DETECT_PRIO)
>  declare_ovr_handler(detect_prio, set_yes_no_undef)
>  declare_ovr_snprint(detect_prio, print_yes_no_undef)
>  declare_hw_handler(detect_prio, set_yes_no_undef)
>  declare_hw_snprint(detect_prio, print_yes_no_undef)
>  
>  declare_def_handler(detect_checker, set_yes_no_undef)
> -declare_def_snprint_defint(detect_checker, print_yes_no_undef, YNU_NO)
> +declare_def_snprint_defint(detect_checker, print_yes_no_undef,
> +DEFAULT_DETECT_CHECKER)
>  declare_ovr_handler(detect_checker, set_yes_no_undef)
>  declare_ovr_snprint(detect_checker, print_yes_no_undef)
>  declare_hw_handler(detect_checker, set_yes_no_undef)
> @@ -396,7 +400,8 @@ declare_def_handler(force_sync, set_yes_no)
>  declare_def_snprint(force_sync, print_yes_no)
>  
>  declare_def_handler(deferred_remove, set_yes_no_undef)
> -declare_def_snprint_defint(deferred_remove, print_yes_no_undef, YNU_NO)
> +declare_def_snprint_defint(deferred_remove, print_yes_no_undef,
> +DEFAULT_DEFERRED_REMOVE)
>  declare_ovr_handler(deferred_remove, set_yes_no_undef)
>  declare_ovr_snprint(deferred_remove, print_yes_no_undef)
>  declare_hw_handler(deferred_remove, set_yes_no_undef)
> @@ -417,7 +422,8 @@ declare_def_handler(strict_timing, set_yes_no)
>  declare_def_snprint(strict_timing, print_yes_no)
>  
>  declare_def_handler(skip_kpartx, set_yes_no_undef)
> -declare_def_snprint_defint(skip_kpartx, print_yes_no_undef, YNU_NO)
> +declare_def_snprint_defint(skip_kpartx, print_yes_no_undef,
> +DEFAULT_SKIP_KPARTX)
>  declare_ovr_handler(skip_kpartx, set_yes_no_undef)
>  declare_ovr_snprint(skip_kpartx, print_yes_no_undef)
>  declare_hw_handler(skip_kpartx, set_yes_no_undef)
> @@ -634,7 +640,8 @@ print_fast_io_fail(char * buff, int len, void *ptr)
>  }
>  
>  declare_def_handler(fast_io_fail, set_fast_io_fail)
> -declare_def_snprint_defint(fast_io_fail, print_fast_io_fail, 
> DEFAULT_FAST_IO_FAIL)
> +declare_def_snprint_defint(fast_io_fail, print_fast_io_fail,
> +DEFAULT_FAST_IO_FAIL)
>  declare_ovr_handler(fast_io_fail, set_fast_io_fail)
>  declare_ovr_snprint(fast_io_fail, print_fast_io_fail)
>  declare_hw_handler(fast_io_fail, set_fast_io_fail)
> @@ -828,7 +835,7 @@ print_rr_weight (char * buff, int len, void *ptr)
>  }
>  
>  declare_def_handler(rr_we

Re: [dm-devel] [RFC PATCH 01/20] multipath(d)/Makefile: add explicit dependency on libraries

2018-02-28 Thread Benjamin Marzinski
On Tue, Feb 20, 2018 at 02:26:39PM +0100, Martin Wilck wrote:
> Otherwise the binaries won't be re-linked if the libraries change.
> 

Reviewed-by: Benjamin Marzinski 

> Signed-off-by: Martin Wilck 
> ---
>  multipath/Makefile  | 2 +-
>  multipathd/Makefile | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/multipath/Makefile b/multipath/Makefile
> index 654568af3576..0828a8f72db7 100644
> --- a/multipath/Makefile
> +++ b/multipath/Makefile
> @@ -14,7 +14,7 @@ OBJS = main.o
>  
>  all: $(EXEC)
>  
> -$(EXEC): $(OBJS)
> +$(EXEC): $(OBJS) $(multipathdir)/libmultipath.so 
> $(mpathcmddir)/libmpathcmd.so
>   $(CC) $(CFLAGS) $(OBJS) -o $(EXEC) $(LDFLAGS) $(LIBDEPS)
>   $(GZIP) $(EXEC).8 > $(EXEC).8.gz
>   $(GZIP) $(EXEC).conf.5 > $(EXEC).conf.5.gz
> diff --git a/multipathd/Makefile b/multipathd/Makefile
> index 251690ec5e2a..4c9d29634160 100644
> --- a/multipathd/Makefile
> +++ b/multipathd/Makefile
> @@ -28,7 +28,7 @@ EXEC = multipathd
>  
>  all : $(EXEC)
>  
> -$(EXEC): $(OBJS)
> +$(EXEC): $(OBJS) $(multipathdir)/libmultipath.so 
> $(mpathcmddir)/libmpathcmd.so
>   $(CC) $(CFLAGS) $(OBJS) $(LDFLAGS) -o $(EXEC) $(LIBDEPS)
>   $(GZIP) $(EXEC).8 > $(EXEC).8.gz
>  
> -- 
> 2.16.1

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel


Re: [dm-devel] [RFC PATCH 02/20] libmultipath: remove unused "stdout helpers"

2018-02-28 Thread Benjamin Marzinski
On Tue, Feb 20, 2018 at 02:26:40PM +0100, Martin Wilck wrote:
> Signed-off-by: Martin Wilck 

Reviewed-by: Benjamin Marzinski 

> ---
>  libmultipath/print.c | 26 --
>  libmultipath/print.h |  5 -
>  2 files changed, 31 deletions(-)
> 
> diff --git a/libmultipath/print.c b/libmultipath/print.c
> index 65a98247a753..27636c35e5ff 100644
> --- a/libmultipath/print.c
> +++ b/libmultipath/print.c
> @@ -1765,32 +1765,6 @@ void print_path(struct path *pp, char *style)
>   printf("%s", line);
>  }
>  
> -void print_multipath(struct multipath *mpp, char *style)
> -{
> - char line[MAX_LINE_LEN];
> -
> - memset(&line[0], 0, MAX_LINE_LEN);
> - snprint_multipath(&line[0], MAX_LINE_LEN, style, mpp, 1);
> - printf("%s", line);
> -}
> -
> -void print_pathgroup(struct pathgroup *pgp, char *style)
> -{
> - char line[MAX_LINE_LEN];
> -
> - memset(&line[0], 0, MAX_LINE_LEN);
> - snprint_pathgroup(&line[0], MAX_LINE_LEN, style, pgp);
> - printf("%s", line);
> -}
> -
> -void print_map(struct multipath *mpp, char *params)
> -{
> - if (mpp->size && params)
> - printf("0 %llu %s %s\n",
> -  mpp->size, TGT_MPATH, params);
> - return;
> -}
> -
>  void print_all_paths(vector pathvec, int banner)
>  {
>   print_all_paths_custo(pathvec, banner, PRINT_PATH_LONG);
> diff --git a/libmultipath/print.h b/libmultipath/print.h
> index b8c343679e15..734f43fd4cb6 100644
> --- a/libmultipath/print.h
> +++ b/libmultipath/print.h
> @@ -119,10 +119,5 @@ int snprint_tgt_wwnn (char *, size_t, struct path *);
>  int snprint_tgt_wwpn (char *, size_t, struct path *);
>  
>  void print_multipath_topology (struct multipath * mpp, int verbosity);
> -void print_path (struct path * pp, char * style);
> -void print_multipath (struct multipath * mpp, char * style);
> -void print_pathgroup (struct pathgroup * pgp, char * style);
> -void print_map (struct multipath * mpp, char * params);
>  void print_all_paths (vector pathvec, int banner);
>  void print_all_paths_custo (vector pathvec, int banner, char *fmt);
> -void print_hwtable (vector hwtable);
> -- 
> 2.16.1

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel


Re: [dm-devel] [RFC PATCH 04/20] libmultipath: parser: use call-by-value for "snprint" methods

2018-02-28 Thread Benjamin Marzinski
On Tue, Feb 20, 2018 at 02:26:42PM +0100, Martin Wilck wrote:
> Convert the snprint methods for all keywords to call-by-value,
> and use "const" qualifier for the "data" argument. This makes sure
> that "snprint" type functions don't modify the data they're print,
> helps compile-time correctness checking, and allows more proper
> "const" cleanups in the future.
> 

Reviewed-by: Benjamin Marzinski 

> Signed-off-by: Martin Wilck 
> ---
>  libmultipath/dict.c   | 206 
> --
>  libmultipath/parser.c |   9 ++-
>  libmultipath/parser.h |  12 ++-
>  3 files changed, 112 insertions(+), 115 deletions(-)
> 
> diff --git a/libmultipath/dict.c b/libmultipath/dict.c
> index e52f1f798f7a..47dc2a38f1ac 100644
> --- a/libmultipath/dict.c
> +++ b/libmultipath/dict.c
> @@ -92,46 +92,35 @@ set_yes_no_undef(vector strvec, void *ptr)
>  }
>  
>  static int
> -print_int (char *buff, int len, void *ptr)
> +print_int (char *buff, int len, long v)
>  {
> - int *int_ptr = (int *)ptr;
> - return snprintf(buff, len, "%i", *int_ptr);
> + return snprintf(buff, len, "%li", v);
>  }
>  
>  static int
> -print_nonzero (char *buff, int len, void *ptr)
> +print_nonzero (char *buff, int len, long v)
>  {
> - int *int_ptr = (int *)ptr;
> - if (!*int_ptr)
> - return 0;
> - return snprintf(buff, len, "%i", *int_ptr);
> + return snprintf(buff, len, "%li", v);
>  }
>  
>  static int
> -print_str (char *buff, int len, void *ptr)
> +print_str (char *buff, int len, const char *ptr)
>  {
> - char **str_ptr = (char **)ptr;
> - if (!*str_ptr)
> - return 0;
> - return snprintf(buff, len, "\"%s\"", *str_ptr);
> + return snprintf(buff, len, "\"%s\"", ptr);
>  }
>  
>  static int
> -print_yes_no (char *buff, int len, void *ptr)
> +print_yes_no (char *buff, int len, long v)
>  {
> - int *int_ptr = (int *)ptr;
>   return snprintf(buff, len, "\"%s\"",
> - (*int_ptr == YN_NO)? "no" : "yes");
> + (v == YN_NO)? "no" : "yes");
>  }
>  
>  static int
> -print_yes_no_undef (char *buff, int len, void *ptr)
> +print_yes_no_undef (char *buff, int len, long v)
>  {
> - int *int_ptr = (int *)ptr;
> - if (!*int_ptr)
> - return 0;
>   return snprintf(buff, len, "\"%s\"",
> - (*int_ptr == YNU_NO)? "no" : "yes");
> + (v == YNU_NO)? "no" : "yes");
>  }
>  
>  #define declare_def_handler(option, function)
> \
> @@ -143,29 +132,32 @@ def_ ## option ## _handler (struct config *conf, vector 
> strvec) \
>  
>  #define declare_def_snprint(option, function)
> \
>  static int   \
> -snprint_def_ ## option (struct config *conf, char * buff, int len, void * 
> data) \
> +snprint_def_ ## option (struct config *conf, char * buff, int len,   \
> + const void * data)  \
>  {\
> - return function (buff, len, &conf->option); \
> + return function (buff, len, conf->option);  \
>  }
>  
>  #define declare_def_snprint_defint(option, function, value)  \
>  static int   \
> -snprint_def_ ## option (struct config *conf, char * buff, int len, void * 
> data) \
> +snprint_def_ ## option (struct config *conf, char * buff, int len,   \
> + const void * data)  \
>  {\
>   int i = value;  \
>   if (!conf->option)  \
> - return function (buff, len, &i);\
> - return function (buff, len, &conf->option); \
> + return function (buff, len, i); \
> + return function (buff, len, conf->option);  \
>  }
>  
>  #define declare_def_snprint_defstr(option, function, value)  \
>  static int   \
> -snprint_def_ ## option (struct config *conf, char * buff, int len, void * 
> data) \
> +snprint_def_ ## option (struct config *conf, char * buff, int len,   \
> + const void * data)  \
>  {\
> - char *s = value;\
> + static const char *s = value;   \
>   if (!conf->option)  \
> - return function (buff, len, &s);\
> - return function (buff, len, &conf->option);   

Re: [dm-devel] [RFC PATCH 03/20] libmultipath: get rid of selector "hack" in print.c

2018-02-28 Thread Benjamin Marzinski
On Tue, Feb 20, 2018 at 02:26:41PM +0100, Martin Wilck wrote:
> By properly linking the path groups with their parent multipath,
> we don't need this "hack" any more.
> 

Reviewed-by: Benjamin Marzinski 

> Signed-off-by: Martin Wilck 
> ---
>  libmultipath/dmparser.c   |  2 +-
>  libmultipath/pgpolicies.c | 11 ++-
>  libmultipath/print.c  |  6 +++---
>  libmultipath/structs.c| 10 ++
>  libmultipath/structs.h|  3 ++-
>  5 files changed, 22 insertions(+), 10 deletions(-)
> 
> diff --git a/libmultipath/dmparser.c b/libmultipath/dmparser.c
> index 027ae989781e..783c934f1154 100644
> --- a/libmultipath/dmparser.c
> +++ b/libmultipath/dmparser.c
> @@ -267,7 +267,7 @@ int disassemble_map(vector pathvec, char *params, struct 
> multipath *mpp,
>   if (!pgp)
>   goto out;
>  
> - if (store_pathgroup(mpp->pg, pgp))
> + if (add_pathgroup(mpp, pgp))
>   goto out;
>  
>   p += get_word(p, &word);
> diff --git a/libmultipath/pgpolicies.c b/libmultipath/pgpolicies.c
> index 4ae4afbccdb7..ac2596ada442 100644
> --- a/libmultipath/pgpolicies.c
> +++ b/libmultipath/pgpolicies.c
> @@ -120,7 +120,7 @@ int group_by_node_name(struct multipath * mp)
>   if (!pgp)
>   goto out1;
>  
> - if (store_pathgroup(mp->pg, pgp))
> + if (add_pathgroup(mp, pgp))
>   goto out2;
>  
>   /* feed the first path */
> @@ -196,7 +196,7 @@ int group_by_serial(struct multipath * mp)
>   if (!pgp)
>   goto out1;
>  
> - if (store_pathgroup(mp->pg, pgp))
> + if (add_pathgroup(mp, pgp))
>   goto out2;
>  
>   /* feed the first path */
> @@ -254,7 +254,7 @@ int one_path_per_group(struct multipath *mp)
>   if (!pgp)
>   goto out;
>  
> - if (store_pathgroup(mp->pg, pgp))
> + if (add_pathgroup(mp, pgp))
>   goto out1;
>  
>   if (store_path(pgp->paths, pp))
> @@ -293,7 +293,7 @@ int one_group(struct multipath *mp)   /* aka multibus 
> */
>  
>   vector_free(pgp->paths);
>  
> - if (store_pathgroup(mp->pg, pgp))
> + if (add_pathgroup(mp, pgp))
>   goto out1;
>  
>   pgp->paths = mp->paths;
> @@ -367,8 +367,9 @@ int group_by_prio(struct multipath *mp)
>   if (i < VECTOR_SIZE(mp->pg)) {
>   if (!vector_insert_slot(mp->pg, i, pgp))
>   goto out2;
> + pgp->mpp = mp;
>   } else {
> - if (store_pathgroup(mp->pg, pgp))
> + if (add_pathgroup(mp, pgp))
>   goto out2;
>   }
>  
> diff --git a/libmultipath/print.c b/libmultipath/print.c
> index 27636c35e5ff..8fb5c5058965 100644
> --- a/libmultipath/print.c
> +++ b/libmultipath/print.c
> @@ -476,7 +476,9 @@ snprint_pri (char * buff, size_t len, struct path * pp)
>  static int
>  snprint_pg_selector (char * buff, size_t len, struct pathgroup * pgp)
>  {
> - return snprint_str(buff, len, pgp->selector);
> + const char *s = pgp->mpp->selector;
> +
> + return snprint_str(buff, len, s ? s : "");
>  }
>  
>  static int
> @@ -1030,7 +1032,6 @@ int snprint_multipath_topology(char *buff, int len, 
> struct multipath *mpp,
>  
>   vector_foreach_slot (mpp->pg, pgp, j) {
>   f=fmt;
> - pgp->selector = mpp->selector; /* hack */
>   if (j + 1 < VECTOR_SIZE(mpp->pg)) {
>   strcpy(f, "|-+- " PRINT_PG_INDENT);
>   } else
> @@ -1122,7 +1123,6 @@ snprint_multipath_fields_json (char * buff, int len,
>  
>   vector_foreach_slot (mpp->pg, pgp, i) {
>  
> - pgp->selector = mpp->selector;
>   fwd += snprint_pathgroup(buff + fwd, len - fwd, 
> PRINT_JSON_GROUP, pgp);
>   if (fwd >= len)
>   return fwd;
> diff --git a/libmultipath/structs.c b/libmultipath/structs.c
> index 4caad2a40302..1ade1a6705ad 100644
> --- a/libmultipath/structs.c
> +++ b/libmultipath/structs.c
> @@ -318,6 +318,16 @@ store_pathgroup (vector pgvec, struct pathgroup * pgp)
>   return 0;
>  }
>  
> +int add_pathgroup(struct multipath *mpp, struct pathgroup *pgp)
> +{
> + int ret = store_pathgroup(mpp->pg, pgp);
> +
> + if (ret)
> + return ret;
> + pgp->mpp = mpp;
> + return 0;
> +}
> +
>  int
>  store_hostgroup(vector hostgroupvec, struct host_group * hgp)
>  {
> diff --git a/libmultipath/structs.h b/libmultipath/structs.h
> index b951c7b0e157..71b37cc20674 100644
> --- a/libmultipath/structs.h
> +++ b/libmultipath/structs.h
> @@ -340,7 +340,7 @@ struct pathgroup {
>   int priority;
>   int enabled_paths;
>   vector paths;
> - char * selector;
> + struct multipath *mpp

Re: [dm-devel] [RFC PATCH 06/20] libmultipath/print: use "const" where appropriate

2018-02-28 Thread Benjamin Marzinski
On Tue, Feb 20, 2018 at 02:26:44PM +0100, Martin Wilck wrote:
> Convert the print.h/print.c code to use "const" qualifiers
> properly. This is generally considered good programming practice,
> and the printing code shouldn't change any objects anyway.
> 

Reviewed-by: Benjamin Marzinski 

> Signed-off-by: Martin Wilck 
> ---
>  libmultipath/configure.h |   1 -
>  libmultipath/discovery.c |   6 +--
>  libmultipath/discovery.h |   6 ++-
>  libmultipath/print.c | 133 
> ---
>  libmultipath/print.h |  41 ---
>  5 files changed, 95 insertions(+), 92 deletions(-)
> 
> diff --git a/libmultipath/configure.h b/libmultipath/configure.h
> index 0ffc28efdaf7..0f5d30a540ca 100644
> --- a/libmultipath/configure.h
> +++ b/libmultipath/configure.h
> @@ -35,5 +35,4 @@ int coalesce_paths (struct vectors *vecs, vector curmp, 
> char * refwwid, int forc
>  int get_refwwid (enum mpath_cmds cmd, char * dev, enum devtypes dev_type,
>vector pathvec, char **wwid);
>  int reload_map(struct vectors *vecs, struct multipath *mpp, int refresh, int 
> is_daemon);
> -int sysfs_get_host_adapter_name(struct path *pp, char *adapter_name);
>  struct udev_device *get_udev_device(const char *dev, enum devtypes dev_type);
> diff --git a/libmultipath/discovery.c b/libmultipath/discovery.c
> index 88e9f3b61510..98bddee52c8f 100644
> --- a/libmultipath/discovery.c
> +++ b/libmultipath/discovery.c
> @@ -401,7 +401,7 @@ sysfs_get_tgt_nodename (struct path *pp, char * node)
>   return 0;
>  }
>  
> -int sysfs_get_host_adapter_name(struct path *pp, char *adapter_name)
> +int sysfs_get_host_adapter_name(const struct path *pp, char *adapter_name)
>  {
>   int proto_id;
>  
> @@ -427,7 +427,7 @@ int sysfs_get_host_adapter_name(struct path *pp, char 
> *adapter_name)
>   return sysfs_get_host_pci_name(pp, adapter_name);
>  }
>  
> -int sysfs_get_host_pci_name(struct path *pp, char *pci_name)
> +int sysfs_get_host_pci_name(const struct path *pp, char *pci_name)
>  {
>   struct udev_device *hostdev, *parent;
>   char host_name[HOST_NAME_LEN];
> @@ -466,7 +466,7 @@ int sysfs_get_host_pci_name(struct path *pp, char 
> *pci_name)
>   return 1;
>  }
>  
> -int sysfs_get_iscsi_ip_address(struct path *pp, char *ip_address)
> +int sysfs_get_iscsi_ip_address(const struct path *pp, char *ip_address)
>  {
>   struct udev_device *hostdev;
>   char host_name[HOST_NAME_LEN];
> diff --git a/libmultipath/discovery.h b/libmultipath/discovery.h
> index bd5e6678a26d..9aacf75bfeb0 100644
> --- a/libmultipath/discovery.h
> +++ b/libmultipath/discovery.h
> @@ -44,8 +44,10 @@ int store_pathinfo (vector pathvec, struct config *conf,
>   struct path **pp_ptr);
>  int sysfs_set_scsi_tmo (struct multipath *mpp, int checkint);
>  int sysfs_get_timeout(struct path *pp, unsigned int *timeout);
> -int sysfs_get_host_pci_name(struct path *pp, char *pci_name);
> -int sysfs_get_iscsi_ip_address(struct path *pp, char *ip_address);
> +int sysfs_get_host_pci_name(const struct path *pp, char *pci_name);
> +int sysfs_get_iscsi_ip_address(const struct path *pp, char *ip_address);
> +int sysfs_get_host_adapter_name(const struct path *pp,
> + char *adapter_name);
>  ssize_t sysfs_get_vpd (struct udev_device * udev, int pg, unsigned char * 
> buff,
>  size_t len);
>  int sysfs_get_asymmetric_access_state(struct path *pp,
> diff --git a/libmultipath/print.c b/libmultipath/print.c
> index b5c00bfe69a5..594ca567e22a 100644
> --- a/libmultipath/print.c
> +++ b/libmultipath/print.c
> @@ -28,6 +28,7 @@
>  #include "devmapper.h"
>  #include "uevent.h"
>  #include "debug.h"
> +#include "discovery.h"
>  
>  #define MAX(x,y) (x > y) ? x : y
>  #define TAIL (line + len - 1 - c)
> @@ -97,7 +98,7 @@ snprint_size (char * buff, size_t len, unsigned long long 
> size)
>   * multipath info printing functions
>   */
>  static int
> -snprint_name (char * buff, size_t len, struct multipath * mpp)
> +snprint_name (char * buff, size_t len, const struct multipath * mpp)
>  {
>   if (mpp->alias)
>   return snprintf(buff, len, "%s", mpp->alias);
> @@ -106,7 +107,7 @@ snprint_name (char * buff, size_t len, struct multipath * 
> mpp)
>  }
>  
>  static int
> -snprint_sysfs (char * buff, size_t len, struct multipath * mpp)
> +snprint_sysfs (char * buff, size_t len, const struct multipath * mpp)
>  {
>   if (mpp->dmi)
>   return snprintf(buff, len, "dm-%i", mpp->dmi->minor);
> @@ -115,7 +116,7 @@ snprint_sysfs (char * buff, size_t len, struct multipath 
> * mpp)
>  }
>  
>  static int
> -snprint_ro (char * buff, size_t len, struct multipath * mpp)
> +snprint_ro (char * buff, size_t len, const struct multipath * mpp)
>  {
>   if (!mpp->dmi)
>   return snprintf(buff, len, "undef");
> @@ -154,7 +155,7 @@ out:
>  }
>  
>  static int
> -snprint_failback (char * buff, size_t len, struct multipath * mpp)
> +sn

Re: [dm-devel] [RFC PATCH 07/20] libmultipath: use "const" in devmapper code

2018-02-28 Thread Benjamin Marzinski
On Tue, Feb 20, 2018 at 02:26:45PM +0100, Martin Wilck wrote:
> Improve use of "const" qualifiers in libmultipath's devmapper code.
> 

Reviewed-by: Benjamin Marzinski 

> Signed-off-by: Martin Wilck 
> ---
>  libmultipath/devmapper.c | 32 
>  libmultipath/devmapper.h | 16 
>  2 files changed, 24 insertions(+), 24 deletions(-)
> 
> diff --git a/libmultipath/devmapper.c b/libmultipath/devmapper.c
> index f112e1cb0e66..f61838cbe369 100644
> --- a/libmultipath/devmapper.c
> +++ b/libmultipath/devmapper.c
> @@ -583,7 +583,7 @@ is_mpath_part(const char *part_name, const char *map_name)
>   return 0;
>  }
>  
> -int dm_get_status(char *name, char *outstatus)
> +int dm_get_status(const char *name, char *outstatus)
>  {
>   int r = 1;
>   struct dm_task *dmt;
> @@ -807,7 +807,7 @@ int _dm_flush_map (const char * mapname, int need_sync, 
> int deferred_remove,
>   if (need_suspend &&
>   !dm_get_map(mapname, &mapsize, params) &&
>   strstr(params, "queue_if_no_path")) {
> - if (!dm_queue_if_no_path((char *)mapname, 0))
> + if (!dm_queue_if_no_path(mapname, 0))
>   queue_if_no_path = 1;
>   else
>   /* Leave queue_if_no_path alone if unset failed */
> @@ -850,7 +850,7 @@ int _dm_flush_map (const char * mapname, int need_sync, 
> int deferred_remove,
>   } while (retries-- > 0);
>  
>   if (queue_if_no_path == 1)
> - dm_queue_if_no_path((char *)mapname, 1);
> + dm_queue_if_no_path(mapname, 1);
>  
>   return 1;
>  }
> @@ -938,7 +938,7 @@ out:
>  }
>  
>  int
> -dm_fail_path(char * mapname, char * path)
> +dm_fail_path(const char * mapname, char * path)
>  {
>   char message[32];
>  
> @@ -949,7 +949,7 @@ dm_fail_path(char * mapname, char * path)
>  }
>  
>  int
> -dm_reinstate_path(char * mapname, char * path)
> +dm_reinstate_path(const char * mapname, char * path)
>  {
>   char message[32];
>  
> @@ -960,7 +960,7 @@ dm_reinstate_path(char * mapname, char * path)
>  }
>  
>  int
> -dm_queue_if_no_path(char *mapname, int enable)
> +dm_queue_if_no_path(const char *mapname, int enable)
>  {
>   char *message;
>  
> @@ -973,7 +973,7 @@ dm_queue_if_no_path(char *mapname, int enable)
>  }
>  
>  static int
> -dm_groupmsg (char * msg, char * mapname, int index)
> +dm_groupmsg (const char * msg, const char * mapname, int index)
>  {
>   char message[32];
>  
> @@ -984,19 +984,19 @@ dm_groupmsg (char * msg, char * mapname, int index)
>  }
>  
>  int
> -dm_switchgroup(char * mapname, int index)
> +dm_switchgroup(const char * mapname, int index)
>  {
>   return dm_groupmsg("switch", mapname, index);
>  }
>  
>  int
> -dm_enablegroup(char * mapname, int index)
> +dm_enablegroup(const char * mapname, int index)
>  {
>   return dm_groupmsg("enable", mapname, index);
>  }
>  
>  int
> -dm_disablegroup(char * mapname, int index)
> +dm_disablegroup(const char * mapname, int index)
>  {
>   return dm_groupmsg("disable", mapname, index);
>  }
> @@ -1080,7 +1080,7 @@ out:
>  }
>  
>  int
> -dm_geteventnr (char *name)
> +dm_geteventnr (const char *name)
>  {
>   struct dm_info info;
>  
> @@ -1139,7 +1139,7 @@ dm_mapname(int major, int minor)
>  
>   map = dm_task_get_name(dmt);
>   if (map && strlen(map))
> - response = STRDUP((char *)dm_task_get_name(dmt));
> + response = STRDUP((const char *)dm_task_get_name(dmt));
>  
>   dm_task_destroy(dmt);
>   return response;
> @@ -1264,7 +1264,7 @@ cancel_remove_partmap (const char *name, void *unused)
>  }
>  
>  static int
> -dm_get_deferred_remove (char * mapname)
> +dm_get_deferred_remove (const char * mapname)
>  {
>   struct dm_info info;
>  
> @@ -1412,10 +1412,10 @@ out:
>   return r;
>  }
>  
> -void dm_reassign_deps(char *table, char *dep, char *newdep)
> +void dm_reassign_deps(char *table, const char *dep, const char *newdep)
>  {
> - char *p, *n;
> - char *newtable;
> + char *n;
> + const char *p, *newtable;
>  
>   newtable = strdup(table);
>   if (!newtable)
> diff --git a/libmultipath/devmapper.h b/libmultipath/devmapper.h
> index 558e6914074f..8c8ea6c29b27 100644
> --- a/libmultipath/devmapper.h
> +++ b/libmultipath/devmapper.h
> @@ -38,7 +38,7 @@ int dm_addmap_create (struct multipath *mpp, char *params);
>  int dm_addmap_reload (struct multipath *mpp, char *params, int flush);
>  int dm_map_present (const char *);
>  int dm_get_map(const char *, unsigned long long *, char *);
> -int dm_get_status(char *, char *);
> +int dm_get_status(const char *, char *);
>  int dm_type(const char *, char *);
>  int dm_is_mpath(const char *);
>  int _dm_flush_map (const char *, int, int, int, int);
> @@ -49,14 +49,14 @@ int dm_flush_map_nopaths(const char * mapname, int 
> deferred_remove);
>   _dm_flush_map(mapname, 1, 0, 1, retries)
>  int dm_cancel_deferred_remove(struct multipath *mpp);
>  int 

Re: [dm-devel] [RFC PATCH 08/20] libmultipath: fix compiler warnings for -Wcast-qual

2018-02-28 Thread Benjamin Marzinski
On Tue, Feb 20, 2018 at 02:26:46PM +0100, Martin Wilck wrote:
> Fix the warnings that were caused by adding the -Wcast-qual compiler
> flag in the previous patch.
> 

Reviewed-by: Benjamin Marzinski 

> Signed-off-by: Martin Wilck 
> ---
>  kpartx/devmapper.c  |  3 ++-
>  libmpathcmd/mpath_cmd.c |  2 +-
>  libmultipath/checkers/rbd.c |  4 ++--
>  libmultipath/devmapper.c|  3 ++-
>  libmultipath/discovery.c| 12 ++--
>  libmultipath/list.h |  4 ++--
>  libmultipath/memory.h   |  8 +++-
>  libmultipath/structs.c  |  2 +-
>  libmultipath/structs.h  |  2 +-
>  libmultipath/uevent.c   |  6 +++---
>  libmultipath/util.c |  6 +++---
>  multipathd/main.c   | 10 +-
>  12 files changed, 35 insertions(+), 27 deletions(-)
> 
> diff --git a/kpartx/devmapper.c b/kpartx/devmapper.c
> index 4469fa848de8..eb9dac639175 100644
> --- a/kpartx/devmapper.c
> +++ b/kpartx/devmapper.c
> @@ -11,6 +11,7 @@
>  #include 
>  #include "devmapper.h"
>  
> +#define FREE_CONST(p) do { free((void*)(long)p); p = NULL; } while(0)
>  #define _UUID_PREFIX "part"
>  #define UUID_PREFIX _UUID_PREFIX "%d-"
>  #define _UUID_PREFIX_LEN (sizeof(_UUID_PREFIX) - 1)
> @@ -695,7 +696,7 @@ int dm_find_part(const char *parent, const char *delim, 
> int part,
>   } else
>   *part_uuid = uuid;
>  out:
> - free((void*)tmp);
> + FREE_CONST(tmp);
>   return r;
>  }
>  
> diff --git a/libmpathcmd/mpath_cmd.c b/libmpathcmd/mpath_cmd.c
> index af618cff917c..29d148ce8aff 100644
> --- a/libmpathcmd/mpath_cmd.c
> +++ b/libmpathcmd/mpath_cmd.c
> @@ -64,7 +64,7 @@ static size_t write_all(int fd, const void *buf, size_t len)
>   }
>   if (!n)
>   return total;
> - buf = n + (char *)buf;
> + buf = n + (const char *)buf;
>   len -= n;
>   total += n;
>   }
> diff --git a/libmultipath/checkers/rbd.c b/libmultipath/checkers/rbd.c
> index 2c1800976e40..b1d99b4c81f6 100644
> --- a/libmultipath/checkers/rbd.c
> +++ b/libmultipath/checkers/rbd.c
> @@ -288,7 +288,7 @@ void libcheck_free(struct checker * c)
>  static int rbd_is_blacklisted(struct rbd_checker_context *ct, char *msg)
>  {
>   char *addr_tok, *start, *save;
> - char *cmd[2];
> + const char *cmd[2];
>   char *blklist, *stat;
>   size_t blklist_len, stat_len;
>   int ret;
> @@ -436,7 +436,7 @@ static int sysfs_write_rbd_remove(const char *buf, int 
> buf_len)
>  
>  static int rbd_rm_blacklist(struct rbd_checker_context *ct)
>  {
> - char *cmd[2];
> + const char *cmd[2];
>   char *stat, *cmd_str;
>   size_t stat_len;
>   int ret;
> diff --git a/libmultipath/devmapper.c b/libmultipath/devmapper.c
> index f61838cbe369..607aea8dc1fc 100644
> --- a/libmultipath/devmapper.c
> +++ b/libmultipath/devmapper.c
> @@ -27,6 +27,7 @@
>  #include 
>  #include 
>  
> +#define FREE_CONST(p) do { free((void*)(unsigned long)p); p = NULL; } 
> while(0)
>  #define MAX_WAIT 5
>  #define LOOPS_PER_SEC 5
>  
> @@ -1426,7 +1427,7 @@ void dm_reassign_deps(char *table, const char *dep, 
> const char *newdep)
>   n += strlen(newdep);
>   p += strlen(dep);
>   strcat(n, p);
> - free(newtable);
> + FREE_CONST(newtable);
>  }
>  
>  int dm_reassign_table(const char *name, char *old, char *new)
> diff --git a/libmultipath/discovery.c b/libmultipath/discovery.c
> index 98bddee52c8f..645224c1029c 100644
> --- a/libmultipath/discovery.c
> +++ b/libmultipath/discovery.c
> @@ -121,7 +121,7 @@ path_discover (vector pathvec, struct config * conf,
>   if (!devname)
>   return PATHINFO_FAILED;
>  
> - pp = find_path_by_dev(pathvec, (char *)devname);
> + pp = find_path_by_dev(pathvec, devname);
>   if (!pp) {
>   char devt[BLK_DEV_SIZE];
>   dev_t devnum = udev_device_get_devnum(udevice);
> @@ -905,12 +905,12 @@ static int
>  parse_vpd_pg83(const unsigned char *in, size_t in_len,
>  char *out, size_t out_len)
>  {
> - unsigned char *d;
> - unsigned char *vpd = NULL;
> + const unsigned char *d;
> + const unsigned char *vpd = NULL;
>   int len = -ENODATA, vpd_type, vpd_len, prio = -1, i, naa_prio;
>  
> - d = (unsigned char *)in + 4;
> - while (d < (unsigned char *)in + in_len) {
> + d = in + 4;
> + while (d < in + in_len) {
>   /* Select 'association: LUN' */
>   if ((d[1] & 0x30) != 0) {
>   d += d[3] + 4;
> @@ -1027,7 +1027,7 @@ parse_vpd_pg83(const unsigned char *in, size_t in_len,
>   out[len] = '\0';
>   }
>   } else if (vpd_type == 0x1) {
> - unsigned char *p;
> + const unsigned char *p;
>   int p_len;
>  
>   out[0] = '1';
> diff --git a/libmultipath/list.h b/libmultipath/list.h
> index 2b1dcf396695..c9110ac9de

Re: [dm-devel] [RFC PATCH 09/20] multipath-tools: Makefile.inc: use -Werror=cast-qual

2018-02-28 Thread Benjamin Marzinski
On Tue, Feb 20, 2018 at 02:26:47PM +0100, Martin Wilck wrote:
> Casting "const" away is often an indicator for wrong code.
> Add a compiler flag to warn about such possible breakage.
> 

Reviewed-by: Benjamin Marzinski 

> Signed-off-by: Martin Wilck 
> ---
>  Makefile.inc | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/Makefile.inc b/Makefile.inc
> index eb99c36010c1..a5b9d4e3fa74 100644
> --- a/Makefile.inc
> +++ b/Makefile.inc
> @@ -87,6 +87,7 @@ STACKPROT := $(call 
> TEST_CC_OPTION,-fstack-protector-strong,-fstack-protector)
>  OPTFLAGS = -O2 -g -pipe -Wall -Wextra -Wformat=2 -Werror=implicit-int \
> -Werror=implicit-function-declaration -Werror=format-security 
> \
> -Wno-sign-compare -Wno-unused-parameter -Wno-clobbered \
> +   -Werror=cast-qual -Werror=discarded-qualifiers \
> -Wp,-D_FORTIFY_SOURCE=2 $(STACKPROT) \
> --param=ssp-buffer-size=4
>  
> -- 
> 2.16.1

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel


Re: [dm-devel] [RFC PATCH 10/20] libmultipath: add vector_free_const()

2018-02-28 Thread Benjamin Marzinski
On Tue, Feb 20, 2018 at 02:26:48PM +0100, Martin Wilck wrote:
> ... to dispose of constant vectors (const struct _vector*).
> 

Reviewed-by: Benjamin Marzinski 

> Signed-off-by: Martin Wilck 
> ---
>  libmultipath/vector.h | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/libmultipath/vector.h b/libmultipath/vector.h
> index 5cfd4d060412..3f6e579ae19a 100644
> --- a/libmultipath/vector.h
> +++ b/libmultipath/vector.h
> @@ -46,6 +46,7 @@ typedef struct _vector *vector;
>  extern vector vector_alloc(void);
>  extern void *vector_alloc_slot(vector v);
>  extern void vector_free(vector v);
> +#define vector_free_const(x) vector_free((vector)(long)(x))
>  extern void free_strvec(vector strvec);
>  extern void vector_set_slot(vector v, void *value);
>  extern void vector_del_slot(vector v, int slot);
> -- 
> 2.16.1

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel


Re: [dm-devel] [RFC PATCH 11/20] libmultipath: add vector_convert()

2018-02-28 Thread Benjamin Marzinski
On Tue, Feb 20, 2018 at 02:26:49PM +0100, Martin Wilck wrote:
> This is a handy helper for creating one vector from another,
> mapping each element of the origin vector to an element of
> the target vector with a given conversion function. It can
> also be used to "concatenate" vectors by passing in a non-NULL first
> argument.
> 

Reviewed-by: Benjamin Marzinski 

> Signed-off-by: Martin Wilck 
> ---
>  libmultipath/vector.h | 29 +
>  1 file changed, 29 insertions(+)
> 
> diff --git a/libmultipath/vector.h b/libmultipath/vector.h
> index 3f6e579ae19a..6018b57525bf 100644
> --- a/libmultipath/vector.h
> +++ b/libmultipath/vector.h
> @@ -42,6 +42,35 @@ typedef struct _vector *vector;
>  #define vector_foreach_slot_backwards(v,p,i) \
>   for (i = VECTOR_SIZE(v); i > 0 && ((p) = (v)->slot[i-1]); i--)
>  
> +#define identity(x) (x)
> +/*
> + * Given a vector vec with elements of given type,
> + * return a newly allocated vector with elements conv(e) for each element
> + * e in vec. "conv" may be a macro or a function.
> + * Use "identity" for a simple copy.
> + */
> +#define vector_convert(new, vec, type, conv) \
> + ({  \
> + const struct _vector *__v = (vec);  \
> + vector __t = (new); \
> + type *__j;  \
> + int __i;\
> + \
> + if (__t == NULL)\
> + __t = vector_alloc();   \
> + if (__t != NULL) {  \
> + vector_foreach_slot(__v, __j, __i) {\
> + if (vector_alloc_slot(__t) == NULL) {   \
> + vector_free(__t);   \
> + __t = NULL; \
> + break;  \
> + }   \
> + vector_set_slot(__t, conv(__j));\
> + }   \
> + }   \
> + __t;\
> + })
> +
>  /* Prototypes */
>  extern vector vector_alloc(void);
>  extern void *vector_alloc_slot(vector v);
> -- 
> 2.16.1

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel


Re: [dm-devel] [RFC PATCH 16/20] libmultipath/print: add "%G - foreign" wildcard

2018-02-28 Thread Benjamin Marzinski
On Tue, Feb 20, 2018 at 02:26:54PM +0100, Martin Wilck wrote:
> This adds a format field to identify foreign maps as such, and
> uses it in default-formatted topology output (generic_style()).
> 

Reviewed-by: Benjamin Marzinski 

> Signed-off-by: Martin Wilck 
> ---
>  libmultipath/generic.c |  4 +++-
>  libmultipath/print.c   | 14 ++
>  2 files changed, 17 insertions(+), 1 deletion(-)
> 
> diff --git a/libmultipath/generic.c b/libmultipath/generic.c
> index 61cbffb708b6..5f74427cb5b1 100644
> --- a/libmultipath/generic.c
> +++ b/libmultipath/generic.c
> @@ -33,7 +33,9 @@ int generic_style(const struct gen_multipath* gm,
>   gm->ops->snprint(gm, wwid_buf, sizeof(wwid_buf), 'w');
>  
>   if (strcmp(alias_buf, wwid_buf))
> - n = snprintf(buf, len, " (%%w)");
> + n += snprintf(buf, len, "%%n (%%w) [%%G]");
> + else
> + n += snprintf(buf, len, "%%n [%%G]");
>  
>   return (n < len ? n : len - 1);
>  }
> diff --git a/libmultipath/print.c b/libmultipath/print.c
> index 9a5a6a2f4ad6..a6ff6b297b3f 100644
> --- a/libmultipath/print.c
> +++ b/libmultipath/print.c
> @@ -342,6 +342,12 @@ snprint_multipath_rev (char * buff, size_t len, const 
> struct multipath * mpp)
>   return snprintf(buff, len, "##");
>  }
>  
> +static int
> +snprint_multipath_foreign (char * buff, size_t len, const struct multipath * 
> pp)
> +{
> + return snprintf(buff, len, "%s", "--");
> +}
> +
>  static int
>  snprint_action (char * buff, size_t len, const struct multipath * mpp)
>  {
> @@ -621,6 +627,12 @@ snprint_path_checker (char * buff, size_t len, const 
> struct path * pp)
>   return snprint_str(buff, len, c->name);
>  }
>  
> +static int
> +snprint_path_foreign (char * buff, size_t len, const struct path * pp)
> +{
> + return snprintf(buff, len, "%s", "--");
> +}
> +
>  struct multipath_data mpd[] = {
>   {'n', "name",  0, snprint_name},
>   {'w', "uuid",  0, snprint_multipath_uuid},
> @@ -644,6 +656,7 @@ struct multipath_data mpd[] = {
>   {'v', "vend",  0, snprint_multipath_vend},
>   {'p', "prod",  0, snprint_multipath_prod},
>   {'e', "rev",   0, snprint_multipath_rev},
> + {'G', "foreign",   0, snprint_multipath_foreign},
>   {0, NULL, 0 , NULL}
>  };
>  
> @@ -667,6 +680,7 @@ struct path_data pd[] = {
>   {'R', "host WWPN", 0, snprint_host_wwpn},
>   {'r', "target WWPN",   0, snprint_tgt_wwpn},
>   {'a', "host adapter",  0, snprint_host_adapter},
> + {'G', "foreign",   0, snprint_path_foreign},
>   {0, NULL, 0 , NULL}
>  };
>  
> -- 
> 2.16.1

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel


Re: [dm-devel] [RFC PATCH 14/20] libmultipath: print: use generic API for get_x_layout()

2018-02-28 Thread Benjamin Marzinski
On Tue, Feb 20, 2018 at 02:26:52PM +0100, Martin Wilck wrote:
> Introduce new functions _get_path_layout and _get_multipath_layout
> using the new "generic" API to determine field widths, and map the
> old API to them.
> 
> Furthermore, replace the boolean "header" by an enum with 3 possible
> values. The new value LAYOUT_RESET_NOT allows calling the get_x_layout
> function several times and determine the overall field width.
> 

Reviewed-by: Benjamin Marzinski 

> Signed-off-by: Martin Wilck 
> ---
>  libmultipath/print.c | 73 
> 
>  libmultipath/print.h |  8 ++
>  2 files changed, 65 insertions(+), 16 deletions(-)
> 
> diff --git a/libmultipath/print.c b/libmultipath/print.c
> index 8846765066ef..9a5a6a2f4ad6 100644
> --- a/libmultipath/print.c
> +++ b/libmultipath/print.c
> @@ -698,20 +698,48 @@ snprint_wildcards (char * buff, int len)
>  }
>  
>  void
> -get_path_layout (vector pathvec, int header)
> +get_path_layout(vector pathvec, int header)
> +{
> + vector gpvec = vector_convert(NULL, pathvec, struct path,
> +   dm_path_to_gen);
> + _get_path_layout(gpvec,
> +  header ? LAYOUT_RESET_HEADER : LAYOUT_RESET_ZERO);
> + vector_free(gpvec);
> +}
> +
> +static void
> +reset_width(int *width, enum layout_reset reset, const char *header)
> +{
> + switch (reset) {
> + case LAYOUT_RESET_HEADER:
> + *width = strlen(header);
> + break;
> + case LAYOUT_RESET_ZERO:
> + *width = 0;
> + break;
> + default:
> + /* don't reset */
> + break;
> + }
> +}
> +
> +void
> +_get_path_layout (const struct _vector *gpvec, enum layout_reset reset)
>  {
>   int i, j;
>   char buff[MAX_FIELD_LEN];
> - struct path * pp;
> + const struct gen_path *gp;
>  
>   for (j = 0; pd[j].header; j++) {
> - if (header)
> - pd[j].width = strlen(pd[j].header);
> - else
> - pd[j].width = 0;
>  
> - vector_foreach_slot (pathvec, pp, i) {
> - pd[j].snprint(buff, MAX_FIELD_LEN, pp);
> + reset_width(&pd[j].width, reset, pd[j].header);
> +
> + if (gpvec == NULL)
> + continue;
> +
> + vector_foreach_slot (gpvec, gp, i) {
> + gp->ops->snprint(gp, buff, MAX_FIELD_LEN,
> +  pd[j].wildcard);
>   pd[j].width = MAX(pd[j].width, strlen(buff));
>   }
>   }
> @@ -727,22 +755,35 @@ reset_multipath_layout (void)
>  }
>  
>  void
> -get_multipath_layout (vector mpvec, int header)
> +get_multipath_layout (vector mpvec, int header) {
> + vector gmvec = vector_convert(NULL, mpvec, struct multipath,
> +   dm_multipath_to_gen);
> + _get_multipath_layout(gmvec,
> +  header ? LAYOUT_RESET_HEADER : LAYOUT_RESET_ZERO);
> + vector_free(gmvec);
> +}
> +
> +void
> +_get_multipath_layout (const struct _vector *gmvec,
> + enum layout_reset reset)
>  {
>   int i, j;
>   char buff[MAX_FIELD_LEN];
> - struct multipath * mpp;
> + const struct gen_multipath * gm;
>  
>   for (j = 0; mpd[j].header; j++) {
> - if (header)
> - mpd[j].width = strlen(mpd[j].header);
> - else
> - mpd[j].width = 0;
>  
> - vector_foreach_slot (mpvec, mpp, i) {
> - mpd[j].snprint(buff, MAX_FIELD_LEN, mpp);
> + reset_width(&mpd[j].width, reset, mpd[j].header);
> +
> + if (gmvec == NULL)
> + continue;
> +
> + vector_foreach_slot (gmvec, gm, i) {
> + gm->ops->snprint(gm, buff, MAX_FIELD_LEN,
> +  mpd[j].wildcard);
>   mpd[j].width = MAX(mpd[j].width, strlen(buff));
>   }
> + condlog(4, "%s: width %d", mpd[j].header, mpd[j].width);
>   }
>  }
>  
> diff --git a/libmultipath/print.h b/libmultipath/print.h
> index e71f87722315..1ba364ac19ac 100644
> --- a/libmultipath/print.h
> +++ b/libmultipath/print.h
> @@ -93,7 +93,15 @@ struct pathgroup_data {
>   int (*snprint)(char * buff, size_t len, const struct pathgroup * pgp);
>  };
>  
> +enum layout_reset {
> + LAYOUT_RESET_NOT,
> + LAYOUT_RESET_ZERO,
> + LAYOUT_RESET_HEADER,
> +};
> +
> +void _get_path_layout (const struct _vector *gpvec, enum layout_reset);
>  void get_path_layout (vector pathvec, int header);
> +void _get_multipath_layout (const struct _vector *gmvec, enum layout_reset);
>  void get_multipath_layout (vector mpvec, int header);
>  int snprint_path_header (char *, int, const char *);
>  int snprint_multipath_header (char *, int, const char *);
> -- 
> 2.16.1

--
dm-devel mailing list
dm-devel@redhat.com
https