From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:53176) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Ui7DR-0004VV-Bv for qemu-devel@nongnu.org; Thu, 30 May 2013 14:04:53 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Ui7DK-0004j3-VJ for qemu-devel@nongnu.org; Thu, 30 May 2013 14:04:45 -0400 Received: from e37.co.us.ibm.com ([32.97.110.158]:60984) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Ui7DK-0004TO-N5 for qemu-devel@nongnu.org; Thu, 30 May 2013 14:04:38 -0400 Received: from /spool/local by e37.co.us.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Thu, 30 May 2013 12:03:34 -0600 Received: from d03relay04.boulder.ibm.com (d03relay04.boulder.ibm.com [9.17.195.106]) by d03dlp02.boulder.ibm.com (Postfix) with ESMTP id 05BD63E4006D for ; Thu, 30 May 2013 12:00:52 -0600 (MDT) Received: from d03av04.boulder.ibm.com (d03av04.boulder.ibm.com [9.17.195.170]) by d03relay04.boulder.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id r4UI0rYT132992 for ; Thu, 30 May 2013 12:00:54 -0600 Received: from d03av04.boulder.ibm.com (loopback [127.0.0.1]) by d03av04.boulder.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id r4UI0qdS021779 for ; Thu, 30 May 2013 12:00:53 -0600 Message-ID: <51A793CD.1070304@us.ibm.com> Date: Thu, 30 May 2013 11:00:45 -0700 From: Badari Pulavarty MIME-Version: 1.0 References: <1366381460-6041-1-git-send-email-pbonzini@redhat.com> <1366381460-6041-7-git-send-email-pbonzini@redhat.com> <51A4590F.90503@linux.vnet.ibm.com> <51A4644A.6070002@redhat.com> <20130528083352.GA19799@hj.localdomain> <51A47244.9050203@linux.vnet.ibm.com> <51A5C4DB.9040307@linux.vnet.ibm.com> <51A61A74.7050508@us.ibm.com> <20130529221721.GC19601@hj.localdomain> <1369888147.7364.124.camel@haakon3.risingtidesystems.com> <1369892185.7364.134.camel@haakon3.risingtidesystems.com> In-Reply-To: <1369892185.7364.134.camel@haakon3.risingtidesystems.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit Subject: Re: [Qemu-devel] [PATCH 6/9] vhost-scsi: new device supporting the tcm_vhost Linux kernel module List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Nicholas A. Bellinger" Cc: Anthony Liguori , "Michael S. Tsirkin" , qemu-devel@nongnu.org, target-devel , Paolo Bonzini , Asias He , Wenchao Xia On 05/29/2013 10:36 PM, Nicholas A. Bellinger wrote: > On Wed, 2013-05-29 at 21:29 -0700, Nicholas A. Bellinger wrote: >> On Thu, 2013-05-30 at 06:17 +0800, Asias He wrote: >>> On Wed, May 29, 2013 at 08:10:44AM -0700, Badari Pulavarty wrote: >>>> On 05/29/2013 02:05 AM, Wenchao Xia wrote: >>>>> 于 2013-5-28 17:00, Wenchao Xia 写道: >> >> >>>>> I have done a basic test of vhost-scsi, following is the result I'd >>>>> like to post, generally it seems fine: >>>>> >>>>> Result: >>>>> fdisk/mkfs: fdisk can find it, mke2fs works fine. >>>>> mount: can mount it. >>>>> file I/O: dd 90M zero to a file in that disk succeed. >>>> >>>> >>>> I tried without nested kvm. >>>> >>>>> Issues: >>>>> 1) in fdisk -l, sometime timeout with dmesg "end_request: I/O error, >>>>> dev fd0, sector 0", I guess it is caused by nested KVM that failed >>>>> to kick host kernel? >>>> >>>> I don't see this issue. Are you sure "fd0" is actually the scsi device ? >>>> what is "fd0" ? >>>> >>>>> 2) in fdisk -l, it shows 512 bytes larger than the parameter I >>>>> specified in fd_dev_size parameter in configfs on host.(shows >>>>> 104858112 bytes, see the invocation script below) >>>>> >>>> I see the same. For some reason "fdisk -l" in the VM shows >>>> 512-bytes more than the actual size for the file (on the host). >>> Hmm, interesting. Will look into it. >>> >>> Nick, Any ideas here? >>> >> Mmm, fd_get_blocks() is not returning the expected minus one logical >> blocks with a !S_ISBLK() setup. >> >> This is happening for every other backend ->get_blocks() call already, >> and should be happening for the fd_dev_size case as well. >> >> Applying the following to target-pending.git now. >> > Actually sorry, that last patch is not correct.. > > Here's a better one to properly set fd_dev->fd_block_size at configure > time, and use dev_attrib.block_size in fd_get_blocks() to allow for user > defined block_sizes. > > Thanks, > > --nab > > commit 9e309f9307fe644dee8718980bfcb77de91ce38e > Author: Nicholas Bellinger > Date: Wed May 29 21:35:23 2013 -0700 > > target/file: Fix off-by-one READ_CAPACITY bug for !S_ISBLK export > > This patch fixes a bug where FILEIO was incorrectly reporting the number > of logical blocks (+ 1) when using non struct block_device export mode. > > It changes fd_get_blocks() to follow all other backend ->get_blocks() cases, > and reduces the calculated dev_size by one dev->dev_attrib.block_size > number of bytes, and fixes the initial block_size assignment within > fd_configure_device() > > Reported-by: Wenchao Xia > Reported-by: Badari Pulavarty > Signed-off-by: Nicholas Bellinger > > diff --git a/drivers/target/target_core_file.c b/drivers/target/target_core_file.c > index 1b1d544..b11890d 100644 > --- a/drivers/target/target_core_file.c > +++ b/drivers/target/target_core_file.c > @@ -153,6 +153,7 @@ static int fd_configure_device(struct se_device *dev) > struct request_queue *q = bdev_get_queue(inode->i_bdev); > unsigned long long dev_size; > > + fd_dev->fd_block_size = bdev_logical_block_size(inode->i_bdev); > /* > * Determine the number of bytes from i_size_read() minus > * one (1) logical sector from underlying struct block_device > @@ -199,6 +200,7 @@ static int fd_configure_device(struct se_device *dev) > goto fail; > } > > + fd_dev->fd_block_size = FD_BLOCKSIZE; > /* > * Limit UNMAP emulation to 8k Number of LBAs (NoLB) > */ > @@ -217,9 +219,7 @@ static int fd_configure_device(struct se_device *dev) > dev->dev_attrib.max_write_same_len = 0x1000; > } > > - fd_dev->fd_block_size = dev->dev_attrib.hw_block_size; > - > - dev->dev_attrib.hw_block_size = FD_BLOCKSIZE; > + dev->dev_attrib.hw_block_size = fd_dev->fd_block_size; > dev->dev_attrib.hw_max_sectors = FD_MAX_SECTORS; > dev->dev_attrib.hw_queue_depth = FD_MAX_DEVICE_QUEUE_DEPTH; > > @@ -694,11 +694,12 @@ static sector_t fd_get_blocks(struct se_device *dev) > * to handle underlying block_device resize operations. > */ > if (S_ISBLK(i->i_mode)) > - dev_size = (i_size_read(i) - fd_dev->fd_block_size); > + dev_size = i_size_read(i); > else > dev_size = fd_dev->fd_dev_size; > > - return div_u64(dev_size, dev->dev_attrib.block_size); > + return div_u64(dev_size - dev->dev_attrib.block_size, > + dev->dev_attrib.block_size); > } > > static struct sbc_ops fd_sbc_ops = { > > Verified and it shows the correct value now. Thanks, Badari