diff mbox series

[v2,11/13] libmultipath: assume device is in use if dm_get_opencount fails

Message ID 20241122211133.718861-12-bmarzins@redhat.com (mailing list archive)
State New
Headers show
Series multipath-tools: Handle tableless DM devices | expand

Commit Message

Benjamin Marzinski Nov. 22, 2024, 9:11 p.m. UTC
If dm_get_opencount() fails, open_count will be negative, which means
mpath_in_use() will never fail. However, dm_flush_map__() will
eventually fail. There are multiple callers of dm_get_opencount() that
run when trying to remove a device. All the others treat a failed call
as if the device was in use, including one that will later be called
directly on this device. So save ourselves the work, and exit early

Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
---
 libmultipath/devmapper.c | 5 +++++
 1 file changed, 5 insertions(+)

Comments

Martin Wilck Nov. 22, 2024, 10:17 p.m. UTC | #1
On Fri, 2024-11-22 at 16:11 -0500, Benjamin Marzinski wrote:
> If dm_get_opencount() fails, open_count will be negative, which means
> mpath_in_use() will never fail. However, dm_flush_map__() will
> eventually fail. There are multiple callers of dm_get_opencount()
> that
> run when trying to remove a device. All the others treat a failed
> call
> as if the device was in use, including one that will later be called
> directly on this device. So save ourselves the work, and exit early
> 
> Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>

Side note: I realize that there are other callers of dm_get_opencount()
that take a negative return value for a "yes". That happens to be
correct for the respective callers, but it's quite unclean.

Your patch is fine though.

Martin

> ---
>  libmultipath/devmapper.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/libmultipath/devmapper.c b/libmultipath/devmapper.c
> index 751b45d8..3667c51b 100644
> --- a/libmultipath/devmapper.c
> +++ b/libmultipath/devmapper.c
> @@ -961,6 +961,11 @@ int mpath_in_use(const char *name)
>  {
>  	int open_count = dm_get_opencount(name);
>  
> +	if (open_count < 0) {
> +		condlog(0, "%s: %s: failed to get open count,
> assuming in use",
> +			__func__, name);
> +		return 1;
> +	}
>  	if (open_count) {
>  		int part_count = 0;
>
diff mbox series

Patch

diff --git a/libmultipath/devmapper.c b/libmultipath/devmapper.c
index 751b45d8..3667c51b 100644
--- a/libmultipath/devmapper.c
+++ b/libmultipath/devmapper.c
@@ -961,6 +961,11 @@  int mpath_in_use(const char *name)
 {
 	int open_count = dm_get_opencount(name);
 
+	if (open_count < 0) {
+		condlog(0, "%s: %s: failed to get open count, assuming in use",
+			__func__, name);
+		return 1;
+	}
 	if (open_count) {
 		int part_count = 0;