| Message ID | 1251112316-18971-2-git-send-email-corentincj@iksaif.net |
|---|---|
| State | New, archived |
| Headers | show |
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
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.
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,
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
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
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 ?
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.
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
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; }
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(-)