| Message ID | 1491545798-22183-8-git-send-email-bmarzins@redhat.com (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Delegated to: | christophe varoqui |
| Headers | show |
On Fri, 2017-04-07 at 01:16 -0500, Benjamin Marzinski wrote: > Whenever multipathd tries to reload a device, even if it's because a > path switched from read/write to read-only, it tries to load the > device > read/write first, and then falls back to read-only. When device- > mapper > sees that multipath is using the same devices in the same state in > its > new table, it simply reuses the devices from the old table, instead > of > closing and re-opening them. This means that multipath can > successfully > reload the multipath device read/write, even if a path device has > switched to read-only. To deal with this, multipathd now doesn't try > to > reload a device read/write when it sees that a path device has > switched > to read-only. > > Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com> > --- > libmultipath/devmapper.c | 9 +++++---- > libmultipath/structs.h | 1 + > multipathd/main.c | 3 +++ > 3 files changed, 9 insertions(+), 4 deletions(-) > > diff --git a/multipathd/main.c b/multipathd/main.c > index f4ff13e..995e580 100644 > --- a/multipathd/main.c > +++ b/multipathd/main.c > @@ -1017,7 +1017,10 @@ uev_update_path (struct uevent *uev, struct > vectors * vecs) > if (mpp->wait_for_udev) > mpp->wait_for_udev = 2; > else { > + if (ro == 1) > + pp->mpp->force_readonly = 1; > retval = reload_map(vecs, mpp, 0, > 1); > + pp->mpp->force_readonly = 0; Why don't you leave this set to 1 until all paths have been switched to rw mode? AFAICS, if any uevent arrives except a switch to ro (assume several paths are ro and one switches back to rw), multipathd will reload the map r/w. Or am I overlooking something? Regards, Martin
On Fri, 2017-04-07 at 01:16 -0500, Benjamin Marzinski wrote: > Whenever multipathd tries to reload a device, even if it's because a > path switched from read/write to read-only, it tries to load the > device > read/write first, and then falls back to read-only. When device- > mapper > sees that multipath is using the same devices in the same state in > its > new table, it simply reuses the devices from the old table, instead > of > closing and re-opening them. This means that multipath can > successfully > reload the multipath device read/write, even if a path device has > switched to read-only. To deal with this, multipathd now doesn't try > to > reload a device read/write when it sees that a path device has > switched > to read-only. > > Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com> > --- > libmultipath/devmapper.c | 9 +++++---- > libmultipath/structs.h | 1 + > multipathd/main.c | 3 +++ > 3 files changed, 9 insertions(+), 4 deletions(-) > > diff --git a/multipathd/main.c b/multipathd/main.c > index f4ff13e..995e580 100644 > --- a/multipathd/main.c > +++ b/multipathd/main.c > @@ -1017,7 +1017,10 @@ uev_update_path (struct uevent *uev, struct > vectors * vecs) > if (mpp->wait_for_udev) > mpp->wait_for_udev = 2; > else { > + if (ro == 1) > + pp->mpp->force_readonly = 1; > retval = reload_map(vecs, mpp, 0, > 1); > + pp->mpp->force_readonly = 0; Why don't you leave this set to 1 until all paths have been switched to rw mode? AFAICS, if any uevent arrives except a switch to ro (assume several paths are ro and one switches back to rw), multipathd will reload the map r/w. Or am I overlooking something? Regards, Martin -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
On Thu, Apr 13, 2017 at 04:39:45PM +0200, Martin Wilck wrote: > On Fri, 2017-04-07 at 01:16 -0500, Benjamin Marzinski wrote: > > Whenever multipathd tries to reload a device, even if it's because a > > path switched from read/write to read-only, it tries to load the > > device > > read/write first, and then falls back to read-only. When device- > > mapper > > sees that multipath is using the same devices in the same state in > > its > > new table, it simply reuses the devices from the old table, instead > > of > > closing and re-opening them. This means that multipath can > > successfully > > reload the multipath device read/write, even if a path device has > > switched to read-only. To deal with this, multipathd now doesn't try > > to > > reload a device read/write when it sees that a path device has > > switched > > to read-only. > > > > Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com> > > --- > > libmultipath/devmapper.c | 9 +++++---- > > libmultipath/structs.h | 1 + > > multipathd/main.c | 3 +++ > > 3 files changed, 9 insertions(+), 4 deletions(-) > > > > diff --git a/multipathd/main.c b/multipathd/main.c > > index f4ff13e..995e580 100644 > > --- a/multipathd/main.c > > +++ b/multipathd/main.c > > @@ -1017,7 +1017,10 @@ uev_update_path (struct uevent *uev, struct > > vectors * vecs) > > if (mpp->wait_for_udev) > > mpp->wait_for_udev = 2; > > else { > > + if (ro == 1) > > + pp->mpp->force_readonly = 1; > > retval = reload_map(vecs, mpp, 0, > > 1); > > + pp->mpp->force_readonly = 0; > > Why don't you leave this set to 1 until all paths have been switched to > rw mode? AFAICS, if any uevent arrives except a switch to ro (assume > several paths are ro and one switches back to rw), multipathd will > reload the map r/w. Or am I overlooking something? Once multipath has reloaded the map RO, the path devices that device-mapper has opened for it will all be RO. So, when another uevent comes along that causes multipathd to reload the map, the R/W reload will fail. Since the path devices in the old table are in the wrong state, the kernel is forced to re-open them in the correct state. This will fail if any of them are set RO. The problem that this is fixing is that multipath usually tries reloading the table R/W first. This means that when a path switches from R/W to RO, the path devices in the old table will be opened R/W. When multipath tries to reload the table R/W, it can just reuse them, since ther state matches. Now, we could avoid attempting the useless R/W reload by tracking the RO status of the paths, and only trying R/W when we don't know that a path is RO, but then we would have to track the RO status of paths. But this should be sufficient to make multipath devices turn RO (and stay that way) when a path does. -Ben > > Regards, > Martin > > -- > Dr. Martin Wilck <mwilck@suse.com>, 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 mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
diff --git a/libmultipath/devmapper.c b/libmultipath/devmapper.c index 044be2b..026418f 100644 --- a/libmultipath/devmapper.c +++ b/libmultipath/devmapper.c @@ -371,7 +371,7 @@ int dm_addmap_create (struct multipath *mpp, char * params) int dm_addmap_reload(struct multipath *mpp, char *params, int flush) { - int r; + int r = 0; uint16_t udev_flags = (flush ? 0 : MPATH_UDEV_RELOAD_FLAG) | ((mpp->skip_kpartx == SKIP_KPARTX_ON)? MPATH_UDEV_NO_KPARTX_FLAG : 0) | @@ -383,10 +383,11 @@ int dm_addmap_reload(struct multipath *mpp, char *params, int flush) * DM_DEVICE_RESUME. So call DM_DEVICE_RESUME * after each successful call to DM_DEVICE_RELOAD. */ - r = dm_addmap(DM_DEVICE_RELOAD, TGT_MPATH, mpp, params, ADDMAP_RW, - SKIP_KPARTX_OFF); + if (!mpp->force_readonly) + r = dm_addmap(DM_DEVICE_RELOAD, TGT_MPATH, mpp, params, + ADDMAP_RW, SKIP_KPARTX_OFF); if (!r) { - if (errno != EROFS) + if (!mpp->force_readonly && errno != EROFS) return 0; r = dm_addmap(DM_DEVICE_RELOAD, TGT_MPATH, mpp, params, ADDMAP_RO, SKIP_KPARTX_OFF); diff --git a/libmultipath/structs.h b/libmultipath/structs.h index fdcfc85..98e13e4 100644 --- a/libmultipath/structs.h +++ b/libmultipath/structs.h @@ -271,6 +271,7 @@ struct multipath { int san_path_err_recovery_time; int skip_kpartx; int max_sectors_kb; + int force_readonly; unsigned int dev_loss; uid_t uid; gid_t gid; diff --git a/multipathd/main.c b/multipathd/main.c index f4ff13e..995e580 100644 --- a/multipathd/main.c +++ b/multipathd/main.c @@ -1017,7 +1017,10 @@ uev_update_path (struct uevent *uev, struct vectors * vecs) if (mpp->wait_for_udev) mpp->wait_for_udev = 2; else { + if (ro == 1) + pp->mpp->force_readonly = 1; retval = reload_map(vecs, mpp, 0, 1); + pp->mpp->force_readonly = 0; condlog(2, "%s: map %s reloaded (retval %d)", uev->kernel, mpp->alias, retval); }
Whenever multipathd tries to reload a device, even if it's because a path switched from read/write to read-only, it tries to load the device read/write first, and then falls back to read-only. When device-mapper sees that multipath is using the same devices in the same state in its new table, it simply reuses the devices from the old table, instead of closing and re-opening them. This means that multipath can successfully reload the multipath device read/write, even if a path device has switched to read-only. To deal with this, multipathd now doesn't try to reload a device read/write when it sees that a path device has switched to read-only. Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com> --- libmultipath/devmapper.c | 9 +++++---- libmultipath/structs.h | 1 + multipathd/main.c | 3 +++ 3 files changed, 9 insertions(+), 4 deletions(-)