[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-block] [Qemu-devel] [PATCH v2 0/7] dma-helpers, scsi-block: us
From: |
Paolo Bonzini |
Subject: |
Re: [Qemu-block] [Qemu-devel] [PATCH v2 0/7] dma-helpers, scsi-block: use SG_IO for all I/O on scsi-block |
Date: |
Wed, 25 May 2016 10:45:57 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.0 |
On 25/05/2016 00:59, Mark Cave-Ayland wrote:
> I eventually traced the corruption down to this section of code in
> dma_blk_cb() which was incorrectly truncating the unaligned iovecs:
>
> if (dbs->iov.size & ~BDRV_SECTOR_MASK) {
> qemu_iovec_discard_back(&dbs->iov, dbs->iov.size &
> ~BDRV_SECTOR_MASK);
> }
>
> This was introduced in
> http://lists.gnu.org/archive/html/qemu-devel/2014-07/msg02236.html to
> handle non-sector aligned SG lists, although given that this is a
> restriction of one particular implementation (PRDT) I'm not sure whether
> a plain revert is the correct thing to do or whether an alternative
> solution needs to be found.
Yeah, I have a plan for this bit. It's related to this code I'm adding
in patch 7:
+ /* This is not supported yet. It can only happen if the guest does
+ * reads and writes that are not aligned to one logical sectors
+ * _and_ cover multiple MemoryRegions.
+ */
+ assert(offset % s->qdev.blocksize == 0);
+ assert(iov->size % s->qdev.blocksize == 0);
The idea behind the "if" is that the I/O code cannot deal with a number
of bytes that is not a multiple of the logical sector size. These
assertions could be removed by generalizing the "if" to an arbitrary
mask, in this case s->qdev.blocksize - 1.
There are two things that are wrong however in the logic. First, the
"if" must be moved before the "if (dbs->iov.size & ~BDRV_SECTOR_MASK)",
because the call to qemu_iovec_discard_back can result in a zero-byte
QEMUIOVector. Second, the sg_cur_* variables must be rewound too.
Thanks,
Paolo
- [Qemu-block] [PATCH 3/7] scsi-disk: introduce a common base class, (continued)
- [Qemu-block] [PATCH 3/7] scsi-disk: introduce a common base class, Paolo Bonzini, 2016/05/23
- [Qemu-block] [PATCH 4/7] scsi-disk: introduce dma_readv and dma_writev, Paolo Bonzini, 2016/05/23
- [Qemu-block] [PATCH 6/7] scsi-disk: introduce scsi_disk_req_check_error, Paolo Bonzini, 2016/05/23
- [Qemu-block] [PATCH 7/7] scsi-block: always use SG_IO, Paolo Bonzini, 2016/05/23
- Re: [Qemu-block] [Qemu-devel] [PATCH v2 0/7] dma-helpers, scsi-block: use SG_IO for all I/O on scsi-block, Mark Cave-Ayland, 2016/05/23
Re: [Qemu-block] [PATCH v2 0/7] dma-helpers, scsi-block: use SG_IO for all I/O on scsi-block, Kevin Wolf, 2016/05/25