On Wed, Apr 23, 2008 at 10:25 AM, FUJITA Tomonori
<fujita.tomonori@xxxxxxxxxxxxx> wrote:
> On Wed, 23 Apr 2008 09:32:22 +0200
> Borislav Petkov <petkovbb@xxxxxxxxxxxxxx> wrote:
>
> > Hi,
> >
> > On Tue, Apr 22, 2008 at 09:26:37AM +0900, FUJITA Tomonori wrote:
> > > ide_do_drive_cmd forges an error value (-EIO) instead of
> > > rq->errors. idetape_queue_rw_tail wants rq->errors so this patch makes
> > > ide_do_drive_cmd return rq->errors.
> > >
> > > For compatibility, this patch makes the users of ide_do_drive_cmd
> > > return -EIO instead of a return value of ide_do_drive_cmd
> > > (rq->errors).
> >
> > i don't see the reason for it. In the end, the only error type that is being
> > handed to and fro is -EIO and nobody is interested in rq->errors. So, whether
> > ide_do_drive_cmd or its callers return -EIO is kinda unimportant. In the second
> > case, however, you have simply added more code (as per the diffstat below) with
> > no apparent functionality. It would only make sense, IMHO, if you would
> > differentiate behaviour based on rq->errors...
>
> I did this only because idetape_queue_rw_tail seems to have
> differentiate behaviour based on rq->errors:
>
>
> static int idetape_queue_rw_tail(ide_drive_t *drive, int cmd, int blocks,
> struct idetape_bh *bh)
> {
> ...
>
> (void) ide_do_drive_cmd(drive, &rq, ide_wait);
>
> if ((cmd & (REQ_IDETAPE_READ | REQ_IDETAPE_WRITE)) == 0)
> return 0;
>
> if (tape->merge_stage)
> idetape_init_merge_stage(tape);
> if (rq.errors == IDETAPE_ERROR_GENERAL)
> return -EIO;
>
>
> If we can do something like:
>
>
> ret = ide_do_drive_cmd(drive, &rq, ide_wait);
>
> if ((cmd & (REQ_IDETAPE_READ | REQ_IDETAPE_WRITE)) == 0)
> return 0;
>
> if (tape->merge_stage)
> idetape_init_merge_stage(tape);
> if (ret)
> return -EIO;
>
>
> Then yeah, we don't need this patch.
>
Well, AFAICS, all calls to idetape_queue_rw_tail simply check whether its return
value is negative, and, if so, discontinue further command queueing. So, i think
we're OK if we simply return -EIO. Bart?
--
Regards/Gruß,
Boris
--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
