[PATCH 1/2] libmtd: add `mtd_dev_present()' library function

[PATCH 1/2] libmtd: add `mtd_dev_present()' library function

[Brian Norris]

Will be used for `mtdinfo --all'

Signed-off-by: Brian Norris <computersforpeace@gmail.com>
---
 include/libmtd.h |    9 +++++++++
 lib/libmtd.c     |   22 ++++++++++++----------
 2 files changed, 21 insertions(+), 10 deletions(-)

diff --git a/include/libmtd.h b/include/libmtd.h
index 07c304a..a78c8cb 100644
--- a/include/libmtd.h
+++ b/include/libmtd.h
@@ -106,6 +106,15 @@ libmtd_t libmtd_open(void);
 void libmtd_close(libmtd_t desc);
 
 /**
+ * mtd_dev_present - check whether a MTD device is present.
+ * @desc: MTD library descriptor
+ * @mtd_num: MTD device number to check
+ *
+ * This function returns %1 if MTD device is present and %0 if not.
+ */
+int mtd_dev_present(libmtd_t desc, int mtd_num);
+
+/**
  * mtd_get_info - get general MTD information.
  * @desc: MTD library descriptor
  * @info: the MTD device information is returned here
diff --git a/lib/libmtd.c b/lib/libmtd.c
index c6e5b20..9786e83 100644
--- a/lib/libmtd.c
+++ b/lib/libmtd.c
@@ -713,10 +713,18 @@ out_close:
 	return -1;
 }
 
+int mtd_dev_present(libmtd_t desc, int mtd_num) {
+	struct stat st;
+	struct libmtd *lib = (struct libmtd *)desc;
+	char file[strlen(lib->mtd) + 10];
+
+	sprintf(file, lib->mtd, mtd_num);
+	return !stat(file, &st);
+}
+
 int mtd_get_dev_info1(libmtd_t desc, int mtd_num, struct mtd_dev_info *mtd)
 {
 	int ret;
-	struct stat st;
 	struct libmtd *lib = (struct libmtd *)desc;
 
 	memset(mtd, 0, sizeof(struct mtd_dev_info));
@@ -724,15 +732,9 @@ int mtd_get_dev_info1(libmtd_t desc, int mtd_num, struct mtd_dev_info *mtd)
 
 	if (!lib->sysfs_supported)
 		return legacy_get_dev_info1(mtd_num, mtd);
-	else {
-		char file[strlen(lib->mtd) + 10];
-
-		sprintf(file, lib->mtd, mtd_num);
-		if (stat(file, &st)) {
-			if (errno == ENOENT)
-				errno = ENODEV;
-			return -1;
-		}
+	else if (!mtd_dev_present(desc, mtd_num)) {
+		errno = ENODEV;
+		return -1;
 	}
 
 	if (dev_get_major(lib, mtd_num, &mtd->major, &mtd->minor))
-- 
1.7.5.4

[PATCH 2/2] mtdinfo: fix `--all' for non-consecutive device numbers

[Brian Norris]

When we have assigned non-consecutive device numbers to our MTD devices,
then we run `mtdinfo --all', we get errors once mtdinfo tries to process
the devices in the "hole". For instance, suppose that at boot time, we have
one MTD (/dev/mtd0) then perform a sequence like the following:

 # modprobe mtdram
 # modprobe nandsim
 # rmmod mtdram

Then at this point, we have should have devices 0 and 2 without 1. Then:

 # mtdinfo --all
 ...
 mtdinfo: error!: mtd1 does not correspond to any existing MTD device

We add a check to first see if device is present, then continue to the next
ID if it doesn't exist.

Reported-by: Brian Foster <brian.foster@maxim-ic.com>
Signed-off-by: Brian Norris <computersforpeace@gmail.com>
---
 ubi-utils/mtdinfo.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/ubi-utils/mtdinfo.c b/ubi-utils/mtdinfo.c
index ead4bce..d25595a 100644
--- a/ubi-utils/mtdinfo.c
+++ b/ubi-utils/mtdinfo.c
@@ -378,6 +378,8 @@ static int print_general_info(libmtd_t libmtd, const struct mtd_info *mtd_info,
 
 	for (i = mtd_info->lowest_mtd_num;
 	     i <= mtd_info->highest_mtd_num; i++) {
+		if (!mtd_dev_present(libmtd, i))
+			continue;
 		err = print_dev_info(libmtd, mtd_info, i);
 		if (err)
 			return err;
-- 
1.7.5.4

Re: [PATCH 2/2] mtdinfo: fix `--all' for non-consecutive device numbers

[Brian Norris]

Hmm, didn't CC Brian Foster. Must have mixed up my headers...

