Re: [PATCH 35/44] libmultipath: use libmp_pathinfo() in update_multipath_table()

[Benjamin Marzinski <bmarzins@xxxxxxxxxx>]

On Tue, Jul 09, 2024 at 11:39:26PM +0200, Martin Wilck wrote:
> This allows us to remove dm_get_status(), and dm_get_map(), of which
> update_multipath_table() was the last caller.
> 
> Signed-off-by: Martin Wilck <mwilck@xxxxxxxx>
> ---
>  libmultipath/devmapper.c   | 89 --------------------------------------
>  libmultipath/devmapper.h   |  2 -
>  libmultipath/structs_vec.c | 22 +++++-----
>  3 files changed, 12 insertions(+), 101 deletions(-)
> 
> diff --git a/libmultipath/devmapper.c b/libmultipath/devmapper.c
> index 185c76b..105e838 100644
> --- a/libmultipath/devmapper.c
> +++ b/libmultipath/devmapper.c
> @@ -804,45 +804,6 @@ static int dm_get_dm_uuid(const char *mapname, char uuid[DM_UUID_LEN])
>  			     (mapinfo_t) { .uuid = uuid });
>  }
>  
> -int dm_get_map(const char *name, unsigned long long *size, char **outparams)
> -{
> -	struct dm_task __attribute__((cleanup(cleanup_dm_task))) *dmt = NULL;
> -	uint64_t start, length;
> -	char *target_type = NULL;
> -	char *params = NULL;
> -
> -	if (!(dmt = libmp_dm_task_create(DM_DEVICE_TABLE)))
> -		return DMP_ERR;
> -
> -	if (!dm_task_set_name(dmt, name))
> -		return DMP_ERR;
> -
> -	errno = 0;
> -	if (!libmp_dm_task_run(dmt)) {
> -		dm_log_error(3, DM_DEVICE_TABLE, dmt);
> -		if (dm_task_get_errno(dmt) == ENXIO)
> -			return DMP_NOT_FOUND;
> -		else
> -			return DMP_ERR;
> -	}
> -
> -	/* Fetch 1st target */
> -	if (dm_get_next_target(dmt, NULL, &start, &length,
> -			       &target_type, &params) != NULL || !params)
> -		/* more than one target or not found target */
> -		return DMP_NOT_FOUND;
> -
> -	if (size)
> -		*size = length;
> -
> -	if (!outparams)
> -		return DMP_OK;
> -	else {
> -		*outparams = strdup(params);
> -		return *outparams ? DMP_OK : DMP_ERR;
> -	}
> -}
> -
>  bool is_mpath_uuid(const char uuid[DM_UUID_LEN])
>  {
>  	return !strncmp(uuid, UUID_PREFIX, UUID_PREFIX_LEN);
> @@ -886,56 +847,6 @@ static bool is_mpath_part_uuid(const char part_uuid[DM_UUID_LEN],
>  	return !strcmp(part_uuid + nc, map_uuid);
>  }
>  
> -int dm_get_status(const char *name, char **outstatus)
> -{
> -	int r = DMP_ERR;
> -	struct dm_task __attribute__((cleanup(cleanup_dm_task))) *dmt = NULL;
> -	uint64_t start, length;
> -	char *target_type = NULL;
> -	char *status = NULL;
> -
> -	if (!(dmt = libmp_dm_task_create(DM_DEVICE_STATUS)))
> -		return r;
> -
> -	if (!dm_task_set_name(dmt, name))
> -		goto out;
> -
> -	errno = 0;
> -	if (!libmp_dm_task_run(dmt)) {
> -		dm_log_error(3, DM_DEVICE_STATUS, dmt);
> -		if (dm_task_get_errno(dmt) == ENXIO)
> -			r = DMP_NOT_FOUND;
> -		goto out;
> -	}
> -
> -	r = DMP_NOT_FOUND;
> -	/* Fetch 1st target */
> -	if (dm_get_next_target(dmt, NULL, &start, &length,
> -			       &target_type, &status) != NULL)
> -		goto out;
> -
> -	if (!target_type || strcmp(target_type, TGT_MPATH) != 0)
> -		goto out;
> -
> -	if (!status) {
> -		condlog(2, "got null status.");
> -		goto out;
> -	}
> -
> -	if (!outstatus)
> -		r = DMP_OK;
> -	else {
> -		*outstatus = strdup(status);
> -		r = *outstatus ? DMP_OK : DMP_ERR;
> -	}
> -out:
> -	if (r != DMP_OK)
> -		condlog(0, "%s: %s: error getting map status string: %d",
> -			__func__, name, r);
> -
> -	return r;
> -}
> -
>  int dm_is_mpath(const char *name)
>  {
>  	char uuid[DM_UUID_LEN];
> diff --git a/libmultipath/devmapper.h b/libmultipath/devmapper.h
> index ed5e866..cb5151e 100644
> --- a/libmultipath/devmapper.h
> +++ b/libmultipath/devmapper.h
> @@ -126,8 +126,6 @@ int dm_simplecmd_noflush (int task, const char *name, uint16_t udev_flags);
>  int dm_addmap_create (struct multipath *mpp, char *params);
>  int dm_addmap_reload (struct multipath *mpp, char *params, int flush);
>  int dm_map_present_by_wwid(const char *uuid);
> -int dm_get_map(const char *name, unsigned long long *size, char **outparams);
> -int dm_get_status(const char *name, char **outstatus);
>  
>  enum {
>  	DM_IS_MPATH_NO,
> diff --git a/libmultipath/structs_vec.c b/libmultipath/structs_vec.c
> index ccc4efc..770e498 100644
> --- a/libmultipath/structs_vec.c
> +++ b/libmultipath/structs_vec.c
> @@ -484,30 +484,32 @@ int
>  update_multipath_table (struct multipath *mpp, vector pathvec, int flags)
>  {
>  	int r = DMP_ERR;
> -	char *params = NULL;
> +	char __attribute__((cleanup(cleanup_charp))) *params = NULL;
> +	char __attribute__((cleanup(cleanup_charp))) *status = NULL;
>  
>  	if (!mpp)
>  		return r;
>  
> -	r = dm_get_map(mpp->alias, &mpp->size, &params);

Shouldn't we update mpp->size like we were with dm_get_map()?

-Ben

> +	r = libmp_mapinfo(DM_MAP_BY_NAME,
> +			  (mapid_t) { .str = mpp->alias },
> +			  (mapinfo_t) {
> +				  .target = &params,
> +				  .status = &status,
> +				  .tgt_type = TGT_MPATH
> +			  });
> +
>  	if (r != DMP_OK) {
> -		condlog(2, "%s: %s", mpp->alias, (r == DMP_ERR)? "error getting table" : "map not present");
> +		condlog(2, "%s: %s", mpp->alias, dmp_errstr(r));
>  		return r;
>  	}
>  
>  	if (disassemble_map(pathvec, params, mpp)) {
>  		condlog(2, "%s: cannot disassemble map", mpp->alias);
> -		free(params);
>  		return DMP_ERR;
>  	}
>  
> -	free(params);
> -	params = NULL;
> -	if (dm_get_status(mpp->alias, &params) != DMP_OK)
> -		condlog(2, "%s: %s", mpp->alias, (r == DMP_ERR)? "error getting status" : "map not present");
> -	else if (disassemble_status(params, mpp))
> +	if (disassemble_status(status, mpp))
>  		condlog(2, "%s: cannot disassemble status", mpp->alias);
> -	free(params);
>  
>  	/* FIXME: we should deal with the return value here */
>  	update_pathvec_from_dm(pathvec, mpp, flags);
> -- 
> 2.45.2