From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:41521)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <paolo.bonzini@gmail.com>) id 1aTp7F-0007bk-Cb
	for qemu-devel@nongnu.org; Thu, 11 Feb 2016 06:08:54 -0500
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <paolo.bonzini@gmail.com>) id 1aTp7A-0005Mg-D1
	for qemu-devel@nongnu.org; Thu, 11 Feb 2016 06:08:53 -0500
Received: from mail-wm0-x241.google.com ([2a00:1450:400c:c09::241]:33746)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <paolo.bonzini@gmail.com>) id 1aTp7A-0005Ma-3B
	for qemu-devel@nongnu.org; Thu, 11 Feb 2016 06:08:48 -0500
Received: by mail-wm0-x241.google.com with SMTP id c200so9976322wme.0
	for <qemu-devel@nongnu.org>; Thu, 11 Feb 2016 03:08:48 -0800 (PST)
Sender: Paolo Bonzini <paolo.bonzini@gmail.com>
References: <1450284917-10508-1-git-send-email-apyrgio@arrikto.com>
	<1450284917-10508-4-git-send-email-apyrgio@arrikto.com>
From: Paolo Bonzini <pbonzini@redhat.com>
Message-ID: <56BC6BBD.2070907@redhat.com>
Date: Thu, 11 Feb 2016 12:08:45 +0100
MIME-Version: 1.0
In-Reply-To: <1450284917-10508-4-git-send-email-apyrgio@arrikto.com>
Content-Type: text/plain; charset=windows-1252
Content-Transfer-Encoding: 8bit
Subject: Re: [Qemu-devel] [PATCH 3/9] dma-helpers: Do not truncate small
	qiovs
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Alex Pyrgiotis <apyrgio@arrikto.com>, qemu-devel@nongnu.org, Kevin Wolf <kwolf@redhat.com>Alex Pyrgiotis <apyrgio@arrikto.com>



On 16/12/2015 17:55, Alex Pyrgiotis wrote:
> If the size of the qiov is smaller than the sector size, do not truncate
> the qiov, which would effectively make it empty. Instead, allow it to
> pass as is.
> 
> This is necessary for SCSI requests like READ CAPACITY which have small
> buffers, e.g. 32 bytes.
> 
> Signed-off-by: Alex Pyrgiotis <apyrgio@arrikto.com>
> Signed-off-by: Dimitris Aragiorgis <dimara@arrikto.com>
> 
> diff --git a/dma-helpers.c b/dma-helpers.c
> index e1ea7b3..b8f2ae0 100644
> --- a/dma-helpers.c
> +++ b/dma-helpers.c
> @@ -162,7 +162,16 @@ static void dma_map_sg(DMAAIOCB *dbs)
>          return;
>      }
>  
> -    if (dbs->iov.size & ~BDRV_SECTOR_MASK) {
> +    /*
> +     * If the size of the qiov is not a multiple of the sector size, truncate
> +     * the qiov.
> +     *
> +     * NOTE: If the qiov is less than a sector, we can assume that there is a
> +     * reason for it, e.g., a SCSI request such as READ CAPACITY, and we should
> +     * not truncate it.
> +     */

I'm afraid this is wrong.  It is legal to send SCSI requests that are
e.g. 513 bytes in size.  The sector limit is arbitrary.

I think the "if" statement is wrong too, however.  In particular it has
not broken IDE TRIM only because the ATA standard makes 512 bytes the
unit of DMA transfer.  Rather, it is the IDE device that should discard
extra data in the QEMUSGList.

Paolo

> +    if (dbs->iov.size & ~BDRV_SECTOR_MASK &&
> +            dbs->iov.size > BDRV_SECTOR_SIZE) {
>          qemu_iovec_discard_back(&dbs->iov, dbs->iov.size & ~BDRV_SECTOR_MASK);
>      }
>  }
>