On Fri, Jan 27, 2012 at 10:30 AM, Brian Norris
<computersforpeace@gmail.com> wrote:
> When we have assigned non-consecutive device numbers to our MTD devices,
> then we run `mtdinfo --all', we get errors once mtdinfo tries to process
> the devices in the "hole". For instance, suppose that at boot time, we have
> one MTD (/dev/mtd0) then perform a sequence like the following:
>
>  # modprobe mtdram
>  # modprobe nandsim
>  # rmmod mtdram
>
> Then at this point, we have should have devices 0 and 2 without 1. Then:
>
>  # mtdinfo --all
>  ...
>  mtdinfo: error!: mtd1 does not correspond to any existing MTD device
>
> We add a check to first see if device is present, then continue to the next
> ID if it doesn't exist.
>
> Reported-by: Brian Foster <brian.foster@maxim-ic.com>
> Signed-off-by: Brian Norris <computersforpeace@gmail.com>
> ---
>  ubi-utils/mtdinfo.c |    2 ++
>  1 files changed, 2 insertions(+), 0 deletions(-)
>
> diff --git a/ubi-utils/mtdinfo.c b/ubi-utils/mtdinfo.c
> index ead4bce..d25595a 100644
> --- a/ubi-utils/mtdinfo.c
> +++ b/ubi-utils/mtdinfo.c
> @@ -378,6 +378,8 @@ static int print_general_info(libmtd_t libmtd, const struct mtd_info *mtd_info,
>
>        for (i = mtd_info->lowest_mtd_num;
>             i <= mtd_info->highest_mtd_num; i++) {
> +               if (!mtd_dev_present(libmtd, i))
> +                       continue;
>                err = print_dev_info(libmtd, mtd_info, i);
>                if (err)
>                        return err;
> --
> 1.7.5.4
>

Re: [PATCH 1/2] libmtd: add `mtd_dev_present()' library function

[Artem Bityutskiy]

[-- Attachment #1: Type: text/plain, Size: 1197 bytes --]

On Fri, 2012-01-27 at 10:30 -0800, Brian Norris wrote:

> +int mtd_dev_present(libmtd_t desc, int mtd_num) {
> +	struct stat st;
> +	struct libmtd *lib = (struct libmtd *)desc;
> +	char file[strlen(lib->mtd) + 10];
> +
> +	sprintf(file, lib->mtd, mtd_num);
> +	return !stat(file, &st);
> +}

Thanks! However...

This will only work for relatively newer kernels where MTD has sysfs
support (2.6.30+). Older kernels have no MTD sysfs support and the sysfs
file you are stat()'ing won't exist, so this function will always return
an error.

I've added the following stub to 'mtd_dev_present()':

+       if (!lib->sysfs_supported)
+               /* TODO: add legacy_dev_present() function */
+               return 1;

And pushed your patches to mtd-utils.

This means old kernels won't be fixed, but at least they won't be broken
either. Do you have enough juice to add a 'legacy_def_present()'
function? Ideally, it should parse '/proc/mtd', but I think there are
functions already which just check for '/dev/mtdX' node, so you could
just stat it instead of the sysfs file, I guess. See
'legacy_get_dev_info1()' for example.

-- 
Best Regards,
Artem Bityutskiy

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 1/2] libmtd: add `mtd_dev_present()' library function

[Brian Foster]

On Thursday 02 February 2012 12:33:24 Artem Bityutskiy wrote:
> On Fri, 2012-01-27 at 10:30 -0800, Brian Norris wrote:
> > +int mtd_dev_present(libmtd_t desc, int mtd_num) {  [ ... ]
> 
> This will only work for relatively newer kernels where MTD has sysfs
> support (2.6.30+). Older kernels have no MTD sysfs support and the sysfs
> file you are stat()'ing won't exist, so this function will always return
> an error.

 This is a very plausible concern:  Our older system
 is mostly deployed with 2.6.26(or even older) kernel,
 albeit a 2.6.30 option exists.  It has multiple MTD
 devices, albeit due to the usage/configuration (and
 maybe the use of older mtd-utils?) I suspect the bug
 has never been observed.

 Our latest system uses 2.6.36, and is where the original
 UBI-related bug was observed.

cheers!
	-blf-

-- 
Brian Foster
Principal MTS, Software        |  La Ciotat, France
Maxim Integrated Products      |  Web:  http://www.maxim-ic.com/

Re: [PATCH 1/2] libmtd: add `mtd_dev_present()' library function

[Artem Bityutskiy]

