[PATCH] liblkid: fix probe_nilfs2 I/O error backup

[PATCH] liblkid: fix probe_nilfs2 I/O error backup

[Veeraiyan Chidambaram (RBEI/ECF32)]

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

Hello  All,

In our system recently updated libblkid-2.24 to libblkid-2.26.
After updating  of libblkid, blkid -p -u noraid /dev/sr0  stopped 
working for my Mixed mode CD (audio + data) and doesnt give any output.

libblkid run's probe function to find      
When I  investigated further found probe of nilfs file system is failing with I/O error
(before nilfs file system probe iso9660 probe was successful) .

The nilfs probe function does the following
1. reads for primary partition 
2. read backup partition
3. checks the nilfs related validity checks
   if validate fails returns 1 meaning CD file system is 
   not an nilfs file system.
4. if validation is successful then copy the necessary information and 
   return 0 meaning CD is nilfs file system.

In my case
1. Read of primary portion is successful.
2. Read Backup partition is falls with I/O error.
3. retuns errno -5 

 
here i see a following difference b/w libblkid-2.24 and libblkid-2.26
1. In version 2.24  if backup partition fails they doesn't treat as error
    but in newer version treated as error.
2. On the whole error handling was changed in libblkid-2.26 ,
   if any file probe return errno, then successful found file system
   all so discarded , in my case even through iso9660 probe found
   CD was a iso9660 file system due nilfs probe blkid return errno 
  the successful is9660 results also ignored by liblkid.


I would like to propose the following fix 


In nilfs file system probe
1. read primary partition 
2. validate the primary partition if fails return 1 i.e no file system found for nilfs file system in the CD.
3. if backup partition if validate.
4. then validate backup partition and then copy the necessary information and 
   return 0 meaning CD is nilfs file system.



-- 
Thanks &Regards 


veeraiyan chidambaram 

