[PATCH] ESP : Fix SCSI READ command length

Signed-off-by: Olivier Danet <odanet@caramail.com> --- diff -rup a/drivers/esp.c b/drivers/esp.c --- a/drivers/esp.c 2011-01-27 15:11:52.000000000 +0100 +++ b/drivers/esp.c 2011-01-27 15:13:34.000000000 +0100 @@ -147,7 +147,7 @@ ob_sd_read_sector(esp_private_t *esp, sd sd->id, offset); // Setup command = Read(10) - memset(esp->buffer, 0, 10); + memset(esp->buffer, 0, 11); esp->buffer[0] = 0x80; esp->buffer[1] = READ_10; @@ -159,7 +159,7 @@ ob_sd_read_sector(esp_private_t *esp, sd esp->buffer[8] = 0; esp->buffer[9] = 1; - if (do_command(esp, sd, 10, sd->bs)) + if (do_command(esp, sd, 11, sd->bs)) return 0; return 0;

On 27/01/11 19:05, Olivier DANET wrote:
Signed-off-by: Olivier Danet<odanet@caramail.com> --- diff -rup a/drivers/esp.c b/drivers/esp.c --- a/drivers/esp.c 2011-01-27 15:11:52.000000000 +0100 +++ b/drivers/esp.c 2011-01-27 15:13:34.000000000 +0100 @@ -147,7 +147,7 @@ ob_sd_read_sector(esp_private_t *esp, sd sd->id, offset);
// Setup command = Read(10) - memset(esp->buffer, 0, 10); + memset(esp->buffer, 0, 11); esp->buffer[0] = 0x80; esp->buffer[1] = READ_10;
@@ -159,7 +159,7 @@ ob_sd_read_sector(esp_private_t *esp, sd esp->buffer[8] = 0; esp->buffer[9] = 1;
- if (do_command(esp, sd, 10, sd->bs)) + if (do_command(esp, sd, 11, sd->bs)) return 0;
return 0;
Hi Olivier, While on the surface the patch looks sensible, it's quite difficult to review this (and your associated QEMU patches) in their current form. Could you re-submit with an extra description, describing briefly i) what symptoms did you see without the patch, ii) what does this fix with the patch applied, and iii) a document reference which verifies the command length should be 11 instead of 10. ATB, Mark. -- Mark Cave-Ayland - Senior Technical Architect PostgreSQL - PostGIS Sirius Corporation plc - control through freedom http://www.siriusit.co.uk t: +44 870 608 0063 Sirius Labs: http://www.siriusit.co.uk/labs

On Fri, Jan 28, 2011 at 10:24 AM, Mark Cave-Ayland <mark.cave-ayland@siriusit.co.uk> wrote:
On 27/01/11 19:05, Olivier DANET wrote:
Signed-off-by: Olivier Danet<odanet@caramail.com> --- diff -rup a/drivers/esp.c b/drivers/esp.c --- a/drivers/esp.c 2011-01-27 15:11:52.000000000 +0100 +++ b/drivers/esp.c 2011-01-27 15:13:34.000000000 +0100 @@ -147,7 +147,7 @@ ob_sd_read_sector(esp_private_t *esp, sd sd->id, offset);
// Setup command = Read(10) - memset(esp->buffer, 0, 10); + memset(esp->buffer, 0, 11); esp->buffer[0] = 0x80; esp->buffer[1] = READ_10;
@@ -159,7 +159,7 @@ ob_sd_read_sector(esp_private_t *esp, sd esp->buffer[8] = 0; esp->buffer[9] = 1;
- if (do_command(esp, sd, 10, sd->bs)) + if (do_command(esp, sd, 11, sd->bs)) return 0;
return 0;
Hi Olivier,
While on the surface the patch looks sensible, it's quite difficult to review this (and your associated QEMU patches) in their current form.
Could you re-submit with an extra description, describing briefly i) what symptoms did you see without the patch, ii) what does this fix with the patch applied, and iii) a document reference which verifies the command length should be 11 instead of 10.
For iii): SCSI-3 Block Commands (SBC), 6.1.5 READ(10): the number of bytes is 10 for the command itself. Before the command there is one byte for IDENTIFY (0x80), defined by SCSI Parallel Interface. Thus 11, like READ_CAPACITY below. INQUIRY is 6+1, defined in SCSI Primary Commands.

