Re: [dm-devel] [PATCH v4 09/11] libmultipath: retain_attached_hw_handler obsolete with 4.3+

[Martin Wilck]

On Fri, 2017-06-23 at 19:25 +0200, Xose Vazquez Perez wrote:
> On 06/22/2017 04:59 PM, Martin Wilck wrote:
> 
> > Kernels 4.3 and newer (commit 1bab0de0 "dm-mpath, scsi_dh: don't
> > let dm detach device handlers") imply "retain_attached_hw_handler
> > yes".
> > 
> > Clarify this in the propsel code, log messages, and documentation.
> > 
> > Signed-off-by: Martin Wilck 
> > Reviewed-by: Hannes Reinecke 
> > ---
> >  libmultipath/configure.c   |  3 ++-
> >  libmultipath/dmparser.c|  3 ++-
> >  libmultipath/propsel.c |  7 ++-
> >  libmultipath/util.c| 36
> > 
> >  libmultipath/util.h|  2 ++
> >  multipath/multipath.conf.5 | 15 +++
> >  6 files changed, 59 insertions(+), 7 deletions(-)
> 
> [...]
> > --- a/libmultipath/propsel.c
> > +++ b/libmultipath/propsel.c
> > @@ -628,7 +628,12 @@ int select_retain_hwhandler(struct config
> > *conf, struct multipath *mp)
> >  
> > if (!VERSION_GE(conf->version, minv_dm_retain)) {
> > mp->retain_hwhandler = RETAIN_HWHANDLER_OFF;
> > -   origin = "(setting: WARNING, requires kernel
> > version >= 1.5.0)";
> > +   origin = "(setting: WARNING, requires kernel dm-
> > mpath version >= 1.5.0)";
> 
> It would be more informative replace the dm-mpath version with the
> kernel version. No one cares about subsystems versions.

I disagree. This code should also work for vendor kernels which may
e.g. contain patches to update dm-mpath without updating the main
kernel (utsname) version.

The reason I used get_linux_version_code() for the new check my patch
introduced was that unfortunately, the dm-mpath version has not been
changed when the "retain_attached_hwhandler" feature was removed in
4.3. The next dm-mpath version change (to 1.10) happened in 4.4.
Thus I couldn't use the dm-mpath version and had to fallback to
utsname.

Thinking about it, the new check should probably be (dm_mpath version
>= 1.10 OR kernel verson >= 4.3). IMO that can be handled in an
incremental patch.

Regards,
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 v4 09/11] libmultipath: retain_attached_hw_handler obsolete with 4.3+

[Xose Vazquez Perez]

On 06/22/2017 04:59 PM, Martin Wilck wrote:

> Kernels 4.3 and newer (commit 1bab0de0 "dm-mpath, scsi_dh: don't
> let dm detach device handlers") imply "retain_attached_hw_handler yes".
> 
> Clarify this in the propsel code, log messages, and documentation.
> 
> Signed-off-by: Martin Wilck 
> Reviewed-by: Hannes Reinecke 
> ---
>  libmultipath/configure.c   |  3 ++-
>  libmultipath/dmparser.c|  3 ++-
>  libmultipath/propsel.c |  7 ++-
>  libmultipath/util.c| 36 
>  libmultipath/util.h|  2 ++
>  multipath/multipath.conf.5 | 15 +++
>  6 files changed, 59 insertions(+), 7 deletions(-)
[...]
> --- a/libmultipath/propsel.c
> +++ b/libmultipath/propsel.c
> @@ -628,7 +628,12 @@ int select_retain_hwhandler(struct config *conf, struct 
> multipath *mp)
>  
>   if (!VERSION_GE(conf->version, minv_dm_retain)) {
>   mp->retain_hwhandler = RETAIN_HWHANDLER_OFF;
> - origin = "(setting: WARNING, requires kernel version >= 1.5.0)";
> + origin = "(setting: WARNING, requires kernel dm-mpath version 
> >= 1.5.0)";
It would be more informative replace the dm-mpath version with the
kernel version. No one cares about subsystems versions.

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

Re: [dm-devel] [PATCH v4 09/11] libmultipath: retain_attached_hw_handler obsolete with 4.3+

[Benjamin Marzinski]

On Thu, Jun 22, 2017 at 04:59:11PM +0200, Martin Wilck wrote:
> Kernels 4.3 and newer (commit 1bab0de0 "dm-mpath, scsi_dh: don't
> let dm detach device handlers") imply "retain_attached_hw_handler yes".
> 
> Clarify this in the propsel code, log messages, and documentation.

ACK

-Ben

> 
> Signed-off-by: Martin Wilck 
> Reviewed-by: Hannes Reinecke 
> ---
>  libmultipath/configure.c   |  3 ++-
>  libmultipath/dmparser.c|  3 ++-
>  libmultipath/propsel.c |  7 ++-
>  libmultipath/util.c| 36 
>  libmultipath/util.h|  2 ++
>  multipath/multipath.conf.5 | 15 +++
>  6 files changed, 59 insertions(+), 7 deletions(-)
> 
> diff --git a/libmultipath/configure.c b/libmultipath/configure.c
> index a7f2b443..74b6f52a 100644
> --- a/libmultipath/configure.c
> +++ b/libmultipath/configure.c
> @@ -572,7 +572,8 @@ select_action (struct multipath * mpp, vector curmp, int 
> force_reload)
>   }
>  
>   if (mpp->retain_hwhandler != RETAIN_HWHANDLER_UNDEF &&
> - mpp->retain_hwhandler != cmpp->retain_hwhandler) {
> + mpp->retain_hwhandler != cmpp->retain_hwhandler &&
> + get_linux_version_code() < KERNEL_VERSION(4, 3, 0)) {
>   mpp->action = ACT_RELOAD;
>   condlog(3, "%s: set ACT_RELOAD (retain_hwhandler change)",
>   mpp->alias);
> diff --git a/libmultipath/dmparser.c b/libmultipath/dmparser.c
> index 1121c715..b647c256 100644
> --- a/libmultipath/dmparser.c
> +++ b/libmultipath/dmparser.c
> @@ -97,7 +97,8 @@ assemble_map (struct multipath * mp, char * params, int len)
>   } else if (mp->no_path_retry != NO_PATH_RETRY_UNDEF) {
>   add_feature(, no_path_retry);
>   }
> - if (mp->retain_hwhandler == RETAIN_HWHANDLER_ON)
> + if (mp->retain_hwhandler == RETAIN_HWHANDLER_ON &&
> + get_linux_version_code() < KERNEL_VERSION(4, 3, 0))
>   add_feature(, retain_hwhandler);
>  
>   APPEND(p, end, "%s %s %i %i", f, mp->hwhandler, nr_priority_groups,
> diff --git a/libmultipath/propsel.c b/libmultipath/propsel.c
> index e885a845..d609394e 100644
> --- a/libmultipath/propsel.c
> +++ b/libmultipath/propsel.c
> @@ -628,7 +628,12 @@ int select_retain_hwhandler(struct config *conf, struct 
> multipath *mp)
>  
>   if (!VERSION_GE(conf->version, minv_dm_retain)) {
>   mp->retain_hwhandler = RETAIN_HWHANDLER_OFF;
> - origin = "(setting: WARNING, requires kernel version >= 1.5.0)";
> + origin = "(setting: WARNING, requires kernel dm-mpath version 
> >= 1.5.0)";
> + goto out;
> + }
> + if (get_linux_version_code() >= KERNEL_VERSION(4, 3, 0)) {
> + mp->retain_hwhandler = RETAIN_HWHANDLER_ON;
> + origin = "(setting: implied in kernel >= 4.3.0)";
>   goto out;
>   }
>   mp_set_ovr(retain_hwhandler);
> diff --git a/libmultipath/util.c b/libmultipath/util.c
> index b90cd8b0..dff2ed3c 100644
> --- a/libmultipath/util.c
> +++ b/libmultipath/util.c
> @@ -6,6 +6,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -380,3 +381,38 @@ int systemd_service_enabled(const char *dev)
>   found = systemd_service_enabled_in(dev, "/run");
>   return found;
>  }
> +
> +static int _linux_version_code;
> +static pthread_once_t _lvc_initialized = PTHREAD_ONCE_INIT;
> +
> +/* Returns current kernel version encoded as major*65536 + minor*256 + patch,
> + * so, for example,  to check if the kernel is greater than 2.2.11:
> + *
> + * if (get_linux_version_code() > KERNEL_VERSION(2,2,11)) {  }
> + *
> + * Copyright (C) 1999-2004 by Erik Andersen 
> + * Code copied from busybox (GPLv2 or later)
> + */
> +static void
> +_set_linux_version_code(void)
> +{
> + struct utsname name;
> + char *t;
> + int i, r;
> +
> + uname(); /* never fails */
> + t = name.release;
> + r = 0;
> + for (i = 0; i < 3; i++) {
> + t = strtok(t, ".");
> + r = r * 256 + (t ? atoi(t) : 0);
> + t = NULL;
> + }
> + _linux_version_code = r;
> +}
> +
> +int get_linux_version_code(void)
> +{
> + pthread_once(&_lvc_initialized, _set_linux_version_code);
> + return _linux_version_code;
> +}
> diff --git a/libmultipath/util.h b/libmultipath/util.h
> index b087e32e..45291be8 100644
> --- a/libmultipath/util.h
> +++ b/libmultipath/util.h
> @@ -15,6 +15,8 @@ char *convert_dev(char *dev, int is_path_device);
>  char *parse_uid_attribute_by_attrs(char *uid_attrs, char *path_dev);
>  void setup_thread_attr(pthread_attr_t *attr, size_t stacksize, int detached);
>  int systemd_service_enabled(const char *dev);
> +int get_linux_version_code(void);
> +#define KERNEL_VERSION(maj, min, ptc) maj) * 256) + (min)) * 256 + (ptc))
>  
>  #define safe_sprintf(var, format, args...)   \
>   snprintf(var, sizeof(var), 

[dm-devel] [PATCH v4 09/11] libmultipath: retain_attached_hw_handler obsolete with 4.3+

[Martin Wilck]

Kernels 4.3 and newer (commit 1bab0de0 "dm-mpath, scsi_dh: don't
let dm detach device handlers") imply "retain_attached_hw_handler yes".

Clarify this in the propsel code, log messages, and documentation.

Signed-off-by: Martin Wilck 
Reviewed-by: Hannes Reinecke 
---
 libmultipath/configure.c   |  3 ++-
 libmultipath/dmparser.c|  3 ++-
 libmultipath/propsel.c |  7 ++-
 libmultipath/util.c| 36 
 libmultipath/util.h|  2 ++
 multipath/multipath.conf.5 | 15 +++
 6 files changed, 59 insertions(+), 7 deletions(-)

diff --git a/libmultipath/configure.c b/libmultipath/configure.c
index a7f2b443..74b6f52a 100644
--- a/libmultipath/configure.c
+++ b/libmultipath/configure.c
@@ -572,7 +572,8 @@ select_action (struct multipath * mpp, vector curmp, int 
force_reload)
}
 
if (mpp->retain_hwhandler != RETAIN_HWHANDLER_UNDEF &&
-   mpp->retain_hwhandler != cmpp->retain_hwhandler) {
+   mpp->retain_hwhandler != cmpp->retain_hwhandler &&
+   get_linux_version_code() < KERNEL_VERSION(4, 3, 0)) {
mpp->action = ACT_RELOAD;
condlog(3, "%s: set ACT_RELOAD (retain_hwhandler change)",
mpp->alias);
diff --git a/libmultipath/dmparser.c b/libmultipath/dmparser.c
index 1121c715..b647c256 100644
--- a/libmultipath/dmparser.c
+++ b/libmultipath/dmparser.c
@@ -97,7 +97,8 @@ assemble_map (struct multipath * mp, char * params, int len)
} else if (mp->no_path_retry != NO_PATH_RETRY_UNDEF) {
add_feature(, no_path_retry);
}
-   if (mp->retain_hwhandler == RETAIN_HWHANDLER_ON)
+   if (mp->retain_hwhandler == RETAIN_HWHANDLER_ON &&
+   get_linux_version_code() < KERNEL_VERSION(4, 3, 0))
add_feature(, retain_hwhandler);
 
APPEND(p, end, "%s %s %i %i", f, mp->hwhandler, nr_priority_groups,
diff --git a/libmultipath/propsel.c b/libmultipath/propsel.c
index e885a845..d609394e 100644
--- a/libmultipath/propsel.c
+++ b/libmultipath/propsel.c
@@ -628,7 +628,12 @@ int select_retain_hwhandler(struct config *conf, struct 
multipath *mp)
 
if (!VERSION_GE(conf->version, minv_dm_retain)) {
mp->retain_hwhandler = RETAIN_HWHANDLER_OFF;
-   origin = "(setting: WARNING, requires kernel version >= 1.5.0)";
+   origin = "(setting: WARNING, requires kernel dm-mpath version 
>= 1.5.0)";
+   goto out;
+   }
+   if (get_linux_version_code() >= KERNEL_VERSION(4, 3, 0)) {
+   mp->retain_hwhandler = RETAIN_HWHANDLER_ON;
+   origin = "(setting: implied in kernel >= 4.3.0)";
goto out;
}
mp_set_ovr(retain_hwhandler);
diff --git a/libmultipath/util.c b/libmultipath/util.c
index b90cd8b0..dff2ed3c 100644
--- a/libmultipath/util.c
+++ b/libmultipath/util.c
@@ -6,6 +6,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -380,3 +381,38 @@ int systemd_service_enabled(const char *dev)
found = systemd_service_enabled_in(dev, "/run");
return found;
 }
