* Regression due to "Return correct error number in ubi_get_vol_info1" in mtd-utils 2.0.1+
@ 2018-06-07 20:35 Christian Lamparter
2018-06-10 5:24 ` David Oberhollenzer
0 siblings, 1 reply; 2+ messages in thread
From: Christian Lamparter @ 2018-06-07 20:35 UTC (permalink / raw)
To: linux-mtd; +Cc: david.oberhollenzer
Hello,
the commit "Return correct error number in ubi_get_vol_info1" [0]
causes an regression within the libubi of mtd-utils. Affected
by this is ubinfo, ubirmvol and ubirsvol (basically all tools that
use "ubi_get_vol_info1" and check for ENOENT error code).
steps to reproduce: have mtd-utils 2.0.1 or 2.0.2
0. make a bunch of ubi volumes in sequential order
# ubimkvol /dev/ubi0 -s 64KiB -N test1
# ubimkvol /dev/ubi0 -s 64KiB -N test2
# ubimkvol /dev/ubi0 -s 64KiB -N test3
# ubimkvol /dev/ubi0 -s 64KiB -N test4
..
(The idea is that the volume ID of test4 is bigger than
of test1).
1. delete the test1 volume
# ubirmvol /dev/ubi0 -N test1
(this makes a hole in the volume table)
2. try an affected tool (i.e. "ubirmvol /dev/ubi0 -N test4" )
|root@mr24:/# ubirmvol /dev/ubi0 -N test4
|ubirmvol: error!: cannot find UBI volume "test4"
| error 19 (No such device)
or "ubinfo -a"
| root@mr24:/# ubinfo -a
| UBI version: 1
| Count of UBI devices: 1
| UBI control device major/minor: 10:59
| Present UBI devices: ubi0
|
| ubi0
| Volumes count: 11
| Logical eraseblock size: 15872 bytes, 15.5 KiB
| Total amount of logical eraseblocks: 1952 (30982144 bytes, 29.5 MiB)
| Amount of available logical eraseblocks: 75 (1190400 bytes, 1.1 MiB)
| Maximum count of volumes 92
| Count of bad physical eraseblocks: 0
| Count of reserved physical eraseblocks: 40
| Current maximum erase counter value: 984
| Minimum input/output unit size: 512 bytes
| Character device major/minor: 251:0
| ubinfo: error!: libubi failed to probe volume 5 on ubi0
| error 19 (No such device)
| Present volumes: 0, 1, 2, 3, 4root@mr24:/#
The problem with this patch is, that it updated ubi_get_vol_info1() to
return a different errno code (ENODEV instead of ENOENT).
|- if (vol_get_major(lib, dev_num, vol_id, &info->major, &info->minor))
|+ if (vol_get_major(lib, dev_num, vol_id, &info->major, &info->minor)) {
|+ if (errno == ENOENT)
|+ errno = ENODEV;
| return -1;
|+ }
However, the patch didn't update any existing code. For example this
loop @ line 1327 in ubi_get_vol_info1_nm() [1]
|1327 for (i = dev_info.lowest_vol_id;
|1328 i <= dev_info.highest_vol_id; i++) {
|1329 err = ubi_get_vol_info1(desc, dev_num, i, info);
|1330 if (err == -1) {
|1331 if (errno == ENOENT)
|1332 continue;
|1333 return -1;
|1334 }
|1335
|1336 if (nlen == strlen(info->name) && !strcmp(name, info->name))
|1337 return 0;
|1338 }
will now fail with the aforementioned ENODEV error, instead of
skipping deleted entry.
the same is true for ubinfo [2]:
| 266 for (i = dev_info.lowest_vol_id;
| 267 i <= dev_info.highest_vol_id; i++) {
| 268 err = ubi_get_vol_info1(libubi, dev_info.dev_num, i, &vol_info);
| 269 if (err == -1) {
| 270 if (errno == ENOENT)
| 271 continue;
| 272
| 273 return sys_errmsg("libubi failed to probe volume %d on ubi%d",
| 274 i, dev_info.dev_num);
| 275 }
reverting the patch [0] would probably be the easiest
way to fix the issue, but I'm not sure if that's the
best course of action. If somebody has an idea or
different opinion, please: hit "reply all" ;-).
Regards,
Christian Lamparter
[0] <http://git.infradead.org/mtd-utils.git/commitdiff/dede98ffb706676309488d7cc660f569548d5930>
[1] <http://git.infradead.org/mtd-utils.git/blob/bc63d36e39f389c8c17f6a8e9db47f2acc884659:/lib/libubi.c#l1327>
[2] <http://git.infradead.org/mtd-utils.git/blob/HEAD:/ubi-utils/ubinfo.c#l268>
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: Regression due to "Return correct error number in ubi_get_vol_info1" in mtd-utils 2.0.1+
2018-06-07 20:35 Regression due to "Return correct error number in ubi_get_vol_info1" in mtd-utils 2.0.1+ Christian Lamparter
@ 2018-06-10 5:24 ` David Oberhollenzer
0 siblings, 0 replies; 2+ messages in thread
From: David Oberhollenzer @ 2018-06-10 5:24 UTC (permalink / raw)
To: Christian Lamparter, linux-mtd
Hi,
the commit you mentioned was part of a series that tried to fix some odd errors
encountered when using mtd/ubi utilities if the MTD subsystem wasn't present,
or MTD was present but UBI missing. When investigating the issue last year,
it was discovered that the documented behavior does not match what the function
actually does. Unfortunately it was overlooked that some other parts of the code
actually rely on the behavior as implemented.
Some odd error messages if UBI isn't present are probably the lesser issue,
so I reverted the commit for now.
Thanks,
David
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2018-06-10 5:25 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-06-07 20:35 Regression due to "Return correct error number in ubi_get_vol_info1" in mtd-utils 2.0.1+ Christian Lamparter
2018-06-10 5:24 ` David Oberhollenzer
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.