From nobody Mon Feb 9 12:47:48 2026 Delivered-To: importer@patchew.org Authentication-Results: mx.zohomail.com; dkim=fail; spf=pass (zohomail.com: domain of gnu.org designates 209.51.188.17 as permitted sender) smtp.mailfrom=qemu-devel-bounces+importer=patchew.org@nongnu.org; dmarc=fail(p=none dis=none) header.from=redhat.com ARC-Seal: i=1; a=rsa-sha256; t=1613679577; cv=none; d=zohomail.com; s=zohoarc; b=mnnfut78ekBk1SrLE7BBlRUMN/uJ8cN0ZpzYcp0pVwjdXlDNap9efaFb167Opm8lLJ5msKgxEeQCH0s+dAHP0AW5ccHSabsyiPFHfYW8sLuNTDHfgKgGMIwGLCsobzSz+bOWAHCeLnLi/pINDwyI0lAhoLRgoJ1DjqA76IL6rwc= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=zohomail.com; s=zohoarc; t=1613679577; h=Content-Type:Content-Transfer-Encoding:Cc:Date:From:In-Reply-To:List-Subscribe:List-Post:List-Id:List-Archive:List-Help:List-Unsubscribe:MIME-Version:Message-ID:References:Sender:Subject:To; bh=2PgYbSCTOsTLlBB4//5cUx+3IxOfNqMe3enY2o2akKc=; b=EzS2f+i2hHlK3cXq+B9WmmH3ZnF1TUb0j9KqSLHBIJD5rrw67rpTXCDXLaZ4Co5ec8y/W6P4sL3TDSf9g+AiVWnSNeo6hNcSD/Lp43zg8SbOA7rNEzI8k12+J9rB3ii5SHqvl062oqUEnjkRMYLF9dhSEzFfYjPSB09lCYIzngY= ARC-Authentication-Results: i=1; mx.zohomail.com; dkim=fail; spf=pass (zohomail.com: domain of gnu.org designates 209.51.188.17 as permitted sender) smtp.mailfrom=qemu-devel-bounces+importer=patchew.org@nongnu.org; dmarc=fail header.from= (p=none dis=none) header.from= Return-Path: Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) by mx.zohomail.com with SMTPS id 1613679577508283.4323063916871; Thu, 18 Feb 2021 12:19:37 -0800 (PST) Received: from localhost ([::1]:47694 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1lCplw-0007IW-AX for importer@patchew.org; Thu, 18 Feb 2021 15:19:36 -0500 Received: from eggs.gnu.org ([2001:470:142:3::10]:46192) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1lCpi7-00058A-CL for qemu-devel@nongnu.org; Thu, 18 Feb 2021 15:15:39 -0500 Received: from us-smtp-delivery-124.mimecast.com ([63.128.21.124]:60727) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA1:256) (Exim 4.90_1) (envelope-from ) id 1lCpi3-0000Bt-4N for qemu-devel@nongnu.org; Thu, 18 Feb 2021 15:15:39 -0500 Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-204-TLaXAl0QPRSSKNS04OF4RQ-1; Thu, 18 Feb 2021 15:15:32 -0500 Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 1C1541020C20; Thu, 18 Feb 2021 20:15:31 +0000 (UTC) Received: from blue.redhat.com (ovpn-113-156.phx2.redhat.com [10.3.113.156]) by smtp.corp.redhat.com (Postfix) with ESMTP id A45FB5B4B2; Thu, 18 Feb 2021 20:15:30 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1613679334; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=2PgYbSCTOsTLlBB4//5cUx+3IxOfNqMe3enY2o2akKc=; b=IoSgwlfy3Gl/xUdWm8HUc6W0wq4xOWsGkCTOyS7REUKlCNt1oE2P/gWph8Vx9guE+4cLRY 29+s9NoiW9zD9+pLMHWKSYLsdH6oTpU2PBPxR4/0iJlD0JREcUhZJh+YmBDVA8uBffIHic 5GjiMG7REasBwaQt3bGrKgpcIh/NINs= X-MC-Unique: TLaXAl0QPRSSKNS04OF4RQ-1 From: Eric Blake To: qemu-devel@nongnu.org Subject: [PATCH 1/5] iotests: Update 241 to expose backing layer fragmentation Date: Thu, 18 Feb 2021 14:15:24 -0600 Message-Id: <20210218201528.127099-2-eblake@redhat.com> In-Reply-To: <20210218201528.127099-1-eblake@redhat.com> References: <20210218201528.127099-1-eblake@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.11 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=eblake@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Transfer-Encoding: quoted-printable Received-SPF: pass (zohomail.com: domain of gnu.org designates 209.51.188.17 as permitted sender) client-ip=209.51.188.17; envelope-from=qemu-devel-bounces+importer=patchew.org@nongnu.org; helo=lists.gnu.org; Received-SPF: pass client-ip=63.128.21.124; envelope-from=eblake@redhat.com; helo=us-smtp-delivery-124.mimecast.com X-Spam_score_int: -27 X-Spam_score: -2.8 X-Spam_bar: -- X-Spam_report: (-2.8 / 5.0 requ) BAYES_00=-1.9, DKIMWL_WL_HIGH=-0.001, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_DNSWL_LOW=-0.7, RCVD_IN_MSPIKE_H4=0.001, RCVD_IN_MSPIKE_WL=0.001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 autolearn=unavailable autolearn_force=no X-Spam_action: no action X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Kevin Wolf , vsementsov@virtuozzo.com, qemu-block@nongnu.org, Max Reitz Errors-To: qemu-devel-bounces+importer=patchew.org@nongnu.org Sender: "Qemu-devel" X-ZohoMail-DKIM: fail (Header signature does not verify) Content-Type: text/plain; charset="utf-8" Previous commits (such as 6e280648, 75d34eb9) have mentioned that our NBD server still sends unaligned fragments when an active layer with large advertised minimum block size is backed by another layer with a smaller block size. Expand the test to actually cover these scenario, by using two different approaches: qcow2 encryption (which forces 512-byte alignment) with an unaligned raw backing file, and blkdebug with a 4k alignment. The encryption test passes with the desired results, but only because the client side works around the server's non-compliance; if you repeat the test manually with tracing turned on, you will see the server sending a status for 1000 bytes of data then 1048 bytes of hole, which is not aligned. But reverting commit 737d3f5244 shows that it is indeed the client working around the bug in the server. Meanwhile, the blkdebug test gives incorrect results: remember, when using x-dirty-bitmap with qemu-img map as a way to sniff alternative metadata contexts, the meanings of "data" and "zero" are determined by that context. Our client workaround is assuming that the fragmented replies can be merged according to base:allocation rules, but those rules do not work for other contexts (merging dirty and clean bitmap should produce dirty; merging allocated and unallocated should produce allocated; see the FIXME for more about the decoded values we expect). Signed-off-by: Eric Blake Reviewed-by: Vladimir Sementsov-Ogievskiy --- tests/qemu-iotests/241 | 60 ++++++++++++++++++++++++++++++++++---- tests/qemu-iotests/241.out | 21 +++++++++++++ 2 files changed, 76 insertions(+), 5 deletions(-) diff --git a/tests/qemu-iotests/241 b/tests/qemu-iotests/241 index c962c8b6075d..5217af82dc65 100755 --- a/tests/qemu-iotests/241 +++ b/tests/qemu-iotests/241 @@ -3,7 +3,7 @@ # # Test qemu-nbd vs. unaligned images # -# Copyright (C) 2018-2019 Red Hat, Inc. +# Copyright (C) 2018-2021 Red Hat, Inc. # # This program is free software; you can redistribute it and/or modify # it under the terms of the GNU General Public License as published by @@ -27,7 +27,7 @@ status=3D1 # failure is the default! _cleanup() { _cleanup_test_img - rm -f "$TEST_DIR/server.log" + rm -f "$TEST_DIR/server.log" "$TEST_IMG_FILE.mid" "$TEST_IMG_FILE.qcow= 2" nbd_server_stop } trap "_cleanup; exit \$status" 0 1 2 3 15 @@ -37,7 +37,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15 . ./common.filter . ./common.nbd -_supported_fmt raw +_supported_fmt raw # although the test also requires use of qcow2 _supported_proto nbd _supported_os Linux _require_command QEMU_NBD @@ -89,11 +89,61 @@ $QEMU_IMG map --output=3Djson "$TEST_IMG" | _filter_qem= u_img_map $QEMU_IO -c map "$TEST_IMG" nbd_server_stop -# Not tested yet: we also want to ensure that qemu as NBD client does +echo +echo "=3D=3D=3D Encrypted qcow2 file backed by unaligned raw image =3D=3D= =3D" +echo + +# Enabling encryption in qcow2 forces 512-alignment +SECRET=3Dsecret,id=3Dsec0,data=3D12345 +$QEMU_IMG create -f qcow2 -b "$TEST_IMG_FILE" -F raw --object "$SECRET" \ + -o encrypt.format=3Dluks,encrypt.key-secret=3Dsec0,encrypt.iter-time=3D1= 0 \ + "$TEST_IMG_FILE.qcow2" 2k | _filter_img_create +nbd_server_start_unix_socket --object "$SECRET" --image-opts \ + driver=3Dqcow2,file.filename=3D"$TEST_IMG_FILE.qcow2",encrypt.key-secret= =3Dsec0 + +$QEMU_NBD_PROG --list -k $nbd_unix_socket | grep '\(size\|min\)' +$QEMU_IMG map --output=3Djson "$TEST_IMG" | _filter_qemu_img_map +$QEMU_IO -c map "$TEST_IMG" +nbd_server_stop + +echo +echo "=3D=3D=3D Use blkdebug for larger alignment than backing layer =3D= =3D=3D" +echo + +$QEMU_IMG create -f qcow2 -o cluster_size=3D1024 \ + "$TEST_IMG_FILE" 2k | _filter_img_create +$QEMU_IMG create -f qcow2 -b "$TEST_IMG_FILE" -F qcow2 -o cluster_size=3D5= 12 \ + "$TEST_IMG_FILE.mid" | _filter_img_create +$QEMU_IMG bitmap --add -g 512 --enable "$TEST_IMG_FILE.mid" b0 +$QEMU_IO -f qcow2 -c 'w 512 512' "$TEST_IMG_FILE.mid" | _filter_qemu_io +$QEMU_IMG create -f qcow2 -b "$TEST_IMG_FILE.mid" -F qcow2 \ + "$TEST_IMG_FILE.qcow2" 4k | _filter_img_create +FILE=3Dimage.file.filename=3D"$TEST_IMG_FILE.qcow2" +nbd_server_start_unix_socket -B b0 -A --image-opts \ + driver=3Dblkdebug,align=3D4096,image.driver=3Dqcow2,image.file.driver=3D= file,"$FILE" + +TEST_IMG=3D"driver=3Dnbd,server.type=3Dunix,server.path=3D$nbd_unix_socket" +$QEMU_NBD_PROG --list -k $nbd_unix_socket | grep '\(size\|min\)' +# FIXME: this should report a single 4k block of "data":false which transl= ates +# to the dirty bitmap being set for at least part of the region; "data":tr= ue +# is wrong unless the entire 4k is clean. +$QEMU_IMG map --output=3Djson --image-opts \ + "$TEST_IMG",x-dirty-bitmap=3Dqemu:dirty-bitmap:b0 | _filter_qemu_img_map + +# FIXME: this should report a single 4k block of "zero":true,"data":true, +# meaning allocated from the backing chain. Using "zero":false,"data":fal= se +# (allocated in active layer) or "zero":false,"data":true (entire region +# unallocated) is wrong. +$QEMU_IMG map --output=3Djson --image-opts \ + "$TEST_IMG",x-dirty-bitmap=3Dqemu:allocation-depth | _filter_qemu_img_map +nbd_server_stop + +# Not tested here: we also want to ensure that qemu as NBD client does # not access beyond the end of a server's advertised unaligned size: # nbdkit -U - memory size=3D513 --run 'qemu-io -f raw -c "r 512 512" $nbd' # However, since qemu as server always rounds up to a sector alignment, -# we would have to use nbdkit to provoke the current client failures. +# we would have to use nbdkit to demonstrate this scenario (see +# commit 9cf638508c for more information). # success, all done echo '*** done' diff --git a/tests/qemu-iotests/241.out b/tests/qemu-iotests/241.out index 75f9f465e522..67aaeed34f50 100644 --- a/tests/qemu-iotests/241.out +++ b/tests/qemu-iotests/241.out @@ -25,4 +25,25 @@ WARNING: Image format was not specified for 'TEST_DIR/t.= raw' and probing guessed [{ "start": 0, "length": 1000, "depth": 0, "zero": false, "data": true, "o= ffset": OFFSET}, { "start": 1000, "length": 24, "depth": 0, "zero": true, "data": true, "of= fset": OFFSET}] 1 KiB (0x400) bytes allocated at offset 0 bytes (0x0) + +=3D=3D=3D Encrypted qcow2 file backed by unaligned raw image =3D=3D=3D + +Formatting 'TEST_DIR/t.IMGFMT.qcow2', fmt=3Dqcow2 size=3D2048 backing_file= =3DTEST_DIR/t.IMGFMT backing_fmt=3DIMGFMT + size: 2048 + min block: 512 +[{ "start": 0, "length": 1024, "depth": 0, "zero": false, "data": true, "o= ffset": OFFSET}, +{ "start": 1024, "length": 1024, "depth": 0, "zero": true, "data": true, "= offset": OFFSET}] +2 KiB (0x800) bytes allocated at offset 0 bytes (0x0) + +=3D=3D=3D Use blkdebug for larger alignment than backing layer =3D=3D=3D + +Formatting 'TEST_DIR/t.IMGFMT', fmt=3Dqcow2 size=3D2048 +Formatting 'TEST_DIR/t.IMGFMT.mid', fmt=3Dqcow2 size=3D2048 backing_file= =3DTEST_DIR/t.IMGFMT backing_fmt=3Dqcow2 +wrote 512/512 bytes at offset 512 +512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) +Formatting 'TEST_DIR/t.IMGFMT.qcow2', fmt=3Dqcow2 size=3D4096 backing_file= =3DTEST_DIR/t.IMGFMT.mid backing_fmt=3Dqcow2 + size: 4096 + min block: 4096 +[{ "start": 0, "length": 4096, "depth": 0, "zero": false, "data": true, "o= ffset": OFFSET}] +[{ "start": 0, "length": 4096, "depth": 0, "zero": false, "data": true, "o= ffset": OFFSET}] *** done --=20 2.30.1 From nobody Mon Feb 9 12:47:48 2026 Delivered-To: importer@patchew.org Authentication-Results: mx.zohomail.com; dkim=fail; spf=pass (zohomail.com: domain of gnu.org designates 209.51.188.17 as permitted sender) smtp.mailfrom=qemu-devel-bounces+importer=patchew.org@nongnu.org; dmarc=fail(p=none dis=none) header.from=redhat.com Return-Path: Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) by mx.zohomail.com with SMTPS id 161367957266088.9521551873936; Thu, 18 Feb 2021 12:19:32 -0800 (PST) Received: from localhost ([::1]:47260 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1lCplr-00076z-6L for importer@patchew.org; Thu, 18 Feb 2021 15:19:31 -0500 Received: from eggs.gnu.org ([2001:470:142:3::10]:46216) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1lCpi9-0005BV-M2 for qemu-devel@nongnu.org; Thu, 18 Feb 2021 15:15:41 -0500 Received: from us-smtp-delivery-124.mimecast.com ([63.128.21.124]:47036) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA1:256) (Exim 4.90_1) (envelope-from ) id 1lCpi6-0000Ex-Jf for qemu-devel@nongnu.org; Thu, 18 Feb 2021 15:15:41 -0500 Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-214-ncKaCDrLN2eI2Oj3cQ3TFg-1; Thu, 18 Feb 2021 15:15:35 -0500 Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id CAA63107ACE4; Thu, 18 Feb 2021 20:15:34 +0000 (UTC) Received: from blue.redhat.com (ovpn-113-156.phx2.redhat.com [10.3.113.156]) by smtp.corp.redhat.com (Postfix) with ESMTP id 5F8B650C0E; Thu, 18 Feb 2021 20:15:31 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1613679337; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=+M4YMKl+P7NllAPbt944VL6IzDLVPLHmcKIYqeBHbN4=; b=YOP5NWcZid7ml6nLFU+hevZqG1VvgZjIzJiLhvvnkJ3eodoylmlTKPDDWHP5xMJIW33MY+ LiFajk2JLH/l4GjOIqv9uODCin5cPSDSwKP6kS/SkACXzTbD5x7zSTD7XjNIPo/jl7+h9R +6YfMwr5t/bTGMsMwumCntMYte6kZvc= X-MC-Unique: ncKaCDrLN2eI2Oj3cQ3TFg-1 From: Eric Blake To: qemu-devel@nongnu.org Subject: [PATCH 2/5] block: Fix BDRV_BLOCK_RAW status to honor alignment Date: Thu, 18 Feb 2021 14:15:25 -0600 Message-Id: <20210218201528.127099-3-eblake@redhat.com> In-Reply-To: <20210218201528.127099-1-eblake@redhat.com> References: <20210218201528.127099-1-eblake@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.11 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=eblake@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Transfer-Encoding: quoted-printable Received-SPF: pass (zohomail.com: domain of gnu.org designates 209.51.188.17 as permitted sender) client-ip=209.51.188.17; envelope-from=qemu-devel-bounces+importer=patchew.org@nongnu.org; helo=lists.gnu.org; Received-SPF: pass client-ip=63.128.21.124; envelope-from=eblake@redhat.com; helo=us-smtp-delivery-124.mimecast.com X-Spam_score_int: -27 X-Spam_score: -2.8 X-Spam_bar: -- X-Spam_report: (-2.8 / 5.0 requ) BAYES_00=-1.9, DKIMWL_WL_HIGH=-0.001, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_DNSWL_LOW=-0.7, RCVD_IN_MSPIKE_H4=0.001, RCVD_IN_MSPIKE_WL=0.001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Fam Zheng , Kevin Wolf , vsementsov@virtuozzo.com, qemu-block@nongnu.org, Max Reitz , Stefan Hajnoczi Errors-To: qemu-devel-bounces+importer=patchew.org@nongnu.org Sender: "Qemu-devel" X-ZohoMail-DKIM: fail (Header signature does not verify) Content-Type: text/plain; charset="utf-8" Previous patches mentioned how the blkdebug filter driver demonstrates a bug present in our NBD server (for example, commit b0245d64); the bug is also present with the raw format driver when probing occurs. Basically, if we specify a particular alignment > 1, but defer the actual block status to the underlying file, and the underlying file has a smaller alignment, then the use of BDRV_BLOCK_RAW to defer to the underlying file can end up with status split at an alignment unacceptable to our layer. Many callers don't care, but NBD does - it is a violation of the NBD protocol to send status or read results split on an unaligned boundary (in 737d3f5244, we taught our 4.0 client to be tolerant of such violations because the problem was even more pronounced with qemu 3.1 as server; but we still need to fix our server for the sake of other stricter clients). This patch lays the groundwork - it adjusts bdrv_block_status to ensure that any time one layer defers to another via BDRV_BLOCK_RAW, the deferral is either truncated down to an aligned boundary, or multiple sub-aligned requests are coalesced into a single representative answer (using an implicit hole beyond EOF as needed). Iotest 241 exposes the effect (when format probing occurred, we don't want block status to subdivide the initial sector, and thus not any other sector either). Similarly, iotest 221 is a good candidate to amend to specifically track the effects; a change to a hole at EOF is not visible if an upper layer does not want alignment smaller than 512. However, this patch alone is not a complete fix - it is still possible to have an active layer with large alignment constraints backed by another layer with smaller constraints; so the next patch will complete the task. In particular, the next patch will introduce some mutual recursion (bdrv_co_common_block_status_above will call this new function rather than directly into bdrv_co_block_status), so some conditions added here (such as a NULL pointer for map or handling a length-0 request) are not reachable until that point. There is one interesting corner case: prior to this patch, ALLOCATED was never returned without either DATA or ZERO also set. But now, if we have two subregions where the first reports status 0 (not allocated), and the second reports ZERO|ALLOCATED but not DATA (preallocated, read sees zero but underlying file has indeterminate contents), then we can end up with a result of just ALLOCATED. However, looking at callers of bdrv_is_allocated does not find any problem with this new return value. What's more, this situation doesn't really happen until the next patch adds support for getting aligned status from backing files (as the use of aligned status in this patch tends to be limited to just the protocol child of a format driver, yet protocol drivers tend to report fully allocated, and only format drivers have unallocated clusters that defer to a backing file in the first place). Signed-off-by: Eric Blake --- block/io.c | 142 +++++++++++++++++++++++++++++++++++-- tests/qemu-iotests/221 | 13 ++++ tests/qemu-iotests/221.out | 9 +++ tests/qemu-iotests/241.out | 3 +- 4 files changed, 161 insertions(+), 6 deletions(-) diff --git a/block/io.c b/block/io.c index ca2dca30070e..4bca775c96b4 100644 --- a/block/io.c +++ b/block/io.c @@ -2325,6 +2325,132 @@ int bdrv_flush_all(void) return result; } +static int coroutine_fn bdrv_co_block_status(BlockDriverState *bs, + bool want_zero, + int64_t offset, int64_t bytes, + int64_t *pnum, int64_t *map, + BlockDriverState **file); + +/* + * Returns an aligned allocation status of the specified disk region. + * + * Wrapper around bdrv_co_block_status() which requires the initial + * @offset and @count to be aligned to @align (must be power of 2), + * and guarantees the resulting @pnum will also be aligned; this may + * require piecing together multiple sub-aligned queries into an + * appropriate coalesced answer, as follows: + * + * - BDRV_BLOCK_DATA is set if the flag is set for at least one subregion + * - BDRV_BLOCK_ZERO is set only if the flag is set for all subregions + * - BDRV_BLOCK_OFFSET_VALID is set only if all subregions are contiguous + * from the same file (@map and @file are then from the first subregion) + * - BDRV_BLOCK_ALLOCATED is set if the flag is set for at least one subre= gion + * - BDRV_BLOCK_EOF is set if the last subregion queried set it (any + * remaining bytes to reach alignment are treated as an implicit hole) + * - BDRV_BLOCK_RAW is never set + */ +static int coroutine_fn bdrv_co_block_status_aligned(BlockDriverState *bs, + uint32_t align, + bool want_zero, + int64_t offset, + int64_t bytes, + int64_t *pnum, + int64_t *map, + BlockDriverState **fi= le) +{ + int ret; + + assert(is_power_of_2(align) && QEMU_IS_ALIGNED(offset | bytes, align)); + ret =3D bdrv_co_block_status(bs, want_zero, offset, bytes, pnum, map, = file); + if (ret < 0) { + return ret; + } + /* 0-length return only possible for 0-length query or beyond EOF */ + if (!*pnum) { + assert(!bytes || ret & BDRV_BLOCK_EOF); + return ret; + } + assert(!(ret & BDRV_BLOCK_RAW)); + + /* + * If initial query ended at EOF, round up to align: the post-EOF + * tail is an implicit hole, but our rules say we can treat that + * like the initial subregion. + */ + if (ret & BDRV_BLOCK_EOF) { + *pnum =3D QEMU_ALIGN_UP(*pnum, align); + assert(*pnum <=3D bytes); + return ret; + } + + /* + * If result is unaligned but not at EOF, it's easier to return + * the aligned subset and then compute the coalesced version over + * just align bytes. + */ + if (*pnum >=3D align) { + *pnum =3D QEMU_ALIGN_DOWN(*pnum, align); + return ret; + } + + /* + * If we got here, we have to merge status for multiple + * subregions. We can't detect if offsets are contiguous unless + * map and file are non-NULL. + */ + if (!map || !file) { + ret &=3D ~BDRV_BLOCK_OFFSET_VALID; + } + while (*pnum < align) { + int ret2; + int64_t pnum2; + int64_t map2; + BlockDriverState *file2; + + ret2 =3D bdrv_co_block_status(bs, want_zero, offset + *pnum, + align - *pnum, &pnum2, &map2, &file2); + if (ret2 < 0) { + return ret2; + } + assert(!(ret2 & BDRV_BLOCK_RAW)); + /* + * A 0-length answer here is a bug - we should not be querying + * beyond EOF. Our rules allow any further bytes in implicit + * hole past EOF to have same treatment as the subregion just + * before EOF. + */ + assert(pnum2 && pnum2 <=3D align - *pnum); + if (ret2 & BDRV_BLOCK_EOF) { + ret |=3D BDRV_BLOCK_EOF; + pnum2 =3D align - *pnum; + } + + /* Now merge the status */ + if (ret2 & BDRV_BLOCK_DATA) { + ret |=3D BDRV_BLOCK_DATA; + } + if (!(ret2 & BDRV_BLOCK_ZERO)) { + ret &=3D ~BDRV_BLOCK_ZERO; + } + if ((ret & BDRV_BLOCK_OFFSET_VALID) && + (!(ret2 & BDRV_BLOCK_OFFSET_VALID) || + *map + *pnum !=3D map2 || *file !=3D file2)) { + ret &=3D ~BDRV_BLOCK_OFFSET_VALID; + if (map) { + *map =3D 0; + } + if (file) { + *file =3D NULL; + } + } + if (ret2 & BDRV_BLOCK_ALLOCATED) { + ret |=3D BDRV_BLOCK_ALLOCATED; + } + *pnum +=3D pnum2; + } + return ret; +} + /* * Returns the allocation status of the specified sectors. * Drivers not implementing the functionality are assumed to not support @@ -2438,7 +2564,17 @@ static int coroutine_fn bdrv_co_block_status(BlockDr= iverState *bs, */ assert(*pnum && QEMU_IS_ALIGNED(*pnum, align) && align > offset - aligned_offset); - if (ret & BDRV_BLOCK_RECURSE) { + if (ret & BDRV_BLOCK_RAW) { + assert(local_file); + ret =3D bdrv_co_block_status_aligned(local_file, align, want_zero, + local_map, *pnum, pnum, &local_= map, + &local_file); + if (ret < 0) { + goto out; + } + assert(!(ret & BDRV_BLOCK_RAW)); + ret |=3D BDRV_BLOCK_RAW; + } else if (ret & BDRV_BLOCK_RECURSE) { assert(ret & BDRV_BLOCK_DATA); assert(ret & BDRV_BLOCK_OFFSET_VALID); assert(!(ret & BDRV_BLOCK_ZERO)); @@ -2453,9 +2589,7 @@ static int coroutine_fn bdrv_co_block_status(BlockDri= verState *bs, } if (ret & BDRV_BLOCK_RAW) { - assert(ret & BDRV_BLOCK_OFFSET_VALID && local_file); - ret =3D bdrv_co_block_status(local_file, want_zero, local_map, - *pnum, pnum, &local_map, &local_file); + ret &=3D ~BDRV_BLOCK_RAW; goto out; } diff --git a/tests/qemu-iotests/221 b/tests/qemu-iotests/221 index c463fd4b113e..6a15e0160b24 100755 --- a/tests/qemu-iotests/221 +++ b/tests/qemu-iotests/221 @@ -46,6 +46,12 @@ echo echo "=3D=3D=3D Check mapping of unaligned raw image =3D=3D=3D" echo +# Note that when we enable format probing by omitting -f, the raw +# layer forces 512-byte alignment and the bytes past EOF take on the +# same status as the rest of the sector; otherwise, we can see the +# implicit hole visible past EOF thanks to the block layer rounding +# sizes up. + _make_test_img 65537 # qemu-img create rounds size up # file-posix allocates the first block of any images when it is created; @@ -55,15 +61,22 @@ _make_test_img 65537 # qemu-img create rounds size up $QEMU_IO -c 'discard 0 65537' "$TEST_IMG" | _filter_qemu_io $QEMU_IMG map --output=3Djson "$TEST_IMG" | _filter_qemu_img_map +$QEMU_IMG map -f $IMGFMT --output=3Djson "$TEST_IMG" | _filter_qemu_img_map +echo truncate --size=3D65537 "$TEST_IMG" # so we resize it and check again $QEMU_IMG map --output=3Djson "$TEST_IMG" | _filter_qemu_img_map +$QEMU_IMG map -f $IMGFMT --output=3Djson "$TEST_IMG" | _filter_qemu_img_map +echo $QEMU_IO -c 'w 65536 1' "$TEST_IMG" | _filter_qemu_io # writing also round= s up $QEMU_IMG map --output=3Djson "$TEST_IMG" | _filter_qemu_img_map +$QEMU_IMG map -f $IMGFMT --output=3Djson "$TEST_IMG" | _filter_qemu_img_map +echo truncate --size=3D65537 "$TEST_IMG" # so we resize it and check again $QEMU_IMG map --output=3Djson "$TEST_IMG" | _filter_qemu_img_map +$QEMU_IMG map -f $IMGFMT --output=3Djson "$TEST_IMG" | _filter_qemu_img_map # success, all done echo '*** done' diff --git a/tests/qemu-iotests/221.out b/tests/qemu-iotests/221.out index 93846c7dabb6..d22b5e00d4f8 100644 --- a/tests/qemu-iotests/221.out +++ b/tests/qemu-iotests/221.out @@ -7,11 +7,20 @@ discard 65537/65537 bytes at offset 0 64.001 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) [{ "start": 0, "length": 66048, "depth": 0, "zero": true, "data": false, "= offset": OFFSET}] [{ "start": 0, "length": 66048, "depth": 0, "zero": true, "data": false, "= offset": OFFSET}] + +[{ "start": 0, "length": 66048, "depth": 0, "zero": true, "data": false, "= offset": OFFSET}] +[{ "start": 0, "length": 66048, "depth": 0, "zero": true, "data": false, "= offset": OFFSET}] + wrote 1/1 bytes at offset 65536 1 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) [{ "start": 0, "length": 65536, "depth": 0, "zero": true, "data": false, "= offset": OFFSET}, +{ "start": 65536, "length": 512, "depth": 0, "zero": false, "data": true, = "offset": OFFSET}] +[{ "start": 0, "length": 65536, "depth": 0, "zero": true, "data": false, "= offset": OFFSET}, { "start": 65536, "length": 1, "depth": 0, "zero": false, "data": true, "o= ffset": OFFSET}, { "start": 65537, "length": 511, "depth": 0, "zero": true, "data": false, = "offset": OFFSET}] + +[{ "start": 0, "length": 65536, "depth": 0, "zero": true, "data": false, "= offset": OFFSET}, +{ "start": 65536, "length": 512, "depth": 0, "zero": false, "data": true, = "offset": OFFSET}] [{ "start": 0, "length": 65536, "depth": 0, "zero": true, "data": false, "= offset": OFFSET}, { "start": 65536, "length": 1, "depth": 0, "zero": false, "data": true, "o= ffset": OFFSET}, { "start": 65537, "length": 511, "depth": 0, "zero": true, "data": false, = "offset": OFFSET}] diff --git a/tests/qemu-iotests/241.out b/tests/qemu-iotests/241.out index 67aaeed34f50..56d3796cf3ac 100644 --- a/tests/qemu-iotests/241.out +++ b/tests/qemu-iotests/241.out @@ -22,8 +22,7 @@ WARNING: Image format was not specified for 'TEST_DIR/t.r= aw' and probing guessed size: 1024 min block: 1 -[{ "start": 0, "length": 1000, "depth": 0, "zero": false, "data": true, "o= ffset": OFFSET}, -{ "start": 1000, "length": 24, "depth": 0, "zero": true, "data": true, "of= fset": OFFSET}] +[{ "start": 0, "length": 1024, "depth": 0, "zero": false, "data": true, "o= ffset": OFFSET}] 1 KiB (0x400) bytes allocated at offset 0 bytes (0x0) =3D=3D=3D Encrypted qcow2 file backed by unaligned raw image =3D=3D=3D --=20 2.30.1 From nobody Mon Feb 9 12:47:48 2026 Delivered-To: importer@patchew.org Authentication-Results: mx.zohomail.com; dkim=fail; spf=pass (zohomail.com: domain of gnu.org designates 209.51.188.17 as permitted sender) smtp.mailfrom=qemu-devel-bounces+importer=patchew.org@nongnu.org; dmarc=fail(p=none dis=none) header.from=redhat.com ARC-Seal: i=1; a=rsa-sha256; t=1613679790; cv=none; d=zohomail.com; s=zohoarc; b=YuubVh/1I/QXnWaXpp7sex4rH5NvGmkgEx0hVGrJTZR7el7rgypcaNmq5e9TVxBE0iW4nQe6din8ZSW/BIbD8Oh3fHgCbfpqAu0ATxLacli6/M/oqTisDGgRe42tOJ8jspEmM7cTmgYHKDVi/0ceiEVY0d77lwD6sIM5C3fCLXc= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=zohomail.com; s=zohoarc; t=1613679790; h=Content-Type:Content-Transfer-Encoding:Cc:Date:From:In-Reply-To:List-Subscribe:List-Post:List-Id:List-Archive:List-Help:List-Unsubscribe:MIME-Version:Message-ID:References:Sender:Subject:To; bh=pbxOVseHrBBO3iTKLsPMDeo7I0onuR+B2h9y93y/HgE=; b=Pbl+aiGYJ3wV8g7huve63ftfFLL+NrLJPilCDxfJMH8wfmagvtc2lKz3bxxgyjxrWKvMvtZQbx86/MsWw3tLq6Xj/Q70i+T7p7KwYbH8dWtCnei5M4DEwjMc3Jo1T17dwhaCXF+ituaPK8IsDag6NCfpdMPKoh5hysFd6YoU1QE= ARC-Authentication-Results: i=1; mx.zohomail.com; dkim=fail; spf=pass (zohomail.com: domain of gnu.org designates 209.51.188.17 as permitted sender) smtp.mailfrom=qemu-devel-bounces+importer=patchew.org@nongnu.org; dmarc=fail header.from= (p=none dis=none) header.from= Return-Path: Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) by mx.zohomail.com with SMTPS id 1613679789867412.35003023357353; Thu, 18 Feb 2021 12:23:09 -0800 (PST) Received: from localhost ([::1]:56954 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1lCppM-0002nP-KW for importer@patchew.org; Thu, 18 Feb 2021 15:23:08 -0500 Received: from eggs.gnu.org ([2001:470:142:3::10]:46260) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1lCpiD-0005HR-BW for qemu-devel@nongnu.org; Thu, 18 Feb 2021 15:15:46 -0500 Received: from us-smtp-delivery-124.mimecast.com ([216.205.24.124]:51291) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA1:256) (Exim 4.90_1) (envelope-from ) id 1lCpiA-0000HN-UT for qemu-devel@nongnu.org; Thu, 18 Feb 2021 15:15:45 -0500 Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-365-NvoyDVBKPoWJQAtUhkIcwQ-1; Thu, 18 Feb 2021 15:15:38 -0500 Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id A36A3427E3; Thu, 18 Feb 2021 20:15:36 +0000 (UTC) Received: from blue.redhat.com (ovpn-113-156.phx2.redhat.com [10.3.113.156]) by smtp.corp.redhat.com (Postfix) with ESMTP id 1489950C0E; Thu, 18 Feb 2021 20:15:35 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1613679342; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=pbxOVseHrBBO3iTKLsPMDeo7I0onuR+B2h9y93y/HgE=; b=APRzaypP+Ovn7mGnW3Kk9FkxLsXZ8rZvkKDD9KBgJS4GOzWhOjiRxKw9cViYkTwvJP3BJD sCczav66AhHDpOqkrvL7pDJFUKkHRuCU4xqFIDEOkGMrROp0T9mk7e5ZL03rtekwrdzx2d Qv+YizHiHsPsMWqp52iaM3VAGAQEy5s= X-MC-Unique: NvoyDVBKPoWJQAtUhkIcwQ-1 From: Eric Blake To: qemu-devel@nongnu.org Subject: [PATCH 3/5] nbd/server: Avoid unaligned read/block_status from backing Date: Thu, 18 Feb 2021 14:15:26 -0600 Message-Id: <20210218201528.127099-4-eblake@redhat.com> In-Reply-To: <20210218201528.127099-1-eblake@redhat.com> References: <20210218201528.127099-1-eblake@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.11 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=eblake@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Transfer-Encoding: quoted-printable Received-SPF: pass (zohomail.com: domain of gnu.org designates 209.51.188.17 as permitted sender) client-ip=209.51.188.17; envelope-from=qemu-devel-bounces+importer=patchew.org@nongnu.org; helo=lists.gnu.org; Received-SPF: pass client-ip=216.205.24.124; envelope-from=eblake@redhat.com; helo=us-smtp-delivery-124.mimecast.com X-Spam_score_int: -27 X-Spam_score: -2.8 X-Spam_bar: -- X-Spam_report: (-2.8 / 5.0 requ) BAYES_00=-1.9, DKIMWL_WL_HIGH=-0.001, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_DNSWL_LOW=-0.7, RCVD_IN_MSPIKE_H3=0.001, RCVD_IN_MSPIKE_WL=0.001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 autolearn=unavailable autolearn_force=no X-Spam_action: no action X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Kevin Wolf , Fam Zheng , vsementsov@virtuozzo.com, Alberto Garcia , qemu-block@nongnu.org, Max Reitz , Stefan Hajnoczi Errors-To: qemu-devel-bounces+importer=patchew.org@nongnu.org Sender: "Qemu-devel" X-ZohoMail-DKIM: fail (Header signature does not verify) Content-Type: text/plain; charset="utf-8" The NBD server code used bdrv_block_status_above() to determine where to fragment structured read and block status replies, and similarly used bdrv_is_allocated_above() for the qemu:allocation-depth context. However, the protocol can only advertise the active layer's minimum block size; if the active layer is backed by another file with smaller alignment, then we can end up leaking unaligned results back through to the client, in violation of the spec. Fix this by exposing a new bdrv_block_status_aligned() function around the recently-added internal bdrv_co_block_status_aligned, to guarantee that all block status answers from backing layers are rounded up to the alignment of the current layer. Note that the underlying function still requires aligned boundaries, but the public function copes with unaligned inputs. The portion of iotest 241 using an encrypted qcow2 file does not change in output, but running it manually with traces shows the improved behavior; furthermore, reverting 737d3f5244 but leaving this patch in place lets the test pass (whereas before the test would fail because the client had to work around the server's non-compliance). Meanwhile, the portion running with blkdebug shows that qemu:allocation-depth now shows the desired output. Note that while this fixes NBD_CMD_READ and NBD_CMD_BLOCK_STATUS for base:allocation and qemu:allocation-depth (because those rely on bdrv_block_status*), we still have a compliance problem with NBD_CMD_BLOCK_STATUS for qemu:dirty-bitmap:NN when visiting a bitmap created at a smaller granularity than what we advertised. That will be addressed in the next patch. Signed-off-by: Eric Blake --- block/coroutines.h | 2 ++ include/block/block.h | 2 ++ block/io.c | 68 +++++++++++++++++++++++++++++++++----- block/quorum.c | 7 ++-- nbd/server.c | 12 +++---- tests/qemu-iotests/241 | 9 +++-- tests/qemu-iotests/241.out | 2 +- 7 files changed, 77 insertions(+), 25 deletions(-) diff --git a/block/coroutines.h b/block/coroutines.h index 4cfb4946e65e..1c0d761c669e 100644 --- a/block/coroutines.h +++ b/block/coroutines.h @@ -41,6 +41,7 @@ bdrv_pwritev(BdrvChild *child, int64_t offset, unsigned i= nt bytes, int coroutine_fn bdrv_co_common_block_status_above(BlockDriverState *bs, BlockDriverState *base, + uint32_t align, bool include_base, bool want_zero, int64_t offset, @@ -52,6 +53,7 @@ bdrv_co_common_block_status_above(BlockDriverState *bs, int generated_co_wrapper bdrv_common_block_status_above(BlockDriverState *bs, BlockDriverState *base, + uint32_t align, bool include_base, bool want_zero, int64_t offset, diff --git a/include/block/block.h b/include/block/block.h index b3f6e509d49d..fcfd3514701e 100644 --- a/include/block/block.h +++ b/include/block/block.h @@ -517,6 +517,8 @@ int bdrv_block_status(BlockDriverState *bs, int64_t off= set, int bdrv_block_status_above(BlockDriverState *bs, BlockDriverState *base, int64_t offset, int64_t bytes, int64_t *pnum, int64_t *map, BlockDriverState **file); +int bdrv_block_status_aligned(BlockDriverState *bs, int64_t offset, + int64_t bytes, int64_t *pnum); int bdrv_is_allocated(BlockDriverState *bs, int64_t offset, int64_t bytes, int64_t *pnum); int bdrv_is_allocated_above(BlockDriverState *top, BlockDriverState *base, diff --git a/block/io.c b/block/io.c index 4bca775c96b4..d239282b4763 100644 --- a/block/io.c +++ b/block/io.c @@ -2656,6 +2656,7 @@ early_out: int coroutine_fn bdrv_co_common_block_status_above(BlockDriverState *bs, BlockDriverState *base, + uint32_t align, bool include_base, bool want_zero, int64_t offset, @@ -2698,8 +2699,8 @@ bdrv_co_common_block_status_above(BlockDriverState *b= s, for (p =3D bdrv_filter_or_cow_bs(bs); include_base || p !=3D base; p =3D bdrv_filter_or_cow_bs(p)) { - ret =3D bdrv_co_block_status(p, want_zero, offset, bytes, pnum, ma= p, - file); + ret =3D bdrv_co_block_status_aligned(p, align, want_zero, offset, = bytes, + pnum, map, file); ++*depth; if (ret < 0) { return ret; @@ -2758,8 +2759,8 @@ int bdrv_block_status_above(BlockDriverState *bs, Blo= ckDriverState *base, int64_t offset, int64_t bytes, int64_t *pnum, int64_t *map, BlockDriverState **file) { - return bdrv_common_block_status_above(bs, base, false, true, offset, b= ytes, - pnum, map, file, NULL); + return bdrv_common_block_status_above(bs, base, 1, false, true, offset, + bytes, pnum, map, file, NULL); } int bdrv_block_status(BlockDriverState *bs, int64_t offset, int64_t bytes, @@ -2786,7 +2787,7 @@ int coroutine_fn bdrv_co_is_zero_fast(BlockDriverStat= e *bs, int64_t offset, return 1; } - ret =3D bdrv_common_block_status_above(bs, NULL, false, false, offset, + ret =3D bdrv_common_block_status_above(bs, NULL, 1, false, false, offs= et, bytes, &pnum, NULL, NULL, NULL); if (ret < 0) { @@ -2796,13 +2797,47 @@ int coroutine_fn bdrv_co_is_zero_fast(BlockDriverSt= ate *bs, int64_t offset, return (pnum =3D=3D bytes) && (ret & BDRV_BLOCK_ZERO); } +/* + * Similar to bdrv_block_status_above(bs, NULL, ...), but ensures that + * the answer matches the minimum alignment of bs (smaller alignments + * in layers above will not leak through to the active layer). It is + * assumed that callers do not care about the resulting mapping of + * offsets to an underlying BDS. + */ +int bdrv_block_status_aligned(BlockDriverState *bs, int64_t offset, + int64_t bytes, int64_t *pnum) +{ + /* Widen the request to aligned boundaries */ + int64_t aligned_offset, aligned_bytes; + uint32_t align =3D bs->bl.request_alignment; + int ret; + + assert(pnum); + aligned_offset =3D QEMU_ALIGN_DOWN(offset, align); + aligned_bytes =3D ROUND_UP(offset + bytes, align) - aligned_offset; + ret =3D bdrv_common_block_status_above(bs, NULL, align, false, true, + aligned_offset, aligned_bytes, + pnum, NULL, NULL, NULL); + if (ret < 0) { + *pnum =3D 0; + return ret; + } + assert(*pnum && QEMU_IS_ALIGNED(*pnum, align) && + align > offset - aligned_offset); + *pnum -=3D offset - aligned_offset; + if (*pnum > bytes) { + *pnum =3D bytes; + } + return ret; +} + int coroutine_fn bdrv_is_allocated(BlockDriverState *bs, int64_t offset, int64_t bytes, int64_t *pnum) { int ret; int64_t dummy; - ret =3D bdrv_common_block_status_above(bs, bs, true, false, offset, + ret =3D bdrv_common_block_status_above(bs, bs, 1, true, false, offset, bytes, pnum ? pnum : &dummy, NULL, NULL, NULL); if (ret < 0) { @@ -2833,13 +2868,28 @@ int bdrv_is_allocated_above(BlockDriverState *top, bool include_base, int64_t offset, int64_t bytes, int64_t *pnum) { + /* Widen the request to aligned boundaries */ + int64_t aligned_offset, aligned_bytes; + uint32_t align =3D top->bl.request_alignment; int depth; - int ret =3D bdrv_common_block_status_above(top, base, include_base, fa= lse, - offset, bytes, pnum, NULL, NU= LL, - &depth); + int ret; + + assert(pnum); + aligned_offset =3D QEMU_ALIGN_DOWN(offset, align); + aligned_bytes =3D ROUND_UP(offset + bytes, align) - aligned_offset; + ret =3D bdrv_common_block_status_above(top, base, align, include_base,= false, + aligned_offset, aligned_bytes, pn= um, + NULL, NULL, &depth); if (ret < 0) { + *pnum =3D 0; return ret; } + assert(*pnum && QEMU_IS_ALIGNED(*pnum, align) && + align > offset - aligned_offset); + *pnum -=3D offset - aligned_offset; + if (*pnum > bytes) { + *pnum =3D bytes; + } if (ret & BDRV_BLOCK_ALLOCATED) { return depth; diff --git a/block/quorum.c b/block/quorum.c index 0bd75450de97..feea9ad8fa87 100644 --- a/block/quorum.c +++ b/block/quorum.c @@ -1230,9 +1230,10 @@ static int coroutine_fn quorum_co_block_status(Block= DriverState *bs, for (i =3D 0; i < s->num_children; i++) { int64_t bytes; - ret =3D bdrv_co_common_block_status_above(s->children[i]->bs, NULL= , false, - want_zero, offset, count, - &bytes, NULL, NULL, NULL); + ret =3D bdrv_co_common_block_status_above(s->children[i]->bs, NULL= , 1, + false, want_zero, offset, + count, &bytes, NULL, NULL, + NULL); if (ret < 0) { quorum_report_bad(QUORUM_OP_TYPE_READ, offset, count, s->children[i]->bs->node_name, ret); diff --git a/nbd/server.c b/nbd/server.c index 7229f487d296..40847276ca64 100644 --- a/nbd/server.c +++ b/nbd/server.c @@ -1923,7 +1923,7 @@ static int coroutine_fn nbd_co_send_structured_error(= NBDClient *client, } /* Do a sparse read and send the structured reply to the client. - * Returns -errno if sending fails. bdrv_block_status_above() failure is + * Returns -errno if sending fails. bdrv_block_status_aligned() failure is * reported to the client, at which point this function succeeds. */ static int coroutine_fn nbd_co_send_sparse_read(NBDClient *client, @@ -1939,10 +1939,9 @@ static int coroutine_fn nbd_co_send_sparse_read(NBDC= lient *client, while (progress < size) { int64_t pnum; - int status =3D bdrv_block_status_above(blk_bs(exp->common.blk), NU= LL, - offset + progress, - size - progress, &pnum, NULL, - NULL); + int status =3D bdrv_block_status_aligned(blk_bs(exp->common.blk), + offset + progress, + size - progress, &pnum); bool final; if (status < 0) { @@ -2080,8 +2079,7 @@ static int blockstatus_to_extents(BlockDriverState *b= s, uint64_t offset, while (bytes) { uint32_t flags; int64_t num; - int ret =3D bdrv_block_status_above(bs, NULL, offset, bytes, &num, - NULL, NULL); + int ret =3D bdrv_block_status_aligned(bs, offset, bytes, &num); if (ret < 0) { return ret; diff --git a/tests/qemu-iotests/241 b/tests/qemu-iotests/241 index 5217af82dc65..49e2bc09e5bc 100755 --- a/tests/qemu-iotests/241 +++ b/tests/qemu-iotests/241 @@ -129,11 +129,10 @@ $QEMU_NBD_PROG --list -k $nbd_unix_socket | grep '\(s= ize\|min\)' # is wrong unless the entire 4k is clean. $QEMU_IMG map --output=3Djson --image-opts \ "$TEST_IMG",x-dirty-bitmap=3Dqemu:dirty-bitmap:b0 | _filter_qemu_img_map - -# FIXME: this should report a single 4k block of "zero":true,"data":true, -# meaning allocated from the backing chain. Using "zero":false,"data":fal= se -# (allocated in active layer) or "zero":false,"data":true (entire region -# unallocated) is wrong. +# Reports a single 4k block of "zero":true,"data":true, meaning allocated +# from the backing chain. Reporting "zero":false,"data":false would be wr= ong +# (nothing is allocated in the active layer), and as would reporting +# "zero":false,"data":true (the entire region is not unallocated). $QEMU_IMG map --output=3Djson --image-opts \ "$TEST_IMG",x-dirty-bitmap=3Dqemu:allocation-depth | _filter_qemu_img_map nbd_server_stop diff --git a/tests/qemu-iotests/241.out b/tests/qemu-iotests/241.out index 56d3796cf3ac..12a899ba9181 100644 --- a/tests/qemu-iotests/241.out +++ b/tests/qemu-iotests/241.out @@ -44,5 +44,5 @@ Formatting 'TEST_DIR/t.IMGFMT.qcow2', fmt=3Dqcow2 size=3D= 4096 backing_file=3DTEST_DIR/ size: 4096 min block: 4096 [{ "start": 0, "length": 4096, "depth": 0, "zero": false, "data": true, "o= ffset": OFFSET}] -[{ "start": 0, "length": 4096, "depth": 0, "zero": false, "data": true, "o= ffset": OFFSET}] +[{ "start": 0, "length": 4096, "depth": 0, "zero": true, "data": true, "of= fset": OFFSET}] *** done --=20 2.30.1 From nobody Mon Feb 9 12:47:48 2026 Delivered-To: importer@patchew.org Authentication-Results: mx.zohomail.com; dkim=fail; spf=pass (zohomail.com: domain of gnu.org designates 209.51.188.17 as permitted sender) smtp.mailfrom=qemu-devel-bounces+importer=patchew.org@nongnu.org; dmarc=fail(p=none dis=none) header.from=redhat.com ARC-Seal: i=1; a=rsa-sha256; t=1613679769; cv=none; d=zohomail.com; s=zohoarc; b=oCEknbmWtOWBoWSUWmK4zWdXN6H9qLV/DgVlkxCwcl0kku26zjrXJrjgli5BWOpIDmkwiYQpT3LimOqTlDPiBym5BnULgq28FZRCOTFxeGv0L6rTsGw3okSZ0/bU/jlmvSyQ2CmYFPtoPs1fJaQCBzHDh42end5od27GABWcv9o= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=zohomail.com; s=zohoarc; t=1613679769; h=Content-Type:Content-Transfer-Encoding:Cc:Date:From:In-Reply-To:List-Subscribe:List-Post:List-Id:List-Archive:List-Help:List-Unsubscribe:MIME-Version:Message-ID:References:Sender:Subject:To; bh=oCOEY2aazGCOIFvFQIRJGGlpCSBGnj3Ux92aLuQ3nr0=; b=MZHxF+feWpoWzR+Jn/ofke80faL8VYkxGLHyEWQKtSNHKykKE5QkvKKbp0gm0GqR9Ry0qQvlbO/LB7ICHnXNfVxCsvFesnICoi4ZviuDIcvJLgGBO53CP5eVul43r4b/7u/Og3mhEjXA8KXldeRm1W3B14UCKtBcTLtuNQ134ds= ARC-Authentication-Results: i=1; mx.zohomail.com; dkim=fail; spf=pass (zohomail.com: domain of gnu.org designates 209.51.188.17 as permitted sender) smtp.mailfrom=qemu-devel-bounces+importer=patchew.org@nongnu.org; dmarc=fail header.from= (p=none dis=none) header.from= Return-Path: Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) by mx.zohomail.com with SMTPS id 1613679768600748.625141497251; Thu, 18 Feb 2021 12:22:48 -0800 (PST) Received: from localhost ([::1]:55742 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1lCpp1-0002JU-Gs for importer@patchew.org; Thu, 18 Feb 2021 15:22:47 -0500 Received: from eggs.gnu.org ([2001:470:142:3::10]:46300) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1lCpiP-0005Kd-Bt for qemu-devel@nongnu.org; Thu, 18 Feb 2021 15:16:02 -0500 Received: from us-smtp-delivery-124.mimecast.com ([63.128.21.124]:58594) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA1:256) (Exim 4.90_1) (envelope-from ) id 1lCpiD-0000Iw-F9 for qemu-devel@nongnu.org; Thu, 18 Feb 2021 15:15:55 -0500 Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-222-AoDuBSwhMnu8XdtqFE8cMw-1; Thu, 18 Feb 2021 15:15:43 -0500 Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 1D103427C1; Thu, 18 Feb 2021 20:15:42 +0000 (UTC) Received: from blue.redhat.com (ovpn-113-156.phx2.redhat.com [10.3.113.156]) by smtp.corp.redhat.com (Postfix) with ESMTP id D0D045B4B0; Thu, 18 Feb 2021 20:15:36 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1613679344; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=oCOEY2aazGCOIFvFQIRJGGlpCSBGnj3Ux92aLuQ3nr0=; b=BM1h4rIfghQNfhjB4bUyFtXz9QVeTZC8eOoNeJIxaW8Pyuk1Xx+TqWMvU0O73tjz4v+V9f 0GINFRsuucvIaMbiNWDtKp9kFOasMv6xHPieO2lsC1F9Fpllpp4p6a5d2HwhMejr96hpep zKZh74yup6Ct563Bo6EmG15Gw5qIQ+s= X-MC-Unique: AoDuBSwhMnu8XdtqFE8cMw-1 From: Eric Blake To: qemu-devel@nongnu.org Subject: [PATCH 4/5] nbd/server: Avoid unaligned dirty-bitmap status Date: Thu, 18 Feb 2021 14:15:27 -0600 Message-Id: <20210218201528.127099-5-eblake@redhat.com> In-Reply-To: <20210218201528.127099-1-eblake@redhat.com> References: <20210218201528.127099-1-eblake@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.11 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=eblake@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Transfer-Encoding: quoted-printable Received-SPF: pass (zohomail.com: domain of gnu.org designates 209.51.188.17 as permitted sender) client-ip=209.51.188.17; envelope-from=qemu-devel-bounces+importer=patchew.org@nongnu.org; helo=lists.gnu.org; Received-SPF: pass client-ip=63.128.21.124; envelope-from=eblake@redhat.com; helo=us-smtp-delivery-124.mimecast.com X-Spam_score_int: -27 X-Spam_score: -2.8 X-Spam_bar: -- X-Spam_report: (-2.8 / 5.0 requ) BAYES_00=-1.9, DKIMWL_WL_HIGH=-0.001, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_DNSWL_LOW=-0.7, RCVD_IN_MSPIKE_H4=0.001, RCVD_IN_MSPIKE_WL=0.001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Kevin Wolf , vsementsov@virtuozzo.com, qemu-block@nongnu.org, Max Reitz Errors-To: qemu-devel-bounces+importer=patchew.org@nongnu.org Sender: "Qemu-devel" X-ZohoMail-DKIM: fail (Header signature does not verify) Content-Type: text/plain; charset="utf-8" The NBD spec requires that responses to NBD_CMD_BLOCK_STATUS be aligned to the server's advertised min_block alignment, if the client agreed to abide by alignments. In general, since dirty bitmap granularity cannot go below 512, and it is hard to provoke qcow2 to go above an alignment of 512, this is not an issue. But now that the block layer can see through filters, it is possible to use blkdebug to show a scenario where where the server is provoked into supplying a non-compliant reply, as show in iotest 241. Thus, it is time to fix the dirty bitmap code to round requests to aligned boundaries. Signed-off-by: Eric Blake --- nbd/server.c | 30 ++++++++++++++++++++++++++---- tests/qemu-iotests/241 | 5 ++--- tests/qemu-iotests/241.out | 2 +- 3 files changed, 29 insertions(+), 8 deletions(-) diff --git a/nbd/server.c b/nbd/server.c index 40847276ca64..31462abaee63 100644 --- a/nbd/server.c +++ b/nbd/server.c @@ -1,5 +1,5 @@ /* - * Copyright (C) 2016-2020 Red Hat, Inc. + * Copyright (C) 2016-2021 Red Hat, Inc. * Copyright (C) 2005 Anthony Liguori * * Network Block Device Server Side @@ -2175,7 +2175,8 @@ static int nbd_co_send_block_status(NBDClient *client= , uint64_t handle, } /* Populate @ea from a dirty bitmap. */ -static void bitmap_to_extents(BdrvDirtyBitmap *bitmap, +static void bitmap_to_extents(uint32_t align, + BdrvDirtyBitmap *bitmap, uint64_t offset, uint64_t length, NBDExtentArray *es) { @@ -2186,10 +2187,23 @@ static void bitmap_to_extents(BdrvDirtyBitmap *bitm= ap, bdrv_dirty_bitmap_lock(bitmap); for (start =3D offset; - bdrv_dirty_bitmap_next_dirty_area(bitmap, start, end, INT32_MAX, + bdrv_dirty_bitmap_next_dirty_area(bitmap, start, end, + INT32_MAX - align + 1, &dirty_start, &dirty_count); start =3D dirty_start + dirty_count) { + /* + * Round unaligned bits: any transition mid-alignment makes + * that entire aligned region appear dirty. + */ + assert(QEMU_IS_ALIGNED(start, align)); + if (!QEMU_IS_ALIGNED(dirty_start, align)) { + dirty_count +=3D dirty_start - QEMU_ALIGN_DOWN(dirty_start, al= ign); + dirty_start =3D QEMU_ALIGN_DOWN(dirty_start, align); + } + if (!QEMU_IS_ALIGNED(dirty_count, align)) { + dirty_count =3D QEMU_ALIGN_UP(dirty_count, align); + } if ((nbd_extent_array_add(es, dirty_start - start, 0) < 0) || (nbd_extent_array_add(es, dirty_count, NBD_STATE_DIRTY) < 0)) { @@ -2214,7 +2228,15 @@ static int nbd_co_send_bitmap(NBDClient *client, uin= t64_t handle, unsigned int nb_extents =3D dont_fragment ? 1 : NBD_MAX_BLOCK_STATUS_E= XTENTS; g_autoptr(NBDExtentArray) ea =3D nbd_extent_array_new(nb_extents); - bitmap_to_extents(bitmap, offset, length, ea); + /* Easiest to just refuse to answer an unaligned query */ + if (client->check_align && + !QEMU_IS_ALIGNED(offset | length, client->check_align)) { + return nbd_co_send_structured_error(client, handle, -EINVAL, + "unaligned dirty bitmap reques= t", + errp); + } + + bitmap_to_extents(client->check_align ?: 1, bitmap, offset, length, ea= ); return nbd_co_send_extents(client, handle, ea, last, context_id, errp); } diff --git a/tests/qemu-iotests/241 b/tests/qemu-iotests/241 index 49e2bc09e5bc..b4d2e1934d66 100755 --- a/tests/qemu-iotests/241 +++ b/tests/qemu-iotests/241 @@ -124,9 +124,8 @@ nbd_server_start_unix_socket -B b0 -A --image-opts \ TEST_IMG=3D"driver=3Dnbd,server.type=3Dunix,server.path=3D$nbd_unix_socket" $QEMU_NBD_PROG --list -k $nbd_unix_socket | grep '\(size\|min\)' -# FIXME: this should report a single 4k block of "data":false which transl= ates -# to the dirty bitmap being set for at least part of the region; "data":tr= ue -# is wrong unless the entire 4k is clean. +# Reports a single 4k block of "data":false, meaning dirty. Reporting +# "data":true would be wrong (the entire region is not clean). $QEMU_IMG map --output=3Djson --image-opts \ "$TEST_IMG",x-dirty-bitmap=3Dqemu:dirty-bitmap:b0 | _filter_qemu_img_map # Reports a single 4k block of "zero":true,"data":true, meaning allocated diff --git a/tests/qemu-iotests/241.out b/tests/qemu-iotests/241.out index 12a899ba9181..2368b71054d3 100644 --- a/tests/qemu-iotests/241.out +++ b/tests/qemu-iotests/241.out @@ -43,6 +43,6 @@ wrote 512/512 bytes at offset 512 Formatting 'TEST_DIR/t.IMGFMT.qcow2', fmt=3Dqcow2 size=3D4096 backing_file= =3DTEST_DIR/t.IMGFMT.mid backing_fmt=3Dqcow2 size: 4096 min block: 4096 -[{ "start": 0, "length": 4096, "depth": 0, "zero": false, "data": true, "o= ffset": OFFSET}] +[{ "start": 0, "length": 4096, "depth": 0, "zero": false, "data": false}] [{ "start": 0, "length": 4096, "depth": 0, "zero": true, "data": true, "of= fset": OFFSET}] *** done --=20 2.30.1 From nobody Mon Feb 9 12:47:48 2026 Delivered-To: importer@patchew.org Authentication-Results: mx.zohomail.com; dkim=fail; spf=pass (zohomail.com: domain of gnu.org designates 209.51.188.17 as permitted sender) smtp.mailfrom=qemu-devel-bounces+importer=patchew.org@nongnu.org; dmarc=fail(p=none dis=none) header.from=redhat.com ARC-Seal: i=1; a=rsa-sha256; t=1613680106; cv=none; d=zohomail.com; s=zohoarc; b=efM+CBjprnMfR64KhETzr8p+LXZS6oTatasvKjMUKSCwUxr/w4dW0biv6+0CDPdNy2PSqsGN6DerP0aWFHR9wFoCc6UM7vF0EbO+uhHbezVMB9FO4RIpV0IlI/tihNg1HyFn+tBoMuyOnNTOofVebAG6KlShGAIUFJeuBJ3oN18= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=zohomail.com; s=zohoarc; t=1613680106; h=Content-Type:Content-Transfer-Encoding:Cc:Date:From:In-Reply-To:List-Subscribe:List-Post:List-Id:List-Archive:List-Help:List-Unsubscribe:MIME-Version:Message-ID:References:Sender:Subject:To; bh=Lb+A7NjLOTHq64qqLSA9PvyrBUIxt4UfWXkvuo5Y8iE=; b=KXSDRHWoefBUQvnpG0N0Ea72JqDORVN5owHKhaLWwvmxbxBNBW6m0DI5aDlPcwUwtE50IE5Edma4gPhEYZYCl09fKMU7Y1CRe1imngrsVjdDN6U6JPdmhd/ZCDygp3tzskAtxTLyd0EcmCRhfsWVVVxxgSUL7ByX9Hfq3/jOeHQ= ARC-Authentication-Results: i=1; mx.zohomail.com; dkim=fail; spf=pass (zohomail.com: domain of gnu.org designates 209.51.188.17 as permitted sender) smtp.mailfrom=qemu-devel-bounces+importer=patchew.org@nongnu.org; dmarc=fail header.from= (p=none dis=none) header.from= Return-Path: Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) by mx.zohomail.com with SMTPS id 1613680105790372.2334511249568; Thu, 18 Feb 2021 12:28:25 -0800 (PST) Received: from localhost ([::1]:40954 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1lCpuS-00085G-JH for importer@patchew.org; Thu, 18 Feb 2021 15:28:24 -0500 Received: from eggs.gnu.org ([2001:470:142:3::10]:46358) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1lCpiX-0005PR-2I for qemu-devel@nongnu.org; Thu, 18 Feb 2021 15:16:05 -0500 Received: from us-smtp-delivery-124.mimecast.com ([216.205.24.124]:52908) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA1:256) (Exim 4.90_1) (envelope-from ) id 1lCpiP-0000L4-3i for qemu-devel@nongnu.org; Thu, 18 Feb 2021 15:16:04 -0500 Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-413-Hbos65TtNxm9zZdJU2_PAw-1; Thu, 18 Feb 2021 15:15:43 -0500 Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id BB93F107ACF9; Thu, 18 Feb 2021 20:15:42 +0000 (UTC) Received: from blue.redhat.com (ovpn-113-156.phx2.redhat.com [10.3.113.156]) by smtp.corp.redhat.com (Postfix) with ESMTP id 4BCFF50C0E; Thu, 18 Feb 2021 20:15:42 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1613679348; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=Lb+A7NjLOTHq64qqLSA9PvyrBUIxt4UfWXkvuo5Y8iE=; b=P3x4NQk7KTssNNnihMlVLywV53/AtUSrEfEFvJKroDLCgg3UTaj8Lr3jl1moMR7FuA9dS8 kd4xnCzdxme4tCJgNcfEOjCdpzuayXB+LowHIFbmQNztx67YPbXJ1m47cNk4WDkS0LSVxO 9pWRg6L9hE0H1bTlSR0z9KV4zHE6QiA= X-MC-Unique: Hbos65TtNxm9zZdJU2_PAw-1 From: Eric Blake To: qemu-devel@nongnu.org Subject: [PATCH 5/5] do not apply: Revert "nbd-client: Work around server BLOCK_STATUS misalignment at EOF" Date: Thu, 18 Feb 2021 14:15:28 -0600 Message-Id: <20210218201528.127099-6-eblake@redhat.com> In-Reply-To: <20210218201528.127099-1-eblake@redhat.com> References: <20210218201528.127099-1-eblake@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.11 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=eblake@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Transfer-Encoding: quoted-printable Received-SPF: pass (zohomail.com: domain of gnu.org designates 209.51.188.17 as permitted sender) client-ip=209.51.188.17; envelope-from=qemu-devel-bounces+importer=patchew.org@nongnu.org; helo=lists.gnu.org; Received-SPF: pass client-ip=216.205.24.124; envelope-from=eblake@redhat.com; helo=us-smtp-delivery-124.mimecast.com X-Spam_score_int: -27 X-Spam_score: -2.8 X-Spam_bar: -- X-Spam_report: (-2.8 / 5.0 requ) BAYES_00=-1.9, DKIMWL_WL_HIGH=-0.001, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_DNSWL_LOW=-0.7, RCVD_IN_MSPIKE_H3=0.001, RCVD_IN_MSPIKE_WL=0.001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Kevin Wolf , vsementsov@virtuozzo.com, qemu-block@nongnu.org, Max Reitz Errors-To: qemu-devel-bounces+importer=patchew.org@nongnu.org Sender: "Qemu-devel" X-ZohoMail-DKIM: fail (Header signature does not verify) Content-Type: text/plain; charset="utf-8" This reverts commit 737d3f524481bb2ef68d3eba1caa636ff143e16a. This is intended only for testing purposes: if you apply this without the rest of the series, iotest 241 no longer benefits from the client side working around server non-compliance. --- block/nbd.c | 30 ++++-------------------------- 1 file changed, 4 insertions(+), 26 deletions(-) diff --git a/block/nbd.c b/block/nbd.c index c26dc5a54f52..34c91f68e150 100644 --- a/block/nbd.c +++ b/block/nbd.c @@ -937,36 +937,14 @@ static int nbd_parse_blockstatus_payload(BDRVNBDState= *s, extent->length =3D payload_advance32(&payload); extent->flags =3D payload_advance32(&payload); - if (extent->length =3D=3D 0) { + if (extent->length =3D=3D 0 || + (s->info.min_block && !QEMU_IS_ALIGNED(extent->length, + s->info.min_block))) { error_setg(errp, "Protocol error: server sent status chunk with " - "zero length"); + "invalid length"); return -EINVAL; } - /* - * A server sending unaligned block status is in violation of the - * protocol, but as qemu-nbd 3.1 is such a server (at least for - * POSIX files that are not a multiple of 512 bytes, since qemu - * rounds files up to 512-byte multiples but lseek(SEEK_HOLE) - * still sees an implicit hole beyond the real EOF), it's nicer to - * work around the misbehaving server. If the request included - * more than the final unaligned block, truncate it back to an - * aligned result; if the request was only the final block, round - * up to the full block and change the status to fully-allocated - * (always a safe status, even if it loses information). - */ - if (s->info.min_block && !QEMU_IS_ALIGNED(extent->length, - s->info.min_block)) { - trace_nbd_parse_blockstatus_compliance("extent length is unaligned= "); - if (extent->length > s->info.min_block) { - extent->length =3D QEMU_ALIGN_DOWN(extent->length, - s->info.min_block); - } else { - extent->length =3D s->info.min_block; - extent->flags =3D 0; - } - } - /* * We used NBD_CMD_FLAG_REQ_ONE, so the server should not have * sent us any more than one extent, nor should it have included --=20 2.30.1