On 18.11.2016 00:01, Eric Blake wrote:
> On 11/17/2016 04:26 PM, Max Reitz wrote:
>> On 17.11.2016 21:13, Eric Blake wrote:
>>> Discard is advisory, so rounding the requests to alignment
>>> boundaries is never semantically wrong from the data that
>>> the guest sees. But at least the Dell Equallogic iSCSI SANs
>>> has an interesting property that its advertised discard
>>> alignment is 15M, yet documents that discarding a sequence
>>> of 1M slices will eventually result in the 15M page being
>>> marked as discarded, and it is possible to observe which
>>> pages have been discarded.
>>>
>
>>> max_pdiscard = QEMU_ALIGN_DOWN(MIN_NON_ZERO(bs->bl.max_pdiscard,
>>> INT_MAX),
>>> align);
>>> - assert(max_pdiscard);
>>> + assert(max_pdiscard >= bs->bl.request_alignment);
>>>
>>> while (count > 0) {
>>> int ret;
>>> - int num = MIN(count, max_pdiscard);
>>> + int num = count;
>>> +
>>> + if (head) {
>>> + /* Make small requests to get to alignment boundaries. */
>>> + num = MIN(count, align - head);
>>> + if (!QEMU_IS_ALIGNED(num, bs->bl.request_alignment)) {
>>> + num %= bs->bl.request_alignment;
>>> + }
>>
>> Could be written as
>>
>> num = num % bs->bl.request_alignment ?: num;
>>
>> But that's up to you.
>>
>> More importantly, is it possible that request_alignment >
>> pdiscard_alignment? In that case, align would be request_alignment, head
>> would be less than request_alignment but could be more than
>> pdiscard_alignment.
>
> pdiscard_alignment can be 0 (no inherent limit); but it if it is
> nonzero, it must be at least request_alignment. The block layer (should
> be, if it is not already) enforcing that as part of the
> .bdrv_refresh_limits() callback contract; at any rate, it is documented
> that way in block_int.hYes, you're right. Thus: Reviewed-by: Max Reitz <mre...@redhat.com>
signature.asc
Description: OpenPGP digital signature
