* [PATCH 1/3] libmultipath: assign variable to make gcc happy
@ 2020-03-24 21:03 Benjamin Marzinski
2020-03-24 21:03 ` [PATCH 2/3] libmutipath: don't close fd on dm_lib_release Benjamin Marzinski
` (2 more replies)
0 siblings, 3 replies; 12+ messages in thread
From: Benjamin Marzinski @ 2020-03-24 21:03 UTC (permalink / raw)
To: Christophe Varoqui; +Cc: device-mapper development, Martin Wilck
There is nothing wrong with is_queueing not being set at the start
of __set_no_path_retry(), it will always get set before it is accessed,
but gcc 8.2.1 is failing with
structs_vec.c: In function ‘__set_no_path_retry’:
structs_vec.c:339:7: error: ‘is_queueing’ may be used uninitialized in
this function [-Werror=maybe-uninitialized]
bool is_queueing;
^~~~~~~~~~~
so, assign a value to make it happy.
Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
---
libmultipath/structs_vec.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/libmultipath/structs_vec.c b/libmultipath/structs_vec.c
index 3dbbaa0f..077f2e42 100644
--- a/libmultipath/structs_vec.c
+++ b/libmultipath/structs_vec.c
@@ -336,7 +336,7 @@ static void leave_recovery_mode(struct multipath *mpp)
void __set_no_path_retry(struct multipath *mpp, bool check_features)
{
- bool is_queueing;
+ bool is_queueing = false; /* assign a value to make gcc happy */
check_features = check_features && mpp->features != NULL;
if (check_features)
--
2.17.2
--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH 2/3] libmutipath: don't close fd on dm_lib_release 2020-03-24 21:03 [PATCH 1/3] libmultipath: assign variable to make gcc happy Benjamin Marzinski @ 2020-03-24 21:03 ` Benjamin Marzinski 2020-03-25 15:16 ` Martin Wilck 2020-03-24 21:03 ` [PATCH 3/3] libmultipath: allow force reload with no active paths Benjamin Marzinski 2020-03-25 15:22 ` [PATCH 1/3] libmultipath: assign variable to make gcc happy Martin Wilck 2 siblings, 1 reply; 12+ messages in thread From: Benjamin Marzinski @ 2020-03-24 21:03 UTC (permalink / raw) To: Christophe Varoqui; +Cc: device-mapper development, Martin Wilck If dm_hold_control_open() isn't set, when dm_lib_release() is called, it will close the control fd. The control fd will get re-opened on the next dm_task_run() call, but if there is a dm_task_run() call already in progress in another thread, it can fail. Since many of the device-mapper callouts happen with the vecs lock held, this wasn't too noticeable, but there is code that calls dm_task_run() without the vecs lock held, notably the dmevent waiter code. Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com> --- libmultipath/devmapper.c | 1 + 1 file changed, 1 insertion(+) diff --git a/libmultipath/devmapper.c b/libmultipath/devmapper.c index bed8ddc6..d96472fe 100644 --- a/libmultipath/devmapper.c +++ b/libmultipath/devmapper.c @@ -254,6 +254,7 @@ void libmp_dm_init(void) memcpy(conf->version, version, sizeof(version)); put_multipath_config(conf); dm_init(verbosity); + dm_hold_control_dev(1); dm_udev_set_sync_support(libmp_dm_udev_sync); } -- 2.17.2 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH 2/3] libmutipath: don't close fd on dm_lib_release 2020-03-24 21:03 ` [PATCH 2/3] libmutipath: don't close fd on dm_lib_release Benjamin Marzinski @ 2020-03-25 15:16 ` Martin Wilck 2020-03-25 20:52 ` Benjamin Marzinski 0 siblings, 1 reply; 12+ messages in thread From: Martin Wilck @ 2020-03-25 15:16 UTC (permalink / raw) To: bmarzins@redhat.com, christophe.varoqui@opensvc.com; +Cc: dm-devel@redhat.com On Tue, 2020-03-24 at 16:03 -0500, Benjamin Marzinski wrote: > If dm_hold_control_open() isn't set, when dm_lib_release() is called, > it > will close the control fd. The control fd will get re-opened on the > next > dm_task_run() call, but if there is a dm_task_run() call already > in progress in another thread, it can fail. Since many of the > device-mapper callouts happen with the vecs lock held, this wasn't > too > noticeable, but there is code that calls dm_task_run() without the > vecs lock held, notably the dmevent waiter code. > > Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com> > --- > libmultipath/devmapper.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/libmultipath/devmapper.c b/libmultipath/devmapper.c > index bed8ddc6..d96472fe 100644 > --- a/libmultipath/devmapper.c > +++ b/libmultipath/devmapper.c > @@ -254,6 +254,7 @@ void libmp_dm_init(void) > memcpy(conf->version, version, sizeof(version)); > put_multipath_config(conf); > dm_init(verbosity); > + dm_hold_control_dev(1); > dm_udev_set_sync_support(libmp_dm_udev_sync); > } AFAICS, this function has been in libdm since 1.02.111. We support 1.02.89 (if all features enabled, otherwise even older). Perhaps we should make this function call conditional on the libdm verson? But perhaps more importantly, why do we still need to call dm_lib_release()? AFAICS it's only needed for systems that have no udev support for creating device nodes (to call update_devs() via dm_lib_release()), and we don't support that anymore anyway, do we? Since 26c4bb0, we're always setting the DM_UDEV_DISABLE_LIBRARY_FALLBACK flag, and the cookie, too (we aren't setting it for DM_DEVICE_RELOAD, but it isn't needed for that, either, since no device nodes need to be created or removed); so dm_lib_release() should really have no effect. Regards Martin -- Dr. Martin Wilck <mwilck@suse.com>, Tel. +49 (0)911 74053 2107 SUSE Software Solutions Germany GmbH HRB 36809, AG Nürnberg GF: Felix Imendörffer ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/3] libmutipath: don't close fd on dm_lib_release 2020-03-25 15:16 ` Martin Wilck @ 2020-03-25 20:52 ` Benjamin Marzinski 2020-03-25 22:00 ` Benjamin Marzinski 0 siblings, 1 reply; 12+ messages in thread From: Benjamin Marzinski @ 2020-03-25 20:52 UTC (permalink / raw) To: Martin Wilck; +Cc: dm-devel@redhat.com On Wed, Mar 25, 2020 at 03:16:50PM +0000, Martin Wilck wrote: > On Tue, 2020-03-24 at 16:03 -0500, Benjamin Marzinski wrote: > > If dm_hold_control_open() isn't set, when dm_lib_release() is called, > > it > > will close the control fd. The control fd will get re-opened on the > > next > > dm_task_run() call, but if there is a dm_task_run() call already > > in progress in another thread, it can fail. Since many of the > > device-mapper callouts happen with the vecs lock held, this wasn't > > too > > noticeable, but there is code that calls dm_task_run() without the > > vecs lock held, notably the dmevent waiter code. > > > > Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com> > > --- > > libmultipath/devmapper.c | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/libmultipath/devmapper.c b/libmultipath/devmapper.c > > index bed8ddc6..d96472fe 100644 > > --- a/libmultipath/devmapper.c > > +++ b/libmultipath/devmapper.c > > @@ -254,6 +254,7 @@ void libmp_dm_init(void) > > memcpy(conf->version, version, sizeof(version)); > > put_multipath_config(conf); > > dm_init(verbosity); > > + dm_hold_control_dev(1); > > dm_udev_set_sync_support(libmp_dm_udev_sync); > > } > > AFAICS, this function has been in libdm since 1.02.111. We support > 1.02.89 (if all features enabled, otherwise even older). Perhaps we > should make this function call conditional on the libdm verson? > > But perhaps more importantly, why do we still need to call > dm_lib_release()? AFAICS it's only needed for systems that have no udev > support for creating device nodes (to call update_devs() via > dm_lib_release()), and we don't support that anymore anyway, do we? > > Since 26c4bb0, we're always setting the > DM_UDEV_DISABLE_LIBRARY_FALLBACK flag, and the cookie, too > (we aren't setting it for DM_DEVICE_RELOAD, but it isn't needed for > that, either, since no device nodes need to be created or removed); so > dm_lib_release() should really have no effect. > > Regards > Martin Good call. I'll redo this patch. -Ben > > -- > Dr. Martin Wilck <mwilck@suse.com>, Tel. +49 (0)911 74053 2107 > SUSE Software Solutions Germany GmbH > HRB 36809, AG Nürnberg GF: Felix > Imendörffer > ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/3] libmutipath: don't close fd on dm_lib_release 2020-03-25 20:52 ` Benjamin Marzinski @ 2020-03-25 22:00 ` Benjamin Marzinski 2020-03-25 22:11 ` Martin Wilck 2020-07-02 11:52 ` Martin Wilck 0 siblings, 2 replies; 12+ messages in thread From: Benjamin Marzinski @ 2020-03-25 22:00 UTC (permalink / raw) To: Martin Wilck; +Cc: dm-devel@redhat.com On Wed, Mar 25, 2020 at 03:52:55PM -0500, Benjamin Marzinski wrote: > On Wed, Mar 25, 2020 at 03:16:50PM +0000, Martin Wilck wrote: > > On Tue, 2020-03-24 at 16:03 -0500, Benjamin Marzinski wrote: > > > > AFAICS, this function has been in libdm since 1.02.111. We support > > 1.02.89 (if all features enabled, otherwise even older). Perhaps we > > should make this function call conditional on the libdm verson? > > > > But perhaps more importantly, why do we still need to call > > dm_lib_release()? AFAICS it's only needed for systems that have no udev > > support for creating device nodes (to call update_devs() via > > dm_lib_release()), and we don't support that anymore anyway, do we? > > > > Since 26c4bb0, we're always setting the > > DM_UDEV_DISABLE_LIBRARY_FALLBACK flag, and the cookie, too > > (we aren't setting it for DM_DEVICE_RELOAD, but it isn't needed for > > that, either, since no device nodes need to be created or removed); so > > dm_lib_release() should really have no effect. > > > > Regards > > Martin > > Good call. I'll redo this patch. Actually, I've changed my mind. Calling dm_lib_release() lets us release the memory that device-mapper uses to store all the node ops that it was saving up. Without calling dm_lib_release(), AFAICS, that memory keeps growing until the daemon exits. -Ben > -Ben > > > > > -- > > Dr. Martin Wilck <mwilck@suse.com>, Tel. +49 (0)911 74053 2107 > > SUSE Software Solutions Germany GmbH > > HRB 36809, AG Nürnberg GF: Felix > > Imendörffer > > > > -- > dm-devel mailing list > dm-devel@redhat.com > https://www.redhat.com/mailman/listinfo/dm-devel ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/3] libmutipath: don't close fd on dm_lib_release 2020-03-25 22:00 ` Benjamin Marzinski @ 2020-03-25 22:11 ` Martin Wilck 2020-07-02 11:52 ` Martin Wilck 1 sibling, 0 replies; 12+ messages in thread From: Martin Wilck @ 2020-03-25 22:11 UTC (permalink / raw) To: Benjamin Marzinski; +Cc: dm-devel@redhat.com On Wed, 2020-03-25 at 17:00 -0500, Benjamin Marzinski wrote: > On Wed, Mar 25, 2020 at 03:52:55PM -0500, Benjamin Marzinski wrote: > > On Wed, Mar 25, 2020 at 03:16:50PM +0000, Martin Wilck wrote: > > > On Tue, 2020-03-24 at 16:03 -0500, Benjamin Marzinski wrote: > > > > > > AFAICS, this function has been in libdm since 1.02.111. We > > > support > > > 1.02.89 (if all features enabled, otherwise even older). Perhaps > > > we > > > should make this function call conditional on the libdm verson? > > > > > > But perhaps more importantly, why do we still need to call > > > dm_lib_release()? AFAICS it's only needed for systems that have > > > no udev > > > support for creating device nodes (to call update_devs() via > > > dm_lib_release()), and we don't support that anymore anyway, do > > > we? > > > > > > Since 26c4bb0, we're always setting the > > > DM_UDEV_DISABLE_LIBRARY_FALLBACK flag, and the cookie, too > > > (we aren't setting it for DM_DEVICE_RELOAD, but it isn't needed > > > for > > > that, either, since no device nodes need to be created or > > > removed); so > > > dm_lib_release() should really have no effect. > > > > > > Regards > > > Martin > > > > Good call. I'll redo this patch. > > Actually, I've changed my mind. Calling dm_lib_release() lets us > release > the memory that device-mapper uses to store all the node ops that it > was saving up. Without calling dm_lib_release(), AFAICS, that memory > keeps growing until the daemon exits. Ok, I see. libdm stacks the node ops, even if it's told to rely on udev. The question is, what for, as it will discard the stacked nodes later on anyway. Fine, then. But please add a check for the libdm version. Thanks, Martin -- Dr. Martin Wilck <mwilck@suse.com>, Tel. +49 (0)911 74053 2107 SUSE Software Solutions Germany GmbH HRB 36809, AG Nürnberg GF: Felix Imendörffer ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/3] libmutipath: don't close fd on dm_lib_release 2020-03-25 22:00 ` Benjamin Marzinski 2020-03-25 22:11 ` Martin Wilck @ 2020-07-02 11:52 ` Martin Wilck 2020-07-02 20:06 ` Benjamin Marzinski 1 sibling, 1 reply; 12+ messages in thread From: Martin Wilck @ 2020-07-02 11:52 UTC (permalink / raw) To: bmarzins@redhat.com; +Cc: dm-devel@redhat.com On Wed, 2020-03-25 at 17:00 -0500, Benjamin Marzinski wrote: > On Wed, Mar 25, 2020 at 03:52:55PM -0500, Benjamin Marzinski wrote: > > On Wed, Mar 25, 2020 at 03:16:50PM +0000, Martin Wilck wrote: > > > On Tue, 2020-03-24 at 16:03 -0500, Benjamin Marzinski wrote: > > > > > > AFAICS, this function has been in libdm since 1.02.111. We > > > support > > > 1.02.89 (if all features enabled, otherwise even older). Perhaps > > > we > > > should make this function call conditional on the libdm verson? > > > > > > But perhaps more importantly, why do we still need to call > > > dm_lib_release()? AFAICS it's only needed for systems that have > > > no udev > > > support for creating device nodes (to call update_devs() via > > > dm_lib_release()), and we don't support that anymore anyway, do > > > we? > > > > > > Since 26c4bb0, we're always setting the > > > DM_UDEV_DISABLE_LIBRARY_FALLBACK flag, and the cookie, too > > > (we aren't setting it for DM_DEVICE_RELOAD, but it isn't needed > > > for > > > that, either, since no device nodes need to be created or > > > removed); so > > > dm_lib_release() should really have no effect. > > > > > > Regards > > > Martin > > > > Good call. I'll redo this patch. > > Actually, I've changed my mind. Calling dm_lib_release() lets us > release > the memory that device-mapper uses to store all the node ops that it > was saving up. Without calling dm_lib_release(), AFAICS, that memory > keeps growing until the daemon exits. Sorry for coming back to this so late. I've just stared at the libdm code again. We always set DM_UDEV_DISABLE_LIBRARY_FALLBACK. In the standard CREATE and REMOVE cases, libdm doesn't stack any operations if this flag is set. The only exceptions are a) RESUME operations with DM_ADD_NODE_ON_RESUME set. This happens implicity when we create new maps b) RENAME operations In both cases, we call dm_udev_wait() after the libdm operation, which calls update_devs() and should thus have the same effect as dm_lib_release(). IOW, I still believe we don't need to call dm_lib_release() any more. Regards, Martin -- Dr. Martin Wilck <mwilck@suse.com>, Tel. +49 (0)911 74053 2107 SUSE Software Solutions Germany GmbH HRB 36809, AG Nürnberg GF: Felix Imendörffer ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/3] libmutipath: don't close fd on dm_lib_release 2020-07-02 11:52 ` Martin Wilck @ 2020-07-02 20:06 ` Benjamin Marzinski 2020-07-03 14:05 ` Martin Wilck 0 siblings, 1 reply; 12+ messages in thread From: Benjamin Marzinski @ 2020-07-02 20:06 UTC (permalink / raw) To: Martin Wilck; +Cc: dm-devel@redhat.com On Thu, Jul 02, 2020 at 11:52:21AM +0000, Martin Wilck wrote: > On Wed, 2020-03-25 at 17:00 -0500, Benjamin Marzinski wrote: > > On Wed, Mar 25, 2020 at 03:52:55PM -0500, Benjamin Marzinski wrote: > > > On Wed, Mar 25, 2020 at 03:16:50PM +0000, Martin Wilck wrote: > > > > On Tue, 2020-03-24 at 16:03 -0500, Benjamin Marzinski wrote: > > > > > > > > AFAICS, this function has been in libdm since 1.02.111. We > > > > support > > > > 1.02.89 (if all features enabled, otherwise even older). Perhaps > > > > we > > > > should make this function call conditional on the libdm verson? > > > > > > > > But perhaps more importantly, why do we still need to call > > > > dm_lib_release()? AFAICS it's only needed for systems that have > > > > no udev > > > > support for creating device nodes (to call update_devs() via > > > > dm_lib_release()), and we don't support that anymore anyway, do > > > > we? > > > > > > > > Since 26c4bb0, we're always setting the > > > > DM_UDEV_DISABLE_LIBRARY_FALLBACK flag, and the cookie, too > > > > (we aren't setting it for DM_DEVICE_RELOAD, but it isn't needed > > > > for > > > > that, either, since no device nodes need to be created or > > > > removed); so > > > > dm_lib_release() should really have no effect. > > > > > > > > Regards > > > > Martin > > > > > > Good call. I'll redo this patch. > > > > Actually, I've changed my mind. Calling dm_lib_release() lets us > > release > > the memory that device-mapper uses to store all the node ops that it > > was saving up. Without calling dm_lib_release(), AFAICS, that memory > > keeps growing until the daemon exits. > > Sorry for coming back to this so late. I've just stared at the libdm > code again. > > We always set DM_UDEV_DISABLE_LIBRARY_FALLBACK. In the standard CREATE > and REMOVE cases, libdm doesn't stack any operations if this flag is > set. The only exceptions are > > a) RESUME operations with DM_ADD_NODE_ON_RESUME set. This happens > implicity when we create new maps > b) RENAME operations > > In both cases, we call dm_udev_wait() after the libdm operation, which > calls update_devs() and should thus have the same effect as > dm_lib_release(). IOW, I still believe we don't need to call > dm_lib_release() any more. Sure. But can we leave this patch as is, and remove those calls in a different patch? -Ben > Regards, > Martin > > -- > Dr. Martin Wilck <mwilck@suse.com>, Tel. +49 (0)911 74053 2107 > SUSE Software Solutions Germany GmbH > HRB 36809, AG Nürnberg GF: Felix > Imendörffer > ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/3] libmutipath: don't close fd on dm_lib_release 2020-07-02 20:06 ` Benjamin Marzinski @ 2020-07-03 14:05 ` Martin Wilck 0 siblings, 0 replies; 12+ messages in thread From: Martin Wilck @ 2020-07-03 14:05 UTC (permalink / raw) To: bmarzins@redhat.com; +Cc: dm-devel@redhat.com On Thu, 2020-07-02 at 15:06 -0500, Benjamin Marzinski wrote: > On Thu, Jul 02, 2020 at 11:52:21AM +0000, Martin Wilck wrote: > > On Wed, 2020-03-25 at 17:00 -0500, Benjamin Marzinski wrote: > > > On Wed, Mar 25, 2020 at 03:52:55PM -0500, Benjamin Marzinski > > > wrote: > > > > On Wed, Mar 25, 2020 at 03:16:50PM +0000, Martin Wilck wrote: > > > > > On Tue, 2020-03-24 at 16:03 -0500, Benjamin Marzinski wrote: > > > > > > > > > > AFAICS, this function has been in libdm since 1.02.111. We > > > > > support > > > > > 1.02.89 (if all features enabled, otherwise even older). > > > > > Perhaps > > > > > we > > > > > should make this function call conditional on the libdm > > > > > verson? > > > > > > > > > > But perhaps more importantly, why do we still need to call > > > > > dm_lib_release()? AFAICS it's only needed for systems that > > > > > have > > > > > no udev > > > > > support for creating device nodes (to call update_devs() via > > > > > dm_lib_release()), and we don't support that anymore anyway, > > > > > do > > > > > we? > > > > > > > > > > Since 26c4bb0, we're always setting the > > > > > DM_UDEV_DISABLE_LIBRARY_FALLBACK flag, and the cookie, too > > > > > (we aren't setting it for DM_DEVICE_RELOAD, but it isn't > > > > > needed > > > > > for > > > > > that, either, since no device nodes need to be created or > > > > > removed); so > > > > > dm_lib_release() should really have no effect. > > > > > > > > > > Regards > > > > > Martin > > > > > > > > Good call. I'll redo this patch. > > > > > > Actually, I've changed my mind. Calling dm_lib_release() lets us > > > release > > > the memory that device-mapper uses to store all the node ops that > > > it > > > was saving up. Without calling dm_lib_release(), AFAICS, that > > > memory > > > keeps growing until the daemon exits. > > > > Sorry for coming back to this so late. I've just stared at the > > libdm > > code again. > > > > We always set DM_UDEV_DISABLE_LIBRARY_FALLBACK. In the standard > > CREATE > > and REMOVE cases, libdm doesn't stack any operations if this flag > > is > > set. The only exceptions are > > > > a) RESUME operations with DM_ADD_NODE_ON_RESUME set. This happens > > implicity when we create new maps > > b) RENAME operations > > > > In both cases, we call dm_udev_wait() after the libdm operation, > > which > > calls update_devs() and should thus have the same effect as > > dm_lib_release(). IOW, I still believe we don't need to call > > dm_lib_release() any more. > > Sure. But can we leave this patch as is, and remove those calls in a > different patch? Of course. It's not important, either. I just wanted to make sure we agree on the technical side. Regards Martin -- Dr. Martin Wilck <mwilck@suse.com>, Tel. +49 (0)911 74053 2107 SUSE Software Solutions Germany GmbH HRB 36809, AG Nürnberg GF: Felix Imendörffer ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 3/3] libmultipath: allow force reload with no active paths 2020-03-24 21:03 [PATCH 1/3] libmultipath: assign variable to make gcc happy Benjamin Marzinski 2020-03-24 21:03 ` [PATCH 2/3] libmutipath: don't close fd on dm_lib_release Benjamin Marzinski @ 2020-03-24 21:03 ` Benjamin Marzinski 2020-03-25 15:27 ` Martin Wilck 2020-03-25 15:22 ` [PATCH 1/3] libmultipath: assign variable to make gcc happy Martin Wilck 2 siblings, 1 reply; 12+ messages in thread From: Benjamin Marzinski @ 2020-03-24 21:03 UTC (permalink / raw) To: Christophe Varoqui; +Cc: device-mapper development, Martin Wilck If the partition information has changed on multipath devices (say, because it was updated on another node that has access to the same storage), users expect that running "multipathd reconfigure" will update that. However, if the checkers for the multipath device are pending for too long when the the device is reconfigured, multipathd will give up waiting for them, and refuse to reload the device, since there are no active paths. This means that no kpartx update will be triggered. Multipath is fully capable of reloading a multipath device that has no active paths. This has been possible for years. If multipath is supposed to reload the device, it should do so, even if there are no active paths. Generally, when multipath is force reloaded, kpartx will be updated. However when a device is reloaded with no paths, the udev rules won't run kpartx. But they also weren't running kpartx when the first valid path appeared, even though the dm activation rules get run in this case. This changes 11-dm-mpath.rules to run kpartx when a device goes from no usable paths to having usable paths. Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com> --- libmultipath/configure.c | 6 ------ multipath/11-dm-mpath.rules | 2 +- 2 files changed, 1 insertion(+), 7 deletions(-) diff --git a/libmultipath/configure.c b/libmultipath/configure.c index c95848a0..96c79610 100644 --- a/libmultipath/configure.c +++ b/libmultipath/configure.c @@ -710,12 +710,6 @@ select_action (struct multipath * mpp, vector curmp, int force_reload) return; } - if (pathcount(mpp, PATH_UP) == 0) { - mpp->action = ACT_IMPOSSIBLE; - condlog(3, "%s: set ACT_IMPOSSIBLE (no usable path)", - mpp->alias); - return; - } if (force_reload) { mpp->force_udev_reload = 1; mpp->action = ACT_RELOAD; diff --git a/multipath/11-dm-mpath.rules b/multipath/11-dm-mpath.rules index 07320a14..cd522e8c 100644 --- a/multipath/11-dm-mpath.rules +++ b/multipath/11-dm-mpath.rules @@ -75,7 +75,7 @@ ENV{MPATH_DEVICE_READY}=="0", ENV{DM_UDEV_DISABLE_OTHER_RULES_FLAG}="1" ENV{MPATH_DEVICE_READY}!="0", ENV{.MPATH_DEVICE_READY_OLD}=="0",\ ENV{DM_UDEV_DISABLE_OTHER_RULES_FLAG}="$env{DM_DISABLE_OTHER_RULES_FLAG_OLD}",\ ENV{DM_DISABLE_OTHER_RULES_FLAG_OLD}="",\ - ENV{DM_ACTIVATION}="1" + ENV{DM_ACTIVATION}="1", ENV{MPATH_UNCHANGED}="0" # The code to check multipath state ends here. We need to set # properties and symlinks regardless whether the map is usable or -- 2.17.2 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH 3/3] libmultipath: allow force reload with no active paths 2020-03-24 21:03 ` [PATCH 3/3] libmultipath: allow force reload with no active paths Benjamin Marzinski @ 2020-03-25 15:27 ` Martin Wilck 0 siblings, 0 replies; 12+ messages in thread From: Martin Wilck @ 2020-03-25 15:27 UTC (permalink / raw) To: bmarzins@redhat.com, christophe.varoqui@opensvc.com; +Cc: dm-devel@redhat.com On Tue, 2020-03-24 at 16:03 -0500, Benjamin Marzinski wrote: > If the partition information has changed on multipath devices (say, > because it was updated on another node that has access to the same > storage), users expect that running "multipathd reconfigure" will > update > that. However, if the checkers for the multipath device are pending > for > too long when the the device is reconfigured, multipathd will give up > waiting for them, and refuse to reload the device, since there are no > active paths. This means that no kpartx update will be triggered. > > Multipath is fully capable of reloading a multipath device that has > no > active paths. This has been possible for years. If multipath is > supposed > to reload the device, it should do so, even if there are no active > paths. > > Generally, when multipath is force reloaded, kpartx will be updated. > However when a device is reloaded with no paths, the udev rules won't > run kpartx. But they also weren't running kpartx when the first > valid > path appeared, even though the dm activation rules get run in this > case. > This changes 11-dm-mpath.rules to run kpartx when a device goes from > no > usable paths to having usable paths. > > Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com> > --- > libmultipath/configure.c | 6 ------ > multipath/11-dm-mpath.rules | 2 +- > 2 files changed, 1 insertion(+), 7 deletions(-) Reviewed-by: Martin Wilck <mwilck@suse.com> -- Dr. Martin Wilck <mwilck@suse.com>, Tel. +49 (0)911 74053 2107 SUSE Software Solutions Germany GmbH HRB 36809, AG Nürnberg GF: Felix Imendörffer ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/3] libmultipath: assign variable to make gcc happy 2020-03-24 21:03 [PATCH 1/3] libmultipath: assign variable to make gcc happy Benjamin Marzinski 2020-03-24 21:03 ` [PATCH 2/3] libmutipath: don't close fd on dm_lib_release Benjamin Marzinski 2020-03-24 21:03 ` [PATCH 3/3] libmultipath: allow force reload with no active paths Benjamin Marzinski @ 2020-03-25 15:22 ` Martin Wilck 2 siblings, 0 replies; 12+ messages in thread From: Martin Wilck @ 2020-03-25 15:22 UTC (permalink / raw) To: bmarzins@redhat.com, christophe.varoqui@opensvc.com; +Cc: dm-devel@redhat.com On Tue, 2020-03-24 at 16:03 -0500, Benjamin Marzinski wrote: > There is nothing wrong with is_queueing not being set at the start > of __set_no_path_retry(), it will always get set before it is > accessed, > but gcc 8.2.1 is failing with > > structs_vec.c: In function ‘__set_no_path_retry’: > structs_vec.c:339:7: error: ‘is_queueing’ may be used uninitialized > in > this function [-Werror=maybe-uninitialized] > bool is_queueing; > ^~~~~~~~~~~ > > so, assign a value to make it happy. > > Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com> > --- > libmultipath/structs_vec.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) That actually looks like a gcc bug to me. Anyway: Reviewed-by: Martin Wilck <mwilck@suse.de> -- Dr. Martin Wilck <mwilck@suse.com>, Tel. +49 (0)911 74053 2107 SUSE Software Solutions Germany GmbH HRB 36809, AG Nürnberg GF: Felix Imendörffer -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2020-07-03 14:05 UTC | newest] Thread overview: 12+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2020-03-24 21:03 [PATCH 1/3] libmultipath: assign variable to make gcc happy Benjamin Marzinski 2020-03-24 21:03 ` [PATCH 2/3] libmutipath: don't close fd on dm_lib_release Benjamin Marzinski 2020-03-25 15:16 ` Martin Wilck 2020-03-25 20:52 ` Benjamin Marzinski 2020-03-25 22:00 ` Benjamin Marzinski 2020-03-25 22:11 ` Martin Wilck 2020-07-02 11:52 ` Martin Wilck 2020-07-02 20:06 ` Benjamin Marzinski 2020-07-03 14:05 ` Martin Wilck 2020-03-24 21:03 ` [PATCH 3/3] libmultipath: allow force reload with no active paths Benjamin Marzinski 2020-03-25 15:27 ` Martin Wilck 2020-03-25 15:22 ` [PATCH 1/3] libmultipath: assign variable to make gcc happy Martin Wilck
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.