Re: [dm-devel] [PATCH] mpathpersist.8: add missing documentation for -K, -C, -l

2017-06-20 Thread Benjamin Marzinski 
On Tue, May 23, 2017 at 12:16:43AM +0200, Martin Wilck wrote:

ACK

-Ben

> Furthermore, add a reference to the sg_persist man page.
> ---
>  mpathpersist/mpathpersist.8 | 21 -
>  1 file changed, 20 insertions(+), 1 deletion(-)
> 
> diff --git a/mpathpersist/mpathpersist.8 b/mpathpersist/mpathpersist.8
> index 4b15666f..a8982e65 100644
> --- a/mpathpersist/mpathpersist.8
> +++ b/mpathpersist/mpathpersist.8
> @@ -34,6 +34,12 @@ attribute must be defined in the \fI/etc/multipath.conf\fR 
> file. Otherwise the
>  \fBmultipathd\fR daemon will not check for persistent reservation for newly
>  discovered paths or reinstated paths.
>  .
> +.LP
> +\fBmpathpersist\fR supports the same command-line options as the
> +\fBsg_persist\fR utility.
> +.
> +Consult the \fBsg_persist (8)\fR manual page for an in-depth discussion of 
> the
> +various options.
>  .
>  .\" 
> 
>  .SH OPTIONS
> @@ -89,6 +95,10 @@ PR Out parameter 'APTPL'.
>  PR In: Read Keys.
>  .
>  .TP
> +.BI \--param-rk=\fIRK\fB|\-K " RK"
> +PR Out parameter reservation key (RK is in hex, up to 8 bytes).
> +.
> +.TP
>  .BI \--param-sark=\fISARK\fB|\-S " SARK"
>  PR Out parameter service action reservation key (SARK is in hex).
>  .
> @@ -97,6 +107,10 @@ PR Out parameter service action reservation key (SARK is 
> in hex).
>  PR Out: Preempt.
>  .
>  .TP
> +.B \--clear|\-C
> +PR Out: Clear registrations.
> +.
> +.TP
>  .B \--preempt-abort|\-A
>  PR Out: Preempt and Abort.
>  .
> @@ -140,6 +154,10 @@ PR Out: Reserve.
>  .BI \--transport-id=\fITIDS\fB|\-X " TIDS"
>  TransportIDs can be mentioned in several forms.
>  .
> +.TP
> +.BI \--alloc-length=\fILEN\fB|\-l " LEN"
> +PR In: maximum allocation length. LEN is a decimal number between 0 and 8192.
> +.
>  .
>  .\" 
> 
>  .SH EXAMPLE
> @@ -164,7 +182,8 @@ Read the reservation status of the /dev/mapper/mpath9 
> device:
>  .\" 
> 
>  .
>  .BR multipath (8),
> -.BR multipathd (8).
> +.BR multipathd (8),
> +.BR sg_persist (8).
>  .
>  .
>  .\" 
> 
> -- 
> 2.12.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] [PATCH] libmultipath: lazy device-mapper initialization

2017-06-20 Thread Benjamin Marzinski 
On Tue, Jun 06, 2017 at 09:33:11PM +0200, Martin Wilck wrote:
> The multipath command fails early if libdevmapper and/or the
> dm_multipath driver are not found. But some multipath operations
> don't require device mapper, notably the path checking operations
> called from udev rules during early boot.
> 
> This patch delays dm initialization until the first device-mapper
> call, allowing such operations to succeed even if the dm_multipath
> driver is not loaded yet.
> 
> This fixes the following problem: during system boot, the dm_multipath
> driver is usually loaded via "ExecStartPre" from the multipathd service
> file. But with commit d7188fcd, this service is started only after
> udev settle and thus the modules are loaded too late for udev
> rule processing, causing "multipath" invocations from udev rules to fail
> and paths to be wrongly classified as non-multipath.

ACK

-Ben

