Virtuozzo QEMU development (svt-core@virtuozzo.com)
 help / color / mirror / Atom feed
From: "Denis V. Lunev" <den@openvz.org>
To: svt-core@virtuozzo.com
Cc: andrey.drobyshev@virtuozzo.com, den@openvz.org
Subject: [PATCH hci-8.0 5/5] iotests/nbd-commands: cover the command flags and sparse replies #VSTOR-119829
Date: Mon, 31 Aug 2026 17:25:28 +0200	[thread overview]
Message-ID: <20260831152528.1350583-6-den@openvz.org> (raw)
In-Reply-To: <20260831152528.1350583-1-den@openvz.org>

From: Denis V. Lunev <den@openvz.org>

No test sends the NBD command flags, so the server paths behind them
go unexercised. Send them from the new client.

A structured read answers a hole with an offset and a length rather
than a cluster of zeroes. Lay out data, a hole and data again, and
read the three arrangements a hole can appear in, since a leading hole
and a hole between two extents take different turns through
nbd_co_send_sparse_read(). NBD_CMD_FLAG_DF asks for one chunk instead,
which is the same layout sent as real zeroes. Check the chunk
boundaries in both forms, and that the data still reads back.

NBD_CMD_FLAG_REQ_ONE caps the extent array at one entry, so the block
status reply covers only the first cluster of the three asked about.
That a reply may describe less than was requested is also how the
server keeps NBD_MAX_BLOCK_STATUS_EXTENTS from being exceeded on a
long fragmented range, and a client assuming full coverage believes
stale status.

NBD_CMD_FLAG_FAST_ZERO becomes BDRV_REQ_NO_FALLBACK. A cluster aligned
zero can be done by marking the cluster, while zeroing part of a
cluster over a backing file needs the read modify write the flag
forbids, so check both the success and the ENOTSUP.

NBD_CMD_FLAG_FUA is added to a write and a trim.

Signed-off-by: Denis V. Lunev <den@openvz.org>
CC: Eric Blake <eblake@redhat.com>
Message-ID: <20260827161002.310688-6-den@openvz.org>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
---
 tests/qemu-iotests/tests/nbd-commands     | 72 ++++++++++++++++++++++-
 tests/qemu-iotests/tests/nbd-commands.out |  4 +-
 2 files changed, 71 insertions(+), 5 deletions(-)

diff --git a/tests/qemu-iotests/tests/nbd-commands b/tests/qemu-iotests/tests/nbd-commands
index 09e811724b1..adfd4a495cb 100755
--- a/tests/qemu-iotests/tests/nbd-commands
+++ b/tests/qemu-iotests/tests/nbd-commands
@@ -25,6 +25,7 @@ nbd: ModuleType
 
 DEPTH_LOCAL = 1
 DEPTH_BACKING = 2
+CLUSTER = 65536
 
 
 class TestNbdCommands(iotests.QMPTestCase):
@@ -90,7 +91,7 @@ class TestNbdCommands(iotests.QMPTestCase):
             self.h.shutdown()
             self.h = None
 
-    def block_status(self, count=size, wanted=None):
+    def block_status(self, count=size, wanted=None, flags=0):
         """Map each meta context in the reply to its list of extents."""
         reply = {}
 
@@ -99,9 +100,9 @@ class TestNbdCommands(iotests.QMPTestCase):
                                                   entries[1::2]))
 
         if wanted is None:
-            self.h.block_status(count, 0, cb)
+            self.h.block_status(count, 0, cb, flags)
         else:
-            self.h.block_status_filter(count, 0, wanted, cb)
+            self.h.block_status_filter(count, 0, wanted, cb, flags)
         return reply
 
     def top_extents(self):
@@ -158,6 +159,71 @@ class TestNbdCommands(iotests.QMPTestCase):
         for wanted in (['base:allocation'], ['qemu:allocation-depth']):
             self.assertEqual(sorted(self.block_status(wanted=wanted)), wanted)
 
