From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:60237) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1b5Wwv-0008T6-If for qemu-devel@nongnu.org; Wed, 25 May 2016 07:26:06 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1b5Wwt-0000nn-EL for qemu-devel@nongnu.org; Wed, 25 May 2016 07:26:04 -0400 References: <1464173965-9694-1-git-send-email-eblake@redhat.com> From: Eric Blake Message-ID: <57458BC3.6000606@redhat.com> Date: Wed, 25 May 2016 05:25:55 -0600 MIME-Version: 1.0 In-Reply-To: <1464173965-9694-1-git-send-email-eblake@redhat.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="6UpbNxSrDtEEB252LJofQjT4mgchCT3BC" Subject: Re: [Qemu-devel] [PATCH v2] nbd: Don't trim unrequested bytes List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: qemu-devel@nongnu.org Cc: kwolf@redhat.com, pbonzini@redhat.com, qemu-stable@nongnu.org, qemu-block@nongnu.org This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --6UpbNxSrDtEEB252LJofQjT4mgchCT3BC Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable 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. >=20 > CC: qemu-stable@nongnu.org > Signed-off-by: Eric Blake > --- >=20 > v2: don't underflow request.len on sub-sector requests >=20 > nbd/server.c | 20 ++++++++++++++------ > 1 file changed, 14 insertions(+), 6 deletions(-) >=20 > 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 =3D blk_co_discard(exp->blk, (request.from + exp->dev_offs= et) > - / BDRV_SECTOR_SIZE, > - request.len / BDRV_SECTOR_SIZE); > - if (ret < 0) { > - LOG("discard failed"); > - reply.error =3D -ret; > + /* Ignore unaligned head or tail, until block layer adds byte > + * interface */ > + if (request.len >=3D BDRV_SECTOR_SIZE) { > + request.len -=3D (request.from + request.len) % BDRV_SECTO= R_SIZE; > + ret =3D 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? --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --6UpbNxSrDtEEB252LJofQjT4mgchCT3BC Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 Comment: Public key at http://people.redhat.com/eblake/eblake.gpg Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQEcBAEBCAAGBQJXRYvDAAoJEKeha0olJ0NqOFAH+gNEwYSV132lWnaV6ckYXDFl WYO3UZuLCwvmgrAjz/kDrhLi7sRjO2VvY/iR7zHCM9bb+aW5WFy1zh9EEbZPNNw3 zyUxTfm9bs90MgC6Ys6vRSjWv6dKjO5WkKH/HJQ0RTkpxLquhY35+eWAuqPUcyUL qr1K6HjS1zbT42PJVvRWMj7lOkJluLHW0fgxdQz5d5uo4IHw38vjaBwcAz30MbXe /gB9yhncMi+0LAi+URLPfb/BQo5PsOcDykfygbo2WnXwoZmKL78ilTKCPZ9V1N8h KuqtLHBI+upepXgP5cZQwxp5n3vChoF+9+QTdtDxAGd646F1V3dCebais1kePo0= =/gAe -----END PGP SIGNATURE----- --6UpbNxSrDtEEB252LJofQjT4mgchCT3BC--