Re: [PATCH v2 02/12] libmultipath: libmp_mapinfo(): return DMP_NO_MATCH for multi-target maps

[Benjamin Marzinski <bmarzins@redhat.com>]

On Tue, Nov 12, 2024 at 04:02:05PM +0100, Martin Wilck wrote:
> multi-target maps are more like maps with a single non-multipath
> target than like maps with no target at all.
> 
> Signed-off-by: Martin Wilck <mwilck@suse.com>
> ---
>  libmultipath/devmapper.c | 2 +-
>  tests/mapinfo.c          | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/libmultipath/devmapper.c b/libmultipath/devmapper.c
> index 52bfe9c..ab6eefc 100644
> --- a/libmultipath/devmapper.c
> +++ b/libmultipath/devmapper.c
> @@ -719,7 +719,7 @@ static int libmp_mapinfo__(int flags, mapid_t id, mapinfo_t info, const char *ma
>  		if (dm_get_next_target(dmt, NULL, &start, &length,
>  				       &target_type, &params) != NULL) {
>  			condlog(2, "%s: map %s has multiple targets", fname__, map_id);
> -			return DMP_NOT_FOUND;
> +			return DMP_NO_MATCH;
>  		}
>  		if (!params) {
>  			condlog(2, "%s: map %s has no targets", fname__, map_id);

I feel like given the choice between DMP_NOT_FOUND and DMP_NO_MATCH, an
empty table should be DMP_NO_MATCH since we did find a device. I
assume you left this case alone because it will get changed with the
addition of a new error code for empty tables (I plan on posting that
shortly). So

Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>

> diff --git a/tests/mapinfo.c b/tests/mapinfo.c
> index fca6462..66c81e8 100644
> --- a/tests/mapinfo.c
> +++ b/tests/mapinfo.c
> @@ -870,7 +870,7 @@ static void test_mapinfo_bad_next_target_01(void **state)
>  	rc = libmp_mapinfo(DM_MAP_BY_NAME,
>  			   (mapid_t) { .str = "foo", },
>  			   (mapinfo_t) { .size = &size });
> -	assert_int_equal(rc, DMP_NOT_FOUND);
> +	assert_int_equal(rc, DMP_NO_MATCH);
>  }
>  
>  static void test_mapinfo_bad_next_target_02(void **state)
> -- 
> 2.47.0