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: Alex Bligh
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 17:23:21 +0100

On 10 May 2016, at 17:04, Quentin Casasnovas <address@hidden> wrote:

> Alright, I assumed by 'length of an NBD request', the specification was
> talking about the length of.. well, the request as opposed to whatever is
> in the length field of the request header.

With NBD_CMD_TRIM the length in the header field is 32 bit and specifies
the length of data to trim, not the length of the data transferred (which
is none).

> Is there a use case where you'd want to split up a single big TRIM request
> in smaller ones (as in some hardware would not support it or something)?
> Even then, it looks like this splitting up would be hardware dependant and
> better implemented in block device drivers.

Part of the point of the block size extension is to push such limits to the
client.

I could make up use cases (e.g. that performing a multi-gigabyte trim in
a single threaded server will effectively block all other I/O), but the
main reason in my book is orthogonality, and the fact the client needs
to do some breaking up anyway.

> I'm just finding odd that something that fits inside the length field can't
> be used.

That's a different point. That's Qemu's 'Denial of Service Attack'
prevention, *not* maximum block sizes. It isn't dropping it because
of a maximum block size parameter. If it doesn't support the block size
extension which the version you're looking at does not, it's meant
to handle requests up to 2^32-1 long EXCEPT that it MAY error requests
so long as to cause a denial of service attack. As this doesn't fit
into that case (it's a TRIM), it shouldn't be erroring it on that grounds.

I agree Qemu should fix that.

(So in a sense Eric and I are arguing about something irrelevant to
your current problem, which is how this would work /with/ the block
size extensions, as Eric is in the process of implementing them).

>  I do agree with your point number 3, obviously if the lenght
> field type doesn't allow something bigger than a u32, then the kernel has
> to do some breaking up in that case.


-- 
Alex Bligh







reply via email to

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