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 <bmarz...@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;
>  


Reply via email to