From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=44269 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1Ol3Zu-0007vA-4n for qemu-devel@nongnu.org; Mon, 16 Aug 2010 13:34:31 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.69) (envelope-from ) id 1Ol3Zs-0000Va-MW for qemu-devel@nongnu.org; Mon, 16 Aug 2010 13:34:30 -0400 Received: from mx1.redhat.com ([209.132.183.28]:21844) by eggs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1Ol3Zs-0000VH-BU for qemu-devel@nongnu.org; Mon, 16 Aug 2010 13:34:28 -0400 Message-ID: <4C6976A8.3070206@redhat.com> Date: Mon, 16 Aug 2010 19:34:32 +0200 From: Kevin Wolf MIME-Version: 1.0 Subject: Re: [Qemu-devel] [PATCH 4/4] scsi-disk: fix the block descriptor returned by the MODE SENSE command References: <1280763089-11829-1-git-send-email-bernhard.kohl@nsn.com> <1280763089-11829-5-git-send-email-bernhard.kohl@nsn.com> In-Reply-To: <1280763089-11829-5-git-send-email-bernhard.kohl@nsn.com> Content-Type: text/plain; charset=ISO-8859-15 Content-Transfer-Encoding: 7bit List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Bernhard Kohl Cc: qemu-devel@nongnu.org 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 > --- > 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