Am 28.01.2011 um 22:47 schrieb Blue Swirl:
On Fri, Jan 28, 2011 at 10:24 AM, Mark Cave-Ayland <mark.cave-ayland@siriusit.co.uk> wrote:
On 27/01/11 19:05, Olivier DANET wrote:
Signed-off-by: Olivier Danet<odanet@caramail.com> --- diff -rup a/drivers/esp.c b/drivers/esp.c --- a/drivers/esp.c 2011-01-27 15:11:52.000000000 +0100 +++ b/drivers/esp.c 2011-01-27 15:13:34.000000000 +0100 @@ -147,7 +147,7 @@ ob_sd_read_sector(esp_private_t *esp, sd sd->id, offset);
// Setup command = Read(10) - memset(esp->buffer, 0, 10); + memset(esp->buffer, 0, 11); esp->buffer[0] = 0x80; esp->buffer[1] = READ_10;
@@ -159,7 +159,7 @@ ob_sd_read_sector(esp_private_t *esp, sd esp->buffer[8] = 0; esp->buffer[9] = 1;
- if (do_command(esp, sd, 10, sd->bs)) + if (do_command(esp, sd, 11, sd->bs)) return 0;
return 0;
While on the surface the patch looks sensible, it's quite difficult to review this (and your associated QEMU patches) in their current form.
Could you re-submit with an extra description, describing briefly i) what symptoms did you see without the patch, ii) what does this fix with the patch applied, and iii) a document reference which verifies the command length should be 11 instead of 10.
For iii): SCSI-3 Block Commands (SBC), 6.1.5 READ(10): the number of bytes is 10 for the command itself. Before the command there is one byte for IDENTIFY (0x80), defined by SCSI Parallel Interface.
Would be nice to get a #define for 0x80 then. It's used as magic value above. Andreas
Thus 11, like READ_CAPACITY below. INQUIRY is 6+1, defined in SCSI Primary Commands.

On 28/01/11 21:47, Blue Swirl wrote:
Could you re-submit with an extra description, describing briefly i) what symptoms did you see without the patch, ii) what does this fix with the patch applied, and iii) a document reference which verifies the command length should be 11 instead of 10.
For iii): SCSI-3 Block Commands (SBC), 6.1.5 READ(10): the number of bytes is 10 for the command itself. Before the command there is one byte for IDENTIFY (0x80), defined by SCSI Parallel Interface. Thus 11, like READ_CAPACITY below. INQUIRY is 6+1, defined in SCSI Primary Commands.
Given that we've had no response on this to date, what shall we do? Blue, if you're happy enough the patch is correct based upon the above then please feel free to apply. ATB, Mark. -- Mark Cave-Ayland - Senior Technical Architect PostgreSQL - PostGIS Sirius Corporation plc - control through freedom http://www.siriusit.co.uk t: +44 870 608 0063 Sirius Labs: http://www.siriusit.co.uk/labs

On Thu, Feb 17, 2011 at 1:19 PM, Mark Cave-Ayland <mark.cave-ayland@siriusit.co.uk> wrote:
On 28/01/11 21:47, Blue Swirl wrote:
Could you re-submit with an extra description, describing briefly i) what symptoms did you see without the patch, ii) what does this fix with the patch applied, and iii) a document reference which verifies the command length should be 11 instead of 10.
For iii): SCSI-3 Block Commands (SBC), 6.1.5 READ(10): the number of bytes is 10 for the command itself. Before the command there is one byte for IDENTIFY (0x80), defined by SCSI Parallel Interface. Thus 11, like READ_CAPACITY below. INQUIRY is 6+1, defined in SCSI Primary Commands.
Given that we've had no response on this to date, what shall we do? Blue, if you're happy enough the patch is correct based upon the above then please feel free to apply.
OK. Thanks, applied.
participants (4)
-
Andreas Färber
-
Blue Swirl
-
Mark Cave-Ayland
-
Olivier DANET