From: Eric Blake <eblake@redhat.com>
To: qemu-devel@nongnu.org
Cc: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>,
qemu-stable@nongnu.org,
"open list:Network Block Dev..." <qemu-block@nongnu.org>
Subject: [PULL 1/9] nbd: Fix large trim/zero requests
Date: Tue, 28 Jul 2020 10:03:59 -0500 [thread overview]
Message-ID: <20200728150408.291299-2-eblake@redhat.com> (raw)
In-Reply-To: <20200728150408.291299-1-eblake@redhat.com>
Although qemu as NBD client limits requests to <2G, the NBD protocol
allows clients to send requests almost all the way up to 4G. But
because our block layer is not yet 64-bit clean, we accidentally wrap
such requests into a negative size, and fail with EIO instead of
performing the intended operation.
The bug is visible in modern systems with something as simple as:
$ qemu-img create -f qcow2 /tmp/image.img 5G
$ sudo qemu-nbd --connect=/dev/nbd0 /tmp/image.img
$ sudo blkdiscard /dev/nbd0
or with user-space only:
$ truncate --size=3G file
$ qemu-nbd -f raw file
$ nbdsh -u nbd://localhost:10809 -c 'h.trim(3*1024*1024*1024,0)'
Although both blk_co_pdiscard and blk_pwrite_zeroes currently return 0
on success, this is also a good time to fix our code to a more robust
paradigm that treats all non-negative values as success.
Alas, our iotests do not currently make it easy to add external
dependencies on blkdiscard or nbdsh, so we have to rely on manual
testing for now.
This patch can be reverted when we later improve the overall block
layer to be 64-bit clean, but for now, a minimal fix was deemed less
risky prior to release.
CC: qemu-stable@nongnu.org
Fixes: 1f4d6d18ed
Fixes: 1c6c4bb7f0
Fixes: https://github.com/systemd/systemd/issues/16242
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <20200722212231.535072-1-eblake@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
[eblake: rework success tests to use >=0]
---
nbd/server.c | 28 +++++++++++++++++++++++-----
1 file changed, 23 insertions(+), 5 deletions(-)
diff --git a/nbd/server.c b/nbd/server.c
index 4752a6c8bc07..c5d71cff1001 100644
--- a/nbd/server.c
+++ b/nbd/server.c
@@ -2378,8 +2378,17 @@ static coroutine_fn int nbd_handle_request(NBDClient *client,
if (request->flags & NBD_CMD_FLAG_FAST_ZERO) {
flags |= BDRV_REQ_NO_FALLBACK;
}
- ret = blk_pwrite_zeroes(exp->blk, request->from + exp->dev_offset,
- request->len, flags);
+ ret = 0;
+ /* FIXME simplify this when blk_pwrite_zeroes switches to 64-bit */
+ while (ret >= 0 && request->len) {
+ int align = client->check_align ?: 1;
+ int len = MIN(request->len, QEMU_ALIGN_DOWN(BDRV_REQUEST_MAX_BYTES,
+ align));
+ ret = blk_pwrite_zeroes(exp->blk, request->from + exp->dev_offset,
+ len, flags);
+ request->len -= len;
+ request->from += len;
+ }
return nbd_send_generic_reply(client, request->handle, ret,
"writing to file failed", errp);
@@ -2393,9 +2402,18 @@ static coroutine_fn int nbd_handle_request(NBDClient *client,
"flush failed", errp);
case NBD_CMD_TRIM:
- ret = blk_co_pdiscard(exp->blk, request->from + exp->dev_offset,
- request->len);
- if (ret == 0 && request->flags & NBD_CMD_FLAG_FUA) {
+ ret = 0;
+ /* FIXME simplify this when blk_co_pdiscard switches to 64-bit */
+ while (ret >= 0 && request->len) {
+ int align = client->check_align ?: 1;
+ int len = MIN(request->len, QEMU_ALIGN_DOWN(BDRV_REQUEST_MAX_BYTES,
+ align));
+ ret = blk_co_pdiscard(exp->blk, request->from + exp->dev_offset,
+ len);
+ request->len -= len;
+ request->from += len;
+ }
+ if (ret >= 0 && request->flags & NBD_CMD_FLAG_FUA) {
ret = blk_co_flush(exp->blk);
}
return nbd_send_generic_reply(client, request->handle, ret,
--
2.27.0
next prev parent reply other threads:[~2020-07-28 15:07 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-07-28 15:03 [PULL 0/9] nbd patches for -rc2, 2020-07-28 Eric Blake
2020-07-28 15:03 ` Eric Blake [this message]
2020-07-28 15:04 ` [PULL 2/9] block: nbd: Fix convert qcow2 compressed to nbd Eric Blake
2020-07-28 15:04 ` [PULL 3/9] iotests: Make qemu_nbd_popen() a contextmanager Eric Blake
2020-07-28 15:04 ` [PULL 4/9] iotests: Add more qemu_img helpers Eric Blake
2020-07-28 15:04 ` [PULL 5/9] iotests: Test convert to qcow2 compressed to NBD Eric Blake
2020-07-28 15:04 ` [PULL 6/9] block/nbd: split nbd_establish_connection out of nbd_client_connect Eric Blake
2020-07-28 15:04 ` [PULL 7/9] block/nbd: allow drain during reconnect attempt Eric Blake
2020-07-28 15:04 ` [PULL 8/9] block/nbd: on shutdown terminate connection attempt Eric Blake
2020-07-28 15:04 ` [PULL 9/9] block/nbd: nbd_co_reconnect_loop(): don't sleep if drained Eric Blake
2020-07-28 20:48 ` [PULL 0/9] nbd patches for -rc2, 2020-07-28 Peter Maydell
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20200728150408.291299-2-eblake@redhat.com \
--to=eblake@redhat.com \
--cc=qemu-block@nongnu.org \
--cc=qemu-devel@nongnu.org \
--cc=qemu-stable@nongnu.org \
--cc=vsementsov@virtuozzo.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).