From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=33776 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1Ol34s-0001RD-Vo for qemu-devel@nongnu.org; Mon, 16 Aug 2010 13:02:27 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.69) (envelope-from ) id 1Ol34q-0004lE-UI for qemu-devel@nongnu.org; Mon, 16 Aug 2010 13:02:26 -0400 Received: from mx1.redhat.com ([209.132.183.28]:33134) by eggs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1Ol34q-0004l4-Ij for qemu-devel@nongnu.org; Mon, 16 Aug 2010 13:02:24 -0400 Message-ID: <4C696F25.707@redhat.com> Date: Mon, 16 Aug 2010 19:02:29 +0200 From: Kevin Wolf MIME-Version: 1.0 Subject: Re: [Qemu-devel] [PATCH 2/4] scsi-disk: fix the mode data header returned by the MODE SENSE(10) command References: <1280763089-11829-1-git-send-email-bernhard.kohl@nsn.com> <1280763089-11829-3-git-send-email-bernhard.kohl@nsn.com> In-Reply-To: <1280763089-11829-3-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 header for the MODE SENSE(10) command is 8 bytes long. > > Signed-off-by: Bernhard Kohl > --- > hw/scsi-disk.c | 35 ++++++++++++++++++++++++++++------- > 1 files changed, 28 insertions(+), 7 deletions(-) > > diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c > index 57439f4..927df54 100644 > --- a/hw/scsi-disk.c > +++ b/hw/scsi-disk.c > @@ -606,6 +606,7 @@ static int scsi_disk_emulate_mode_sense(SCSIRequest *req, uint8_t *outbuf) > uint64_t nb_sectors; > int page, dbd, buflen; > uint8_t *p; > + uint8_t dev_specific_param; > > dbd = req->cmd.buf[1] & 0x8; > page = req->cmd.buf[2] & 0x3f; > @@ -613,16 +614,31 @@ static int scsi_disk_emulate_mode_sense(SCSIRequest *req, uint8_t *outbuf) > memset(outbuf, 0, req->cmd.xfer); > p = outbuf; > > - p[1] = 0; /* Default media type. */ > - p[3] = 0; /* Block descriptor length. */ > - if (bdrv_is_read_only(s->bs)) { > - p[2] = 0x80; /* Readonly. */ > + if (bdrv_get_type_hint(s->bs) == BDRV_TYPE_CDROM || > + bdrv_is_read_only(s->bs)) { This looks like a mismerge. The check for CDROMs was removed when they became read-only by definition. Please don't reintroduce it. > + dev_specific_param = 0x80; /* Readonly. */ > + } else { > + dev_specific_param = 0x00; > + } > + > + if (req->cmd.buf[0] == MODE_SENSE) { > + p[1] = 0; /* Default media type. */ > + p[2] = dev_specific_param; > + p[3] = 0; /* Block descriptor length. */ > + p += 4; > + } else { /* MODE_SENSE_10 */ > + p[2] = 0; /* Default media type. */ > + p[3] = dev_specific_param; > + p[6] = p[7] = 0; /* Block descriptor length. */ > + p += 8; > } > - p += 4; > > bdrv_get_geometry(s->bs, &nb_sectors); > if ((~dbd) & nb_sectors) { > - outbuf[3] = 8; /* Block descriptor length */ > + if (req->cmd.buf[0] == MODE_SENSE) > + outbuf[3] = 8; /* Block descriptor length */ > + else /* MODE_SENSE_10 */ > + outbuf[7] = 8; /* Block descriptor length */ Please add curly braces here (see CODING_STYLE). Kevin