s390: Fix me in dasd_eer.c
Subsystems (2)
PATCH 1/1 s390: Fix me in dasd_eer.c
This patch changes return type to EMEDUIMTYPE in function, dasd_eer_enable for when checking if the medium has no errors according to this function.
--- drivers/s390/block/dasd_eer.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/s390/block/dasd_eer.c +1 -1
@@ -462,7 +462,7 @@ int dasd_eer_enable(struct dasd_device *device)return 0 ;if (!device->discipline ||strcmp (device->discipline->name,"ECKD" )) -return -EPERM ;/* FIXME: -EMEDIUMTYPE ? */ +return -EMEDIUMTYPE ;/* FIXME: -EMEDIUMTYPE ? */ cqr =dasd_kmalloc_request (DASD_ECKD_MAGIC ,1 /* SNSS */ ,SNSS_DATA_SIZE , device);
Signature · 1 line
1.9.1
Quoted 4 lines · On Tue, 22 Jul 2014 02:29:32 -0400 Nicholas Krause <[email protected]> wrote:
This patch changes return type to EMEDUIMTYPE in function,
dasd_eer_enablefor when checking if the medium has no errors according to this function.Signed-off-by Nicholas Krause <[email protected]>
What distributions have you tested this with and what configurations?
Quoted 9 lines · One Thousand Gnomes
Quoted 4 lines · On Tue, 22 Jul 2014 02:29:32 -0400 Nicholas Krause <[email protected]> wrote:
This patch changes return type to EMEDUIMTYPE in function,
dasd_eer_enablefor when checking if the medium has no errors according to this function.Signed-off-by Nicholas Krause <[email protected]>What distributions have you tested this with and what configurations?
I have not tested this patch but based on the logical statements this seems to be
correct.
Nick
Nicholas Krause <[email protected]> wrote on 2014-07-22 08:29:32:
[..]
Quoted 5 lines
Subject:
[PATCH] s390: Fix me in
dasd_eer.cThis patch changes return type to EMEDUIMTYPE in function,
dasd_eer_enable
Quoted 1 line
for when checking if the medium has no errors according to this
function.
Review
Quoted 7 lines
Signed-off-by Nicholas Krause <[email protected]>--- drivers/s390/block/dasd_eer.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)(quoted diff) +0
-0
b/drivers/s390/block/dasd_eer.c
Quoted 15 lines
index 21ef63c..08ee040 100644
drivers/s390/block/dasd_eer.c +1
-1@@ -462,7 +462,7 @@ int dasd_eer_enable(struct dasd_device *device)return 0 ;if (!device->discipline ||strcmp (device->discipline->name,"ECKD" )) -return -EPERM ;/* FIXME: -EMEDIUMTYPE ? */ +return -EMEDIUMTYPE ;/* FIXME: -EMEDIUMTYPE ? */ cqr =dasd_kmalloc_request (DASD_ECKD_MAGIC ,1 /* SNSS */ ,SNSS_DATA_SIZE , device);Signature · 1 line
1.9.1
Hm, after some consideration, I have to disagree with your suggestion. If you try to enable EER on an FBA device, you will get the following result with current code:
$ echo 1 > eer_enabled -bash: echo: write error: Operation not permitted
and with your code the following:
$ echo 1 > eer_enabled -bash: echo: write error: Wrong medium type
When I wrote this code, I was not sure which one is better. But today I say
that the 'Operation not permitted' is more to the point. An FBA device has no (changable) medium, so what could be wrong about its type? Could be confusing.
From your patch description I do not really get why you want to change the return code. Why do you think that EMEDIUMTYPE is better than EPERM?
Mit freundlichen Grüßen / Kind regards
Stefan Weinhuber
Quoted 53 lines · Stefan Weinhuber
Nicholas Krause <[email protected]> wrote on 2014-07-22 08:29:32:
[..]
Quoted 5 lines
Subject:
[PATCH] s390: Fix me in
dasd_eer.cThis patch changes return type to EMEDUIMTYPE in function,
dasd_eer_enableQuoted 1 line
for when checking if the medium has no errors according to this
function.
Review
Quoted 7 lines
Signed-off-by Nicholas Krause <[email protected]>--- drivers/s390/block/dasd_eer.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)(quoted diff) +0
-0
b/drivers/s390/block/dasd_eer.cQuoted 13 lines
index 21ef63c..08ee040 100644
drivers/s390/block/dasd_eer.c +1
-2@@ -462,7 +462,7 @@ int dasd_eer_enable(struct dasd_device *device)return 0 ;if (!device->discipline ||strcmp (device->discipline->name,"ECKD" )) -return -EPERM ;/* FIXME: -EMEDIUMTYPE ? */ +return -EMEDIUMTYPE ;/* FIXME: -EMEDIUMTYPE ? */ cqr =dasd_kmalloc_request (DASD_ECKD_MAGIC ,1 /* SNSS */ ,SNSS_DATA_SIZE , device); --Quoted 2 lines
1.9.1
Hm, after some consideration, I have to disagree with your suggestion. If you try to enable EER on an FBA device, you will get the following result with current code:
$ echo 1 > eer_enabled -bash: echo: write error: Operation not permittedand with your code the following:
$ echo 1 > eer_enabled -bash: echo: write error: Wrong medium typeWhen I wrote this code, I was not sure which one is better. But today I say
that the 'Operation not permitted' is more to the point. An FBA device has no (changable) medium, so what could be wrong about its type? Could be confusing.
Quoted 7 lines
From your patch description I do not really get why you want to change the return code. Why do you think that EMEDIUMTYPE is better than EPERM?
Mit freundlichen Grüßen / Kind regards
Stefan Weinhuber
Stefan,
From my reading seemed more like EMEDUIMTYPE return but seems I was wrong here. If you want I can remove the fix me message in a patch if you want to keep this return type.
Regards Nick