Patchwork [1/3] blkid: add UBI volume support

login
register
mail settings
Submitter Corentin Chary
Date Aug. 24, 2009, 11:11 a.m.
Message ID <1251112316-18971-2-git-send-email-corentincj@iksaif.net>
Download mbox | patch
Permalink /patch/31904/
State New, archived
Headers show

Comments

Corentin Chary - Aug. 24, 2009, 11:11 a.m.
Probe UBI volume under /dev (or /devfs, /devices).
ubi_ctrl skip is hardcoded, maybe we should find a
cleaner way to do that. Also change probe.c to handle
char devices.

Signed-off-by: Corentin Chary <corentincj@iksaif.net>
---
 shlibs/blkid/src/blkidP.h  |    1 +
 shlibs/blkid/src/devname.c |   56 +++++++++++++++++++++++++++++++++++++++++++-
 shlibs/blkid/src/probe.c   |    2 +
 3 files changed, 58 insertions(+), 1 deletions(-)
Karel Zak - Sept. 2, 2009, 9:24 a.m.
Hi Corentin,

On Mon, Aug 24, 2009 at 01:11:54PM +0200, Corentin Chary wrote:
> Probe UBI volume under /dev (or /devfs, /devices).

 that's not elegant. Does it mean that UBI volumes are not in
 /proc/partitions?
 
 Ted, any comment?

> ubi_ctrl skip is hardcoded, maybe we should find a
> cleaner way to do that. Also change probe.c to handle
> char devices.
> 
> Signed-off-by: Corentin Chary <corentincj@iksaif.net>
> ---
>  shlibs/blkid/src/blkidP.h  |    1 +
>  shlibs/blkid/src/devname.c |   56 +++++++++++++++++++++++++++++++++++++++++++-
>  shlibs/blkid/src/probe.c   |    2 +
>  3 files changed, 58 insertions(+), 1 deletions(-)
> 
> diff --git a/shlibs/blkid/src/blkidP.h b/shlibs/blkid/src/blkidP.h
> index e0e5cb8..8db6599 100644
> --- a/shlibs/blkid/src/blkidP.h
> +++ b/shlibs/blkid/src/blkidP.h
> @@ -237,6 +237,7 @@ extern char *blkid_strndup(const char *s, const int length);
>  /*
>   * Priority settings for different types of devices
>   */
> +#define BLKID_PRI_UBI	50
>  #define BLKID_PRI_DM	40
>  #define BLKID_PRI_EVMS	30
>  #define BLKID_PRI_LVM	20
> diff --git a/shlibs/blkid/src/devname.c b/shlibs/blkid/src/devname.c
> index ef686f4..cac13c5 100644
> --- a/shlibs/blkid/src/devname.c
> +++ b/shlibs/blkid/src/devname.c
> @@ -229,7 +229,9 @@ static void probe_one(blkid_cache cache, const char *ptname,
>  		    dev->bid_devno == devno)
>  			goto set_pri;
>  
> -		if (stat(device, &st) == 0 && S_ISBLK(st.st_mode) &&
> +		if (stat(device, &st) == 0 &&
> +		    (S_ISBLK(st.st_mode) ||
> +		     (S_ISCHR(st.st_mode) && !strncmp(ptname, "ubi", 3))) &&
>  		    st.st_rdev == devno) {
>  			devname = blkid_strdup(device);
>  			goto get_dev;
> @@ -388,6 +390,57 @@ evms_probe_all(blkid_cache cache, int only_if_new)
>  	return num;
>  }
>  
> +static void
> +ubi_probe_all(blkid_cache cache, int only_if_new)
> +{
> +	const char **dirname;
> +
> +	for (dirname = dirlist; *dirname; dirname++) {
> +		DBG(DEBUG_DEVNAME, printf("probing UBI volumes under %s\n",
> +					  *dirname));
> +
> +		DIR		*dir;
> +		struct dirent	*iter;
> +
> +		dir = opendir(*dirname);
> +		if (dir == NULL)
> +			continue ;
> +
> +		while ((iter = readdir(dir)) != NULL) {
> +			char		*name, *device;
> +			struct stat	st;
> +			dev_t		dev;
> +
> +			name = iter->d_name;
> +
> +			if (!strcmp(name, ".") || !strcmp(name, "..") ||
> +			    !strstr(name, "ubi"))
> +				continue;
> +			if (!strcmp(name, "ubi_ctrl"))
> +				continue;
> +			device = malloc(strlen(*dirname) + strlen(name) + 2);
> +			if (!device)
> +				break ;
> +			sprintf(device, "%s/%s", *dirname, name);
> +			if (stat(device, &st))
             
 leak:          free(device);

> +				break ;

 I hope one day we will convert whole libblkid to use openat(),
 statat(), ... functions to avoid malloc() and sprintf() for paths :-)

> +			if (!(st.st_rdev & 0xFF)) { // It's an UBI Device
> +				free(device);
> +				continue ;
> +			}
> +			dev = st.st_rdev;
> +			DBG(DEBUG_DEVNAME, printf("UBI vol %s: devno 0x%04X\n",
> +						  device,
> +						  (int) dev));
> +			probe_one(cache, name, dev, BLKID_PRI_UBI,
> +				  only_if_new);
> +			free(device);
> +		}
> +		closedir(dir);
> +	}
> +}

    Karel
