On Tue, May 10, 2016 at 04:41:27PM +0100, Alex Bligh wrote:
On 10 May 2016, at 16:29, Eric Blake <eblake@...696...> wrote:So the kernel is currently one of the clients that does NOT honor block sizes, and as such, servers should be prepared for ANY size up to UINT_MAX (other than DoS handling).Interesting followup question: If the kernel does not fragment TRIM requests at all (in the same way it fragments read and write requests), I suspect something bad may happen with TRIM requests over 2^31 in size (particularly over 2^32 in size), as the length field in nbd only has 32 bits. Whether it supports block size constraints or not, it is going to need to do *some* breaking up of requests.
Doesn't the kernel break TRIM requests at max_discard_sectors? I remember testing the following change in the nbd kmod long time ago: #if 0 disk->queue->limits.max_discard_sectors = UINT_MAX; #else disk->queue->limits.max_discard_sectors = 65536; #endifThe problem with large TRIM requests is exactly the same as with other (READ/WRITE) large requests -- they _may_ take loads of time and if the client wants to support a fast switch over to another replica it must put some constraints on the timeout value... which may be very easily broken if you allow things like a 1GB trim request on the server using DIO on the other side (and say heavily fragmented sparse file over XFS, never mind).
I don't think it's the problem of QEMU NBD server to fix that, the constraint on the server side is perfectly fine, the problem is to note that a change on the client side is required to limit the maximum size of the TRIM requests. The reason for the patch I pasted above was that at the time I looked into it there was no other way to change the TRIM request size via /sys/block/$dev/queue/max_discard_sectors, but maybe the more recent kernels allow that? Not to mention that the NBD client option to set that at NBD queue setup time would be nice...
Just my 2p. -- Michał Belczyk Snr