[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

[dm-devel] RE: [Bug 217014] Setting "user_friendly_names" to "yes" causesdm-mp to occasionally miss a mpath device during configuration



Sorry, but I didn't send it to you directly since I mistakenly
thought you had already merged in the fix after I had
initially sent out this patch to the dm-devel list on 7/17. 

> -----Original Message-----
> From: Christophe Varoqui [mailto:christophe varoqui free fr] 
> Sent: Monday, November 27, 2006 5:10 PM
> To: goggin, edward
> Cc: device-mapper development
> Subject: Re: [Bug 217014] Setting "user_friendly_names" to 
> "yes" causesdm-mp to occasionally miss a mpath device during 
> configuration
> 
> Nice fix, thank you.
> Merged.
> 
> Le lundi 27 novembre 2006 à 11:30 -0500, bugzilla redhat com a écrit :
> > Please do not reply directly to this email. All additional
> > comments should be made in the comments box of this bug report.
> > 
> > Summary: Setting "user_friendly_names" to "yes" causes 
> dm-mp to occasionally miss a mpath device during configuration
> > 
> > 
> > https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=217014
> > 
> > 
> > 
> > 
> > 
> > ------- Additional Comments From egoggin emc com  
> 2006-11-27 11:29 EST -------
> > The posix file byte range locks used to provide atomicity 
> for accessing the 
> > entries in the multipath bindings file get released from 
> whenever __any__ 
> > descriptor or FILE structure for that file is closed.  This 
> patch delays the 
> > fclose() for the FILE structures used within lookup_binding() and 
> > rlookup_binding() until there is no more need for the atomicity.
> > 
> > Without this patch I could fairly easily get two multipath 
> mpath0 entries 
> > in /var/lib/multipath/bindings by running 8 concurrent 
> instances of multipath
> > (8) while with the patch I cannot get this problem to occur.
> > 
> > diff --git a/libmultipath/alias.c b/libmultipath/alias.c
> > index 6d103d7..86cae9b 100644
> > --- a/libmultipath/alias.c
> > +++ b/libmultipath/alias.c
> > @@ -166,28 +166,14 @@ fail:
> >  
> > 
> >  static int
> > -lookup_binding(int fd, char *map_wwid, char **map_alias)
> > +lookup_binding(FILE *f, char *map_wwid, char **map_alias)
> >  {
> >  	char buf[LINE_MAX];
> > -	FILE *f;
> >  	unsigned int line_nr = 0;
> > -	int scan_fd;
> >  	int id = 0;
> >  
> >  	*map_alias = NULL;
> > -	scan_fd = dup(fd);
> > -	if (scan_fd < 0) {
> > -		condlog(0, "Cannot dup bindings file descriptor : %s",
> > -			strerror(errno));
> > -		return -1;
> > -	}
> > -	f = fdopen(scan_fd, "r");
> > -	if (!f) {
> > -		condlog(0, "cannot fdopen on bindings file 
> descriptor : %s",
> > -			strerror(errno));
> > -		close(scan_fd);
> > -		return -1;
> > -	}
> > +
> >  	while (fgets(buf, LINE_MAX, f)) {
> >  		char *c, *alias, *wwid;
> >  		int curr_id;
> > @@ -215,38 +201,22 @@ lookup_binding(int fd, char *map_wwid, c
> >  			if (*map_alias == NULL)
> >  				condlog(0, "Cannot copy alias 
> from bindings "
> >  					"file : %s", strerror(errno));
> > -			fclose(f);
> >  			return id;
> >  		}
> >  	}
> >  	condlog(3, "No matching wwid [%s] in bindings file.", map_wwid);
> > -	fclose(f);
> >  	return id;
> >  }	
> >  
> >  static int
> > -rlookup_binding(int fd, char **map_wwid, char *map_alias)
> > +rlookup_binding(FILE *f, char **map_wwid, char *map_alias)
> >  {
> >  	char buf[LINE_MAX];
> > -	FILE *f;
> >  	unsigned int line_nr = 0;
> > -	int scan_fd;
> >  	int id = 0;
> >  
> >  	*map_wwid = NULL;
> > -	scan_fd = dup(fd);
> > -	if (scan_fd < 0) {
> > -		condlog(0, "Cannot dup bindings file descriptor : %s",
> > -			strerror(errno));
> > -		return -1;
> > -	}
> > -	f = fdopen(scan_fd, "r");
> > -	if (!f) {
> > -		condlog(0, "cannot fdopen on bindings file 
> descriptor : %s",
> > -			strerror(errno));
> > -		close(scan_fd);
> > -		return -1;
> > -	}
> > +
> >  	while (fgets(buf, LINE_MAX, f)) {
> >  		char *c, *alias, *wwid;
> >  		int curr_id;
> > @@ -274,12 +244,10 @@ rlookup_binding(int fd, char **map_wwid,
> >  			if (*map_wwid == NULL)
> >  				condlog(0, "Cannot copy alias 
> from bindings "
> >  					"file : %s", strerror(errno));
> > -			fclose(f);
> >  			return id;
> >  		}
> >  	}
> >  	condlog(3, "No matching alias [%s] in bindings file.", 
> map_alias);
> > -	fclose(f);
> >  	return id;
> >  }	
> >  
> > @@ -327,7 +295,8 @@ char *
> >  get_user_friendly_alias(char *wwid, char *file)
> >  {
> >  	char *alias;
> > -	int fd, id;
> > +	int fd, scan_fd, id;
> > +	FILE *f;
> >  
> >  	if (!wwid || *wwid == '\0') {
> >  		condlog(3, "Cannot find binding for empty WWID");
> > @@ -337,14 +306,37 @@ get_user_friendly_alias(char *wwid, char
> >  	fd = open_bindings_file(file);
> >  	if (fd < 0)
> >  		return NULL;
> > -	id = lookup_binding(fd, wwid, &alias);
> > +
> > +	scan_fd = dup(fd);
> > +	if (scan_fd < 0) {
> > +		condlog(0, "Cannot dup bindings file descriptor : %s",
> > +			strerror(errno));
> > +		close(fd);
> > +		return NULL;
> > +	}
> > +
> > +	f = fdopen(scan_fd, "r");
> > +	if (!f) {
> > +		condlog(0, "cannot fdopen on bindings file 
> descriptor : %s",
> > +			strerror(errno));
> > +		close(scan_fd);
> > +		close(fd);
> > +		return NULL;
> > +	}
> > +
> > +	id = lookup_binding(f, wwid, &alias);
> >  	if (id < 0) {
> > +		fclose(f);
> > +		close(scan_fd);
> >  		close(fd);
> >  		return NULL;
> >  	}
> > +
> >  	if (!alias)
> >  		alias = allocate_binding(fd, wwid, id);
> >  
> > +	fclose(f);
> > +	close(scan_fd);
> >  	close(fd);
> >  	return alias;
> >  }
> > @@ -353,7 +345,8 @@ char *
> >  get_user_friendly_wwid(char *alias, char *file)
> >  {
> >  	char *wwid;
> > -	int fd, id;
> > +	int fd, scan_fd, id;
> > +	FILE *f;
> >  
> >  	if (!alias || *alias == '\0') {
> >  		condlog(3, "Cannot find binding for empty alias");
> > @@ -363,12 +356,34 @@ get_user_friendly_wwid(char *alias, char
> >  	fd = open_bindings_file(file);
> >  	if (fd < 0)
> >  		return NULL;
> > -	id = rlookup_binding(fd, &wwid, alias);
> > +
> > +	scan_fd = dup(fd);
> > +	if (scan_fd < 0) {
> > +		condlog(0, "Cannot dup bindings file descriptor : %s",
> > +			strerror(errno));
> > +		close(fd);
> > +		return NULL;
> > +	}
> > +
> > +	f = fdopen(scan_fd, "r");
> > +	if (!f) {
> > +		condlog(0, "cannot fdopen on bindings file 
> descriptor : %s",
> > +			strerror(errno));
> > +		close(scan_fd);
> > +		close(fd);
> > +		return NULL;
> > +	}
> > +
> > +	id = rlookup_binding(f, &wwid, alias);
> >  	if (id < 0) {
> > +		fclose(f);
> > +		close(scan_fd);
> >  		close(fd);
> >  		return NULL;
> >  	}
> >  
> > +	fclose(f);
> > +	close(scan_fd);
> >  	close(fd);
> >  	return wwid;
> >  }
> > 
> 
> 
> 


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]