Artem Bityutskiy - Sept. 2, 2009, 10:10 a.m.
On Wed, 2009-09-02 at 11:24 +0200, Karel Zak wrote:
> Hi Corentin,
> 
> On Mon, Aug 24, 2009 at 01:11:54PM +0200, Corentin Chary wrote:
> > Probe UBI volume under /dev (or /devfs, /devices).
> 
>  that's not elegant. Does it mean that UBI volumes are not in
>  /proc/partitions?

Yeah, the thing is that neither UBI volumes nor MTD devices are in
/proc/partitions, just because they are not block devices and they
are not members of the blkock device infrastructure. They form a
completely separate subsystem.

Here are some more data:
http://www.linux-mtd.infradead.org/faq/general.html#L_mtd_what
http://www.linux-mtd.infradead.org/doc/ubifs.html#L_raw_vs_ftl

There are already 3 FSes which work with MTD: JFFS2, UBIFS and
(out-of-tree)YAFFS2.
Corentin Chary - Sept. 18, 2009, 10:52 a.m.
On Wednesday 02 September 2009 11:24:02 Karel Zak wrote:
>  Hi Corentin,
> 
> On Mon, Aug 24, 2009 at 01:11:54PM +0200, Corentin Chary wrote:
> > Probe UBI volume under /dev (or /devfs, /devices).
> 
>  that's not elegant. Does it mean that UBI volumes are not in
>  /proc/partitions?
> 
>  Ted, any comment?

Any news for this patch ?
Thanks,
Theodore Ts'o - Sept. 19, 2009, 1:23 a.m.
On Fri, Sep 18, 2009 at 12:52:09PM +0200, Corentin Chary wrote:
> On Wednesday 02 September 2009 11:24:02 Karel Zak wrote:
> >  Hi Corentin,
> > 
> > On Mon, Aug 24, 2009 at 01:11:54PM +0200, Corentin Chary wrote:
> > > Probe UBI volume under /dev (or /devfs, /devices).
> > 
> >  that's not elegant. Does it mean that UBI volumes are not in
> >  /proc/partitions?
> > 
> >  Ted, any comment?
> 
> Any news for this patch ?
> Thanks,

Sorry, I've been travelling and it's the merge window and I have
slides for a Usenix tutorial due soon, so my userspace-related pages
have been swapped out.  And next week is LinuxCon/Plumber's...  

I'll try to look at this either next week if I can find some airplane
hacking time or when I get back from Plumber's Conference.

	     	       	   	     	       - Ted
