| Message ID | 1280763089-11829-5-git-send-email-bernhard.kohl@nsn.com |
|---|---|
| State | New |
| Headers | show |
Am 02.08.2010 17:31, schrieb Bernhard Kohl: > The block descriptor contains the number of blocks, not the highest LBA. > Real hard disks return 0 if the number of blocks exceed the maximum 0xFFFFFF. > > SCSI-Spec: > http://ldkelley.com/SCSI2/SCSI2/SCSI2-08.html#8.3.3 > The number of blocks field specifies the number of logical blocks on the medium > to which the density code and block length fields apply. A value of zero > indicates that all of the remaining logical blocks of the logical unit shall > have the medium characteristics specified. > > Signed-off-by: Bernhard Kohl <bernhard.kohl@nsn.com> > --- > hw/scsi-disk.c | 3 +-- > 1 files changed, 1 insertions(+), 2 deletions(-) > > diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c > index 26f7345..519513c 100644 > --- a/hw/scsi-disk.c > +++ b/hw/scsi-disk.c > @@ -642,9 +642,8 @@ static int scsi_disk_emulate_mode_sense(SCSIRequest *req, uint8_t *outbuf) > else /* MODE_SENSE_10 */ > outbuf[7] = 8; /* Block descriptor length */ > nb_sectors /= s->cluster_size; > - nb_sectors--; > if (nb_sectors > 0xffffff) > - nb_sectors = 0xffffff; > + nb_sectors = 0; > p[0] = 0; /* media density code */ > p[1] = (nb_sectors >> 16) & 0xff; > p[2] = (nb_sectors >> 8) & 0xff; The patch itself looks okay. However, it made me wonder what this line wants to tell us: if ((~dbd) & nb_sectors) { Is it just me or doesn't this make any sense at all? dbd is a single bit, 0x8 if set or 0x0 otherwise. nb_sectors is the number of sectors. Can this operation have any meaningful result? I suppose it was meant to be something like: if (!dbd && nb_sectors) { Can you please check this and add a patch 5/4 if necessary? Kevin
Am 16.08.2010 19:34, schrieb ext Kevin Wolf: > > The patch itself looks okay. However, it made me wonder what this line > wants to tell us: > > if ((~dbd) & nb_sectors) { > > Is it just me or doesn't this make any sense at all? dbd is a single > bit, 0x8 if set or 0x0 otherwise. nb_sectors is the number of sectors. > Can this operation have any meaningful result? I suppose it was meant to > be something like: > > if (!dbd && nb_sectors) { > > Can you please check this and add a patch 5/4 if necessary? > For my opinion it is nonsense too. And it does not work. I tested it: root@grml ~ # sg_modes /dev/sdb -6 -v -H -H -p 0x3f inquiry cdb: 12 00 00 00 24 00 QEMU QEMU HARDDISK 0.13 peripheral_type: disk [0x0] mode sense (6) cdb: 1a 00 3f 00 fc 00 mode sense (6): requested 252 bytes but got 220 bytes Mode parameter header from MODE SENSE(6): 00 1f 00 00 08 Mode data length=32, medium type=0x00, WP=0, DpoFua=0, longlba=0 Block descriptor length=8 > Direct access device block descriptors: Density code=0x0 00 00 a0 00 00 00 00 02 00 >> page_code=0x8, page_control=0 00 08 12 00 00 00 00 00 00 00 00 00 00 00 00 00 00 10 00 00 00 00 root@grml ~ # sg_modes /dev/sdb -6 -v -H -H -p 0x3f -d inquiry cdb: 12 00 00 00 24 00 QEMU QEMU HARDDISK 0.13 peripheral_type: disk [0x0] mode sense (6) cdb: 1a 08 3f 00 fc 00 mode sense (6): requested 252 bytes but got 220 bytes Mode parameter header from MODE SENSE(6): 00 1f 00 00 08 Mode data length=32, medium type=0x00, WP=0, DpoFua=0, longlba=0 Block descriptor length=8 > Direct access device block descriptors: Density code=0x0 00 00 a0 00 00 00 00 02 00 >> page_code=0x8, page_control=0 00 08 12 00 00 00 00 00 00 00 00 00 00 00 00 00 00 10 00 00 00 00 root@grml ~ # I will add a patch 5/5 in v2 as you proposed. Bernhard
diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c index 26f7345..519513c 100644 --- a/hw/scsi-disk.c +++ b/hw/scsi-disk.c @@ -642,9 +642,8 @@ static int scsi_disk_emulate_mode_sense(SCSIRequest *req, uint8_t *outbuf) else /* MODE_SENSE_10 */ outbuf[7] = 8; /* Block descriptor length */ nb_sectors /= s->cluster_size; - nb_sectors--; if (nb_sectors > 0xffffff) - nb_sectors = 0xffffff; + nb_sectors = 0; p[0] = 0; /* media density code */ p[1] = (nb_sectors >> 16) & 0xff; p[2] = (nb_sectors >> 8) & 0xff;
The block descriptor contains the number of blocks, not the highest LBA. Real hard disks return 0 if the number of blocks exceed the maximum 0xFFFFFF. SCSI-Spec: http://ldkelley.com/SCSI2/SCSI2/SCSI2-08.html#8.3.3 The number of blocks field specifies the number of logical blocks on the medium to which the density code and block length fields apply. A value of zero indicates that all of the remaining logical blocks of the logical unit shall have the medium characteristics specified. Signed-off-by: Bernhard Kohl <bernhard.kohl@nsn.com> --- hw/scsi-disk.c | 3 +-- 1 files changed, 1 insertions(+), 2 deletions(-)