On Thu, 2023-02-09 at 11:28 -0600, Benjamin Marzinski wrote:
> Currently, whenever getting the uid_attribute from the udev database
> fails, multipath will try to get it from the environment variables.
> This
> normally isn't a problem, since either multipath -u is getting called
> from a uevent and the environment will have the correct value in that
> variable, or something else is being run and that variable won't be
> set.
> However, when find_multipaths is configured to "smart", this causes
> problems. For maybe devices, multipath needs to get the WWIDs of all
> the
> other block devices, to see if they match the maybe device wwid. If
> one
> of those devices doesn't have uid_attribute set in its udev database,
> multipath will fall back to checking the environment for it, and it
> will
> find that variable set to the WWID of the maybe device that this
> uevent
> is for. This means that all devices with no WWID will end up
> appearing
> to have the same WWID as the maybe device, causing multipath to
> incorrectly claim it.
>
> Signed-off-by: Benjamin Marzinski <bmarzins@xxxxxxxxxx>
I have to say I don't quite understand why we read from the environment
at all if the libudev call fails. This was coded before I joined the
project, so perhaps you can clarify it. Why do we expect the
environment to provide the correct value if libudev cannot?
If we need to keep this fallback, I wonder if we need another field in
"struct path" for it. For example, we could read MAJOR and MINOR from
the environment and use uid_attribute only if the result matches the
path's devt.
Regards
Martin
> ---
> libmultipath/discovery.c | 2 +-
> libmultipath/structs.h | 1 +
> multipath/main.c | 2 ++
> 3 files changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/libmultipath/discovery.c b/libmultipath/discovery.c
> index 67ac0e6d..3a5ba173 100644
> --- a/libmultipath/discovery.c
> +++ b/libmultipath/discovery.c
> @@ -2093,7 +2093,7 @@ get_udev_uid(struct path * pp, const char
> *uid_attribute, struct udev_device *ud
> const char *value;
>
> value = udev_device_get_property_value(udev, uid_attribute);
> - if (!value || strlen(value) == 0)
> + if ((!value || strlen(value) == 0) && pp->can_use_env_uid)
> value = getenv(uid_attribute);
> if (value && strlen(value)) {
> len = strlcpy(pp->wwid, value, WWID_SIZE);
> diff --git a/libmultipath/structs.h b/libmultipath/structs.h
> index e2294323..a1208751 100644
> --- a/libmultipath/structs.h
> +++ b/libmultipath/structs.h
> @@ -368,6 +368,7 @@ struct path {
> unsigned int dev_loss;
> int eh_deadline;
> bool is_checked;
> + bool can_use_env_uid;
> /* configlet pointers */
> vector hwe;
> struct gen_path generic_path;
> diff --git a/multipath/main.c b/multipath/main.c
> index b9f360b4..90f940f1 100644
> --- a/multipath/main.c
> +++ b/multipath/main.c
> @@ -607,6 +607,8 @@ check_path_valid(const char *name, struct config
> *conf, bool is_uevent)
> pp = alloc_path();
> if (!pp)
> return RTVL_FAIL;
> + if (is_uevent)
> + pp->can_use_env_uid = true;
>
> r = is_path_valid(name, conf, pp, is_uevent);
> if (r <= PATH_IS_ERROR || r >= PATH_MAX_VALID_RESULT)
--
dm-devel mailing list
dm-devel@xxxxxxxxxx
https://listman.redhat.com/mailman/listinfo/dm-devel