+
+static int _linux_version_code;
+static pthread_once_t _lvc_initialized = PTHREAD_ONCE_INIT;
+
+/* Returns current kernel version encoded as major*65536 + minor*256 + patch,
+ * so, for example,  to check if the kernel is greater than 2.2.11:
+ *
+ * if (get_linux_version_code() > KERNEL_VERSION(2,2,11)) {  }
+ *
+ * Copyright (C) 1999-2004 by Erik Andersen 
+ * Code copied from busybox (GPLv2 or later)
+ */
+static void
+_set_linux_version_code(void)
+{
+   struct utsname name;
+   char *t;
+   int i, r;
+
+   uname(); /* never fails */
+   t = name.release;
+   r = 0;
+   for (i = 0; i < 3; i++) {
+   t = strtok(t, ".");
+   r = r * 256 + (t ? atoi(t) : 0);
+   t = NULL;
+   }
+   _linux_version_code = r;
+}
+
+int get_linux_version_code(void)
+{
+   pthread_once(&_lvc_initialized, _set_linux_version_code);
+   return _linux_version_code;
+}
diff --git a/libmultipath/util.h b/libmultipath/util.h
index b087e32e..45291be8 100644
--- a/libmultipath/util.h
+++ b/libmultipath/util.h
@@ -15,6 +15,8 @@ char *convert_dev(char *dev, int is_path_device);
 char *parse_uid_attribute_by_attrs(char *uid_attrs, char *path_dev);
 void setup_thread_attr(pthread_attr_t *attr, size_t stacksize, int detached);
 int systemd_service_enabled(const char *dev);
+int get_linux_version_code(void);
+#define KERNEL_VERSION(maj, min, ptc) maj) * 256) + (min)) * 256 + (ptc))
 
 #define safe_sprintf(var, format, args...) \
snprintf(var, sizeof(var), format, ##args) >= sizeof(var)
diff --git a/multipath/multipath.conf.5 b/multipath/multipath.conf.5
index b32d0383..3b4e5187 100644
--- a/multipath/multipath.conf.5
+++ b/multipath/multipath.conf.5
@@ -698,15 +698,16 @@ The default is: \fB\fR
 .