On Wed, Sep 02, 2020 at 03:24:33PM +0800, lixiaokeng wrote:
> The return values of dm_get_map, disassemble_map in get_mpvec
> were not checked. Use update_multipath_table/status to instead
> of them.
>
Looks mostly good. I agree that we should be checking the results of
getting the raw data before we try to disassemble it. But, there's not
really any point to calling continue as the last operation of a loop.
Perhaps
if (update_multipath_table(mpp, pathvec, DI_CHECKER) == DMP_OK)
update_multipath_status(mpp);
makes more sense.
> Signed-off-by: Lixiaokeng <lixiaokeng@xxxxxxxxxx>
> Signed-off-by: Zhiqiang Liu <liuzhiqiang26@xxxxxxxxxx>
> Signed-off-by: Linfeilong <linfeilong@xxxxxxxxxx>
> ---
> libmpathpersist/mpath_persist.c | 11 +++--------
> 1 file changed, 3 insertions(+), 8 deletions(-)
>
> diff --git a/libmpathpersist/mpath_persist.c b/libmpathpersist/mpath_persist.c
> index e7256049..046fda21 100644
> --- a/libmpathpersist/mpath_persist.c
> +++ b/libmpathpersist/mpath_persist.c
> @@ -323,7 +323,6 @@ get_mpvec (vector curmp, vector pathvec, char * refwwid)
> {
> int i;
> struct multipath *mpp;
> - char params[PARAMS_SIZE], status[PARAMS_SIZE];
>
> vector_foreach_slot (curmp, mpp, i){
> /*
> @@ -341,13 +340,9 @@ get_mpvec (vector curmp, vector pathvec, char * refwwid)
> if (refwwid && strncmp (mpp->alias, refwwid, WWID_SIZE - 1))
> continue;
>
> - dm_get_map(mpp->alias, &mpp->size, params);
> - condlog(3, "params = %s", params);
> - dm_get_status(mpp->alias, status);
> - condlog(3, "status = %s", status);
> - disassemble_map (pathvec, params, mpp);
> - update_pathvec_from_dm(pathvec, mpp, DI_CHECKER);
> - disassemble_status (status, mpp);
> + if (update_multipath_table(mpp, pathvec, DI_CHECKER) != DMP_OK ||
> + update_multipath_status(mpp) != DMP_OK)
> + continue;
>
> }
> return MPATH_PR_SUCCESS ;
> --
--
dm-devel mailing list
dm-devel@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/dm-devel
