Re: [Qemu-devel] [PATCH 4/4] scsi-disk: fix the block descriptor returned by the MODE SENSE command

[Kevin Wolf <kwolf@redhat.com>]

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