aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorEric Blake <eblake@redhat.com>2020-07-22 16:22:31 -0500
committerMichael Roth <mdroth@linux.vnet.ibm.com>2020-09-09 18:51:39 -0500
commitb57df52546762549aec6591f5f38523111702fa9 (patch)
tree37b4e5349d5ef4b0f8c0a6bece988d6d49d17be1
parent921352867bc67b2e6bc6fa2e5538b04aadd78caf (diff)
downloadqemu-b57df52546762549aec6591f5f38523111702fa9.zip
qemu-b57df52546762549aec6591f5f38523111702fa9.tar.gz
qemu-b57df52546762549aec6591f5f38523111702fa9.tar.bz2
nbd: Fix large trim/zero requests
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] (cherry picked from commit 890cbccb089db9e646cc1baea3be9dc060e3917b) Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
-rw-r--r--nbd/server.c28
1 files changed, 23 insertions, 5 deletions
diff --git a/nbd/server.c b/nbd/server.c
index 20754e9..0918173 100644
--- a/nbd/server.c
+++ b/nbd/server.c
@@ -2365,8 +2365,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);
@@ -2380,9 +2389,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,