| Message ID | 20240713060506.2015463-2-bmarzins@redhat.com (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Delegated to: | christophe varoqui |
| Headers | show |
| Series | path checker refactor and misc fixes | expand |
Hi Ben, On Sat, 2024-07-13 at 02:04 -0400, Benjamin Marzinski wrote: > add arguments to dm_map_present_by_wwid() to allow optionally > fetching > the device name and minor number for the devices found by WWID. > These > will be used by a later patch. Since it can now also be used to get > the > name and minor number of a device from its WWID, rename it to > dm_find_map_by_wwid() > > Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com> Is this supposed to apply over my v1 patchseries? That patch series needed major amends, and I'm getting merge conflicts trying to apply it on v2. I can untangle that, but it'd be easier to figure out if we'd setted on a final reviewed version of my patch set. Regards Martin
On Sat, 2024-07-13 at 02:04 -0400, Benjamin Marzinski wrote: > add arguments to dm_map_present_by_wwid() to allow optionally > fetching > the device name and minor number for the devices found by WWID. > These > will be used by a later patch. Since it can now also be used to get > the > name and minor number of a device from its WWID, rename it to > dm_find_map_by_wwid() > > Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com> > --- > libmultipath/devmapper.c | 16 ++++++++++++---- > libmultipath/devmapper.h | 2 +- > libmultipath/valid.c | 2 +- > tests/valid.c | 12 ++++++------ > 4 files changed, 20 insertions(+), 12 deletions(-) > > diff --git a/libmultipath/devmapper.h b/libmultipath/devmapper.h > index c28991fb..1de694c9 100644 > --- a/libmultipath/devmapper.h > +++ b/libmultipath/devmapper.h > @@ -132,7 +132,7 @@ int dm_simplecmd_flush (int task, const char > *name, uint16_t udev_flags); > 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_find_map_by_wwid(const char *wwid, char *name, int *minor); > Nitpick: According to my personal taste, it would be cleaner to pass in the entire dm_info structure rather than just the minor number. One day we may need other fields as well. Martin
On Mon, 2024-07-15 at 12:53 +0200, Martin Wilck wrote: > Hi Ben, > > On Sat, 2024-07-13 at 02:04 -0400, Benjamin Marzinski wrote: > > add arguments to dm_map_present_by_wwid() to allow optionally > > fetching > > the device name and minor number for the devices found by WWID. > > These > > will be used by a later patch. Since it can now also be used to get > > the > > name and minor number of a device from its WWID, rename it to > > dm_find_map_by_wwid() > > > > Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com> > > Is this supposed to apply over my v1 patchseries? That patch series > needed major amends, and I'm getting merge conflicts trying to apply > it > on v2. I can untangle that, but it'd be easier to figure out if we'd > setted on a final reviewed version of my patch set. Never mind, my email filter seems to have swallowed part of the series. The full series applies cleanly to my v2 tree. Martin
On Mon, Jul 15, 2024 at 05:14:36PM +0200, Martin Wilck wrote: > On Sat, 2024-07-13 at 02:04 -0400, Benjamin Marzinski wrote: > > add arguments to dm_map_present_by_wwid() to allow optionally > > fetching > > the device name and minor number for the devices found by WWID. > > These > > will be used by a later patch. Since it can now also be used to get > > the > > name and minor number of a device from its WWID, rename it to > > dm_find_map_by_wwid() > > > > Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com> > > --- > > libmultipath/devmapper.c | 16 ++++++++++++---- > > libmultipath/devmapper.h | 2 +- > > libmultipath/valid.c | 2 +- > > tests/valid.c | 12 ++++++------ > > 4 files changed, 20 insertions(+), 12 deletions(-) > > > > diff --git a/libmultipath/devmapper.h b/libmultipath/devmapper.h > > index c28991fb..1de694c9 100644 > > --- a/libmultipath/devmapper.h > > +++ b/libmultipath/devmapper.h > > @@ -132,7 +132,7 @@ int dm_simplecmd_flush (int task, const char > > *name, uint16_t udev_flags); > > 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_find_map_by_wwid(const char *wwid, char *name, int *minor); > > > > Nitpick: According to my personal taste, it would be cleaner to pass in > the entire dm_info structure rather than just the minor number. One day > we may need other fields as well. Sure. > > Martin
diff --git a/libmultipath/devmapper.c b/libmultipath/devmapper.c index bbbadeee..60d786c6 100644 --- a/libmultipath/devmapper.c +++ b/libmultipath/devmapper.c @@ -872,16 +872,24 @@ int dm_is_mpath(const char *name) } } -int dm_map_present_by_wwid(const char *wwid) +/* if name is non-NULL, it must point to an array of WWID_SIZE bytes */ +int dm_find_map_by_wwid(const char *wwid, char *name, int *minor) { char tmp[DM_UUID_LEN]; + struct dm_info dmi; + int rc; if (safe_sprintf(tmp, UUID_PREFIX "%s", wwid)) return DMP_ERR; - return libmp_mapinfo(DM_MAP_BY_UUID, - (mapid_t) { .str = tmp }, - (mapinfo_t) { .name = NULL }); + rc = libmp_mapinfo(DM_MAP_BY_UUID, + (mapid_t) { .str = tmp }, + (mapinfo_t) { .name = name, + .dmi = minor ? &dmi : NULL }); + if (rc == DMP_OK && minor) + *minor = dmi.minor; + + return rc; } static int dm_dev_t (const char *mapname, char *dev_t, int len) diff --git a/libmultipath/devmapper.h b/libmultipath/devmapper.h index c28991fb..1de694c9 100644 --- a/libmultipath/devmapper.h +++ b/libmultipath/devmapper.h @@ -132,7 +132,7 @@ int dm_simplecmd_flush (int task, const char *name, uint16_t udev_flags); 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_find_map_by_wwid(const char *wwid, char *name, int *minor); enum { DM_IS_MPATH_NO, diff --git a/libmultipath/valid.c b/libmultipath/valid.c index 9267cef9..b7e0cc9b 100644 --- a/libmultipath/valid.c +++ b/libmultipath/valid.c @@ -360,7 +360,7 @@ is_path_valid(const char *name, struct config *conf, struct path *pp, if (check_wwids_file(pp->wwid, 0) == 0) return PATH_IS_VALID_NO_CHECK; - if (dm_map_present_by_wwid(pp->wwid) == DMP_OK) + if (dm_find_map_by_wwid(pp->wwid, NULL, NULL) == DMP_OK) return PATH_IS_VALID; /* all these act like FIND_MULTIPATHS_STRICT for finding if a diff --git a/tests/valid.c b/tests/valid.c index a93bbe50..d9325f8b 100644 --- a/tests/valid.c +++ b/tests/valid.c @@ -189,10 +189,10 @@ int __wrap_check_wwids_file(char *wwid, int write_wwid) return -1; } -int __wrap_dm_map_present_by_wwid(const char *uuid) +int __wrap_dm_find_map_by_wwid(const char *wwid, char *name, int *minor) { int ret = mock_type(int); - assert_string_equal(uuid, mock_ptr_type(char *)); + assert_string_equal(wwid, mock_ptr_type(char *)); return ret; } @@ -271,8 +271,8 @@ static void setup_passing(char *name, char *wwid, unsigned int check_multipathd, will_return(__wrap_check_wwids_file, wwid); if (stage == STAGE_CHECK_WWIDS) return; - will_return(__wrap_dm_map_present_by_wwid, 0); - will_return(__wrap_dm_map_present_by_wwid, wwid); + will_return(__wrap_dm_find_map_by_wwid, 0); + will_return(__wrap_dm_find_map_by_wwid, wwid); } static void test_bad_arguments(void **state) @@ -516,8 +516,8 @@ static void test_check_uuid_present(void **state) memset(&pp, 0, sizeof(pp)); conf.find_multipaths = FIND_MULTIPATHS_STRICT; setup_passing(name, wwid, CHECK_MPATHD_RUNNING, STAGE_CHECK_WWIDS); - will_return(__wrap_dm_map_present_by_wwid, 1); - will_return(__wrap_dm_map_present_by_wwid, wwid); + will_return(__wrap_dm_find_map_by_wwid, 1); + will_return(__wrap_dm_find_map_by_wwid, wwid); assert_int_equal(is_path_valid(name, &conf, &pp, true), PATH_IS_VALID); assert_string_equal(pp.dev, name);
add arguments to dm_map_present_by_wwid() to allow optionally fetching the device name and minor number for the devices found by WWID. These will be used by a later patch. Since it can now also be used to get the name and minor number of a device from its WWID, rename it to dm_find_map_by_wwid() Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com> --- libmultipath/devmapper.c | 16 ++++++++++++---- libmultipath/devmapper.h | 2 +- libmultipath/valid.c | 2 +- tests/valid.c | 12 ++++++------ 4 files changed, 20 insertions(+), 12 deletions(-)