From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=54111 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1Ol3ES-00038O-4b for qemu-devel@nongnu.org; Mon, 16 Aug 2010 13:12:21 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.69) (envelope-from ) id 1Ol3EQ-00066N-DI for qemu-devel@nongnu.org; Mon, 16 Aug 2010 13:12:20 -0400 Received: from mx1.redhat.com ([209.132.183.28]:53409) by eggs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1Ol3EQ-00066A-43 for qemu-devel@nongnu.org; Mon, 16 Aug 2010 13:12:18 -0400 Message-ID: <4C697175.5060908@redhat.com> Date: Mon, 16 Aug 2010 19:12:21 +0200 From: Kevin Wolf MIME-Version: 1.0 Subject: Re: [Qemu-devel] [PATCH 3/4] scsi-disk: fix changeable values returned by the MODE SENSE command References: <1280763089-11829-1-git-send-email-bernhard.kohl@nsn.com> <1280763089-11829-4-git-send-email-bernhard.kohl@nsn.com> In-Reply-To: <1280763089-11829-4-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: > If the page control (PC) field in the MODE SENSE command defines Changeable > Values to be returned in the mode pages, don't return any mode page as there > is no support to change any values. > > Signed-off-by: Bernhard Kohl > --- > hw/scsi-disk.c | 9 ++++++--- > 1 files changed, 6 insertions(+), 3 deletions(-) > > diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c > index 927df54..26f7345 100644 > --- a/hw/scsi-disk.c > +++ b/hw/scsi-disk.c > @@ -604,13 +604,15 @@ static int scsi_disk_emulate_mode_sense(SCSIRequest *req, uint8_t *outbuf) > { > SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, req->dev); > uint64_t nb_sectors; > - int page, dbd, buflen; > + int page, dbd, buflen, page_control; > uint8_t *p; > uint8_t dev_specific_param; > > dbd = req->cmd.buf[1] & 0x8; > page = req->cmd.buf[2] & 0x3f; > - DPRINTF("Mode Sense (page %d, len %zd)\n", page, req->cmd.xfer); > + page_control = (req->cmd.buf[2] & 0xc0) >> 6; > + DPRINTF("Mode Sense(%d) (page %d, len %d, page_control %d)\n", > + (req->cmd.buf[0] == MODE_SENSE) ? 6 : 10, page, len, page_control); > memset(outbuf, 0, req->cmd.xfer); > p = outbuf; > > @@ -654,7 +656,8 @@ static int scsi_disk_emulate_mode_sense(SCSIRequest *req, uint8_t *outbuf) > p += 8; > } > > - switch (page) { > + /* Don't return pages if Changeable Values (1) are requested. */ > + if (page_control != 1) switch (page) { Coding style (missing braces, and switch belongs on its own line). > case 0x04: > case 0x05: > case 0x08: I don't think this is enough. Wouldn't this still let the command return successfully? In fact we need it to fail: "If the logical unit does not implement changeable parameters mode pages and the device server receives a MODE SENSE command with 01b in the PC field, then the command shall be terminated with CHECK CONDITION status, with the sense key set to ILLEGAL REQUEST, and the additional sense code set to INVALID FIELD IN CDB." This should do it if I'm not mistaken: if (page_control == 0x01) { return -1; } Kevin