+    def read_chunks(self, count, offset, flags=0):
+        chunks = []
+
+        def cb(subbuf, off, status, _err):
+            chunks.append((off, len(subbuf), status))
+
+        self.h.pread_structured(count, offset, cb, flags)
+        return chunks
+
+    def make_sparse(self):
+        """Lay out data, a hole and data again, one cluster each."""
+        self.h.pwrite(b'z' * CLUSTER, 0)
+        self.h.zero(CLUSTER, CLUSTER)
+        self.h.pwrite(b'z' * CLUSTER, 2 * CLUSTER)
+
+    def test_read_sparse_chunks(self):
+        self.make_sparse()
+
+        first = (0, CLUSTER, nbd.READ_DATA)
+        hole = (CLUSTER, CLUSTER, nbd.READ_HOLE)
+        second = (2 * CLUSTER, CLUSTER, nbd.READ_DATA)
+
+        # A hole is an offset and a length, not a cluster of zeroes,
+        # wherever it falls in the reply
+        self.assertEqual(self.read_chunks(2 * CLUSTER, 0), [first, hole])
+        self.assertEqual(self.read_chunks(2 * CLUSTER, CLUSTER),
+                         [hole, second])
+        self.assertEqual(self.read_chunks(3 * CLUSTER, 0),
+                         [first, hole, second])
+
+        self.assertEqual(self.h.pread(CLUSTER, CLUSTER), bytes(CLUSTER))
+        self.assertEqual(self.h.pread(CLUSTER, 0), b'z' * CLUSTER)
+
+    def test_read_dont_fragment(self):
+        self.make_sparse()
+
+        self.assertEqual(self.read_chunks(3 * CLUSTER, 0, nbd.CMD_FLAG_DF),
+                         [(0, 3 * CLUSTER, nbd.READ_DATA)])
+
+    def test_block_status_req_one(self):
+        hole = nbd.STATE_HOLE | nbd.STATE_ZERO
+        self.make_sparse()
+
+        alloc = self.block_status(3 * CLUSTER)['base:allocation']
+        self.assertEqual(alloc, [(CLUSTER, 0), (CLUSTER, hole), (CLUSTER, 0)])
+
+        # One extent, so the reply covers less than was asked for and the
+        # client has to come back for the rest
+        alloc = self.block_status(3 * CLUSTER, flags=nbd.CMD_FLAG_REQ_ONE)
+        self.assertEqual(alloc['base:allocation'], [(CLUSTER, 0)])
+
+    def test_write_and_trim_fua(self):
+        self.h.pwrite(b'y' * 4096, 4096, nbd.CMD_FLAG_FUA)
+        self.assertEqual(self.h.pread(4096, 4096), b'y' * 4096)
+        self.h.trim(4096, 4096, nbd.CMD_FLAG_FUA)
+
+    def test_fast_zero(self):
+        self.h.zero(CLUSTER, CLUSTER, nbd.CMD_FLAG_FAST_ZERO)
+        self.assertEqual(self.h.pread(CLUSTER, CLUSTER), bytes(CLUSTER))
+
+        # Zeroing part of a cluster needs the fallback the flag forbids
+        with self.assertRaises(nbd.Error) as caught:
+            self.h.zero(4096, 4096, nbd.CMD_FLAG_FAST_ZERO)
+        self.assertEqual(caught.exception.errno, 'ENOTSUP')
+
     def test_cache_past_end_of_export(self):
         self.assertRaises(nbd.Error, self.h.cache, size + 1, 0)
 
diff --git a/tests/qemu-iotests/tests/nbd-commands.out b/tests/qemu-iotests/tests/nbd-commands.out
index 2f7d3902f23..281b69efeac 100644
--- a/tests/qemu-iotests/tests/nbd-commands.out
+++ b/tests/qemu-iotests/tests/nbd-commands.out
@@ -1,5 +1,5 @@
-.......
+............
 ----------------------------------------------------------------------
-Ran 7 tests
+Ran 12 tests
 
 OK
-- 
2.53.0


  parent reply	other threads:[~2026-08-31 15:26 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-31 15:25 [PATCH hci-8.0 0/5] nbd/server: accept a large NBD_CMD_CACHE #VSTOR-119829 Denis V. Lunev
2026-08-31 15:25 ` [PATCH hci-8.0 1/5] iotests: add coverage for NBD transmission commands #VSTOR-119829 Denis V. Lunev
2026-09-01 15:51   ` Andrey Drobyshev
2026-08-31 15:25 ` [PATCH hci-8.0 2/5] nbd/server: accept NBD_CMD_CACHE above the maximum payload size #VSTOR-119829 Denis V. Lunev
2026-08-31 15:25 ` [PATCH hci-8.0 3/5] iotests/nbd-commands: exercise the simple and structured reply modes #VSTOR-119829 Denis V. Lunev
2026-08-31 15:25 ` [PATCH hci-8.0 4/5] iotests/nbd-commands: cover NBD_CMD_BLOCK_STATUS with a payload #VSTOR-119829 Denis V. Lunev
2026-08-31 15:25 ` Denis V. Lunev [this message]
2026-09-01 15:50 ` [QEMU HCI-8.0 PATCH 6/5] VZ: iotests/nbd-commands: do not depend on cluster allocation order #VSTOR-119829 Andrey Drobyshev

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=20260831152528.1350583-6-den@openvz.org \
    --to=den@openvz.org \
    --cc=andrey.drobyshev@virtuozzo.com \
    --cc=svt-core@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