On 26/10/2026 18.59, Peter Xu wrote:
> Cc Stefan & Kevin, for real.
>
> On Mon, Oct 26, 2026 at 01:57:42PM -0400, Peter Xu wrote:
>> On Mon, Oct 26, 2026 at 01:39:58PM +0100, Daniel Gomez wrote:
>>> On 26/10/2026 13.03, Klaus Jensen wrote:
>>>> On Oct 26 11:11, Daniel Gomez wrote:
>> - the existing qemu_iovec_discard_back() was introduced back in 2014 in
>> commit 58f423fbd5, with a mention of fixing
>> /x86_64/ide/bmdma/short_prdt. Then if patch 1 fixed it up due to some
>> regression, first question I have is why it wasn't spot by unit test.
>> Maybe we missed some test? Or others?
I can't seem to find the test sending unaligned requests? discard_back is
only triggered if the caller (the prdt various test) sends unaligned
requests.
For clarity, the patch only restores the behavior back:
The regression (99868af3d0):
@@ -160,8 +161,9 @@ static void dma_blk_cb(void *opaque, int ret)
return;
}
- if (dbs->iov.size & ~BDRV_SECTOR_MASK) {
- qemu_iovec_discard_back(&dbs->iov, dbs->iov.size & ~BDRV_SECTOR_MASK);
+ if (!QEMU_IS_ALIGNED(dbs->iov.size, dbs->align)) {
+ qemu_iovec_discard_back(&dbs->iov,
+ QEMU_ALIGN_DOWN(dbs->iov.size, dbs->align));
Fix:
@@ -174,8 +174,7 @@ static void dma_blk_cb(void *opaque, int ret)
}
if (!QEMU_IS_ALIGNED(dbs->iov.size, dbs->align)) {
- qemu_iovec_discard_back(&dbs->iov,
- QEMU_ALIGN_DOWN(dbs->iov.size, dbs->align));
+ qemu_iovec_discard_back(&dbs->iov, dbs->iov.size % dbs->align);
}
>> - qemu_iovec_discard_back() alone seems risky on its own, which is the
>> fundation of this whole series:
Not really. But this is what triggered patch 2: to ensure requests are
always aligned. This means, qemu_iovec_discard_back() is never in the path
for large IO.
>> - for patch 2, it seems to me it's only fixing the case when discard
>> worked in the middle not at the end.
We don't trigger the discard path. Patch 2, ensures that by sending always
aligned requests.