From patchwork Thu Oct 30 10:38:24 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Paolo Bonzini X-Patchwork-Id: 405010 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id C6EE014007B for ; Thu, 30 Oct 2014 21:39:06 +1100 (AEDT) Received: from localhost ([::1]:52551 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Xjn8B-0000Yc-4p for incoming@patchwork.ozlabs.org; Thu, 30 Oct 2014 06:39:03 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:59826) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Xjn7l-0000Gg-KK for qemu-devel@nongnu.org; Thu, 30 Oct 2014 06:38:43 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Xjn7f-00026d-TN for qemu-devel@nongnu.org; Thu, 30 Oct 2014 06:38:37 -0400 Received: from mail-lb0-x236.google.com ([2a00:1450:4010:c04::236]:49654) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Xjn7f-00025a-EU for qemu-devel@nongnu.org; Thu, 30 Oct 2014 06:38:31 -0400 Received: by mail-lb0-f182.google.com with SMTP id f15so4369940lbj.41 for ; Thu, 30 Oct 2014 03:38:29 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=sender:message-id:date:from:user-agent:mime-version:newsgroups:to :cc:subject:references:in-reply-to:content-type :content-transfer-encoding; bh=Dca4hUegCdy9rO9ej2L7YnUb0q/3W2vysrI9V6f+AsA=; b=JqhWP+ggPh8vvicRYk36UG0XmRKlmTtJ/WMzKukyl0d3tnw61zziuW5/xtiYHdzOyE GPCHp4Nav5P8jUCdAPXqTgv8kj5C/Yz0ct1llqjtfnbSYBP44YKLPptrQAsYJYqd8HXK sOWjzBcoWRIE+WQxyXpcut9gKxQpMC3Byuy2OSuyT9TM5aqZphA4DNOCFWcskUMOhkBh +Nlgmq+4mqIV7Q86vmaDTHW7m+s43QlC9Tn/aDJWa3br3h39CQdOVo4KFQxj6Pmy5VYA zQMboo0zGfWYMiw5n0gYOXCe01/yNu+T0z9gEHEA+NkubUnoIhc41uR8a5Dll33r/HJR WQKw== X-Received: by 10.152.228.140 with SMTP id si12mr17837108lac.66.1414665509832; Thu, 30 Oct 2014 03:38:29 -0700 (PDT) Received: from [192.168.10.150] (net-37-117-142-149.cust.vodafonedsl.it. [37.117.142.149]) by mx.google.com with ESMTPSA id h9sm3038988lae.44.2014.10.30.03.38.27 for (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 30 Oct 2014 03:38:28 -0700 (PDT) Message-ID: <54521520.4000805@redhat.com> Date: Thu, 30 Oct 2014 11:38:24 +0100 From: Paolo Bonzini User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.2.0 MIME-Version: 1.0 Newsgroups: gmane.comp.emulators.qemu To: Hannes Reinecke , qemu-devel@nongnu.org References: <1414569232-21357-1-git-send-email-hare@suse.de> <1414569232-21357-4-git-send-email-hare@suse.de> In-Reply-To: <1414569232-21357-4-git-send-email-hare@suse.de> X-detected-operating-system: by eggs.gnu.org: Error: Malformed IPv6 address (bad octet value). X-Received-From: 2a00:1450:4010:c04::236 Cc: Alexander Graf , Nic Bellinger , Andreas Faerber Subject: Re: [Qemu-devel] [PATCH 03/17] scsi: Rename scsi_cdb_length() to scsi_xfer_length() X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org On 10/29/2014 08:53 AM, Hannes Reinecke wrote: > scsi_cdb_length() does not return the length of the cdb, but > the transfersize encoded in the cdb. So rename it to scsi_xfer_length() > and add a new scsi_cdb_length() which actually does return the > length of the cdb. This makes sense, but it messes up the function names even more. We now have ata_passthrough_*_xfer_size, scsi_xfer_length, scsi_req_length, scsi_req_*_length. Let's standardize on scsi_*_xfer: > With that DEBUG_SCSI can now display the correct CDB buffer. Why would req->cmd.len be wrong though? Are you masking another bug? Paolo > Signed-off-by: Hannes Reinecke > --- > hw/scsi/scsi-bus.c | 31 +++++++++++++++++++------------ > hw/scsi/scsi-disk.c | 2 +- > include/hw/scsi/scsi.h | 3 ++- > 3 files changed, 22 insertions(+), 14 deletions(-) > > diff --git a/hw/scsi/scsi-bus.c b/hw/scsi/scsi-bus.c > index 022a524..919a86c 100644 > --- a/hw/scsi/scsi-bus.c > +++ b/hw/scsi/scsi-bus.c > @@ -879,11 +879,11 @@ uint32_t scsi_data_cdb_length(uint8_t *buf) > if ((buf[0] >> 5) == 0 && buf[4] == 0) { > return 256; > } else { > - return scsi_cdb_length(buf); > + return scsi_xfer_length(buf); > } > } > > -uint32_t scsi_cdb_length(uint8_t *buf) > +uint32_t scsi_xfer_length(uint8_t *buf) > { > switch (buf[0] >> 5) { > case 0: > @@ -906,7 +906,7 @@ uint32_t scsi_cdb_length(uint8_t *buf) > > static int scsi_req_length(SCSICommand *cmd, SCSIDevice *dev, uint8_t *buf) > { > - cmd->xfer = scsi_cdb_length(buf); > + cmd->xfer = scsi_xfer_length(buf); > switch (buf[0]) { > case TEST_UNIT_READY: > case REWIND: > @@ -1213,28 +1213,35 @@ static uint64_t scsi_cmd_lba(SCSICommand *cmd) > return lba; > } > > -int scsi_req_parse_cdb(SCSIDevice *dev, SCSICommand *cmd, uint8_t *buf) > -{ > - int rc; > +int scsi_cdb_length(uint8_t *buf) { > + int cdb_len; > > - cmd->lba = -1; > switch (buf[0] >> 5) { > case 0: > - cmd->len = 6; > + cdb_len = 6; > break; > case 1: > case 2: > - cmd->len = 10; > + cdb_len = 10; > break; > case 4: > - cmd->len = 16; > + cdb_len = 16; > break; > case 5: > - cmd->len = 12; > + cdb_len = 12; > break; > default: > - return -1; > + cdb_len = -1; > } > + return cdb_len; > +} > + > +int scsi_req_parse_cdb(SCSIDevice *dev, SCSICommand *cmd, uint8_t *buf) > +{ > + int rc; > + > + cmd->lba = -1; > + cmd->len = scsi_cdb_length(buf); > > switch (dev->type) { > case TYPE_TAPE: > diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c > index ae9e08d..30e3789 100644 > --- a/hw/scsi/scsi-disk.c > +++ b/hw/scsi/scsi-disk.c > @@ -2393,7 +2393,7 @@ static SCSIRequest *scsi_new_request(SCSIDevice *d, uint32_t tag, uint32_t lun, > DPRINTF("Command: lun=%d tag=0x%x data=0x%02x", lun, tag, buf[0]); > { > int i; > - for (i = 1; i < req->cmd.len; i++) { > + for (i = 1; i < scsi_cdb_length(buf); i++) { > printf(" 0x%02x", buf[i]); > } > printf("\n"); > diff --git a/include/hw/scsi/scsi.h b/include/hw/scsi/scsi.h > index caaa320..4e9bbd1 100644 > --- a/include/hw/scsi/scsi.h > +++ b/include/hw/scsi/scsi.h > @@ -240,7 +240,8 @@ extern const struct SCSISense sense_code_SPACE_ALLOC_FAILED; > #define SENSE_CODE(x) sense_code_ ## x > > uint32_t scsi_data_cdb_length(uint8_t *buf); > -uint32_t scsi_cdb_length(uint8_t *buf); > +uint32_t scsi_xfer_length(uint8_t *buf); > +int scsi_cdb_length(uint8_t *buf); > int scsi_sense_valid(SCSISense sense); > int scsi_build_sense(uint8_t *in_buf, int in_len, > uint8_t *buf, int len, bool fixed); > diff --git a/hw/scsi/scsi-bus.c b/hw/scsi/scsi-bus.c index 7763847..3eccb1b 100644 --- a/hw/scsi/scsi-bus.c +++ b/hw/scsi/scsi-bus.c @@ -820,7 +820,7 @@ static int ata_passthrough_xfer_unit(SCSIDevice *dev, uint8_t *buf) return xfer_unit; } -static int ata_passthrough_12_xfer_size(SCSIDevice *dev, uint8_t *buf) +static int ata_passthrough_12_xfer(SCSIDevice *dev, uint8_t *buf) { int length = buf[2] & 0x3; int xfer; @@ -843,7 +843,7 @@ static int ata_passthrough_12_xfer_size(SCSIDevice *dev, uint8_t *buf) return xfer * unit; } -static int ata_passthrough_16_xfer_size(SCSIDevice *dev, uint8_t *buf) +static int ata_passthrough_16_xfer(SCSIDevice *dev, uint8_t *buf) { int extend = buf[1] & 0x1; int length = buf[2] & 0x3; @@ -874,11 +874,11 @@ uint32_t scsi_data_cdb_length(uint8_t *buf) if ((buf[0] >> 5) == 0 && buf[4] == 0) { return 256; } else { - return scsi_xfer_length(buf); + return scsi_cdb_xfer(buf); } } -uint32_t scsi_xfer_length(uint8_t *buf) +uint32_t scsi_cdb_xfer(uint8_t *buf) { switch (buf[0] >> 5) { case 0: @@ -899,9 +899,9 @@ uint32_t scsi_xfer_length(uint8_t *buf) } } -static int scsi_req_length(SCSICommand *cmd, SCSIDevice *dev, uint8_t *buf) +static int scsi_req_xfer(SCSICommand *cmd, SCSIDevice *dev, uint8_t *buf) { - cmd->xfer = scsi_xfer_length(buf); + cmd->xfer = scsi_cdb_xfer(buf); switch (buf[0]) { case TEST_UNIT_READY: case REWIND: @@ -1032,17 +1032,17 @@ static int scsi_req_length(SCSICommand *cmd, SCSIDevice *dev, uint8_t *buf) /* BLANK command of MMC */ cmd->xfer = 0; } else { - cmd->xfer = ata_passthrough_12_xfer_size(dev, buf); + cmd->xfer = ata_passthrough_12_xfer(dev, buf); } break; case ATA_PASSTHROUGH_16: - cmd->xfer = ata_passthrough_16_xfer_size(dev, buf); + cmd->xfer = ata_passthrough_16_xfer(dev, buf); break; } return 0; } -static int scsi_req_stream_length(SCSICommand *cmd, SCSIDevice *dev, uint8_t *buf) +static int scsi_req_stream_xfer(SCSICommand *cmd, SCSIDevice *dev, uint8_t *buf) { switch (buf[0]) { /* stream commands */ @@ -1097,12 +1097,12 @@ static int scsi_req_stream_length(SCSICommand *cmd, SCSIDevice *dev, uint8_t *bu break; /* generic commands */ default: - return scsi_req_length(cmd, dev, buf); + return scsi_req_xfer(cmd, dev, buf); } return 0; } -static int scsi_req_medium_changer_length(SCSICommand *cmd, SCSIDevice *dev, uint8_t *buf) +static int scsi_req_medium_changer_xfer(SCSICommand *cmd, SCSIDevice *dev, uint8_t *buf) { switch (buf[0]) { /* medium changer commands */ @@ -1119,7 +1119,7 @@ static int scsi_req_medium_changer_length(SCSICommand *cmd, SCSIDevice *dev, uin /* generic commands */ default: - return scsi_req_length(cmd, dev, buf); + return scsi_req_xfer(cmd, dev, buf); } return 0; } @@ -1240,13 +1240,13 @@ int scsi_req_parse_cdb(SCSIDevice *dev, SCSICommand *cmd, uint8_t *buf) switch (dev->type) { case TYPE_TAPE: - rc = scsi_req_stream_length(cmd, dev, buf); + rc = scsi_req_stream_xfer(cmd, dev, buf); break; case TYPE_MEDIUM_CHANGER: - rc = scsi_req_medium_changer_length(cmd, dev, buf); + rc = scsi_req_medium_changer_xfer(cmd, dev, buf); break; default: - rc = scsi_req_length(cmd, dev, buf); + rc = scsi_req_xfer(cmd, dev, buf); break; }