[dm-devel] [PATCH 1/3] pathinfo: call filter_property() after sysfs_pathinfo()
mwilck at suse.com
mwilck at suse.com
Tue Feb 2 21:27:27 UTC 2021
From: Martin Wilck <mwilck at suse.com>
The of filter_property() depends on the value of pp->uid_attribute.
This may in turn depend on pp->hwe, which is initialized in
sysfs_pathinfo(). To obtain consistent results from pathinfo(),
make sure uid_attribute is correctly set before calling filter_property().
filter_property() is now called from pathinfo() with properly set
uid_attribute, thus we don't need to call it from is_path_valid() any more.
Thes changes require modifications to the unit tests. The is_path_valid()
test now wouldn't need to test filter_property() any more, because
is_path_valid() calls filter_property() no more. But that doesn't feel
right. Instead, test_filter_property() is modified to test the behavior
with the filter_property() test called indirectly from pathinfo().
Signed-off-by: Martin Wilck <mwilck at suse.com>
---
libmultipath/discovery.c | 21 +++++++++-
libmultipath/valid.c | 4 --
tests/Makefile | 2 +-
tests/test-lib.c | 5 ++-
tests/valid.c | 91 ++++++++++++++++++++++++++++++++++++----
5 files changed, 105 insertions(+), 18 deletions(-)
diff --git a/libmultipath/discovery.c b/libmultipath/discovery.c
index 15cf641..febcd0a 100644
--- a/libmultipath/discovery.c
+++ b/libmultipath/discovery.c
@@ -2247,9 +2247,17 @@ int pathinfo(struct path *pp, struct config *conf, int mask)
condlog(4, "%s: hidden", pp->dev);
return PATHINFO_SKIPPED;
}
- if (is_claimed_by_foreign(pp->udev) ||
- filter_property(conf, pp->udev, 4, pp->uid_attribute) > 0)
+
+ if (is_claimed_by_foreign(pp->udev))
return PATHINFO_SKIPPED;
+
+ /*
+ * uid_attribute is required for filter_property below,
+ * and needs access to pp->hwe.
+ */
+ if (!(mask & DI_SYSFS) && !pp->uid_attribute &&
+ VECTOR_SIZE(pp->hwe) == 0)
+ mask |= DI_SYSFS;
}
if (strlen(pp->dev) != 0 && filter_devnode(conf->blist_devnode,
@@ -2287,6 +2295,15 @@ int pathinfo(struct path *pp, struct config *conf, int mask)
}
}
+ if (pp->udev) {
+ /* uid_attribute is required for filter_property() */
+ if (!pp->uid_attribute)
+ select_getuid(conf, pp);
+
+ if (filter_property(conf, pp->udev, 4, pp->uid_attribute) > 0)
+ return PATHINFO_SKIPPED;
+ }
+
if (mask & DI_BLACKLIST && mask & DI_SYSFS) {
if (filter_device(conf->blist_device, conf->elist_device,
pp->vendor_id, pp->product_id, pp->dev) > 0 ||
diff --git a/libmultipath/valid.c b/libmultipath/valid.c
index 456b1f6..a6aa921 100644
--- a/libmultipath/valid.c
+++ b/libmultipath/valid.c
@@ -89,10 +89,6 @@ is_path_valid(const char *name, struct config *conf, struct path *pp,
if (pp->wwid[0] == '\0')
return PATH_IS_NOT_VALID;
- if (pp->udev && pp->uid_attribute &&
- filter_property(conf, pp->udev, 3, pp->uid_attribute) > 0)
- return PATH_IS_NOT_VALID;
-
r = is_failed_wwid(pp->wwid);
if (r != WWID_IS_NOT_FAILED) {
if (r == WWID_IS_FAILED)
diff --git a/tests/Makefile b/tests/Makefile
index 50673fa..11ca1be 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -54,7 +54,7 @@ vpd-test_OBJDEPS := ../libmultipath/discovery.o
vpd-test_LIBDEPS := -ludev -lpthread -ldl
alias-test_TESTDEPS := test-log.o
alias-test_LIBDEPS := -lpthread -ldl
-valid-test_OBJDEPS := ../libmultipath/valid.o
+valid-test_OBJDEPS := ../libmultipath/valid.o ../libmultipath/discovery.o
valid-test_LIBDEPS := -ludev -lpthread -ldl
devt-test_LIBDEPS := -ludev
mpathvalid-test_LIBDEPS := -ludev -lpthread -ldl
diff --git a/tests/test-lib.c b/tests/test-lib.c
index e7663f9..960a766 100644
--- a/tests/test-lib.c
+++ b/tests/test-lib.c
@@ -257,6 +257,9 @@ void mock_pathinfo(int mask, const struct mocked_path *mp)
} else
will_return(__wrap_udev_device_get_sysattr_value, "0");
+ if (mask & DI_SYSFS)
+ mock_sysfs_pathinfo(mp);
+
/* filter_property */
will_return(__wrap_udev_device_get_sysname, mp->devnode);
if (mp->flags & BL_BY_PROPERTY) {
@@ -265,8 +268,6 @@ void mock_pathinfo(int mask, const struct mocked_path *mp)
} else
will_return(__wrap_udev_list_entry_get_name,
"SCSI_IDENT_LUN_NAA_EXT");
- if (mask & DI_SYSFS)
- mock_sysfs_pathinfo(mp);
if (mp->flags & BL_BY_DEVICE &&
(mask & DI_BLACKLIST && mask & DI_SYSFS))
diff --git a/tests/valid.c b/tests/valid.c
index 693c72c..8ec803e 100644
--- a/tests/valid.c
+++ b/tests/valid.c
@@ -25,13 +25,18 @@
#include <stdlib.h>
#include <errno.h>
#include <cmocka.h>
+#include <sys/sysmacros.h>
+
#include "globals.c"
#include "util.h"
#include "discovery.h"
#include "wwids.h"
#include "blacklist.h"
+#include "foreign.h"
#include "valid.h"
+#define PATHINFO_REAL 9999
+
int test_fd;
struct udev_device {
int unused;
@@ -78,12 +83,66 @@ struct udev_device *__wrap_udev_device_new_from_subsystem_sysname(struct udev *u
return NULL;
}
+/* For the "hidden" check in pathinfo() */
+const char *__wrap_udev_device_get_sysattr_value(struct udev_device *udev_device,
+ const char *sysattr)
+{
+ check_expected(sysattr);
+ return mock_ptr_type(char *);
+}
+
+/* For pathinfo() -> is_claimed_by_foreign() */
+int __wrap_add_foreign(struct udev_device *udev_device)
+{
+ return mock_type(int);
+}
+
+/* called from pathinfo() */
+int __wrap_filter_devnode(struct config *conf, const struct _vector *elist,
+ const char *vendor, const char * product, const char *dev)
+{
+ return mock_type(int);
+}
+
+/* called from pathinfo() */
+int __wrap_filter_device(const struct _vector *blist, const struct _vector *elist,
+ const char *vendor, const char * product, const char *dev)
+{
+ return mock_type(int);
+}
+
+/* for common_sysfs_pathinfo() */
+dev_t __wrap_udev_device_get_devnum(struct udev_device *ud)
+{
+ return mock_type(dev_t);
+}
+
+/* for common_sysfs_pathinfo() */
+int __wrap_sysfs_get_size(struct path *pp, unsigned long long * size)
+{
+ return mock_type(int);
+}
+
+/* called in pathinfo() before filter_property() */
+int __wrap_select_getuid(struct config *conf, struct path *pp)
+{
+ pp->uid_attribute = mock_ptr_type(char *);
+ return 0;
+}
+
+int __real_pathinfo(struct path *pp, struct config *conf, int mask);
+
int __wrap_pathinfo(struct path *pp, struct config *conf, int mask)
{
int ret = mock_type(int);
+
assert_string_equal(pp->dev, mock_ptr_type(char *));
assert_int_equal(mask, DI_SYSFS | DI_WWID | DI_BLACKLIST);
- if (ret == PATHINFO_OK) {
+ if (ret == PATHINFO_REAL) {
+ /* for test_filter_property() */
+ ret = __real_pathinfo(pp, conf, mask);
+ return ret;
+ } else if (ret == PATHINFO_OK) {
pp->uid_attribute = "ID_TEST";
strlcpy(pp->wwid, mock_ptr_type(char *), WWID_SIZE);
} else
@@ -128,6 +187,7 @@ enum {
STAGE_IS_MULTIPATHED,
STAGE_CHECK_MULTIPATHD,
STAGE_GET_UDEV_DEVICE,
+ STAGE_PATHINFO_REAL,
STAGE_PATHINFO,
STAGE_FILTER_PROPERTY,
STAGE_IS_FAILED,
@@ -167,12 +227,25 @@ static void setup_passing(char *name, char *wwid, unsigned int check_multipathd,
name);
if (stage == STAGE_GET_UDEV_DEVICE)
return;
+ if (stage == STAGE_PATHINFO_REAL) {
+ /* special case for test_filter_property() */
+ will_return(__wrap_pathinfo, PATHINFO_REAL);
+ will_return(__wrap_pathinfo, name);
+ expect_string(__wrap_udev_device_get_sysattr_value,
+ sysattr, "hidden");
+ will_return(__wrap_udev_device_get_sysattr_value, NULL);
+ will_return(__wrap_add_foreign, FOREIGN_IGNORED);
+ will_return(__wrap_filter_devnode, MATCH_NOTHING);
+ will_return(__wrap_udev_device_get_devnum, makedev(259, 0));
+ will_return(__wrap_sysfs_get_size, 0);
+ will_return(__wrap_select_getuid, "ID_TEST");
+ return;
+ }
will_return(__wrap_pathinfo, PATHINFO_OK);
will_return(__wrap_pathinfo, name);
will_return(__wrap_pathinfo, wwid);
if (stage == STAGE_PATHINFO)
return;
- will_return(__wrap_filter_property, MATCH_PROPERTY_BLIST_EXCEPT);
if (stage == STAGE_FILTER_PROPERTY)
return;
will_return(__wrap_is_failed_wwid, WWID_IS_NOT_FAILED);
@@ -317,24 +390,24 @@ static void test_filter_property(void **state)
/* test blacklist property */
memset(&pp, 0, sizeof(pp));
conf.find_multipaths = FIND_MULTIPATHS_STRICT;
- setup_passing(name, wwid, CHECK_MPATHD_SKIP, STAGE_PATHINFO);
+ setup_passing(name, wwid, CHECK_MPATHD_SKIP, STAGE_PATHINFO_REAL);
will_return(__wrap_filter_property, MATCH_PROPERTY_BLIST);
assert_int_equal(is_path_valid(name, &conf, &pp, false),
PATH_IS_NOT_VALID);
assert_ptr_equal(pp.udev, &test_udev);
- assert_string_equal(pp.wwid, wwid);
+
/* test missing property */
memset(&pp, 0, sizeof(pp));
- setup_passing(name, wwid, CHECK_MPATHD_SKIP, STAGE_PATHINFO);
+ setup_passing(name, wwid, CHECK_MPATHD_SKIP, STAGE_PATHINFO_REAL);
will_return(__wrap_filter_property, MATCH_PROPERTY_BLIST_MISSING);
assert_int_equal(is_path_valid(name, &conf, &pp, false),
PATH_IS_NOT_VALID);
- /* test MATCH_NOTHING fail on is_failed_wwid */
+
+ /* test MATCH_NOTHING fail on filter_device */
memset(&pp, 0, sizeof(pp));
- setup_passing(name, wwid, CHECK_MPATHD_SKIP, STAGE_PATHINFO);
+ setup_passing(name, wwid, CHECK_MPATHD_SKIP, STAGE_PATHINFO_REAL);
will_return(__wrap_filter_property, MATCH_NOTHING);
- will_return(__wrap_is_failed_wwid, WWID_IS_FAILED);
- will_return(__wrap_is_failed_wwid, wwid);
+ will_return(__wrap_filter_device, MATCH_DEVICE_BLIST);
assert_int_equal(is_path_valid(name, &conf, &pp, false),
PATH_IS_NOT_VALID);
}
--
2.29.2
More information about the dm-devel
mailing list