> 
> Fixes: d7188fcd "multipathd: start daemon after udev trigger"
> Signed-off-by: Martin Wilck 
> ---
>  libmultipath/devmapper.c | 79 
> ++--
>  libmultipath/devmapper.h |  4 ++-
>  libmultipath/waiter.c|  2 +-
>  multipath/main.c |  8 ++---
>  multipathd/main.c|  4 +--
>  5 files changed, 63 insertions(+), 34 deletions(-)
> 
> diff --git a/libmultipath/devmapper.c b/libmultipath/devmapper.c
> index 69b634bf..4319b381 100644
> --- a/libmultipath/devmapper.c
> +++ b/libmultipath/devmapper.c
> @@ -21,6 +21,7 @@
>  #include "memory.h"
>  #include "devmapper.h"
>  #include "sysfs.h"
> +#include "config.h"
>  
>  #include "log_pthread.h"
>  #include 
> @@ -178,7 +179,7 @@ out:
>  }
>  
>  static int
> -dm_drv_prereq (void)
> +dm_drv_prereq (unsigned int *ver)
>  {
>   unsigned int minv[3] = {1, 0, 3};
>   unsigned int version[3] = {0, 0, 0};
> @@ -193,19 +194,51 @@ dm_drv_prereq (void)
>   condlog(3, "DM multipath kernel driver v%u.%u.%u",
>   v[0], v[1], v[2]);
>  
> - if VERSION_GE(v, minv)
> + if (VERSION_GE(v, minv)) {
> + ver[0] = v[0];
> + ver[1] = v[1];
> + ver[2] = v[2];
>   return 0;
> + }
>  
>   condlog(0, "DM multipath kernel driver must be >= v%u.%u.%u",
>   minv[0], minv[1], minv[2]);
>   return 1;
>  }
>  
> -int dm_prereq(void)
> +static int dm_prereq(unsigned int *v)
>  {
>   if (dm_lib_prereq())
>   return 1;
> - return dm_drv_prereq();
> + return dm_drv_prereq(v);
> +}
> +
> +static int libmp_dm_udev_sync = 0;
> +
> +void libmp_udev_set_sync_support(int on)
> +{
> + libmp_dm_udev_sync = !!on;
> +}
> +
> +void libmp_dm_init(void)
> +{
> + struct config *conf;
> +
> + conf = get_multipath_config();
> + dm_init(conf->verbosity);
> + if (dm_prereq(conf->version))
> + exit(1);
> + put_multipath_config(conf);
> + dm_udev_set_sync_support(libmp_dm_udev_sync);
> +}
> +
> +struct dm_task*
> +libmp_dm_task_create(int task)
> +{
> + static pthread_once_t dm_initialized = PTHREAD_ONCE_INIT;
> +
> + pthread_once(_initialized, libmp_dm_init);
> + return dm_task_create(task);
>  }
>  
>  #define do_deferred(x) ((x) == DEFERRED_REMOVE_ON || (x) == 
> DEFERRED_REMOVE_IN_PROGRESS)
> @@ -219,7 +252,7 @@ dm_simplecmd (int task, const char *name, int no_flush, 
> int need_sync, uint16_t
>   uint32_t cookie = 0;
>   struct dm_task *dmt;
>  
> - if (!(dmt = dm_task_create (task)))
> + if (!(dmt = libmp_dm_task_create (task)))
>   return 0;
>  
>   if (!dm_task_set_name (dmt, name))
> @@ -274,7 +307,7 @@ dm_addmap (int task, const char *target, struct multipath 
> *mpp,
>   uint32_t cookie = 0;
>   uint16_t udev_flags = DM_UDEV_DISABLE_LIBRARY_FALLBACK | ((skip_kpartx 
> == SKIP_KPARTX_ON)? MPATH_UDEV_NO_KPARTX_FLAG : 0);
>  
> - if (!(dmt = dm_task_create (task)))
> + if (!(dmt = libmp_dm_task_create (task)))
>   return 0;
>  
>   if (!dm_task_set_name (dmt, mpp->alias))
> @@ -411,7 +444,7 @@ do_get_info(const char *name, struct dm_info *info)
>   int r = -1;
>   struct dm_task *dmt;
>  
> - if (!(dmt = dm_task_create(DM_DEVICE_INFO)))
> + if (!(dmt = libmp_dm_task_create(DM_DEVICE_INFO)))
>   return r;
>  
>   if (!dm_task_set_name(dmt, name))
> @@ -449,7 +482,7 @@ int dm_get_map(const char *name, unsigned long long 
> *size, char *outparams)
>   char *target_type = NULL;
>   char *params = NULL;
>  
> - if (!(dmt = dm_task_create(DM_DEVICE_TABLE)))
> + if (!(dmt = libmp_dm_task_create(DM_DEVICE_TABLE)))
>   return 1;
>  
>   if (!dm_task_set_name(dmt, name))
> @@ -485,7 +518,7 @@ dm_get_prefixed_uuid(const char *name, char *uuid)
>   const char *uuidtmp;
>   int r = 1;
>  
> - dmt = dm_task_create(DM_DEVICE_INFO);
> + dmt = libmp_dm_task_create(DM_DEVICE_INFO);
>   if (!dmt)
>   return 1;
>  
> @@ -548,7 +581,7 @@

Re: [dm-devel] [PATCH] libmpathpersist: use extern struct udev from main program

2017-06-20 Thread Benjamin Marzinski 
On Sun, Mar 26, 2017 at 03:49:47PM +0200, Martin Wilck wrote:
> Use the global variable "udev" - the internal one is not
> initialized, causing current libudev calls to fail.
> In the main program "mpathpersist", use a globally visible
> variable "udev" rather than a local variable in main().
> This imitates the way the global variable "udev" is used
> in multipath and multipathd.
> 
> Removed the "udev" parameter from mpath_lib_init() to
> clarify that it isn't used.
> 

ACK

-Ben

> Fixes: b87454988 "libmultipath: separate out 'udev' config entry"
> Signed-off-by: Martin Wilck 
> ---
>  libmpathpersist/mpath_persist.c | 4 ++--
>  libmpathpersist/mpath_persist.h | 2 +-
>  mpathpersist/main.c | 5 +++--
>  3 files changed, 6 insertions(+), 5 deletions(-)
> 
> diff --git a/libmpathpersist/mpath_persist.c b/libmpathpersist/mpath_persist.c
> index 982c7954..9bca7764 100644
> --- a/libmpathpersist/mpath_persist.c
> +++ b/libmpathpersist/mpath_persist.c
> @@ -34,10 +34,10 @@
>  
>  #define __STDC_FORMAT_MACROS 1
>  
> -struct udev *udev;
> +extern struct udev *udev;
>  
>  struct config *
> -mpath_lib_init (struct udev *udev)
> +mpath_lib_init (void)
>  {
>   struct config *conf;
>  
> diff --git a/libmpathpersist/mpath_persist.h b/libmpathpersist/mpath_persist.h
> index 79de5b5b..7422322d 100644
> --- a/libmpathpersist/mpath_persist.h
> +++ b/libmpathpersist/mpath_persist.h
> @@ -174,7 +174,7 @@ struct prout_param_descriptor {   /* PROUT 
> parameter descriptor */
>   *
>   * RETURNS: struct config ->Success, NULL->Failed.
>   */
> -extern struct config * mpath_lib_init (struct udev *udev);
> +extern struct config * mpath_lib_init (void);
>  
>  
>  /*
> diff --git a/mpathpersist/main.c b/mpathpersist/main.c
> index 2e0aba3c..e1aac8fa 100644
> --- a/mpathpersist/main.c
> +++ b/mpathpersist/main.c
> @@ -57,6 +57,8 @@ void rcu_register_thread_memb(void) {}
>  
>  void rcu_unregister_thread_memb(void) {}
>  
> +struct udev *udev;
> +
>  int main (int argc, char * argv[])
>  {
>   int fd, c, res;
> @@ -86,7 +88,6 @@ int main (int argc, char * argv[])
>   int num_transport =0;
>   void *resp = NULL;
>   struct transportid * tmp;
> - struct udev *udev = NULL;
>   struct config *conf;
>  
>   if (optind == argc)
> @@ -104,7 +105,7 @@ int main (int argc, char * argv[])
>   }
>  
>   udev = udev_new();
> - conf = mpath_lib_init(udev);
> + conf = mpath_lib_init();
>   if(!conf) {
>   udev_unref(udev);
>   exit(1);
> -- 
> 2.12.0
> 
> --
> 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] [PATCH v2 0/8] multipath-tools: no_path_retry/queue_if_no_path logic

2017-06-20 Thread Martin Wilck 
On Tue, 2017-06-20 at 13:44 -0500, Benjamin Marzinski wrote:
> Review and comments are highly welcome.
> 
> ACK for the set

I found another issue with the patch set, will send v3 shortly.

Martin

-- 
Dr. Martin Wilck , Tel. +49 (0)911 74053 2107
SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)

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

Re: [dm-devel] [PATCH v2 0/8] multipath-tools: no_path_retry/queue_if_no_path logic

2017-06-20 Thread Martin Wilck 
Hello Ben,

On Tue, 2017-06-20 at 13:44 -0500, Benjamin Marzinski wrote:
> 
> > Review and comments are highly welcome.
> 
> ACK for the set

Thanks a lot. I'd be grateful if you could also take a look at my
previous submissions which have seen no review yet:

"libmpathpersist: use extern struct udev from main program"
"libmultipath: lazy device-mapper initialization"
"mpathpersist.8: add missing documentation for -K, -C, -l"

Martin

-- 
Dr. Martin Wilck , Tel. +49 (0)911 74053 2107
SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)

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

[dm-devel] [PATCH] Clarify commit message in select_max_sectors_kb()

2017-06-20 Thread Martin Wilck 
Do you like it better this way?
---
 libmultipath/propsel.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/libmultipath/propsel.c b/libmultipath/propsel.c
index f11052f2..6d628052 100644
--- a/libmultipath/propsel.c
+++ b/libmultipath/propsel.c
@@ -798,8 +798,9 @@ int select_max_sectors_kb(struct config *conf, struct 
multipath * mp)
mp_set_conf(max_sectors_kb);
mp_set_default(max_sectors_kb, DEFAULT_MAX_SECTORS_KB);
/*
-* In the default case, we will not modify max_sectors_kb.
-* Don't print a log message to avoid user confusion.
+* In the default case, we will not modify max_sectors_kb in sysfs
+* (see sysfs_set_max_sectors_kb()).
+* Don't print a log message here to avoid user confusion.
 */
return 0;
 out:
-- 
2.13.1

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

Re: [dm-devel] [PATCH v2 1/8] libmultipath: load_config: skip setting unnecessary defaults

2017-06-20 Thread Benjamin Marzinski 
On Tue, Jun 20, 2017 at 04:36:47PM +0200, Hannes Reinecke wrote:
> On 06/20/2017 01:46 PM, Martin Wilck wrote:
> > We have the logic for setting defaults for paths and maps
> > in propsel.c. By pre-setting conf values with defaults in
> > load_config(), we generate irritating log messages like
> > 'features = "0" (setting: multipath.conf defaults/devices section)'
> > if multipath.conf doesn't contain a features setting at all.
> > 
> > For some config settings, we need to use declare_def_snprint_defint()
> > now to make sure "multipathd show config" prints all defaults correctly.
> > To avoid confusion, we don't do this for "max_sectors", because
> > multipathd leaves this untouched by default.
> > 
> > Signed-off-by: Martin Wilck 
> > ---
> >  libmultipath/config.c  | 16 
> >  libmultipath/dict.c| 11 +++
> >  libmultipath/propsel.c |  5 +
> >  3 files changed, 12 insertions(+), 20 deletions(-)
> > 
> > diff --git a/libmultipath/config.c b/libmultipath/config.c
> > index bb6619b3..61bbba91 100644
> > --- a/libmultipath/config.c
> > +++ b/libmultipath/config.c
> > @@ -599,40 +599,24 @@ load_config (char * file)
> > if (!conf->verbosity)
> > conf->verbosity = DEFAULT_VERBOSITY;
> >  
> > -   conf->minio = DEFAULT_MINIO;
> > -   conf->minio_rq = DEFAULT_MINIO_RQ;
> > get_sys_max_fds(>max_fds);
> > conf->bindings_file = set_default(DEFAULT_BINDINGS_FILE);
> > conf->wwids_file = set_default(DEFAULT_WWIDS_FILE);
> > conf->multipath_dir = set_default(DEFAULT_MULTIPATHDIR);
> > -   conf->features = set_default(DEFAULT_FEATURES);
> > -   conf->flush_on_last_del = DEFAULT_FLUSH;
> > conf->attribute_flags = 0;
> > conf->reassign_maps = DEFAULT_REASSIGN_MAPS;
> > conf->checkint = DEFAULT_CHECKINT;
> > conf->max_checkint = 0;
> > -   conf->pgfailback = DEFAULT_FAILBACK;
> > -   conf->fast_io_fail = DEFAULT_FAST_IO_FAIL;
> > -   conf->retain_hwhandler = DEFAULT_RETAIN_HWHANDLER;
> > -   conf->detect_prio = DEFAULT_DETECT_PRIO;
> > -   conf->detect_checker = DEFAULT_DETECT_CHECKER;
> > conf->force_sync = DEFAULT_FORCE_SYNC;
> > conf->partition_delim = DEFAULT_PARTITION_DELIM;
> > conf->processed_main_config = 0;
> > conf->find_multipaths = DEFAULT_FIND_MULTIPATHS;
> > conf->uxsock_timeout = DEFAULT_REPLY_TIMEOUT;
> > -   conf->uid_attribute = set_default(DEFAULT_UID_ATTRIBUTE);
> > conf->retrigger_tries = DEFAULT_RETRIGGER_TRIES;
> > conf->retrigger_delay = DEFAULT_RETRIGGER_DELAY;
> > conf->uev_wait_timeout = DEFAULT_UEV_WAIT_TIMEOUT;
> > -   conf->deferred_remove = DEFAULT_DEFERRED_REMOVE;
> > -   conf->skip_kpartx = DEFAULT_SKIP_KPARTX;
> > conf->disable_changed_wwids = DEFAULT_DISABLE_CHANGED_WWIDS;
> > conf->remove_retries = 0;
> > -   conf->max_sectors_kb = DEFAULT_MAX_SECTORS_KB;
> > -   conf->san_path_err_threshold = DEFAULT_ERR_CHECKS;
> > -   conf->san_path_err_forget_rate = DEFAULT_ERR_CHECKS;
> > -   conf->san_path_err_recovery_time = DEFAULT_ERR_CHECKS;
> >  
> > /*
> >  * preload default hwtable
> > diff --git a/libmultipath/dict.c b/libmultipath/dict.c
> > index 82066f67..9dc10904 100644
> > --- a/libmultipath/dict.c
> > +++ b/libmultipath/dict.c
> > @@ -630,7 +630,7 @@ print_fast_io_fail(char * buff, int len, void *ptr)
> >  }
> >  
> >  declare_def_handler(fast_io_fail, set_fast_io_fail)
> > -declare_def_snprint(fast_io_fail, print_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)
> > @@ -1082,7 +1082,8 @@ declare_hw_snprint(delay_wait_checks, 
> > print_off_int_undef)
> >  declare_mp_handler(delay_wait_checks, set_off_int_undef)
> >  declare_mp_snprint(delay_wait_checks, print_off_int_undef)
> >  declare_def_handler(san_path_err_threshold, set_off_int_undef)
> > -declare_def_snprint(san_path_err_threshold, print_off_int_undef)
> > +declare_def_snprint_defint(san_path_err_threshold, print_off_int_undef,
> > +  DEFAULT_ERR_CHECKS)
> >  declare_ovr_handler(san_path_err_threshold, set_off_int_undef)
> >  declare_ovr_snprint(san_path_err_threshold, print_off_int_undef)
> >  declare_hw_handler(san_path_err_threshold, set_off_int_undef)
> > @@ -1090,7 +1091,8 @@ declare_hw_snprint(san_path_err_threshold, 
> > print_off_int_undef)
> >  declare_mp_handler(san_path_err_threshold, set_off_int_undef)
> >  declare_mp_snprint(san_path_err_threshold, print_off_int_undef)
> >  declare_def_handler(san_path_err_forget_rate, set_off_int_undef)
> > -declare_def_snprint(san_path_err_forget_rate, print_off_int_undef)
> > +declare_def_snprint_defint(san_path_err_forget_rate, print_off_int_undef,
> > +  DEFAULT_ERR_CHECKS)
> >  declare_ovr_handler(san_path_err_forget_rate, set_off_int_undef)
> >

Re: [dm-devel] [PATCH v2 2/8] libmultipath: add/remove_feature: use const char* for feature

2017-06-20 Thread Hannes Reinecke 
On 06/20/2017 01:46 PM, Martin Wilck wrote:
> Change the argument type for the feature to add or remove to
> const char*, making it possible to pass const strings without
> warnings.
> 
> Signed-off-by: Martin Wilck 
> ---
>  libmultipath/structs.c | 30 --
>  libmultipath/structs.h |  4 ++--
>  2 files changed, 18 insertions(+), 16 deletions(-)
> 
> diff --git a/libmultipath/structs.c b/libmultipath/structs.c
> index e225f8b4..28704676 100644
> --- a/libmultipath/structs.c
> +++ b/libmultipath/structs.c
> @@ -513,10 +513,11 @@ void setup_feature(struct multipath *mpp, char *feature)
>   }
>  }
>  
> -int add_feature(char **f, char *n)
> +int add_feature(char **f, const char *n)
>  {
>   int c = 0, d, l = 0;
>   char *e, *p, *t;
> + const char *q;
>  
>   if (!f)
>   return 1;
> @@ -554,14 +555,14 @@ int add_feature(char **f, char *n)
>   if ((c % 10) == 9)
>   l++;
>   c++;
> - p = n;
> - while (*p != '\0') {
> - if (*p == ' ' && p[1] != '\0' && p[1] != ' ') {
> + q = n;
> + while (*q != '\0') {
> + if (*q == ' ' && q[1] != '\0' && q[1] != ' ') {
>   if ((c % 10) == 9)
>   l++;
>   c++;
>   }
> - p++;
> + q++;
>   }
>  
>   t = MALLOC(l + 1);
> @@ -601,10 +602,11 @@ int add_feature(char **f, char *n)
>   return 0;
>  }
>  
> -int remove_feature(char **f, char *o)
> +int remove_feature(char **f, const char *o)
>  {
>   int c = 0, d, l;
>   char *e, *p, *n;
> + const char *q;
>  
>   if (!f || !*f)
>   return 1;
> @@ -630,18 +632,18 @@ int remove_feature(char **f, char *o)
>   /* Just spaces, return */
>   if (*o == '\0')
>   return 0;
> - e = o + strlen(o);
> - while (*e == ' ')
> - e--;
> - d = (int)(e - o);
> + q = o + strlen(o);
> + while (*q == ' ')
> + q--;
> + d = (int)(q - o);
>  
>   /* Update feature count */
>   c--;
> - p = o;
> - while (p[0] != '\0') {
> - if (p[0] == ' ' && p[1] != ' ' && p[1] != '\0')
> + q = o;
> + while (q[0] != '\0') {
> + if (q[0] == ' ' && q[1] != ' ' && q[1] != '\0')
>   c--;
> - p++;
> + q++;
>   }
>  
>   /* Quick exit if all features have been removed */
> diff --git a/libmultipath/structs.h b/libmultipath/structs.h
> index 01e031ad..8ea984d9 100644
> --- a/libmultipath/structs.h
> +++ b/libmultipath/structs.h
> @@ -369,8 +369,8 @@ int pathcountgr (struct pathgroup *, int);
>  int pathcount (struct multipath *, int);
>  int pathcmp (struct pathgroup *, struct pathgroup *);
>  void setup_feature(struct multipath *, char *);
> -int add_feature (char **, char *);
> -int remove_feature (char **, char *);
> +int add_feature (char **, const char *);
> +int remove_feature (char **, const char *);
>  
>  extern char sysfs_path[PATH_SIZE];
>  
> 
Reviewed-by: Hannes Reinecke 

Cheers,

Hannes

-- 
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de   +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)

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

Re: [dm-devel] [RFC PATCH 0/4] Allow file systems to selectively bypass dm-crypt

2017-06-20 Thread Mike Snitzer 
On Fri, Jun 16 2017 at  2:42pm -0400,
Michael Halcrow  wrote:

> On Thu, Jun 15, 2017 at 08:17:02PM +0200, Milan Broz wrote:
> > On 06/15/2017 07:24 PM, Michael Halcrow wrote:
> > ...
> > >> If this is accepted, we basically allow attacker to trick system to
> > >> write plaintext to media just by setting this flag. This must never
> > >> ever happen with FDE - BY DESIGN.
> > > 
> > > That's an important point.  This expands the attack surface to include
> > > the file system, so if an adversary can provide a bad encryption key
> > > or policy at the file system layer or if there's a bug in the file
> > > system that an adversary can exploit, then users setting the
> > > allow_encrypt_override option on dmcrypt would be vulnerable.
> > > 
> > >> For me, ABSOLUTE NACK to this approach.
> > > 
> > > I can understand where you're coming from.  Given our challenges on
> > > mobile, do you have any suggestions for an alternative approach?
> > 
> > Well, what I really do not like here that you are solving problem
> > of missing properly designed cryptographic filesystem by hacking
> > some layers together that were not designed to it.
> 
> Agreed.  I enthusiastically withdraw this proposal.
> 
> I think I'm instead going to look into proposing something new in the
> block layer to address performance concerns with file system
> encryption and inline cryptographic engine support.

As should have been done from the start.  The emergence of ICE support
for mobile/embedded platforms should result in a properly designed
solution to enable dm-crypt to leverage them (easier said than done!).
And if only certain files need to be encrypted then dm-crypt may or may
not be configured in the stack.

> > It would better to go with some model that actually increases security.
> > 
> > For example, if your patch marks IO operation that comes from fs as
> > REQ_NOENCRYPT, could fs send some metadata information in bio of
> > owner (user, translated to key id) instead and encrypt the sector
> > with a different key?
> 
> I really like this idea.  However because users can access the dmcrypt
> device without the file system, I wouldn't want to try to do it
> without dmcrypt maintaining additional metadata about which keys
> protect which blocks.  Even then, the user directly accessing the
> dmcrypt device would be surprised when dmcrypt returns EIO because it
> doesn't have a key for the blocks at offsets 42 and 128.
> 
> At this point I do think something new at the block layer for file
> system managed encryption and inline cryptographic engine support will
> be necessary.

Yes.

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

Re: [dm-devel] [PATCH v2 1/8] libmultipath: load_config: skip setting unnecessary defaults

2017-06-20 Thread Hannes Reinecke 
On 06/20/2017 01:46 PM, Martin Wilck wrote:
> We have the logic for setting defaults for paths and maps
> in propsel.c. By pre-setting conf values with defaults in
> load_config(), we generate irritating log messages like
> 'features = "0" (setting: multipath.conf defaults/devices section)'
> if multipath.conf doesn't contain a features setting at all.
> 
> For some config settings, we need to use declare_def_snprint_defint()
> now to make sure "multipathd show config" prints all defaults correctly.
> To avoid confusion, we don't do this for "max_sectors", because
> multipathd leaves this untouched by default.
> 
> Signed-off-by: Martin Wilck 
> ---
>  libmultipath/config.c  | 16 
>  libmultipath/dict.c| 11 +++
>  libmultipath/propsel.c |  5 +
>  3 files changed, 12 insertions(+), 20 deletions(-)
> 
> diff --git a/libmultipath/config.c b/libmultipath/config.c
> index bb6619b3..61bbba91 100644
> --- a/libmultipath/config.c
> +++ b/libmultipath/config.c
> @@ -599,40 +599,24 @@ load_config (char * file)
>   if (!conf->verbosity)
>   conf->verbosity = DEFAULT_VERBOSITY;
>  
> - conf->minio = DEFAULT_MINIO;
> - conf->minio_rq = DEFAULT_MINIO_RQ;
>   get_sys_max_fds(>max_fds);
>   conf->bindings_file = set_default(DEFAULT_BINDINGS_FILE);
>   conf->wwids_file = set_default(DEFAULT_WWIDS_FILE);
>   conf->multipath_dir = set_default(DEFAULT_MULTIPATHDIR);
> - conf->features = set_default(DEFAULT_FEATURES);
> - conf->flush_on_last_del = DEFAULT_FLUSH;
>   conf->attribute_flags = 0;
>   conf->reassign_maps = DEFAULT_REASSIGN_MAPS;
>   conf->checkint = DEFAULT_CHECKINT;
>   conf->max_checkint = 0;
> - conf->pgfailback = DEFAULT_FAILBACK;
> - conf->fast_io_fail = DEFAULT_FAST_IO_FAIL;
> - conf->retain_hwhandler = DEFAULT_RETAIN_HWHANDLER;
> - conf->detect_prio = DEFAULT_DETECT_PRIO;
> - conf->detect_checker = DEFAULT_DETECT_CHECKER;
>   conf->force_sync = DEFAULT_FORCE_SYNC;
>   conf->partition_delim = DEFAULT_PARTITION_DELIM;
>   conf->processed_main_config = 0;
>   conf->find_multipaths = DEFAULT_FIND_MULTIPATHS;
>   conf->uxsock_timeout = DEFAULT_REPLY_TIMEOUT;
> - conf->uid_attribute = set_default(DEFAULT_UID_ATTRIBUTE);
>   conf->retrigger_tries = DEFAULT_RETRIGGER_TRIES;
>   conf->retrigger_delay = DEFAULT_RETRIGGER_DELAY;
>   conf->uev_wait_timeout = DEFAULT_UEV_WAIT_TIMEOUT;
> - conf->deferred_remove = DEFAULT_DEFERRED_REMOVE;
> - conf->skip_kpartx = DEFAULT_SKIP_KPARTX;
>   conf->disable_changed_wwids = DEFAULT_DISABLE_CHANGED_WWIDS;
>   conf->remove_retries = 0;
> - conf->max_sectors_kb = DEFAULT_MAX_SECTORS_KB;
> - conf->san_path_err_threshold = DEFAULT_ERR_CHECKS;
> - conf->san_path_err_forget_rate = DEFAULT_ERR_CHECKS;
> - conf->san_path_err_recovery_time = DEFAULT_ERR_CHECKS;
>  
>   /*
>* preload default hwtable
> diff --git a/libmultipath/dict.c b/libmultipath/dict.c
> index 82066f67..9dc10904 100644
> --- a/libmultipath/dict.c
> +++ b/libmultipath/dict.c
> @@ -630,7 +630,7 @@ print_fast_io_fail(char * buff, int len, void *ptr)
>  }
>  
>  declare_def_handler(fast_io_fail, set_fast_io_fail)
> -declare_def_snprint(fast_io_fail, print_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)
> @@ -1082,7 +1082,8 @@ declare_hw_snprint(delay_wait_checks, 
> print_off_int_undef)
>  declare_mp_handler(delay_wait_checks, set_off_int_undef)
>  declare_mp_snprint(delay_wait_checks, print_off_int_undef)
>  declare_def_handler(san_path_err_threshold, set_off_int_undef)
> -declare_def_snprint(san_path_err_threshold, print_off_int_undef)
> +declare_def_snprint_defint(san_path_err_threshold, print_off_int_undef,
> +DEFAULT_ERR_CHECKS)
>  declare_ovr_handler(san_path_err_threshold, set_off_int_undef)
>  declare_ovr_snprint(san_path_err_threshold, print_off_int_undef)
>  declare_hw_handler(san_path_err_threshold, set_off_int_undef)
> @@ -1090,7 +1091,8 @@ declare_hw_snprint(san_path_err_threshold, 
> print_off_int_undef)
>  declare_mp_handler(san_path_err_threshold, set_off_int_undef)
>  declare_mp_snprint(san_path_err_threshold, print_off_int_undef)
>  declare_def_handler(san_path_err_forget_rate, set_off_int_undef)
> -declare_def_snprint(san_path_err_forget_rate, print_off_int_undef)
> +declare_def_snprint_defint(san_path_err_forget_rate, print_off_int_undef,
> +DEFAULT_ERR_CHECKS)
>  declare_ovr_handler(san_path_err_forget_rate, set_off_int_undef)
>  declare_ovr_snprint(san_path_err_forget_rate, print_off_int_undef)
>  declare_hw_handler(san_path_err_forget_rate, set_off_int_undef)
> @@ -1098,7 +1100,8 @@ declare_hw_snprint(san_path_err_forget_rate, 
>

[dm-devel] [PATCH v2 7/8] multipath.conf.5: Remove ??? and other minor fixes

2017-06-20 Thread Martin Wilck 
Remove the FIXME markers by filling in missing content,
and make some other minor fixes.

Signed-off-by: Martin Wilck 
---
 multipath/multipath.conf.5 | 48 +-
 1 file changed, 26 insertions(+), 22 deletions(-)

diff --git a/multipath/multipath.conf.5 b/multipath/multipath.conf.5
index 6959ba5c..d8856577 100644
--- a/multipath/multipath.conf.5
+++ b/multipath/multipath.conf.5
@@ -258,7 +258,7 @@ Return a constant priority of \fI1\fR.
 .I sysfs
 Use the sysfs attributes \fIaccess_state\fR and \fIpreferred_path\fR to
 generate the path priority. This prioritizer accepts the optional prio_arg
-\fIexclusive_pref_bit\fR
+\fIexclusive_pref_bit\fR.
 .TP
 .I emc
 (Hardware-dependent)
@@ -296,14 +296,19 @@ Generate the path priority based on the regular 
expression and the
 priority provided as argument. Requires prio_args keyword.
 .TP
 .I datacore
-.\" XXX
-???. Requires prio_args keyword.
+(Hardware-dependent)
+Generate the path priority for some Datacore storage arrays. Requires prio_args
+keyword.
 .TP
 .I iet
-.\" XXX
-???. Requires prio_args keyword.
-.TP
-The default is: \fBconst\fR
+(iSCSI only)
+Generate path priority for iSCSI targets based on IP address. Requires
+prio_args keyword.
+.PP
+The default depends on the \fBdetect_prio\fR setting: If \fBdetect_prio\fR is
+\fByes\fR (default), the default priority algorithm is \fBsysfs\fR (except for
+NetAPP E-Series, where it is \fBalua\fR). If \fBdetect_prio\fR is
+\fBno\fR, the default priority algorithm is \fBconst\fR.
 .RE
 .
 .
@@ -344,12 +349,12 @@ If \fIexclusive_pref_bit\fR is set, paths with the 
\fIpreferred path\fR bit
 set will always be in their own path group.
 .TP
 .I datacore
-.\" XXX
-\fIpreferredsds\fR ???.
+\fIpreferredsds\fR (required) denotes the preferred "SDS name" for datacore
+arrays. \fItimeout\fR (optional) is the timeout for the INQUIRY, in ms.
 .TP
 .I iet
-.\" XXX
-\fIpreferredip\fR ???.
+\fIpreferredip=...\fR (required) denotes the preferred IP address (in dotted 
decimal
+notation) for iSCSI targets.
 .TP
 The default is: \fB\fR
 .RE
@@ -364,29 +369,28 @@ Possible values for the feature list are:
 .TP 12
 .\" XXX
 .I queue_if_no_path
-(Superseded by \fIno_path_retry\fR) (Since ??? kernel) Queue I/O if no path is 
active.
+(Superseded by \fIno_path_retry\fR) Queue I/O if no path is active.
 Identical to the \fIno_path_retry\fR with \fIqueue\fR value. If both this
 feature and \fIno_path_retry\fR are set, the latter value takes
 precedence. See KNOWN ISSUES.
 .TP
-.I no_partitions
-Disable automatic partitions generation via kpartx.
-.TP
 .\" XXX
 .I pg_init_retries 
-(Since ??? kernel) Number of times to retry pg_init, it must be between 1 and 
50.
+(Since kernel 2.6.24) Number of times to retry pg_init, it must be between 1 
and 50.
 .TP
 .\" XXX
 .I pg_init_delay_msecs 
-(Since ??? kernel) Number of msecs before pg_init retry, it must be between 0 
and 6.
+(Since kernel 2.6.38) Number of msecs before pg_init retry, it must be between 
0 and 6.
 .TP
 .\" XXX
 .I queue_mode 
-(Since ??? kernel) Select the the queue_mode per multipath device.
-Where  can be \fIbio\fR, \fIrq\fR or \fImq\fR. Which corresponds to
-bio-based, request_fn rq-based, and blk-mq rq-based respectively.
-.TP
-The default is: \fB0\fR
+(Since kernel 4.8) Select the the queueing mode per multipath device.
+ can be \fIbio\fR, \fIrq\fR or \fImq\f, which corresponds to
+bio-based, request-based, and block-multiqueue (blk-mq) request-based,
+respectively.
+
+The default depends on the kernel parameter \fBdm_mod.use_blk_mq\fR. It is
+\fImq\fR if the latter is set, and \fIrq\fR otherwise.
 .RE
 .
 .
-- 
2.13.1

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

[dm-devel] [PATCH v2 6/8] multipath.conf.5: document no_path_retry vs. queue_if_no_path

2017-06-20 Thread Martin Wilck 
Clarify the documentation about option precedence.

Signed-off-by: Martin Wilck 
---
 multipath/multipath.conf.5 | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/multipath/multipath.conf.5 b/multipath/multipath.conf.5
index f04ff194..6959ba5c 100644
--- a/multipath/multipath.conf.5
+++ b/multipath/multipath.conf.5
@@ -365,7 +365,9 @@ Possible values for the feature list are:
 .\" XXX
 .I queue_if_no_path
 (Superseded by \fIno_path_retry\fR) (Since ??? kernel) Queue I/O if no path is 
active.
-Identical to the \fIno_path_retry\fR with \fIqueue\fR value. See KNOWN ISSUES.
+Identical to the \fIno_path_retry\fR with \fIqueue\fR value. If both this
+feature and \fIno_path_retry\fR are set, the latter value takes
+precedence. See KNOWN ISSUES.
 .TP
 .I no_partitions
 Disable automatic partitions generation via kpartx.
-- 
2.13.1

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

[dm-devel] [PATCH v2 4/8] libmultipath: merge_hwe: fix queue_if_no_path logic

2017-06-20 Thread Martin Wilck 
The logic applied here should match the logic in select_features().
This is achieved by calling reconcile_features_with_options().

Signed-off-by: Martin Wilck 
---
 libmultipath/config.c | 20 +++-
 1 file changed, 11 insertions(+), 9 deletions(-)

diff --git a/libmultipath/config.c b/libmultipath/config.c
index 61bbba91..4f1ecee3 100644
--- a/libmultipath/config.c
+++ b/libmultipath/config.c
@@ -25,6 +25,7 @@
 #include "prio.h"
 #include "devmapper.h"
 #include "mpath_cmd.h"
+#include "propsel.h"
 
 static int
 hwe_strmatch (struct hwentry *hwe1, struct hwentry *hwe2)
@@ -318,6 +319,8 @@ set_param_str(char * str)
 static int
 merge_hwe (struct hwentry * dst, struct hwentry * src)
 {
+   int id_len;
+   char *id;
merge_str(vendor);
merge_str(product);
merge_str(revision);
@@ -353,15 +356,14 @@ merge_hwe (struct hwentry * dst, struct hwentry * src)
merge_num(san_path_err_forget_rate);
merge_num(san_path_err_recovery_time);
 
-   /*
-* Make sure features is consistent with
-* no_path_retry
-*/
-   if (dst->no_path_retry == NO_PATH_RETRY_FAIL)
-   remove_feature(>features, "queue_if_no_path");
-   else if (dst->no_path_retry != NO_PATH_RETRY_UNDEF)
-   add_feature(>features, "queue_if_no_path");
-
+   id_len = strlen(dst->vendor) + strlen(dst->product) + 2;
+   id = MALLOC(id_len);
+   if (id != NULL)
+   snprintf(id, id_len, "%s/%s", dst->vendor, dst->product);
+   reconcile_features_with_options(id, >features,
+   >no_path_retry,
+   >retain_hwhandler);
+   FREE(id);
return 0;
 }
 
-- 
2.13.1

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

[dm-devel] [PATCH v2 2/8] libmultipath: add/remove_feature: use const char* for feature

2017-06-20 Thread Martin Wilck 
Change the argument type for the feature to add or remove to
const char*, making it possible to pass const strings without
warnings.

Signed-off-by: Martin Wilck 
---
 libmultipath/structs.c | 30 --
 libmultipath/structs.h |  4 ++--
 2 files changed, 18 insertions(+), 16 deletions(-)

diff --git a/libmultipath/structs.c b/libmultipath/structs.c
index e225f8b4..28704676 100644
--- a/libmultipath/structs.c
+++ b/libmultipath/structs.c
@@ -513,10 +513,11 @@ void setup_feature(struct multipath *mpp, char *feature)
}
 }
 
-int add_feature(char **f, char *n)
+int add_feature(char **f, const char *n)
 {
int c = 0, d, l = 0;
char *e, *p, *t;
+   const char *q;
 
if (!f)
return 1;
@@ -554,14 +555,14 @@ int add_feature(char **f, char *n)
if ((c % 10) == 9)
l++;
c++;
-   p = n;
-   while (*p != '\0') {
-   if (*p == ' ' && p[1] != '\0' && p[1] != ' ') {
+   q = n;
+   while (*q != '\0') {
+   if (*q == ' ' && q[1] != '\0' && q[1] != ' ') {
if ((c % 10) == 9)
l++;
c++;
}
-   p++;
+   q++;
}
 
t = MALLOC(l + 1);
@@ -601,10 +602,11 @@ int add_feature(char **f, char *n)
return 0;
 }
 
-int remove_feature(char **f, char *o)
+int remove_feature(char **f, const char *o)
 {
int c = 0, d, l;
char *e, *p, *n;
+   const char *q;
 
if (!f || !*f)
return 1;
@@ -630,18 +632,18 @@ int remove_feature(char **f, char *o)
/* Just spaces, return */
if (*o == '\0')
return 0;
-   e = o + strlen(o);
-   while (*e == ' ')
-   e--;
-   d = (int)(e - o);
+   q = o + strlen(o);
+   while (*q == ' ')
+   q--;
+   d = (int)(q - o);
 
/* Update feature count */
c--;
-   p = o;
-   while (p[0] != '\0') {
-   if (p[0] == ' ' && p[1] != ' ' && p[1] != '\0')
+   q = o;
+   while (q[0] != '\0') {
+   if (q[0] == ' ' && q[1] != ' ' && q[1] != '\0')
c--;
-   p++;
+   q++;
}
 
/* Quick exit if all features have been removed */
diff --git a/libmultipath/structs.h b/libmultipath/structs.h
index 01e031ad..8ea984d9 100644
--- a/libmultipath/structs.h
+++ b/libmultipath/structs.h
@@ -369,8 +369,8 @@ int pathcountgr (struct pathgroup *, int);
 int pathcount (struct multipath *, int);
 int pathcmp (struct pathgroup *, struct pathgroup *);
 void setup_feature(struct multipath *, char *);
-int add_feature (char **, char *);
-int remove_feature (char **, char *);
+int add_feature (char **, const char *);
+int remove_feature (char **, const char *);
 
 extern char sysfs_path[PATH_SIZE];
 
-- 
2.13.1

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

[dm-devel] [PATCH v2 5/8] libmultipath: assemble_map: fix queue_if_no_path logic

2017-06-20 Thread Martin Wilck 
It is wrong to remove the queue_if_no_path feature if no_path_retry
is unset. Rather, in this case the feature should neither be added
nor removed.

Signed-off-by: Martin Wilck 
---
 libmultipath/dmparser.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/libmultipath/dmparser.c b/libmultipath/dmparser.c
index 469e60d2..8d0c7af1 100644
--- a/libmultipath/dmparser.c
+++ b/libmultipath/dmparser.c
@@ -74,13 +74,12 @@ assemble_map (struct multipath * mp, char * params, int len)
 * We have to set 'queue_if_no_path' here even
 * to avoid path failures during map reload.
 */
-   if (mp->no_path_retry == NO_PATH_RETRY_UNDEF ||
-   mp->no_path_retry == NO_PATH_RETRY_FAIL) {
+   if (mp->no_path_retry == NO_PATH_RETRY_FAIL) {
/* remove queue_if_no_path settings */
condlog(3, "%s: remove queue_if_no_path from '%s'",
mp->alias, mp->features);
remove_feature(, no_path_retry);
-   } else {
+   } else if (mp->no_path_retry != NO_PATH_RETRY_UNDEF) {
add_feature(, no_path_retry);
}
if (mp->retain_hwhandler == RETAIN_HWHANDLER_ON)
-- 
2.13.1

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

[dm-devel] [PATCH v2 0/8] multipath-tools: no_path_retry/queue_if_no_path logic

2017-06-20 Thread Martin Wilck 
This patch set attempts to sanitize the logic used for consistently handling
options that can be set both via the "features" string and explicit 
multipath.conf
options. This is most prominently "no_path_retry" vs. "queue_if_no_path", but 
also
"retain_attached_hw_handler" vs. the feature of the same name.

The logic this patch set follows is:
 - If "no_path_retry" is set to any value, "queue_if_no_path" is ignored
   (this is the case nowadays for almost all "real" storage arrays, thanks to 
hwtable).
 - Otherwise, if "queue_if_no_path" is set, treat it like "no_path_retry queue".

... likewise for "retain_attached_hw_handler".

In the long run, we should get rid of the "features" settings duplicating
configuration options altogether; the patch set prepares this by printing
warning messages.

The logic is implemented in the new function reconcile_features_with_options,
which is called from both select_features() and merge_hwe(). In setup_map(),
we need to call select_features() after select_no_path_retry() to make this 
work.
The actual feature setting for device-mapper is made in assemble_map(), the
patch set also fixes the logic there.

The patch set documents the behavior in the man page, and adds some more
man page fixes.

Finally, by skipping superfluous default initializations in load_config(), the
log messages for the respective config settings become more appropriate.

Review and comments are highly welcome.

Changes wrt v1:
  - Suggestions from Ben Marzinski:
* Made sure "multipathd show config" still works (1/8).
* Fixed logging for default setting of "max_sectors" (1/8)
* Consistent internal treatment of mp->features (3/8, 4/8)
* Fixed whitespace error (6/8)
* Added deprecated warnings (8/8)
  - Made sure the same logic is used in propsel.c and config.c by
calling the same function (3/8, 4/8)
  - Added deprecated warnings (8/8)

Martin Wilck (8):
  libmultipath: load_config: skip setting unnecessary defaults
  libmultipath: add/remove_feature: use const char* for feature
  libmultipath: clarify option conflicts for "features"
  libmultipath: merge_hwe: fix queue_if_no_path logic
  libmultipath: assemble_map: fix queue_if_no_path logic
  multipath.conf.5: document no_path_retry vs. queue_if_no_path
  multipath.conf.5: Remove ??? and other minor fixes
  libmultipath: add deprecated warning for some features settings

 libmultipath/config.c  | 36 +++--
 libmultipath/configure.c   |  4 +--
 libmultipath/dict.c| 11 ---
 libmultipath/dmparser.c|  5 ++-
 libmultipath/propsel.c | 79 +-
 libmultipath/propsel.h |  3 ++
 libmultipath/structs.c | 30 ++
 libmultipath/structs.h |  4 +--
 multipath/multipath.conf.5 | 52 --
 9 files changed, 136 insertions(+), 88 deletions(-)

-- 
2.13.1

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

[dm-devel] [PATCH v2 8/8] libmultipath: add deprecated warning for some features settings

2017-06-20 Thread Martin Wilck 
The device-mapper features "queue_if_no_path" and
"retain_attached_hw_handler" should be set via the configuration
keywords "no_path_retry" and "retain_attached_hw_handler",
respectively, not via "features".
Print a warning if these "features" settings are encountered.

So far these "features" settings will only be ignored if the
respective other keyword is set, so in particular
'features "1 queue_if_no_path"' will still work as expected, but
cause the warning to be printed.

We should consider ignoring these completely in a future version.

Signed-off-by: Martin Wilck 
---
 libmultipath/propsel.c | 6 ++
 multipath/multipath.conf.5 | 2 +-
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/libmultipath/propsel.c b/libmultipath/propsel.c
index ccd067f5..f11052f2 100644
--- a/libmultipath/propsel.c
+++ b/libmultipath/propsel.c
@@ -290,6 +290,9 @@ void reconcile_features_with_options(const char *id, char 
**features, int* no_pa
 * it's translated into "no_path_retry queue" here.
 */
if (strstr(*features, q_i_n_p)) {
+   condlog(0, "%s: option 'features \"1 %s\"' is deprecated, "
+   "please use 'no_path_retry queue' instead",
+   id, q_i_n_p);
if (*no_path_retry == NO_PATH_RETRY_UNDEF) {
*no_path_retry = NO_PATH_RETRY_QUEUE;
print_no_path_retry(buff, sizeof(buff),
@@ -307,6 +310,9 @@ void reconcile_features_with_options(const char *id, char 
**features, int* no_pa
remove_feature(features, q_i_n_p);
}
if (strstr(*features, r_a_h_h)) {
+   condlog(0, "%s: option 'features \"1 %s\"' is deprecated, "
+   "please use '%s yes' instead",
+   id, r_a_h_h, r_a_h_h);
if (*retain_hwhandler == RETAIN_HWHANDLER_UNDEF) {
condlog(3, "%s: %s = on (inherited setting from feature 
'%s')",
id, r_a_h_h, r_a_h_h);
diff --git a/multipath/multipath.conf.5 b/multipath/multipath.conf.5
index d8856577..bacd5e30 100644
--- a/multipath/multipath.conf.5
+++ b/multipath/multipath.conf.5
@@ -369,7 +369,7 @@ Possible values for the feature list are:
 .TP 12
 .\" XXX
 .I queue_if_no_path
-(Superseded by \fIno_path_retry\fR) Queue I/O if no path is active.
+(Deprecated, superseded by \fIno_path_retry\fR) Queue I/O if no path is active.
 Identical to the \fIno_path_retry\fR with \fIqueue\fR value. If both this
 feature and \fIno_path_retry\fR are set, the latter value takes
 precedence. See KNOWN ISSUES.
-- 
2.13.1

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