[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2] nbd: Don't trim unrequested bytes
From: |
Eric Blake |
Subject: |
Re: [Qemu-devel] [PATCH v2] nbd: Don't trim unrequested bytes |
Date: |
Wed, 25 May 2016 05:25:55 -0600 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.3.0 |
On 05/25/2016 04:59 AM, Eric Blake wrote:
> Similar to commit df7b97ff, we are mishandling clients that
> give an unaligned NBD_CMD_TRIM request, and potentially
> trimming bytes that occur before their request; which in turn
> can cause potential unintended data loss (unlikely in
> practice, since most clients are sane and issue aligned trim
> requests). However, while we fixed read and write by switching
> to the byte interfaces of blk_, we don't yet have a byte
> interface for discard. On the other hand, trim is advisory, so
> rounding the user's request to simply ignore the first and last
> unaligned sectors (or the entire request, if it is sub-sector
> in length) is just fine.
>
> CC: address@hidden
> Signed-off-by: Eric Blake <address@hidden>
> ---
>
> v2: don't underflow request.len on sub-sector requests
>
> nbd/server.c | 20 ++++++++++++++------
> 1 file changed, 14 insertions(+), 6 deletions(-)
>
> diff --git a/nbd/server.c b/nbd/server.c
> index fa862cd..b2cfeb9 100644
> --- a/nbd/server.c
> +++ b/nbd/server.c
> @@ -1153,12 +1153,20 @@ static void nbd_trip(void *opaque)
> break;
> case NBD_CMD_TRIM:
> TRACE("Request type is TRIM");
> - ret = blk_co_discard(exp->blk, (request.from + exp->dev_offset)
> - / BDRV_SECTOR_SIZE,
> - request.len / BDRV_SECTOR_SIZE);
> - if (ret < 0) {
> - LOG("discard failed");
> - reply.error = -ret;
> + /* Ignore unaligned head or tail, until block layer adds byte
> + * interface */
> + if (request.len >= BDRV_SECTOR_SIZE) {
> + request.len -= (request.from + request.len) % BDRV_SECTOR_SIZE;
> + ret = blk_co_discard(exp->blk,
> + DIV_ROUND_UP(request.from + exp->dev_offset,
> + BDRV_SECTOR_SIZE),
> + request.len / BDRV_SECTOR_SIZE);
This can end up calling blk_co_discard(, , 0) - do we need to audit
whether all underlying drivers handle a 0-length request, and/or
special-case this in blk_co_discard() to be an explicit no-op?
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature