* [PATCH 4/5] md: Return proper error rather than EIO.
@ 2012-10-27 2:28 majianpeng
2012-10-28 21:43 ` NeilBrown
0 siblings, 1 reply; 3+ messages in thread
From: majianpeng @ 2012-10-27 2:28 UTC (permalink / raw)
To: NeilBrown; +Cc: linux-raid
Signed-off-by: Jianpeng Ma <majianpeng@gmail.com>
---
drivers/md/raid1.c | 4 ++--
drivers/md/raid5.c | 4 ++--
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
index 8034fbd..e2f3783 100644
--- a/drivers/md/raid1.c
+++ b/drivers/md/raid1.c
@@ -2799,12 +2799,12 @@ static int run(struct mddev *mddev)
if (mddev->level != 1) {
printk(KERN_ERR "md/raid1:%s: raid level not set to mirroring (%d)\n",
mdname(mddev), mddev->level);
- return -EIO;
+ return -EINVAL;
}
if (mddev->reshape_position != MaxSector) {
printk(KERN_ERR "md/raid1:%s: reshape_position set but not supported\n",
mdname(mddev));
- return -EIO;
+ return -EINVAL;
}
/*
* copy the already verified devices into our private RAID1
diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
index c5439dc..684ca76 100644
--- a/drivers/md/raid5.c
+++ b/drivers/md/raid5.c
@@ -5088,7 +5088,7 @@ static struct r5conf *setup_conf(struct mddev *mddev)
&& mddev->new_level != 6) {
printk(KERN_ERR "md/raid:%s: raid level not set to 4/5/6 (%d)\n",
mdname(mddev), mddev->new_level);
- return ERR_PTR(-EIO);
+ return ERR_PTR(-EINVAL);
}
if ((mddev->new_level == 5
&& !algorithm_valid_raid5(mddev->new_layout)) ||
@@ -5096,7 +5096,7 @@ static struct r5conf *setup_conf(struct mddev *mddev)
&& !algorithm_valid_raid6(mddev->new_layout))) {
printk(KERN_ERR "md/raid:%s: layout %d not supported\n",
mdname(mddev), mddev->new_layout);
- return ERR_PTR(-EIO);
+ return ERR_PTR(-EINVAL);
}
if (mddev->new_level == 6 && mddev->raid_disks < 4) {
printk(KERN_ERR "md/raid:%s: not enough configured devices (%d, minimum 4)\n",
--
1.7.9.5
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH 4/5] md: Return proper error rather than EIO.
2012-10-27 2:28 [PATCH 4/5] md: Return proper error rather than EIO majianpeng
@ 2012-10-28 21:43 ` NeilBrown
2012-10-29 1:16 ` majianpeng
0 siblings, 1 reply; 3+ messages in thread
From: NeilBrown @ 2012-10-28 21:43 UTC (permalink / raw)
To: majianpeng; +Cc: linux-raid
[-- Attachment #1: Type: text/plain, Size: 2201 bytes --]
On Sat, 27 Oct 2012 10:28:30 +0800 majianpeng <majianpeng@gmail.com> wrote:
> Signed-off-by: Jianpeng Ma <majianpeng@gmail.com>
> ---
> drivers/md/raid1.c | 4 ++--
> drivers/md/raid5.c | 4 ++--
> 2 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
> index 8034fbd..e2f3783 100644
> --- a/drivers/md/raid1.c
> +++ b/drivers/md/raid1.c
> @@ -2799,12 +2799,12 @@ static int run(struct mddev *mddev)
> if (mddev->level != 1) {
> printk(KERN_ERR "md/raid1:%s: raid level not set to mirroring (%d)\n",
> mdname(mddev), mddev->level);
> - return -EIO;
> + return -EINVAL;
> }
> if (mddev->reshape_position != MaxSector) {
> printk(KERN_ERR "md/raid1:%s: reshape_position set but not supported\n",
> mdname(mddev));
> - return -EIO;
> + return -EINVAL;
> }
> /*
> * copy the already verified devices into our private RAID1
> diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
> index c5439dc..684ca76 100644
> --- a/drivers/md/raid5.c
> +++ b/drivers/md/raid5.c
> @@ -5088,7 +5088,7 @@ static struct r5conf *setup_conf(struct mddev *mddev)
> && mddev->new_level != 6) {
> printk(KERN_ERR "md/raid:%s: raid level not set to 4/5/6 (%d)\n",
> mdname(mddev), mddev->new_level);
> - return ERR_PTR(-EIO);
> + return ERR_PTR(-EINVAL);
> }
> if ((mddev->new_level == 5
> && !algorithm_valid_raid5(mddev->new_layout)) ||
> @@ -5096,7 +5096,7 @@ static struct r5conf *setup_conf(struct mddev *mddev)
> && !algorithm_valid_raid6(mddev->new_layout))) {
> printk(KERN_ERR "md/raid:%s: layout %d not supported\n",
> mdname(mddev), mddev->new_layout);
> - return ERR_PTR(-EIO);
> + return ERR_PTR(-EINVAL);
> }
> if (mddev->new_level == 6 && mddev->raid_disks < 4) {
> printk(KERN_ERR "md/raid:%s: not enough configured devices (%d, minimum 4)\n",
One doesn't just randomly change error return codes.
Maybe the current ones are "wrong", but they have been that way for many
years so they are effectively part of the API. So they are "right" be
definition, even if they seem wrong.
NeilBrown
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 828 bytes --]
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: Re: [PATCH 4/5] md: Return proper error rather than EIO.
2012-10-28 21:43 ` NeilBrown
@ 2012-10-29 1:16 ` majianpeng
0 siblings, 0 replies; 3+ messages in thread
From: majianpeng @ 2012-10-29 1:16 UTC (permalink / raw)
To: NeilBrown; +Cc: linux-raid
>On Sat, 27 Oct 2012 10:28:30 +0800 majianpeng <majianpeng@gmail.com> wrote:
>
>> Signed-off-by: Jianpeng Ma <majianpeng@gmail.com>
>> ---
>> drivers/md/raid1.c | 4 ++--
>> drivers/md/raid5.c | 4 ++--
>> 2 files changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
>> index 8034fbd..e2f3783 100644
>> --- a/drivers/md/raid1.c
>> +++ b/drivers/md/raid1.c
>> @@ -2799,12 +2799,12 @@ static int run(struct mddev *mddev)
>> if (mddev->level != 1) {
>> printk(KERN_ERR "md/raid1:%s: raid level not set to mirroring (%d)\n",
>> mdname(mddev), mddev->level);
>> - return -EIO;
>> + return -EINVAL;
>> }
>> if (mddev->reshape_position != MaxSector) {
>> printk(KERN_ERR "md/raid1:%s: reshape_position set but not supported\n",
>> mdname(mddev));
>> - return -EIO;
>> + return -EINVAL;
>> }
>> /*
>> * copy the already verified devices into our private RAID1
>> diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
>> index c5439dc..684ca76 100644
>> --- a/drivers/md/raid5.c
>> +++ b/drivers/md/raid5.c
>> @@ -5088,7 +5088,7 @@ static struct r5conf *setup_conf(struct mddev *mddev)
>> && mddev->new_level != 6) {
>> printk(KERN_ERR "md/raid:%s: raid level not set to 4/5/6 (%d)\n",
>> mdname(mddev), mddev->new_level);
>> - return ERR_PTR(-EIO);
>> + return ERR_PTR(-EINVAL);
>> }
>> if ((mddev->new_level == 5
>> && !algorithm_valid_raid5(mddev->new_layout)) ||
>> @@ -5096,7 +5096,7 @@ static struct r5conf *setup_conf(struct mddev *mddev)
>> && !algorithm_valid_raid6(mddev->new_layout))) {
>> printk(KERN_ERR "md/raid:%s: layout %d not supported\n",
>> mdname(mddev), mddev->new_layout);
>> - return ERR_PTR(-EIO);
>> + return ERR_PTR(-EINVAL);
>> }
>> if (mddev->new_level == 6 && mddev->raid_disks < 4) {
>> printk(KERN_ERR "md/raid:%s: not enough configured devices (%d, minimum 4)\n",
>
>
>One doesn't just randomly change error return codes.
>Maybe the current ones are "wrong", but they have been that way for many
>years so they are effectively part of the API. So they are "right" be
>definition, even if they seem wrong.
>
Yes, i agree with your point.
For func setup_conf,there was alreday EINVAL, so modify EIO to EINVAL isn't add a new return code.
I think for upper caller no effect.
For func run, the caller alse had EINVAL.So modify had no effect.
Jianpeng
>NeilBrown
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2012-10-29 1:16 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-10-27 2:28 [PATCH 4/5] md: Return proper error rather than EIO majianpeng
2012-10-28 21:43 ` NeilBrown
2012-10-29 1:16 ` majianpeng
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).