diff mbox

[68/78] libmultipath: unset 'uid_attribute' on failure

Message ID 1426509425-15978-69-git-send-email-hare@suse.de (mailing list archive)
State Not Applicable, archived
Delegated to: christophe varoqui
Headers show

Commit Message

Hannes Reinecke March 16, 2015, 12:36 p.m. UTC
Due to a race condition within udev the 'uid_attribute'
might not always be set. So we should be zeroing the
'uid_attribute' when retrieving the uid by other means,
otherwise the discovery process will blacklist the device.

Signed-off-by: Hannes Reinecke <hare@suse.de>
---
 libmultipath/discovery.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

Comments

Benjamin Marzinski March 27, 2015, 4:10 a.m. UTC | #1
On Mon, Mar 16, 2015 at 01:36:55PM +0100, Hannes Reinecke wrote:
> Due to a race condition within udev the 'uid_attribute'
> might not always be set. So we should be zeroing the
> 'uid_attribute' when retrieving the uid by other means,
> otherwise the discovery process will blacklist the device.

Possibly I'm just missing obvious here, but I don't get the point of
zeroing out the uid_attribute.  Won't it just get reset on the next call
to get_uid?

-Ben

> 
> Signed-off-by: Hannes Reinecke <hare@suse.de>
> ---
>  libmultipath/discovery.c | 10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/libmultipath/discovery.c b/libmultipath/discovery.c
> index d5dda2c..8e6b228 100644
> --- a/libmultipath/discovery.c
> +++ b/libmultipath/discovery.c
> @@ -1497,13 +1497,17 @@ get_uid (struct path * pp)
>  		}
>  		if (len <= 0) {
>  			len = get_vpd_uid(pp);
> -			if (len > 0)
> +			if (len > 0) {
>  				origin = "sysfs";
> +				pp->uid_attribute = NULL;
> +			}
>  		}
>  		if (len <= 0) {
>  			len = get_udev_uid(pp, DEFAULT_UID_ATTRIBUTE);
> -			if (len > 0)
> -				origin = "udev";
> +			if (len > 0) {
> +				origin = "default";
> +				pp->uid_attribute = DEFAULT_UID_ATTRIBUTE;
> +			}
>  		}
>  	}
>  	if ( len < 0 ) {
> -- 
> 1.8.4.5

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel
Hannes Reinecke March 27, 2015, 7:17 a.m. UTC | #2
On 03/27/2015 05:10 AM, Benjamin Marzinski wrote:
> On Mon, Mar 16, 2015 at 01:36:55PM +0100, Hannes Reinecke wrote:
>> Due to a race condition within udev the 'uid_attribute'
>> might not always be set. So we should be zeroing the
>> 'uid_attribute' when retrieving the uid by other means,
>> otherwise the discovery process will blacklist the device.
> 
> Possibly I'm just missing obvious here, but I don't get the point of
> zeroing out the uid_attribute.  Won't it just get reset on the next call
> to get_uid?
> 
The uid_attribute setting is evaluated later on, so when we're not
zeroing it the code assumes we've got the UID from the udev
attribute, which is untrue.

Cheers,

Hannes
diff mbox

Patch

diff --git a/libmultipath/discovery.c b/libmultipath/discovery.c
index d5dda2c..8e6b228 100644
--- a/libmultipath/discovery.c
+++ b/libmultipath/discovery.c
@@ -1497,13 +1497,17 @@  get_uid (struct path * pp)
 		}
 		if (len <= 0) {
 			len = get_vpd_uid(pp);
-			if (len > 0)
+			if (len > 0) {
 				origin = "sysfs";
+				pp->uid_attribute = NULL;
+			}
 		}
 		if (len <= 0) {
 			len = get_udev_uid(pp, DEFAULT_UID_ATTRIBUTE);
-			if (len > 0)
-				origin = "udev";
+			if (len > 0) {
+				origin = "default";
+				pp->uid_attribute = DEFAULT_UID_ATTRIBUTE;
+			}
 		}
 	}
 	if ( len < 0 ) {