|
From: | Paolo Bonzini |
Subject: | Re: [PATCH v3 3/7] block: add max_hw_transfer to BlockLimits |
Date: | Wed, 16 Jun 2021 15:18:49 +0200 |
User-agent: | Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.10.1 |
On 15/06/21 18:18, Max Reitz wrote:
}+/* Returns the maximum hardware transfer length, in bytes; guaranteed nonzero */+uint64_t blk_get_max_hw_transfer(BlockBackend *blk) +{ + BlockDriverState *bs = blk_bs(blk); + uint64_t max = INT_MAX; + + if (bs) { + max = MIN_NON_ZERO(bs->bl.max_hw_transfer, bs->bl.max_transfer); + } + return max;Both `max_hw_transfer` and `max_transfer` can be 0, so this could return 0, contrary to what the comment above promises.Should `max` be initialized to 0 with a `MIN_NON_ZERO(max, INT_MAX)` here (like `blk_get_max_transfer()` does it)?
Yes, something to that effect.
(As for the rest, I think aligning to the request alignment makes sense, but then again we don’t do that for max_transfer either, so... this at least wouldn’t be a new bug.
Ok, will do. I will also add a new patch to align max_transfer to the request alignment.
Regarding the comment, checkpatch complains about it, so it should be fixed so that /* is on its own line.
That makes it different from every other comment in block_int.h though. Is it okay to fix all of them in a follow-up?
Paolo
Speaking of checkpatch, now that I ran it, it also complains about the new line in bdrv_merge_limits() exceeding 80 characters, so that should be fixed, too.)
[Prev in Thread] | Current Thread | [Next in Thread] |