diff mbox series

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

Message ID 20240709213935.177028-36-mwilck@suse.com (mailing list archive)
State Not Applicable, archived
Delegated to: christophe varoqui
Headers show
Series multipath-tools: devmapper API refactored | expand

Commit Message

Martin Wilck July 9, 2024, 9:39 p.m. UTC 
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@suse.com>
---
 libmultipath/devmapper.c   | 89 --------------------------------------
 libmultipath/devmapper.h   |  2 -
 libmultipath/structs_vec.c | 22 +++++-----
 3 files changed, 12 insertions(+), 101 deletions(-)

Comments

Benjamin Marzinski July 11, 2024, 5:16 a.m. UTC | #1 
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@suse.com>
> ---
>  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
Martin Wilck July 11, 2024, 3:29 p.m. UTC | #2 
On Thu, 2024-07-11 at 01:16 -0400, Benjamin Marzinski wrote:
> 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@suse.com>
> > ---
> >  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()?

Yes. Thanks for catching this.
Although if we do we should probably check whether it changed...

Martin
diff mbox series

Patch

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);
+	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);