qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [Nbd] [PATCH] nbd: fix trim/discard commands with a len


From: Michał Belczyk
Subject: Re: [Qemu-devel] [Nbd] [PATCH] nbd: fix trim/discard commands with a length bigger than NBD_MAX_BUFFER_SIZE
Date: Tue, 10 May 2016 21:13:24 +0200
User-agent: Mutt/1.6.1 (2016-04-27)

On Tue, May 10, 2016 at 04:41:27PM +0100, Alex Bligh wrote:

On 10 May 2016, at 16:29, Eric Blake <address@hidden> 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;
#endif

The 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



reply via email to

[Prev in Thread] Current Thread [Next in Thread]