[PATCH 1/3] libmultipath: assign variable to make gcc happy

[PATCH 1/3] libmultipath: assign variable to make gcc happy

[Benjamin Marzinski]

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

[PATCH 2/3] libmutipath: don't close fd on dm_lib_release

[Benjamin Marzinski]

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

[PATCH 3/3] libmultipath: allow force reload with no active paths

[Benjamin Marzinski]

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

Re: [PATCH 2/3] libmutipath: don't close fd on dm_lib_release

[Martin Wilck]

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

Re: [PATCH 1/3] libmultipath: assign variable to make gcc happy

[Martin Wilck]

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

Re: [PATCH 3/3] libmultipath: allow force reload with no active paths

[Martin Wilck]

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

Re: [PATCH 2/3] libmutipath: don't close fd on dm_lib_release

[Benjamin Marzinski]

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
> 

Re: [PATCH 2/3] libmutipath: don't close fd on dm_lib_release

[Benjamin Marzinski]

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

Re: [PATCH 2/3] libmutipath: don't close fd on dm_lib_release

[Martin Wilck]

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

Re: [PATCH 2/3] libmutipath: don't close fd on dm_lib_release

[Martin Wilck]

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

Re: [PATCH 2/3] libmutipath: don't close fd on dm_lib_release

[Benjamin Marzinski]

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
> 

Re: [PATCH 2/3] libmutipath: don't close fd on dm_lib_release

[Martin Wilck]

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