[Ocfs2-tools-devel] [PATCH] adding code to mounted.c to translate /dev/dm-[N] names to /dev/mapper/xxx names.

[xiaowei hu]

On Fri, 2009-12-04 at 10:15 +0800, Tristan wrote:
> xiaowei hu wrote:
> > thanks for your comments.
> > answers inline.
> >
> > will resend a patch following your suggestions.:)
> >
> > On Thu, 2009-12-03 at 13:20 -0800, Sunil Mushran wrote:
> >   
> >> Please make the title (first line) shorter followed by a description of
> >> what the patch does.
> >>
> >> Comments inline.
> >>
> >> XiaoweiHu wrote:
> >>     
> >>> Signed-off-by: XiaoweiHu <xiaowei.hu at oracle.com>
> >>> ---
> >>>  mounted.ocfs2/mounted.c |   76 +++++++++++++++++++++++++++++++++++++++++++++-
> >>>  1 files changed, 74 insertions(+), 2 deletions(-)
> >>>
> >>> diff --git a/mounted.ocfs2/mounted.c b/mounted.ocfs2/mounted.c
> >>> index e4c5877..c691a3d 100644
> >>> --- a/mounted.ocfs2/mounted.c
> >>> +++ b/mounted.ocfs2/mounted.c
> >>> @@ -33,6 +33,7 @@
> >>>  #include <linux/fd.h>
> >>>  #include <string.h>
> >>>  #include <sys/stat.h>
> >>> +#include <dirent.h>
> >>>  
> >>>  #include <uuid/uuid.h>
> >>>  
> >>> @@ -155,12 +156,70 @@ static void ocfs2_print_quick_detect(struct list_head *dev_list)
> >>>  	}
> >>>  }
> >>>  
> >>> +char *devname_strndup(const char *s, int length)
> >>> +{
> >>> +        char *ret;
> >>> +
> >>> +        if (!s)
> >>> +                return NULL;
> >>> +
> >>> +        if (!length)
> >>> +                length = strlen(s);
> >>> +
> >>> +        ret = malloc(length + 1);
> >>> +        if (ret) {
> >>> +                strncpy(ret, s, length);
> >>> +                ret[length] = '\0';
> >>> +        }
> >>> +        return ret;
> >>> +}
> >>>       
> >> Why not use strndup()?
> >>     
> > yes, use this lib function is better:).
> >
> >   
> >>> +
> >>> +char *devname_strdup(const char *s)
> >>> +{
> >>> +        return devname_strndup(s, 0);
> >>> +}
> >>>       
> >> Please use tabs.
> >> http://oss.oracle.com/osswiki/OCFS2/CodingStyle
> >>
> >> Easiest if you added the following at the top of the file.
> >> /* -*- mode: c; c-basic-offset: 8; -*-
> >> * vim: noexpandtab sw=8 ts=8 sts=0:
> >> *
> >>
> >>     
> >>> +
> >>> +void scan_dir_for_dev(char *dirname, dev_t devno, char **devname)
> >>> +{
> >>> +        DIR     *dir;
> >>> +        struct dirent *dp;
> >>> +        char    path[1024];
> >>>       
> >> Use PATH_MAX.
> >>     
> >
> >   
> >>> +        int     dirlen;
> >>> +        struct stat st;
> >>> +
> >>> +        if ((dir = opendir(dirname)) == NULL)
> >>> +                return;
> >>> +        dirlen = strlen(dirname) + 2;
> >>> +        while ((dp = readdir(dir)) != 0) {
> >>> +                if (dirlen + strlen(dp->d_name) >= sizeof(path))
> >>> +                        continue;
> >>> +
> >>> +                if (dp->d_name[0] == '.' &&
> >>> +                    ((dp->d_name[1] == 0) ||
> >>> +                     ((dp->d_name[1] == '.') && (dp->d_name[2] == 0))))
> >>> +                        continue;
> >>>       
> >> You will skip filename "A.". ;)
> >>     
> > actually this won't skip "A." if the dp-d_name[0] == '.' is fause it
> > won't compare the following characters ,not matter what the following
> > character is '.' or not.
> >
> >   
> I saw you just want to skip the '.' and '..', why not use a separate 
> func to clearly clarify this:
> 
> int is_dot_entry(const char *d_name)
> {
> if (!d_name)
> return 0;
> 
> if (strlen(d_name) == 1 && d_name[0] = '.')
> return 1;
> if (strlen(d_name) == 2 && d_name[0] ='.' && d_name[1] == '.')
> return 1;
> 
> return 0;
> }
> 
> then,
> 
> if (is_dot_entry(dp->d_name))
> contiune;
> ...
> 
Yes, that's more readable:) thanks

> >> Why not strncmp "." and "..". Compare only if the strlen matches.
> >>
> >>     
> >>> +
> >>> +                sprintf(path, "%s/%s", dirname, dp->d_name);
> >>> +                if (stat(path, &st) < 0)
> >>> +                        continue;
> >>> +
> >>> +                if (S_ISBLK(st.st_mode) && st.st_rdev == devno) {
> >>> +			*devname = devname_strdup(path);
> >>> +                        break;
> >>> +                }
> >>>       
> >> This may not work for multipath. In this case, you will find a mpath
> >> for one dm device but not the other. Have you tested this on a multipath
> >> setup?
> >>     
> >
> > Yes ,I did this to solving a CT bug, and I tested this patch on a
> > multipath setup,it works fine.
> >   
> >>> +        }
> >>> +        closedir(dir);
> >>> +        return;
> >>> +}
> >>> +
> >>>  static errcode_t ocfs2_partition_list (struct list_head *dev_list)
> >>>  {
> >>>  	errcode_t ret = 0;
> >>>  	FILE *proc;
> >>>  	char line[256];
> >>>  	char name[256];
> >>> +	char *devname = NULL;
> >>> +	int major,minor;
> >>>  	ocfs2_devices *dev;
> >>>  
> >>>  	proc = fopen ("/proc/partitions", "r");
> >>> @@ -170,14 +229,27 @@ static errcode_t ocfs2_partition_list (struct list_head *dev_list)
> >>>  	}
> >>>  
> >>>  	while (fgets (line, sizeof(line), proc) != NULL) {
> >>> -		if (sscanf(line, "%*d %*d %*d %99[^ \t\n]", name) != 1)
> >>> +		if (sscanf(line, "%d %d %*d %99[^ \t\n]", &major, &minor, name) != 3)
> >>>  			continue;
> >>>  
> >>>  		ret = ocfs2_malloc0(sizeof(ocfs2_devices), &dev);
> >>>  		if (ret)
> >>>  			goto bail;
> >>>  
> >>> -		snprintf(dev->dev_name, sizeof(dev->dev_name), "/dev/%s", name);
> >>> +		/* Try to translate private device-mapper dm-<N> names
> >>> +         	 * to standard /dev/mapper/<name>.
> >>> +         	 */
> >>> +        	if (!strncmp(name, "dm-", 3) && isdigit(name[3])) {
> >>> +                        scan_dir_for_dev("/dev/mapper", makedev(major, minor), &devname);
> >>> +                	if (devname)
> >>> +				snprintf(dev->dev_name, sizeof(dev->dev_name), "%s", devname);
> >>> +			else
> >>> +				snprintf(dev->dev_name, sizeof(dev->dev_name), "/dev/%s", name);
> >>> +
> >>> +        	} else {
> >>> +			snprintf(dev->dev_name, sizeof(dev->dev_name), "/dev/%s", name);
> >>> +		}
> >>> +
> >>>  		list_add_tail(&(dev->list), dev_list);
> >>>  	}
> >>>  
> >>>       
> >
> >
> >
> > _______________________________________________
> > Ocfs2-tools-devel mailing list
> > Ocfs2-tools-devel at oss.oracle.com
> > http://oss.oracle.com/mailman/listinfo/ocfs2-tools-devel
> >   
>