Karel Zak - Sept. 24, 2009, 2:14 p.m.
On Mon, Aug 24, 2009 at 01:11:54PM +0200, Corentin Chary wrote:
> --- a/shlibs/blkid/src/devname.c
> +static void
> +ubi_probe_all(blkid_cache cache, int only_if_new)
> +{
> +	const char **dirname;
> +
> +	for (dirname = dirlist; *dirname; dirname++) {
> +		DBG(DEBUG_DEVNAME, printf("probing UBI volumes under %s\n",
> +					  *dirname));
> +
> +		DIR		*dir;
> +		struct dirent	*iter;
> +
> +		dir = opendir(*dirname);
> +		if (dir == NULL)
> +			continue ;
> +
> +		while ((iter = readdir(dir)) != NULL) {
> +			char		*name, *device;
> +			struct stat	st;
> +			dev_t		dev;
> +
> +			name = iter->d_name;
> +
> +			if (!strcmp(name, ".") || !strcmp(name, "..") ||
> +			    !strstr(name, "ubi"))
> +				continue;
> +			if (!strcmp(name, "ubi_ctrl"))
> +				continue;
> +			device = malloc(strlen(*dirname) + strlen(name) + 2);
> +			if (!device)
> +				break ;
> +			sprintf(device, "%s/%s", *dirname, name);
> +			if (stat(device, &st))
> +				break ;
> +
> +			if (!(st.st_rdev & 0xFF)) { // It's an UBI Device
> +				free(device);
> +				continue ;
> +			}

Wouldn't be better to 

  #define is_ubi_device(s)    (S_ISCHR(s->st_mode) && (s->st_rdev & 0xFF))

(or major() instead magic 0xFF constant?) and use it everywhere in
code? I guess UBI is always a char device.

> +++ b/shlibs/blkid/src/probe.c
> @@ -284,6 +284,8 @@ int blkid_probe_set_device(blkid_probe pr, int fd,
>  
>  		if (S_ISBLK(sb.st_mode))
>  			blkdev_get_size(fd, (unsigned long long *) &pr->size);
> +		else if (S_ISCHR(sb.st_mode))
> +			pr->size = 1;
>  		else
>  			pr->size = sb.st_size;
>  	}

this is the same situation, this code will for all char devices, but we
want to support UBI only.

    Karel
Corentin Chary - Sept. 24, 2009, 9:57 p.m.
On Thu, Sep 24, 2009 at 4:14 PM, Karel Zak <kzak@redhat.com> wrote:
> On Mon, Aug 24, 2009 at 01:11:54PM +0200, Corentin Chary wrote:
>> --- a/shlibs/blkid/src/devname.c
>> +static void
>> +ubi_probe_all(blkid_cache cache, int only_if_new)
>> +{
>> +     const char **dirname;
>> +
>> +     for (dirname = dirlist; *dirname; dirname++) {
>> +             DBG(DEBUG_DEVNAME, printf("probing UBI volumes under %s\n",
>> +                                       *dirname));
>> +
>> +             DIR             *dir;
>> +             struct dirent   *iter;
>> +
>> +             dir = opendir(*dirname);
>> +             if (dir == NULL)
>> +                     continue ;
>> +
>> +             while ((iter = readdir(dir)) != NULL) {
>> +                     char            *name, *device;
>> +                     struct stat     st;
>> +                     dev_t           dev;
>> +
>> +                     name = iter->d_name;
>> +
>> +                     if (!strcmp(name, ".") || !strcmp(name, "..") ||
>> +                         !strstr(name, "ubi"))
>> +                             continue;
>> +                     if (!strcmp(name, "ubi_ctrl"))
>> +                             continue;
>> +                     device = malloc(strlen(*dirname) + strlen(name) + 2);
>> +                     if (!device)
>> +                             break ;
>> +                     sprintf(device, "%s/%s", *dirname, name);
>> +                     if (stat(device, &st))
>> +                             break ;
>> +
>> +                     if (!(st.st_rdev & 0xFF)) { // It's an UBI Device
>> +                             free(device);
>> +                             continue ;
>> +                     }
>
> Wouldn't be better to
>
>  #define is_ubi_device(s)    (S_ISCHR(s->st_mode) && (s->st_rdev & 0xFF))
>
> (or major() instead magic 0xFF constant?) and use it everywhere in
> code? I guess UBI is always a char device.

Maybe something like that:
#define is_ubi_device(s)    (S_ISCHR(s->st_mode) && !minor(s->st_rdev))

>> +++ b/shlibs/blkid/src/probe.c
>> @@ -284,6 +284,8 @@ int blkid_probe_set_device(blkid_probe pr, int fd,
>>
>>               if (S_ISBLK(sb.st_mode))
>>                       blkdev_get_size(fd, (unsigned long long *) &pr->size);
>> +             else if (S_ISCHR(sb.st_mode))
>> +                     pr->size = 1;
>>               else
>>                       pr->size = sb.st_size;
>>       }
>
> this is the same situation, this code will for all char devices, but we
> want to support UBI only.

I don't know if there is a "good" way to detect an ubi device using
only struct stat.

Artem, any idea ?
Artem Bityutskiy - Sept. 25, 2009, 7:19 a.m.
On Thu, 2009-09-24 at 23:57 +0200, Corentin Chary wrote:
> >> +                     if (!(st.st_rdev & 0xFF)) { // It's an UBI Device
> >> +                             free(device);
> >> +                             continue ;
> >> +                     }
> >
> > Wouldn't be better to
> >
> >  #define is_ubi_device(s)    (S_ISCHR(s->st_mode) && (s->st_rdev & 0xFF))
> >
> > (or major() instead magic 0xFF constant?) and use it everywhere in
> > code? I guess UBI is always a char device.
> 
> Maybe something like that:
> #define is_ubi_device(s)    (S_ISCHR(s->st_mode) && !minor(s->st_rdev))

Well, if you want to be really nice, you may implement this as a
function which scans sysfs or looks at /proc/devices and makes sure the
device really belongs to UBI. See below.

> 
> >> +++ b/shlibs/blkid/src/probe.c
> >> @@ -284,6 +284,8 @@ int blkid_probe_set_device(blkid_probe pr, int fd,
> >>
> >>               if (S_ISBLK(sb.st_mode))
> >>                       blkdev_get_size(fd, (unsigned long long *) &pr->size);
> >> +             else if (S_ISCHR(sb.st_mode))
> >> +                     pr->size = 1;
> >>               else
> >>                       pr->size = sb.st_size;
> >>       }
> >
> > this is the same situation, this code will for all char devices, but we
> > want to support UBI only.
> 
> I don't know if there is a "good" way to detect an ubi device using
> only struct stat.
> 
> Artem, any idea ?

UBI char devices' major:minor numbers are allocated dynamically, so they
cannot be used.

To check whether this char. device node is an UBI volume and not
something else, you should scan all /sys/class/ubi/ubiX/Y/dev files, and
see if there is a device with the same major:minor.

The other possibility would be to look at /proc/devices, and make sure
there is an ubiX device with the same major number as your /dev/ubiX_Y
device.
Karel Zak - Sept. 25, 2009, 7:56 a.m.
On Fri, Sep 25, 2009 at 10:19:19AM +0300, Artem Bityutskiy wrote:
> To check whether this char. device node is an UBI volume and not
> something else, you should scan all /sys/class/ubi/ubiX/Y/dev files, and
> see if there is a device with the same major:minor.
> 
> The other possibility would be to look at /proc/devices, and make sure
> there is an ubiX device with the same major number as your /dev/ubiX_Y
> device.

 Yes, we have such function (blkid_driver_has_major()), but I think
 it's overkill in this context. It seems that check for the "ubi" 
 device name in ubi_probe_all() is enough.

 Thanks.

    Karel

Patch

diff --git a/shlibs/blkid/src/blkidP.h b/shlibs/blkid/src/blkidP.h
index e0e5cb8..8db6599 100644
--- a/shlibs/blkid/src/blkidP.h
+++ b/shlibs/blkid/src/blkidP.h
@@ -237,6 +237,7 @@  extern char *blkid_strndup(const char *s, const int length);
 /*
  * Priority settings for different types of devices
  */
+#define BLKID_PRI_UBI	50
 #define BLKID_PRI_DM	40
 #define BLKID_PRI_EVMS	30
 #define BLKID_PRI_LVM	20
diff --git a/shlibs/blkid/src/devname.c b/shlibs/blkid/src/devname.c
index ef686f4..cac13c5 100644
--- a/shlibs/blkid/src/devname.c
+++ b/shlibs/blkid/src/devname.c
@@ -229,7 +229,9 @@  static void probe_one(blkid_cache cache, const char *ptname,
 		    dev->bid_devno == devno)
 			goto set_pri;
 
-		if (stat(device, &st) == 0 && S_ISBLK(st.st_mode) &&
+		if (stat(device, &st) == 0 &&
+		    (S_ISBLK(st.st_mode) ||
+		     (S_ISCHR(st.st_mode) && !strncmp(ptname, "ubi", 3))) &&
 		    st.st_rdev == devno) {
 			devname = blkid_strdup(device);
 			goto get_dev;
@@ -388,6 +390,57 @@  evms_probe_all(blkid_cache cache, int only_if_new)
 	return num;
 }
 
+static void
+ubi_probe_all(blkid_cache cache, int only_if_new)
+{
+	const char **dirname;
+
+	for (dirname = dirlist; *dirname; dirname++) {
+		DBG(DEBUG_DEVNAME, printf("probing UBI volumes under %s\n",
+					  *dirname));
+
+		DIR		*dir;
+		struct dirent	*iter;
+
+		dir = opendir(*dirname);
+		if (dir == NULL)
+			continue ;
+
+		while ((iter = readdir(dir)) != NULL) {
+			char		*name, *device;
+			struct stat	st;
+			dev_t		dev;
+
+			name = iter->d_name;
+
+			if (!strcmp(name, ".") || !strcmp(name, "..") ||
+			    !strstr(name, "ubi"))
+				continue;
+			if (!strcmp(name, "ubi_ctrl"))
+				continue;
+			device = malloc(strlen(*dirname) + strlen(name) + 2);
+			if (!device)
+				break ;
+			sprintf(device, "%s/%s", *dirname, name);
+			if (stat(device, &st))
+				break ;
+
+			if (!(st.st_rdev & 0xFF)) { // It's an UBI Device
+				free(device);
+				continue ;
+			}
+			dev = st.st_rdev;
+			DBG(DEBUG_DEVNAME, printf("UBI vol %s: devno 0x%04X\n",
+						  device,
+						  (int) dev));
+			probe_one(cache, name, dev, BLKID_PRI_UBI,
+				  only_if_new);
+			free(device);
+		}
+		closedir(dir);
+	}
+}
+
 /*
  * Read the device data for all available block devices in the system.
  */
@@ -419,6 +472,7 @@  static int probe_all(blkid_cache cache, int only_if_new)
 #ifdef VG_DIR
 	lvm_probe_all(cache, only_if_new);
 #endif
+	ubi_probe_all(cache, only_if_new);
 
 	proc = fopen(PROC_PARTITIONS, "r");
 	if (!proc)
diff --git a/shlibs/blkid/src/probe.c b/shlibs/blkid/src/probe.c
index 10fcb00..fc293fc 100644
--- a/shlibs/blkid/src/probe.c
+++ b/shlibs/blkid/src/probe.c
@@ -284,6 +284,8 @@  int blkid_probe_set_device(blkid_probe pr, int fd,
 
 		if (S_ISBLK(sb.st_mode))
 			blkdev_get_size(fd, (unsigned long long *) &pr->size);
+		else if (S_ISCHR(sb.st_mode))
+			pr->size = 1;
 		else
 			pr->size = sb.st_size;
 	}