* [RFC] PATCH to fix rescan_partitions to return errors properly
@ 2006-10-04 1:00 Suzuki Kp
2006-10-04 13:09 ` Erik Mouw
0 siblings, 1 reply; 13+ messages in thread
From: Suzuki Kp @ 2006-10-04 1:00 UTC (permalink / raw)
To: lkml, linux-fsdevel; +Cc: Andrew Morton
[resending]
Hi,
Currently the rescan_partition returns 0 (success), even if it is unable
to rescan the partition information ( may be due to disks offline, I/O
error reading the table or unknown partition ). This would make ioctl()
calls succeed for BLKRRPART requests even if partitions were not scanned
properly, which is not a good thing to do.
Attached here is patch to fix the issue. The patch makes
rescan_partition to return -EINVAL for unknown partitions and -EIO for
disk I/O errors ( or when disks are offline ).
Comments ?
Thanks,
Suzuki K P <suzuki@in.ibm.com>
Linux Technology Centre,
IBM Systems & Technology Labs.
* Fix recscan_partitions to return error when the partition is unknown
or there is an I/O error reading the partition information.
Signed Off by : Suzuki K P <suzuki@in.ibm.com>
Index: linux-2.6.18/fs/partitions/check.c
===================================================================
--- linux-2.6.18.orig/fs/partitions/check.c 2006-09-26
04:41:55.000000000 +0530
+++ linux-2.6.18/fs/partitions/check.c 2006-09-26 05:09:29.000000000 +0530
@@ -177,7 +177,7 @@
else if (warn_no_part)
printk(" unable to read partition table\n");
kfree(state);
- return NULL;
+ return ERR_PTR(res);
}
/*
@@ -458,8 +458,13 @@
delete_partition(disk, p);
if (disk->fops->revalidate_disk)
disk->fops->revalidate_disk(disk);
- if (!get_capacity(disk) || !(state = check_partition(disk, bdev)))
- return 0;
+ if (!get_capacity(disk))
+ return -EINVAL;
+ state = check_partition(disk, bdev);
+ if (!state)
+ return -EINVAL;
+ if (IS_ERR(state))
+ return -EIO;
for (p = 1; p < state->limit; p++) {
sector_t size = state->parts[p].size;
sector_t from = state->parts[p].from;
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [RFC] PATCH to fix rescan_partitions to return errors properly 2006-10-04 1:00 [RFC] PATCH to fix rescan_partitions to return errors properly Suzuki Kp @ 2006-10-04 13:09 ` Erik Mouw 2006-10-04 16:50 ` Suzuki Kp 0 siblings, 1 reply; 13+ messages in thread From: Erik Mouw @ 2006-10-04 13:09 UTC (permalink / raw) To: Suzuki Kp; +Cc: lkml, linux-fsdevel, Andrew Morton On Tue, Oct 03, 2006 at 06:00:36PM -0700, Suzuki Kp wrote: > Currently the rescan_partition returns 0 (success), even if it is unable > to rescan the partition information ( may be due to disks offline, I/O > error reading the table or unknown partition ). This would make ioctl() > calls succeed for BLKRRPART requests even if partitions were not scanned > properly, which is not a good thing to do. > > Attached here is patch to fix the issue. The patch makes > rescan_partition to return -EINVAL for unknown partitions and -EIO for > disk I/O errors ( or when disks are offline ). I don't think it's a good idea to return an error when there's an unknown partition table. How do you differentiate between a device that isn't partitioned at all and a device with an unknown partition table? Better return 0 on an unknown partition table. Erik -- +-- Erik Mouw -- www.harddisk-recovery.com -- +31 70 370 12 90 -- | Lab address: Delftechpark 26, 2628 XH, Delft, The Netherlands ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [RFC] PATCH to fix rescan_partitions to return errors properly 2006-10-04 13:09 ` Erik Mouw @ 2006-10-04 16:50 ` Suzuki Kp 2006-10-04 17:08 ` Erik Mouw 0 siblings, 1 reply; 13+ messages in thread From: Suzuki Kp @ 2006-10-04 16:50 UTC (permalink / raw) To: Erik Mouw; +Cc: lkml, linux-fsdevel, Andrew Morton, andmike Erik Mouw wrote: > On Tue, Oct 03, 2006 at 06:00:36PM -0700, Suzuki Kp wrote: > >>Currently the rescan_partition returns 0 (success), even if it is unable >>to rescan the partition information ( may be due to disks offline, I/O >>error reading the table or unknown partition ). This would make ioctl() >>calls succeed for BLKRRPART requests even if partitions were not scanned >>properly, which is not a good thing to do. >> >>Attached here is patch to fix the issue. The patch makes >>rescan_partition to return -EINVAL for unknown partitions and -EIO for >>disk I/O errors ( or when disks are offline ). > > > I don't think it's a good idea to return an error when there's an > unknown partition table. How do you differentiate between a device that > isn't partitioned at all and a device with an unknown partition table? Returning -EINVAL in both the above cases would inform the user that, the partition table was not read properly due to an invalid arguemnt (the disk) passed, which holds good for both the above cases. What do you think ? > Better return 0 on an unknown partition table. > > > Erik > Suzuki ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [RFC] PATCH to fix rescan_partitions to return errors properly 2006-10-04 16:50 ` Suzuki Kp @ 2006-10-04 17:08 ` Erik Mouw 2006-10-04 17:37 ` Suzuki Kp 0 siblings, 1 reply; 13+ messages in thread From: Erik Mouw @ 2006-10-04 17:08 UTC (permalink / raw) To: Suzuki Kp; +Cc: lkml, linux-fsdevel, Andrew Morton, andmike On Wed, Oct 04, 2006 at 09:50:51AM -0700, Suzuki Kp wrote: > Erik Mouw wrote: > >On Tue, Oct 03, 2006 at 06:00:36PM -0700, Suzuki Kp wrote: > > > >>Currently the rescan_partition returns 0 (success), even if it is unable > >>to rescan the partition information ( may be due to disks offline, I/O > >>error reading the table or unknown partition ). This would make ioctl() > >>calls succeed for BLKRRPART requests even if partitions were not scanned > >>properly, which is not a good thing to do. > >> > >>Attached here is patch to fix the issue. The patch makes > >>rescan_partition to return -EINVAL for unknown partitions and -EIO for > >>disk I/O errors ( or when disks are offline ). > > > >I don't think it's a good idea to return an error when there's an > >unknown partition table. How do you differentiate between a device that > >isn't partitioned at all and a device with an unknown partition table? > > Returning -EINVAL in both the above cases would inform the user that, > the partition table was not read properly due to an invalid arguemnt > (the disk) passed, which holds good for both the above cases. > > What do you think ? I disagree. It's perfectly valid for a disk not to have a partition table (for example: components of a RAID5 MD device) and we shouldn't scare users about that. Also an unrecognised partition table format (DEC VMS, Novell Netware, etc.) is not a reason to throw an error, it's just unrecognised and as far as the kernel knows it's unpartioned. It's OK to tell the user that there was some kind of device error when scanning the partition table but IMHO an unpartitioned disk is not a device error. Erik -- +-- Erik Mouw -- www.harddisk-recovery.com -- +31 70 370 12 90 -- | Lab address: Delftechpark 26, 2628 XH, Delft, The Netherlands ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [RFC] PATCH to fix rescan_partitions to return errors properly 2006-10-04 17:08 ` Erik Mouw @ 2006-10-04 17:37 ` Suzuki Kp 2006-10-05 10:40 ` Erik Mouw 0 siblings, 1 reply; 13+ messages in thread From: Suzuki Kp @ 2006-10-04 17:37 UTC (permalink / raw) To: Erik Mouw; +Cc: lkml, linux-fsdevel, Andrew Morton, andmike Erik Mouw wrote: > On Wed, Oct 04, 2006 at 09:50:51AM -0700, Suzuki Kp wrote: > >>Erik Mouw wrote: >> >>>On Tue, Oct 03, 2006 at 06:00:36PM -0700, Suzuki Kp wrote: >>> >>> >>>>Currently the rescan_partition returns 0 (success), even if it is unable >>>>to rescan the partition information ( may be due to disks offline, I/O >>>>error reading the table or unknown partition ). This would make ioctl() >>>>calls succeed for BLKRRPART requests even if partitions were not scanned >>>>properly, which is not a good thing to do. >>>> >>>>Attached here is patch to fix the issue. The patch makes >>>>rescan_partition to return -EINVAL for unknown partitions and -EIO for >>>>disk I/O errors ( or when disks are offline ). >>> >>>I don't think it's a good idea to return an error when there's an >>>unknown partition table. How do you differentiate between a device that >>>isn't partitioned at all and a device with an unknown partition table? >> >>Returning -EINVAL in both the above cases would inform the user that, >>the partition table was not read properly due to an invalid arguemnt >>(the disk) passed, which holds good for both the above cases. >> >>What do you think ? > > > I disagree. It's perfectly valid for a disk not to have a partition > table (for example: components of a RAID5 MD device) and we shouldn't > scare users about that. Also an unrecognised partition table format > (DEC VMS, Novell Netware, etc.) is not a reason to throw an error, it's > just unrecognised and as far as the kernel knows it's unpartioned. Erik, Thank you very much for the clarification. But don't you think that, we should let know the user that he/she is trying to re-read a partition table from a device, which doesn't have it or is not useful for the kernel or to him, rather than making him happy with a success ? > > It's OK to tell the user that there was some kind of device error when > scanning the partition table but IMHO an unpartitioned disk is not a > device error. > > > Erik > Thanks, Suzuki ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [RFC] PATCH to fix rescan_partitions to return errors properly 2006-10-04 17:37 ` Suzuki Kp @ 2006-10-05 10:40 ` Erik Mouw 2006-10-05 20:32 ` [RFC] PATCH to fix rescan_partitions to return errors properly - take 2 Suzuki Kp 0 siblings, 1 reply; 13+ messages in thread From: Erik Mouw @ 2006-10-05 10:40 UTC (permalink / raw) To: Suzuki Kp; +Cc: lkml, linux-fsdevel, Andrew Morton, andmike On Wed, Oct 04, 2006 at 10:37:49AM -0700, Suzuki Kp wrote: > Erik Mouw wrote: > >I disagree. It's perfectly valid for a disk not to have a partition > >table (for example: components of a RAID5 MD device) and we shouldn't > >scare users about that. Also an unrecognised partition table format > >(DEC VMS, Novell Netware, etc.) is not a reason to throw an error, it's > >just unrecognised and as far as the kernel knows it's unpartioned. > > Thank you very much for the clarification. > > But don't you think that, we should let know the user that he/she is > trying to re-read a partition table from a device, which doesn't have it > or is not useful for the kernel or to him, rather than making him happy > with a success ? But we already do that. This is a part of the log from a 2.6.18 kernel: SCSI device sda: 390721968 512-byte hdwr sectors (200050 MB) sda: Write Protect is off sda: Mode Sense: 00 3a 00 00 SCSI device sda: drive cache: write back sda: unknown partition table (SATA disk without partition table. It's one out of four components for a RAID5 MD array). Reading the partition table from an unpartitioned device *is* a success. If that's what the user wanted, then yes, that will make him happy. If we let the kernel return an error like you propose, we will get a lot of bug reports from users plugging in their new and shiny disk: Kernel finds no partition table and throws an error. User thinks his shiny disk is faulty, lkml tells him it's not, user then asks why the kernel throws an error... Erik -- +-- Erik Mouw -- www.harddisk-recovery.com -- +31 70 370 12 90 -- | Lab address: Delftechpark 26, 2628 XH, Delft, The Netherlands ^ permalink raw reply [flat|nested] 13+ messages in thread
* [RFC] PATCH to fix rescan_partitions to return errors properly - take 2 2006-10-05 10:40 ` Erik Mouw @ 2006-10-05 20:32 ` Suzuki Kp 2006-10-05 22:07 ` Andrew Morton 2006-10-06 12:53 ` Erik Mouw 0 siblings, 2 replies; 13+ messages in thread From: Suzuki Kp @ 2006-10-05 20:32 UTC (permalink / raw) To: Erik Mouw; +Cc: lkml, linux-fsdevel, Andrew Morton, andmike [-- Attachment #1: Type: text/plain, Size: 949 bytes --] Erik, Erik Mouw wrote: > On Wed, Oct 04, 2006 at 10:37:49AM -0700, Suzuki Kp wrote: > >>Erik Mouw wrote: >> >>>I disagree. It's perfectly valid for a disk not to have a partition >>>table (for example: components of a RAID5 MD device) and we shouldn't >>>scare users about that. Also an unrecognised partition table format >>>(DEC VMS, Novell Netware, etc.) is not a reason to throw an error, it's >>>just unrecognised and as far as the kernel knows it's unpartioned. >> [...] Thank you very much for the inputs. As per the discussion I have made the changes to the patch. This change needs to be implemented in some of the partition checkers which doesn't do that already. Btw, do you think it is a good idea to let the other partition checkers run, even if one of them has failed ? Right now, the check_partition runs the partition checkers in a sequential manner, until it finds a success or an error. Comments ? Thanks, Suzuki [-- Attachment #2: fix-rescan_partitions-take2.diff --] [-- Type: text/x-patch, Size: 1007 bytes --] * Fix rescan_partition to propagate the low level I/O error. Signed Off by: Suzuki K P <suzuki@in.ibm.com> Index: linux-2.6.18/fs/partitions/check.c =================================================================== --- linux-2.6.18.orig/fs/partitions/check.c 2006-09-26 04:41:55.000000000 +0530 +++ linux-2.6.18/fs/partitions/check.c 2006-10-06 01:22:06.000000000 +0530 @@ -177,7 +177,7 @@ else if (warn_no_part) printk(" unable to read partition table\n"); kfree(state); - return NULL; + return ERR_PTR(res); } /* @@ -460,6 +460,9 @@ disk->fops->revalidate_disk(disk); if (!get_capacity(disk) || !(state = check_partition(disk, bdev))) return 0; + if (IS_ERR(state)) + /* I/O error reading the partition table */ + return -EIO; for (p = 1; p < state->limit; p++) { sector_t size = state->parts[p].size; sector_t from = state->parts[p].from; ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [RFC] PATCH to fix rescan_partitions to return errors properly - take 2 2006-10-05 20:32 ` [RFC] PATCH to fix rescan_partitions to return errors properly - take 2 Suzuki Kp @ 2006-10-05 22:07 ` Andrew Morton 2006-10-06 12:53 ` Erik Mouw 1 sibling, 0 replies; 13+ messages in thread From: Andrew Morton @ 2006-10-05 22:07 UTC (permalink / raw) To: Suzuki Kp; +Cc: Erik Mouw, lkml, linux-fsdevel, andmike On Thu, 05 Oct 2006 13:32:34 -0700 Suzuki Kp <suzuki@in.ibm.com> wrote: > Erik, > > > Erik Mouw wrote: > > On Wed, Oct 04, 2006 at 10:37:49AM -0700, Suzuki Kp wrote: > > > >>Erik Mouw wrote: > >> > >>>I disagree. It's perfectly valid for a disk not to have a partition > >>>table (for example: components of a RAID5 MD device) and we shouldn't > >>>scare users about that. Also an unrecognised partition table format > >>>(DEC VMS, Novell Netware, etc.) is not a reason to throw an error, it's > >>>just unrecognised and as far as the kernel knows it's unpartioned. > >> > > [...] > > > Thank you very much for the inputs. > > As per the discussion I have made the changes to the patch. > > This change needs to be implemented in some of the partition checkers > which doesn't do that already. > > Btw, do you think it is a good idea to let the other partition checkers > run, even if one of them has failed ? > > Right now, the check_partition runs the partition checkers in a > sequential manner, until it finds a success or an error. This is all important information to capture in the patch changelog: it covers user-visible changes, it covers user-affecting problems with the present kernel, it describes the implications of making this change to the kernel, etc. All important stuff. So could you please send a complete changelog for this patch? > > * Fix rescan_partition to propagate the low level I/O error. > Not enough ;) > > > Signed Off by: Suzuki K P <suzuki@in.ibm.com> > Please use "Signed-off-by:" This patch had tabs replaced with spaces, despite the fact that it was an attachment - that's a new one. Please get that fixed up for future patches, thanks. ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [RFC] PATCH to fix rescan_partitions to return errors properly - take 2 2006-10-05 20:32 ` [RFC] PATCH to fix rescan_partitions to return errors properly - take 2 Suzuki Kp 2006-10-05 22:07 ` Andrew Morton @ 2006-10-06 12:53 ` Erik Mouw 2006-10-06 17:43 ` Suzuki K P 1 sibling, 1 reply; 13+ messages in thread From: Erik Mouw @ 2006-10-06 12:53 UTC (permalink / raw) To: Suzuki Kp; +Cc: lkml, linux-fsdevel, Andrew Morton, andmike On Thu, Oct 05, 2006 at 01:32:34PM -0700, Suzuki Kp wrote: > Btw, do you think it is a good idea to let the other partition checkers > run, even if one of them has failed ? Yes, just let them run. Partition information doesn't need to be on the very first sector of the drive. If the first sector is bad and the partition table for your funky XYZ partition table format lives on the tenth sector, then a checker that checks the first sector would fail and prevent your checker from running. OTOH: having ten partition checkers check the same bad first sector doesn't really speed up the partion check process (for that reason we disable partition checking for drives we get for recovery). A way to solve that would be to keep a list of bad sectors: if the first checker finds a bad sector, it notes it down in the list so the next checker wouldn't have to try to read that particular sector. Maybe that's too much work to do in kernel and we'd better move the partition checking to userland. > Right now, the check_partition runs the partition checkers in a > sequential manner, until it finds a success or an error. I think it's best not to change the current behaviour and let all partition checkers run, even if one of them failed due to device errors. I wouldn't mind if the behaviour changed like you propose, though. Erik -- +-- Erik Mouw -- www.harddisk-recovery.com -- +31 70 370 12 90 -- | Lab address: Delftechpark 26, 2628 XH, Delft, The Netherlands ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [RFC] PATCH to fix rescan_partitions to return errors properly - take 2 2006-10-06 12:53 ` Erik Mouw @ 2006-10-06 17:43 ` Suzuki K P 2006-10-06 21:07 ` Erik Mouw 0 siblings, 1 reply; 13+ messages in thread From: Suzuki K P @ 2006-10-06 17:43 UTC (permalink / raw) To: Erik Mouw; +Cc: lkml, linux-fsdevel, Andrew Morton, andmike Erik Mouw wrote: > On Thu, Oct 05, 2006 at 01:32:34PM -0700, Suzuki Kp wrote: > >>Btw, do you think it is a good idea to let the other partition checkers >>run, even if one of them has failed ? > > > Yes, just let them run. Partition information doesn't need to be on the > very first sector of the drive. If the first sector is bad and the > partition table for your funky XYZ partition table format lives on the > tenth sector, then a checker that checks the first sector would fail > and prevent your checker from running. > > OTOH: having ten partition checkers check the same bad first sector > doesn't really speed up the partion check process (for that reason we > disable partition checking for drives we get for recovery). A way to > solve that would be to keep a list of bad sectors: if the first checker > finds a bad sector, it notes it down in the list so the next checker > wouldn't have to try to read that particular sector. Maybe that's too > much work to do in kernel and we'd better move the partition checking > to userland. > > >>Right now, the check_partition runs the partition checkers in a >>sequential manner, until it finds a success or an error. > > > I think it's best not to change the current behaviour and let all > partition checkers run, even if one of them failed due to device > errors. I wouldn't mind if the behaviour changed like you propose, > though. > At present, the partition checkers doesn't run, if one of the preceeding checker has reported an error ! *But*, some of the checkers doesn't report the I/O error which they came across! So, this may let others run. Thats not we want, right. We would like them to return I/O errors, and and the check_partition should let other partition checkers continue. Comments ? Thanks, Suzuki > > Erik > ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [RFC] PATCH to fix rescan_partitions to return errors properly - take 2 2006-10-06 17:43 ` Suzuki K P @ 2006-10-06 21:07 ` Erik Mouw 2006-10-07 1:46 ` [RFC] Fix check_partition routines ( was Re: [RFC] PATCH to fix rescan_partitions to return errors properly - take 2) Suzuki K P 0 siblings, 1 reply; 13+ messages in thread From: Erik Mouw @ 2006-10-06 21:07 UTC (permalink / raw) To: Suzuki K P; +Cc: lkml, linux-fsdevel, Andrew Morton, andmike On Fri, Oct 06, 2006 at 10:43:42AM -0700, Suzuki K P wrote: > Erik Mouw wrote: > >I think it's best not to change the current behaviour and let all > >partition checkers run, even if one of them failed due to device > >errors. I wouldn't mind if the behaviour changed like you propose, > >though. > > > At present, the partition checkers doesn't run, if one of the preceeding > checker has reported an error ! *But*, some of the checkers doesn't > report the I/O error which they came across! So, this may let others > run. Thats not we want, right. We would like them to return I/O errors, > and and the check_partition should let other partition checkers continue. Indeed, we want them to behave the same. I.e.: a partition checker should tell when it encounters an I/O error. Erik -- +-- Erik Mouw -- www.harddisk-recovery.nl -- +31 70 370 12 90 -- | Lab address: Delftechpark 26, 2628 XH, Delft, The Netherlands ^ permalink raw reply [flat|nested] 13+ messages in thread
* [RFC] Fix check_partition routines ( was Re: [RFC] PATCH to fix rescan_partitions to return errors properly - take 2) 2006-10-06 21:07 ` Erik Mouw @ 2006-10-07 1:46 ` Suzuki K P 2006-10-07 2:45 ` Andrew Morton 0 siblings, 1 reply; 13+ messages in thread From: Suzuki K P @ 2006-10-07 1:46 UTC (permalink / raw) To: Erik Mouw; +Cc: lkml, linux-fsdevel, Andrew Morton, andmike [-- Attachment #1: Type: text/plain, Size: 1235 bytes --] Hi, Here is a followup patch, which cleans up the check_partition() as well as some of the partition check routines. check_partition would now record the I/O error notice and proceed with the other partitions. The error will only be reported when all of the partitions have left the disk as "Unknown". Cleanup for partition checkers for ibm, atari and amiga. Comments ? Thanks Suzuki Erik Mouw wrote: > On Fri, Oct 06, 2006 at 10:43:42AM -0700, Suzuki K P wrote: > >>Erik Mouw wrote: >> >>>I think it's best not to change the current behaviour and let all >>>partition checkers run, even if one of them failed due to device >>>errors. I wouldn't mind if the behaviour changed like you propose, >>>though. >>> >> >>At present, the partition checkers doesn't run, if one of the preceeding >>checker has reported an error ! *But*, some of the checkers doesn't >>report the I/O error which they came across! So, this may let others >>run. Thats not we want, right. We would like them to return I/O errors, >>and and the check_partition should let other partition checkers continue. > > > Indeed, we want them to behave the same. I.e.: a partition checker > should tell when it encounters an I/O error. > > > Erik > [-- Attachment #2: fix-check_partition-methods.diff --] [-- Type: text/x-patch, Size: 4758 bytes --] * The check_partition() stops its probe once it hits an I/O error from the partition checkers. This would prevent the actual partition checker getting a chance to verify the partition. So, this patch lets the check_partition continue probing untill it hits a success while recording the I/O error which might have been reported by the checking routines. Also, it does some cleanup of the partition methods for ibm, atari and amiga to return -1 upon hitting an I/O error. Signed-Off-by: Suzuki K P <suzuki@in.ibm.com> Index: linux-2.6.18/fs/partitions/check.c =================================================================== --- linux-2.6.18.orig/fs/partitions/check.c 2006-10-07 06:10:16.000000000 +0530 +++ linux-2.6.18/fs/partitions/check.c 2006-10-07 06:21:39.000000000 +0530 @@ -153,7 +153,7 @@ check_partition(struct gendisk *hd, struct block_device *bdev) { struct parsed_partitions *state; - int i, res; + int i, res, err; state = kmalloc(sizeof(struct parsed_partitions), GFP_KERNEL); if (!state) @@ -165,13 +165,24 @@ sprintf(state->name, "p"); state->limit = hd->minors; - i = res = 0; + i = res = err = 0; while (!res && check_part[i]) { memset(&state->parts, 0, sizeof(state->parts)); res = check_part[i++](state, bdev); + if (res < 0) { + /* We have hit an I/O error which we don't report now. + * But record it, and let the others do their job. + */ + err = res; + res = 0; + } + } if (res > 0) return state; + if (!err) + /* The partition is unrecognized. So report I/O errors if there were any */ + res = err; if (!res) printk(" unknown partition table\n"); else if (warn_no_part) Index: linux-2.6.18/fs/partitions/amiga.c =================================================================== --- linux-2.6.18.orig/fs/partitions/amiga.c 2006-09-20 09:12:06.000000000 +0530 +++ linux-2.6.18/fs/partitions/amiga.c 2006-10-07 06:57:44.000000000 +0530 @@ -43,6 +43,7 @@ if (warn_no_part) printk("Dev %s: unable to read RDB block %d\n", bdevname(bdev, b), blk); + res = -1; goto rdb_done; } if (*(__be32 *)data != cpu_to_be32(IDNAME_RIGIDDISK)) @@ -79,6 +80,7 @@ if (warn_no_part) printk("Dev %s: unable to read partition block %d\n", bdevname(bdev, b), blk); + res = -1; goto rdb_done; } pb = (struct PartitionBlock *)data; Index: linux-2.6.18/fs/partitions/ibm.c =================================================================== --- linux-2.6.18.orig/fs/partitions/ibm.c 2006-09-20 09:12:06.000000000 +0530 +++ linux-2.6.18/fs/partitions/ibm.c 2006-10-07 06:55:23.000000000 +0530 @@ -43,7 +43,7 @@ int ibm_partition(struct parsed_partitions *state, struct block_device *bdev) { - int blocksize, offset, size; + int blocksize, offset, size,res; loff_t i_size; dasd_information_t *info; struct hd_geometry *geo; @@ -55,16 +55,17 @@ } *label; unsigned char *data; Sector sect; - + + res = 0; blocksize = bdev_hardsect_size(bdev); if (blocksize <= 0) - return 0; + goto out_exit; i_size = i_size_read(bdev->bd_inode); if (i_size == 0) - return 0; + goto out_exit; if ((info = kmalloc(sizeof(dasd_information_t), GFP_KERNEL)) == NULL) - goto out_noinfo; + goto out_exit; if ((geo = kmalloc(sizeof(struct hd_geometry), GFP_KERNEL)) == NULL) goto out_nogeo; if ((label = kmalloc(sizeof(union label_t), GFP_KERNEL)) == NULL) @@ -72,7 +73,7 @@ if (ioctl_by_bdev(bdev, BIODASDINFO, (unsigned long)info) != 0 || ioctl_by_bdev(bdev, HDIO_GETGEO, (unsigned long)geo) != 0) - goto out_noioctl; + goto out_freeall; /* * Get volume label, extract name and type. @@ -91,6 +92,8 @@ EBCASC(type, 4); EBCASC(name, 6); + + res = 1; /* * Three different types: CMS1, VOL1 and LNX1/unlabeled @@ -156,6 +159,9 @@ counter++; blk++; } + if (!data) + /* Are we not supposed to report this ? */ + goto out_readerr; } else { /* * Old style LNX1 or unlabeled disk @@ -171,18 +177,17 @@ } printk("\n"); - kfree(label); - kfree(geo); - kfree(info); - return 1; + goto out_freeall; + out_readerr: -out_noioctl: + res = -1; +out_freeall: kfree(label); out_nolab: kfree(geo); out_nogeo: kfree(info); -out_noinfo: - return 0; +out_exit: + return res; } Index: linux-2.6.18/fs/partitions/atari.c =================================================================== --- linux-2.6.18.orig/fs/partitions/atari.c 2006-09-20 09:12:06.000000000 +0530 +++ linux-2.6.18/fs/partitions/atari.c 2006-10-07 06:58:22.000000000 +0530 @@ -88,7 +88,7 @@ if (!xrs) { printk (" block %ld read failed\n", partsect); put_dev_sector(sect); - return 0; + return -1; } /* ++roman: sanity check: bit 0 of flg field must be set */ ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [RFC] Fix check_partition routines ( was Re: [RFC] PATCH to fix rescan_partitions to return errors properly - take 2) 2006-10-07 1:46 ` [RFC] Fix check_partition routines ( was Re: [RFC] PATCH to fix rescan_partitions to return errors properly - take 2) Suzuki K P @ 2006-10-07 2:45 ` Andrew Morton 0 siblings, 0 replies; 13+ messages in thread From: Andrew Morton @ 2006-10-07 2:45 UTC (permalink / raw) To: Suzuki K P; +Cc: Erik Mouw, lkml, linux-fsdevel, andmike On Fri, 06 Oct 2006 18:46:18 -0700 Suzuki K P <suzuki@in.ibm.com> wrote: > * The check_partition() stops its probe once it hits an I/O error from the partition checkers. This would prevent the actual partition checker getting a chance to verify the partition. So, this patch lets the check_partition continue probing untill it hits a success while recording the I/O error which might have been reported by the checking routines. > > Also, it does some cleanup of the partition methods for ibm, atari and amiga to return -1 upon hitting an I/O error. What is the significance of a `return 1', a `return 0' and a `return -EFOO' in these functions? It all looks rather odd. ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2006-10-07 2:46 UTC | newest] Thread overview: 13+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2006-10-04 1:00 [RFC] PATCH to fix rescan_partitions to return errors properly Suzuki Kp 2006-10-04 13:09 ` Erik Mouw 2006-10-04 16:50 ` Suzuki Kp 2006-10-04 17:08 ` Erik Mouw 2006-10-04 17:37 ` Suzuki Kp 2006-10-05 10:40 ` Erik Mouw 2006-10-05 20:32 ` [RFC] PATCH to fix rescan_partitions to return errors properly - take 2 Suzuki Kp 2006-10-05 22:07 ` Andrew Morton 2006-10-06 12:53 ` Erik Mouw 2006-10-06 17:43 ` Suzuki K P 2006-10-06 21:07 ` Erik Mouw 2006-10-07 1:46 ` [RFC] Fix check_partition routines ( was Re: [RFC] PATCH to fix rescan_partitions to return errors properly - take 2) Suzuki K P 2006-10-07 2:45 ` Andrew Morton
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).