From: Xose Vazquez Perez <xose.vazquez@gmail.com>
To: Benjamin Marzinski <bmarzins@redhat.com>,
Christophe Varoqui <christophe.varoqui@opensvc.com>
Cc: device-mapper development <dm-devel@redhat.com>
Subject: Re: [PATCH 09/10] add disable_changed_wwids option
Date: Sun, 6 Nov 2016 01:03:31 +0100 [thread overview]
Message-ID: <e090f9ec-41b9-856c-4fc0-b2ced90fd9a8@gmail.com> (raw)
In-Reply-To: <1477709726-5442-10-git-send-email-bmarzins@redhat.com>
On 10/29/2016 04:55 AM, Benjamin Marzinski wrote:
> If a LUN on a storage device gets remapped while in-use by multipath,
> it's possible that the multipath device will continue writing to this
> new LUN, causing corruption. This is not multipath's fault (users
> should go remapping in-use LUNs), but it's possible for multipath to
> detect this and disable IO to the device. If disable_changed_wwids
> is set to "yes", multipathd will detect when a LUN changes wwids when it
> receives the uevent for this, and will disable access to the device
> until the LUN is mapped back.
Some info is needed at multipath.conf.5 for this new keyword.
> Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
> ---
> libmultipath/config.c | 1 +
> libmultipath/config.h | 1 +
> libmultipath/defaults.h | 1 +
> libmultipath/dict.c | 4 ++++
> libmultipath/discovery.c | 15 +++++++--------
> libmultipath/discovery.h | 1 +
> libmultipath/structs.h | 1 +
> multipathd/main.c | 32 ++++++++++++++++++++++++++++++++
> 8 files changed, 48 insertions(+), 8 deletions(-)
>
> diff --git a/libmultipath/config.c b/libmultipath/config.c
> index bdcad80..a97b318 100644
> --- a/libmultipath/config.c
> +++ b/libmultipath/config.c
> @@ -618,6 +618,7 @@ load_config (char * file)
> 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;
>
> /*
> * preload default hwtable
> diff --git a/libmultipath/config.h b/libmultipath/config.h
> index d59a993..dbdaa44 100644
> --- a/libmultipath/config.h
> +++ b/libmultipath/config.h
> @@ -144,6 +144,7 @@ struct config {
> int delayed_reconfig;
> int uev_wait_timeout;
> int skip_kpartx;
> + int disable_changed_wwids;
> unsigned int version[3];
>
> char * multipath_dir;
> diff --git a/libmultipath/defaults.h b/libmultipath/defaults.h
> index a1fee9b..a72078f 100644
> --- a/libmultipath/defaults.h
> +++ b/libmultipath/defaults.h
> @@ -36,6 +36,7 @@
> #define DEFAULT_FORCE_SYNC 0
> #define DEFAULT_PARTITION_DELIM NULL
> #define DEFAULT_SKIP_KPARTX SKIP_KPARTX_OFF
> +#define DEFAULT_DISABLE_CHANGED_WWIDS 0
>
> #define DEFAULT_CHECKINT 5
> #define MAX_CHECKINT(a) (a << 2)
> diff --git a/libmultipath/dict.c b/libmultipath/dict.c
> index e0a3014..61b6910 100644
> --- a/libmultipath/dict.c
> +++ b/libmultipath/dict.c
> @@ -412,6 +412,9 @@ declare_hw_snprint(skip_kpartx, print_yes_no_undef)
> declare_mp_handler(skip_kpartx, set_yes_no_undef)
> declare_mp_snprint(skip_kpartx, print_yes_no_undef)
>
> +declare_def_handler(disable_changed_wwids, set_yes_no)
> +declare_def_snprint(disable_changed_wwids, print_yes_no)
> +
> static int
> def_config_dir_handler(struct config *conf, vector strvec)
> {
> @@ -1395,6 +1398,7 @@ init_keywords(vector keywords)
> install_keyword("retrigger_delay", &def_retrigger_delay_handler, &snprint_def_retrigger_delay);
> install_keyword("missing_uev_wait_timeout", &def_uev_wait_timeout_handler, &snprint_def_uev_wait_timeout);
> install_keyword("skip_kpartx", &def_skip_kpartx_handler, &snprint_def_skip_kpartx);
> + install_keyword("disable_changed_wwids", &def_disable_changed_wwids_handler, &snprint_def_disable_changed_wwids);
> __deprecated install_keyword("default_selector", &def_selector_handler, NULL);
> __deprecated install_keyword("default_path_grouping_policy", &def_pgpolicy_handler, NULL);
> __deprecated install_keyword("default_uid_attribute", &def_uid_attribute_handler, NULL);
> diff --git a/libmultipath/discovery.c b/libmultipath/discovery.c
> index bb3116d..756344f 100644
> --- a/libmultipath/discovery.c
> +++ b/libmultipath/discovery.c
> @@ -1538,13 +1538,12 @@ get_prio (struct path * pp)
> }
>
> static int
> -get_udev_uid(struct path * pp, char *uid_attribute)
> +get_udev_uid(struct path * pp, char *uid_attribute, struct udev_device *udev)
> {
> ssize_t len;
> const char *value;
>
> - value = udev_device_get_property_value(pp->udev,
> - uid_attribute);
> + value = udev_device_get_property_value(udev, uid_attribute);
> if (!value || strlen(value) == 0)
> value = getenv(uid_attribute);
> if (value && strlen(value)) {
> @@ -1625,8 +1624,8 @@ get_vpd_uid(struct path * pp)
> return get_vpd_sysfs(parent, 0x83, pp->wwid, WWID_SIZE);
> }
>
> -static int
> -get_uid (struct path * pp, int path_state)
> +int
> +get_uid (struct path * pp, int path_state, struct udev_device *udev)
> {
> char *c;
> const char *origin = "unknown";
> @@ -1639,7 +1638,7 @@ get_uid (struct path * pp, int path_state)
> put_multipath_config(conf);
> }
>
> - if (!pp->udev) {
> + if (!udev) {
> condlog(1, "%s: no udev information", pp->dev);
> return 1;
> }
> @@ -1669,7 +1668,7 @@ get_uid (struct path * pp, int path_state)
> int retrigger;
>
> if (pp->uid_attribute) {
> - len = get_udev_uid(pp, pp->uid_attribute);
> + len = get_udev_uid(pp, pp->uid_attribute, udev);
> origin = "udev";
> if (len <= 0)
> condlog(1,
> @@ -1798,7 +1797,7 @@ pathinfo (struct path *pp, struct config *conf, int mask)
> }
>
> if ((mask & DI_WWID) && !strlen(pp->wwid)) {
> - get_uid(pp, path_state);
> + get_uid(pp, path_state, pp->udev);
> if (!strlen(pp->wwid)) {
> pp->initialized = INIT_MISSING_UDEV;
> pp->tick = conf->retrigger_delay;
> diff --git a/libmultipath/discovery.h b/libmultipath/discovery.h
> index 0f5b1e6..176eac1 100644
> --- a/libmultipath/discovery.h
> +++ b/libmultipath/discovery.h
> @@ -49,6 +49,7 @@ 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,
> char *buff, int buflen);
> +int get_uid(struct path * pp, int path_state, struct udev_device *udev);
>
> /*
> * discovery bitmask
> diff --git a/libmultipath/structs.h b/libmultipath/structs.h
> index 3a716d8..58508f6 100644
> --- a/libmultipath/structs.h
> +++ b/libmultipath/structs.h
> @@ -217,6 +217,7 @@ struct path {
> int fd;
> int initialized;
> int retriggers;
> + int wwid_changed;
>
> /* configlet pointers */
> struct hwentry * hwe;
> diff --git a/multipathd/main.c b/multipathd/main.c
> index dbb4554..bb96cca 100644
> --- a/multipathd/main.c
> +++ b/multipathd/main.c
> @@ -958,6 +958,12 @@ uev_update_path (struct uevent *uev, struct vectors * vecs)
> {
> int ro, retval = 0;
> struct path * pp;
> + struct config *conf;
> + int disable_changed_wwids;
> +
> + conf = get_multipath_config();
> + disable_changed_wwids = conf->disable_changed_wwids;
> + put_multipath_config(conf);
>
> ro = uevent_get_disk_ro(uev);
>
> @@ -969,6 +975,25 @@ uev_update_path (struct uevent *uev, struct vectors * vecs)
> if (pp) {
> struct multipath *mpp = pp->mpp;
>
> + if (disable_changed_wwids &&
> + (strlen(pp->wwid) || pp->wwid_changed)) {
> + char wwid[WWID_SIZE];
> +
> + strcpy(wwid, pp->wwid);
> + get_uid(pp, pp->state, uev->udev);
> + if (strcmp(wwid, pp->wwid) != 0) {
> + condlog(0, "%s: path wwid changed from '%s' to '%s'. disallowing", uev->kernel, wwid, pp->wwid);
> + strcpy(pp->wwid, wwid);
> + if (!pp->wwid_changed) {
> + pp->wwid_changed = 1;
> + pp->tick = 1;
> + dm_fail_path(pp->mpp->alias, pp->dev_t);
> + }
> + goto out;
> + } else
> + pp->wwid_changed = 0;
> + }
> +
> if (pp->initialized == INIT_REQUESTED_UDEV)
> retval = uev_add_path(uev, vecs);
> else if (mpp && ro >= 0) {
> @@ -983,6 +1008,7 @@ uev_update_path (struct uevent *uev, struct vectors * vecs)
> }
> }
> }
> +out:
> lock_cleanup_pop(vecs->lock);
> if (!pp)
> condlog(0, "%s: spurious uevent, path not found", uev->kernel);
> @@ -1509,6 +1535,12 @@ check_path (struct vectors * vecs, struct path * pp, int ticks)
> } else
> checker_clear_message(&pp->checker);
>
> + if (pp->wwid_changed) {
> + condlog(2, "%s: path wwid has changed. Refusing to use",
> + pp->dev);
> + newstate = PATH_DOWN;
> + }
> +
> if (newstate == PATH_WILD || newstate == PATH_UNCHECKED) {
> condlog(2, "%s: unusable path", pp->dev);
> conf = get_multipath_config();
>
next prev parent reply other threads:[~2016-11-06 0:03 UTC|newest]
Thread overview: 29+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-10-29 2:55 [PATCH 00/10] misc. multipath patches Benjamin Marzinski
2016-10-29 2:55 ` [PATCH 01/10] libmultipath: add skip_kpartx option Benjamin Marzinski
2016-10-30 13:42 ` Hannes Reinecke
2016-10-29 2:55 ` [PATCH 02/10] kpartx.rules: respect skip_kpartx flag Benjamin Marzinski
2016-10-30 13:42 ` Hannes Reinecke
2016-10-29 2:55 ` [PATCH 03/10] do not allow in-use path to change wwid Benjamin Marzinski
2016-10-30 13:45 ` Hannes Reinecke
2016-10-31 14:30 ` Benjamin Marzinski
2016-10-29 2:55 ` [PATCH 04/10] multipathd: add "map failures" format wildcard Benjamin Marzinski
2016-10-30 13:45 ` Hannes Reinecke
2016-10-29 2:55 ` [PATCH 05/10] mpath: don't wait for udev if all paths are down Benjamin Marzinski
2016-10-30 13:46 ` Hannes Reinecke
2016-10-29 2:55 ` [PATCH 06/10] multipath: set cookie before using it Benjamin Marzinski
2016-10-30 13:46 ` Hannes Reinecke
2016-10-29 2:55 ` [PATCH 07/10] recover from errors in multipathd startup Benjamin Marzinski
2016-10-30 13:47 ` Hannes Reinecke
2016-10-29 2:55 ` [PATCH 08/10] fix INIT_REQUESTED_UDEV code Benjamin Marzinski
2016-10-30 13:48 ` Hannes Reinecke
2016-10-29 2:55 ` [PATCH 09/10] add disable_changed_wwids option Benjamin Marzinski
2016-10-30 13:54 ` Hannes Reinecke
2016-11-04 15:32 ` Benjamin Marzinski
2017-02-28 12:27 ` Zhangguanghui
2016-11-06 0:03 ` Xose Vazquez Perez [this message]
2016-11-07 14:47 ` Benjamin Marzinski
2016-10-29 2:55 ` [PATCH 10/10] set retrigger_tries to 0 for multipath Benjamin Marzinski
2016-10-30 13:54 ` Hannes Reinecke
2016-12-06 15:11 ` Xose Vazquez Perez
2016-12-06 15:22 ` Benjamin Marzinski
2017-04-25 14:07 ` Xose Vazquez Perez
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=e090f9ec-41b9-856c-4fc0-b2ced90fd9a8@gmail.com \
--to=xose.vazquez@gmail.com \
--cc=bmarzins@redhat.com \
--cc=christophe.varoqui@opensvc.com \
--cc=dm-devel@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).