[-- Attachment #1: Type: text/plain, Size: 4272 bytes --]

On Thu, 2012-02-02 at 14:20 +0100, Brian Foster wrote:
> On Thursday 02 February 2012 12:33:24 Artem Bityutskiy wrote:
> > On Fri, 2012-01-27 at 10:30 -0800, Brian Norris wrote:
> > > +int mtd_dev_present(libmtd_t desc, int mtd_num) {  [ ... ]
> > 
> > This will only work for relatively newer kernels where MTD has sysfs
> > support (2.6.30+). Older kernels have no MTD sysfs support and the sysfs
> > file you are stat()'ing won't exist, so this function will always return
> > an error.
> 
>  This is a very plausible concern:  Our older system
>  is mostly deployed with 2.6.26(or even older) kernel,
>  albeit a 2.6.30 option exists.  It has multiple MTD
>  devices, albeit due to the usage/configuration (and
>  maybe the use of older mtd-utils?) I suspect the bug
>  has never been observed.
> 
>  Our latest system uses 2.6.36, and is where the original
>  UBI-related bug was observed.

Ok, I've just prepared the below patch. Compile-tested only. Anyone
volunteers to verify?

From d3e5e0a2aeb61dbd99c41c11e1c268510a4334c2 Mon Sep 17 00:00:00 2001
From: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
Date: Fri, 3 Feb 2012 09:15:31 +0200
Subject: [PATCH] limbtd: implement mtd_dev_present for old kernels

Implement the 'legacy_dev_present()' function which will check whether an MTD
device is present by scanning the /proc/mtd file when the MTD subsystem does
not support sysfs (the case for pre-2.6.30 kernels).

This patch also moves the 'mtd_dev_present()' function to a slightly more
logical position.

Signed-off-by: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
---
 lib/libmtd.c        |   25 ++++++++++++-------------
 lib/libmtd_int.h    |    1 +
 lib/libmtd_legacy.c |   25 +++++++++++++++++++++++++
 3 files changed, 38 insertions(+), 13 deletions(-)

diff --git a/lib/libmtd.c b/lib/libmtd.c
index 888d118..ecf182f 100644
--- a/lib/libmtd.c
+++ b/lib/libmtd.c
@@ -641,6 +641,18 @@ void libmtd_close(libmtd_t desc)
 	free(lib);
 }
 
+int mtd_dev_present(libmtd_t desc, int mtd_num) {
+	struct stat st;
+	struct libmtd *lib = (struct libmtd *)desc;
+	char file[strlen(lib->mtd) + 10];
+
+	if (!lib->sysfs_supported)
+		return legacy_dev_present(mtd_num);
+
+	sprintf(file, lib->mtd, mtd_num);
+	return !stat(file, &st);
+}
+
 int mtd_get_info(libmtd_t desc, struct mtd_info *info)
 {
 	DIR *sysfs_mtd;
@@ -713,19 +725,6 @@ out_close:
 	return -1;
 }
 
-int mtd_dev_present(libmtd_t desc, int mtd_num) {
-	struct stat st;
-	struct libmtd *lib = (struct libmtd *)desc;
-	char file[strlen(lib->mtd) + 10];
-
-	if (!lib->sysfs_supported)
-		/* TODO: add legacy_dev_present() function */
-		return 1;
-
-	sprintf(file, lib->mtd, mtd_num);
-	return !stat(file, &st);
-}
-
 int mtd_get_dev_info1(libmtd_t desc, int mtd_num, struct mtd_dev_info *mtd)
 {
 	int ret;
diff --git a/lib/libmtd_int.h b/lib/libmtd_int.h
index bb48d35..7913e67 100644
--- a/lib/libmtd_int.h
+++ b/lib/libmtd_int.h
@@ -95,6 +95,7 @@ struct libmtd
 };
 
 int legacy_libmtd_open(void);
+int legacy_dev_present(int mtd_num);
 int legacy_mtd_get_info(struct mtd_info *info);
 int legacy_get_dev_info(const char *node, struct mtd_dev_info *mtd);
 int legacy_get_dev_info1(int dev_num, struct mtd_dev_info *mtd);
diff --git a/lib/libmtd_legacy.c b/lib/libmtd_legacy.c
index d6c3938..d3f1672 100644
--- a/lib/libmtd_legacy.c
+++ b/lib/libmtd_legacy.c
@@ -170,6 +170,31 @@ int legacy_libmtd_open(void)
 }
 
 /**
+ * legacy_dev_presentl - legacy version of 'mtd_dev_present()'.
+ * @info: the MTD device information is returned here
+ *
+ * When the kernel does not provide sysfs files for the MTD subsystem,
+ * fall-back to parsing the /proc/mtd file to determine whether an mtd device
+ * number @mtd_num is present.
+ */
+int legacy_dev_present(int mtd_num)
+{
+	int ret;
+	struct proc_parse_info pi;
+
+	ret = proc_parse_start(&pi);
+	if (ret)
+		return -1;
+
+	while (proc_parse_next(&pi)) {
+		if (pi.mtd_num == mtd_num)
+			return 1;
+	}
+
+	return 0;
+}
+
+/**
  * legacy_mtd_get_info - legacy version of 'mtd_get_info()'.
  * @info: the MTD device information is returned here
  *
-- 
1.7.9



-- 
Best Regards,
Artem Bityutskiy


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 1/2] libmtd: add `mtd_dev_present()' library function

[Brian Norris]

On Fri, Feb 3, 2012 at 1:17 AM, Artem Bityutskiy <dedekind1@gmail.com> wrote:
> Ok, I've just prepared the below patch. Compile-tested only. Anyone
> volunteers to verify?

I've tested your patch and it's basically OK, but there are a few
issues with the rest of how libmtd/mtdinfo handles legacy stuff. I'm
sending a separate patch series on top of your patch that solves other
issues (partly fixing up my code and partly fixing up old bugs).

Brian