[-- Attachment #2: 0001-liblkid-fix-probe_nilfs2-I-O-error-backup.patch --]
[-- Type: application/octet-stream, Size: 1766 bytes --]

From da5e9ddb67bb020295af339b8b475c9edd450a61 Mon Sep 17 00:00:00 2001
From: Veeraiyan Chidambaram <veeraiyan.chidambaram@in.bosch.com>
Date: Mon, 11 Jul 2016 13:02:10 +0530
Subject: [PATCH] liblkid: fix probe_nilfs2 I/O error backup

 In mixed mode CD iso9660 file system type, reading backup
 ends-up in I/O error and finally ends up in error in finding
 file system. To avoid this validate primary before reading backup


Signed-off-by: Veeraiyan Chidambaram <veeraiyan.chidambaram@in.bosch.com>
---
 libblkid/src/superblocks/nilfs.c |   10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/libblkid/src/superblocks/nilfs.c b/libblkid/src/superblocks/nilfs.c
index cd93d7b..a635566 100644
--- a/libblkid/src/superblocks/nilfs.c
+++ b/libblkid/src/superblocks/nilfs.c
@@ -108,7 +108,14 @@ static int probe_nilfs2(blkid_probe pr,
 			pr, NILFS_SB_OFFSET, sizeof(struct nilfs_super_block));
 	if (!sbp)
 		return errno ? -errno : 1;
+	/* In mixed mode CD iso9660 file system type, reading backup ends-up in I/O
+	 * error and finally ends up in error in finding file system. To avoid this
+	 * validate primary before reading backup
+	 */
 
+	valid[0] = nilfs_valid_sb(pr, sbp, 0);
+	if(!valid[0])
+		return 1;
 	/* backup */
 	sbb = (struct nilfs_super_block *) blkid_probe_get_buffer(
 			pr, NILFS_SBB_OFFSET(pr->size), sizeof(struct nilfs_super_block));
@@ -119,9 +126,8 @@ static int probe_nilfs2(blkid_probe pr,
 	 * Compare two super blocks and set 1 in swp if the secondary
 	 * super block is valid and newer.  Otherwise, set 0 in swp.
 	 */
-	valid[0] = nilfs_valid_sb(pr, sbp, 0);
 	valid[1] = nilfs_valid_sb(pr, sbb, 1);
-	if (!valid[0] && !valid[1])
+	if (!valid[1])
 		return 1;
 
 	swp = valid[1] && (!valid[0] ||
-- 
1.7.9.5

Re: [PATCH] liblkid: fix probe_nilfs2 I/O error backup

[Karel Zak]

On Mon, Jul 11, 2016 at 07:40:59AM +0000, Veeraiyan Chidambaram (RBEI/ECF32) wrote:
> Hello  All,
> 
> In our system recently updated libblkid-2.24 to libblkid-2.26.
> After updating  of libblkid, blkid -p -u noraid /dev/sr0  stopped 
> working for my Mixed mode CD (audio + data) and doesnt give any output.
> 
> libblkid run's probe function to find      
> When I  investigated further found probe of nilfs file system is failing with I/O error
> (before nilfs file system probe iso9660 probe was successful) .
> 
> The nilfs probe function does the following
> 1. reads for primary partition 
> 2. read backup partition
> 3. checks the nilfs related validity checks
>    if validate fails returns 1 meaning CD file system is 
>    not an nilfs file system.
> 4. if validation is successful then copy the necessary information and 
>    return 0 meaning CD is nilfs file system.
> 
> In my case
> 1. Read of primary portion is successful.
> 2. Read Backup partition is falls with I/O error.
> 3. retuns errno -5 

Would be enough to ignore the backup block errors if the primary block
is valid?

> I would like to propose the following fix 
> 
> 
> In nilfs file system probe
> 1. read primary partition 
> 2. validate the primary partition if fails return 1 i.e no file system found for nilfs file system in the CD.
> 3. if backup partition if validate.
> 4. then validate backup partition and then copy the necessary information and 
>    return 0 meaning CD is nilfs file system.

Your patch completely ignores backup block if the primary is not
valid. I think it's not expected behavior.

Please, try the patch below.

    Karel


diff --git a/libblkid/src/superblocks/nilfs.c b/libblkid/src/superblocks/nilfs.c
index cd93d7b..ab0f74c 100644
--- a/libblkid/src/superblocks/nilfs.c
+++ b/libblkid/src/superblocks/nilfs.c
@@ -109,18 +109,29 @@ static int probe_nilfs2(blkid_probe pr,
 	if (!sbp)
 		return errno ? -errno : 1;
 
+	valid[0] = nilfs_valid_sb(pr, sbp, 0);
+
+
 	/* backup */
 	sbb = (struct nilfs_super_block *) blkid_probe_get_buffer(
 			pr, NILFS_SBB_OFFSET(pr->size), sizeof(struct nilfs_super_block));
-	if (!sbb)
-		return errno ? -errno : 1;
+	if (!sbb) {
+		valid[1] = 0;
+
+		/* If the primary block is valid then continue and ignore also
+		 * I/O errors for backup block. Note the this is probably CD
+		 * where I/O errors and the end of the disk/session are "normal".
+		 */
+		if (!valid[0])
+			return errno ? -errno : 1;
+	} else
+		valid[1] = nilfs_valid_sb(pr, sbb, 1);
+
 
 	/*
 	 * Compare two super blocks and set 1 in swp if the secondary
 	 * super block is valid and newer.  Otherwise, set 0 in swp.
 	 */
-	valid[0] = nilfs_valid_sb(pr, sbp, 0);
-	valid[1] = nilfs_valid_sb(pr, sbb, 1);
 	if (!valid[0] && !valid[1])
 		return 1;
 

Re: [PATCH] liblkid: fix probe_nilfs2 I/O error backup

[Karel Zak]

On Fri, Jul 15, 2016 at 10:39:20AM +0200, Karel Zak wrote:
> Would be enough to ignore the backup block errors if the primary block
> is valid?
... 
> Please, try the patch below.

Applied.

    Karel

-- 
 Karel Zak  <kzak@redhat.com>
 http://karelzak.blogspot.com

RE: [PATCH] liblkid: fix probe_nilfs2 I/O error backup

[Veeraiyan Chidambaram (RBEI/ECF32)]

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

Hello All 

Please find the attached patch .

Best regards

Veeraiyan Chidambaram
RBEI/ECF 

Tel. +91(422)676-4495



-----Original Message-----
From: Veeraiyan Chidambaram (RBEI/ECF32)
Sent: Monday, July 11, 2016 1:11 PM
To: 'util-linux@vger.kernel.org' <util-linux@vger.kernel.org>; Veeraiyan Chidambaram (RBEI/ECF32) <Veeraiyan.Chidambaram@in.bosch.com>
Subject: [PATCH] liblkid: fix probe_nilfs2 I/O error backup

Hello  All,

In our system recently updated libblkid-2.24 to libblkid-2.26.
After updating  of libblkid, blkid -p -u noraid /dev/sr0  stopped
working for my Mixed mode CD (audio + data) and doesnt give any output.

libblkid run's probe function to find     
When I  investigated further found probe of nilfs file system is failing with I/O error
(before nilfs file system probe iso9660 probe was successful) .

The nilfs probe function does the following
1. reads for primary partition
2. read backup partition
3. checks the nilfs related validity checks
   if validate fails returns 1 meaning CD file system is
   not an nilfs file system.
4. if validation is successful then copy the necessary information and
   return 0 meaning CD is nilfs file system.

In my case
1. Read of primary portion is successful.
2. Read Backup partition is falls with I/O error.
3. retuns errno -5

 
here i see a following difference b/w libblkid-2.24 and libblkid-2.26
1. In version 2.24  if backup partition fails they doesn't treat as error
    but in newer version treated as error.
2. On the whole error handling was changed in libblkid-2.26 ,
   if any file probe return errno, then successful found file system
   all so discarded , in my case even through iso9660 probe found
   CD was a iso9660 file system due nilfs probe blkid return errno
  the successful is9660 results also ignored by liblkid.


I would like to propose the following fix


In nilfs file system probe
1. read primary partition
2. validate the primary partition if fails return 1 i.e no file system found for nilfs file system in the CD.
3. if backup partition if validate.
4. then validate backup partition and then copy the necessary information and
   return 0 meaning CD is nilfs file system.



--
Thanks &Regards


veeraiyan chidambaram

[-- Attachment #2: 0001-liblkid-fix-probe_nilfs2-I-O-error-backup.patch --]
[-- Type: application/octet-stream, Size: 1766 bytes --]

From da5e9ddb67bb020295af339b8b475c9edd450a61 Mon Sep 17 00:00:00 2001
From: Veeraiyan Chidambaram <veeraiyan.chidambaram@in.bosch.com>
Date: Mon, 11 Jul 2016 13:02:10 +0530
Subject: [PATCH] liblkid: fix probe_nilfs2 I/O error backup

 In mixed mode CD iso9660 file system type, reading backup
 ends-up in I/O error and finally ends up in error in finding
 file system. To avoid this validate primary before reading backup


Signed-off-by: Veeraiyan Chidambaram <veeraiyan.chidambaram@in.bosch.com>
---
 libblkid/src/superblocks/nilfs.c |   10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/libblkid/src/superblocks/nilfs.c b/libblkid/src/superblocks/nilfs.c
index cd93d7b..a635566 100644
--- a/libblkid/src/superblocks/nilfs.c
+++ b/libblkid/src/superblocks/nilfs.c
@@ -108,7 +108,14 @@ static int probe_nilfs2(blkid_probe pr,
 			pr, NILFS_SB_OFFSET, sizeof(struct nilfs_super_block));
 	if (!sbp)
 		return errno ? -errno : 1;
+	/* In mixed mode CD iso9660 file system type, reading backup ends-up in I/O
+	 * error and finally ends up in error in finding file system. To avoid this
+	 * validate primary before reading backup
+	 */
 
+	valid[0] = nilfs_valid_sb(pr, sbp, 0);
+	if(!valid[0])
+		return 1;
 	/* backup */
 	sbb = (struct nilfs_super_block *) blkid_probe_get_buffer(
 			pr, NILFS_SBB_OFFSET(pr->size), sizeof(struct nilfs_super_block));
@@ -119,9 +126,8 @@ static int probe_nilfs2(blkid_probe pr,
 	 * Compare two super blocks and set 1 in swp if the secondary
 	 * super block is valid and newer.  Otherwise, set 0 in swp.
 	 */
-	valid[0] = nilfs_valid_sb(pr, sbp, 0);
 	valid[1] = nilfs_valid_sb(pr, sbb, 1);
-	if (!valid[0] && !valid[1])
+	if (!valid[1])
 		return 1;
 
 	swp = valid[1] && (!valid[0] ||
-- 
1.7.9.5