diff --git a/scripts/vm/hypervisor/kvm/imageserver/backends/nbd.py b/scripts/vm/hypervisor/kvm/imageserver/backends/nbd.py index c68a3b5188b6..002d777080e3 100644 --- a/scripts/vm/hypervisor/kvm/imageserver/backends/nbd.py +++ b/scripts/vm/hypervisor/kvm/imageserver/backends/nbd.py @@ -32,6 +32,24 @@ from .base import BackendSession, ImageBackend +def _entries_to_pairs(entries: Any) -> List[Tuple[int, int]]: + """ + Normalize block status callback entries to (length, flags) pairs. + block_status delivers a flat list [length, flags, ...] while + block_status_64 delivers a list of (length, flags) tuples. + """ + raw = list(entries) + if not raw: + return [] + if isinstance(raw[0], (tuple, list)): + return [(int(length), int(flags)) for length, flags in raw] + if isinstance(raw[0], int): + return [(int(raw[i]), int(raw[i + 1])) for i in range(0, len(raw) - 1, 2)] + raise TypeError( + f"unrecognized block status entry type: {type(raw[0]).__name__}" + ) + + class NbdConnection: """ Low-level helper to connect to an NBD server over a Unix socket. @@ -190,19 +208,16 @@ def extent_cb(*args: Any, **kwargs: Any) -> int: return 0 current = off try: - flat = list(entries) - for i in range(0, len(flat), 2): - if i + 1 >= len(flat): - break - length = int(flat[i]) - flags = int(flat[i + 1]) + for length, flags in _entries_to_pairs(entries): zero = (flags & (NBD_STATE_HOLE | NBD_STATE_ZERO)) != 0 allocation_extents.append( {"start": current, "length": length, "zero": zero} ) current += length - except (TypeError, ValueError, IndexError): - pass + except (TypeError, ValueError, IndexError) as e: + logging.warning( + "get_allocation_extents: unparseable block status entries: %r", e + ) return 0 block_status_fn = getattr( @@ -261,12 +276,7 @@ def extent_cb(*args: Any, **kwargs: Any) -> int: return 0 current = off try: - flat = list(entries) - for i in range(0, len(flat), 2): - if i + 1 >= len(flat): - break - length = int(flat[i]) - flags = int(flat[i + 1]) + for length, flags in _entries_to_pairs(entries): if metacontext == "base:allocation": zero = (flags & (NBD_STATE_HOLE | NBD_STATE_ZERO)) != 0 allocation_extents.append((current, length, zero)) @@ -274,8 +284,11 @@ def extent_cb(*args: Any, **kwargs: Any) -> int: dirty = (flags & NBD_STATE_DIRTY) != 0 dirty_extents.append((current, length, dirty)) current += length - except (TypeError, ValueError, IndexError): - pass + except (TypeError, ValueError, IndexError) as e: + logging.warning( + "get_extents_dirty_and_zero: unparseable block status entries: %r", + e, + ) return 0 block_status_fn = getattr( diff --git a/scripts/vm/hypervisor/kvm/imageserver/handler.py b/scripts/vm/hypervisor/kvm/imageserver/handler.py index 53bef34b95e4..fcdc0b680773 100644 --- a/scripts/vm/hypervisor/kvm/imageserver/handler.py +++ b/scripts/vm/hypervisor/kvm/imageserver/handler.py @@ -746,6 +746,11 @@ def _handle_get_extents_with_backend( dirty_bitmap_ctx = f"qemu:dirty-bitmap:{export_bitmap}" extents = backend.get_dirty_extents(dirty_bitmap_ctx) if is_fallback_dirty_response(extents): + logging.warning( + "EXTENTS image_id=%s: no usable dirty bitmap data for " + "context %r; reporting all allocated extents as dirty", + image_id, dirty_bitmap_ctx, + ) allocation = backend.get_allocation_extents() extents = [ { diff --git a/scripts/vm/hypervisor/kvm/imageserver/tests/test_nbd_entries.py b/scripts/vm/hypervisor/kvm/imageserver/tests/test_nbd_entries.py new file mode 100644 index 000000000000..8654aa5dad6c --- /dev/null +++ b/scripts/vm/hypervisor/kvm/imageserver/tests/test_nbd_entries.py @@ -0,0 +1,194 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +""" +Tests for NBD block status entry parsing. +""" + +import os +import shutil +import subprocess +import tempfile +import unittest + +from imageserver.backends.nbd import NbdConnection, _entries_to_pairs +from imageserver.util import is_fallback_dirty_response + +class TestRealLibnbdBlockStatus(unittest.TestCase): + """ + Creates a 1 MiB qcow2 with a persistent dirty bitmap, dirties two + 64 KiB regions, exports it via qemu-nbd --bitmap, and asserts both the + raw callback entry format and the parsed extents. Fails loudly if a + new libnbd version changes the block status response. + """ + + REAL_BITMAP = "bm0" + KB64 = 65536 + REAL_SIZE = 16 * KB64 # 1 MiB + + @classmethod + def setUpClass(cls): + cls._tmp = tempfile.mkdtemp(prefix="nbd_entries_test_") + cls._img = os.path.join(cls._tmp, "img.qcow2") + cls._sock = os.path.join(cls._tmp, "nbd.sock") + cls._proc = None + + def run(*cmd): + subprocess.run(cmd, check=True, capture_output=True) + + try: + run("qemu-img", "create", "-f", "qcow2", cls._img, str(cls.REAL_SIZE)) + # allocate 0-192 KiB before the bitmap exists (clean, allocated) + run("qemu-io", "-f", "qcow2", "-c", "write -P 0xaa 0 192k", cls._img) + run("qemu-img", "bitmap", "--add", cls._img, cls.REAL_BITMAP) + # dirty two 64 KiB regions while the bitmap is recording + run("qemu-io", "-f", "qcow2", "-c", "write -P 0xbb 128k 64k", cls._img) + run("qemu-io", "-f", "qcow2", "-c", "write -P 0xcc 512k 64k", cls._img) + + cls._proc = subprocess.Popen( + [ + "qemu-nbd", + "--socket", cls._sock, + "--format", "qcow2", + "--persistent", + "--shared=0", + "--read-only", + f"--bitmap={cls.REAL_BITMAP}", + cls._img, + ], + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + ) + from .test_base import _wait_for_nbd_socket + + _wait_for_nbd_socket(cls._sock) + except BaseException: + cls._teardown() + raise + + @classmethod + def tearDownClass(cls): + cls._teardown() + + @classmethod + def _teardown(cls): + if cls._proc is not None: + for pipe in (cls._proc.stdout, cls._proc.stderr): + if pipe: + pipe.close() + cls._proc.terminate() + try: + cls._proc.wait(timeout=5) + except subprocess.TimeoutExpired: + cls._proc.kill() + cls._proc.wait(timeout=5) + cls._proc = None + shutil.rmtree(cls._tmp, ignore_errors=True) + + def _connect(self): + return NbdConnection( + self._sock, + None, + need_block_status=True, + extra_meta_contexts=[f"qemu:dirty-bitmap:{self.REAL_BITMAP}"], + ) + + def test_raw_entries_format_is_known(self): + """The raw callback entries must be flat ints or (length, flags) pairs.""" + with self._connect() as conn: + size = conn.size() + captured = [] + + def cb(*args, **kwargs): + self.assertGreaterEqual(len(args), 3) + captured.append((args[0], list(args[2]))) + return 0 + + fn = getattr( + conn._nbd, + "block_status_64", + getattr(conn._nbd, "block_status", None), + ) + self.assertIsNotNone( + fn, "libnbd no longer exposes block_status/block_status_64" + ) + fn(size, 0, cb) + + self.assertTrue(captured, "block status delivered no callbacks") + contexts = {ctx for ctx, _ in captured} + self.assertIn("base:allocation", contexts) + self.assertIn(f"qemu:dirty-bitmap:{self.REAL_BITMAP}", contexts) + + for ctx, entries in captured: + self.assertTrue(entries, f"empty entries for context {ctx}") + if isinstance(entries[0], (tuple, list)): + for item in entries: + self.assertIsInstance(item, (tuple, list)) + self.assertEqual(len(item), 2) + self.assertIsInstance(item[0], int) + self.assertIsInstance(item[1], int) + else: + self.assertTrue(all(isinstance(v, int) for v in entries)) + self.assertEqual(len(entries) % 2, 0) + # entries must parse and tile part of the queried range + pairs = _entries_to_pairs(entries) + self.assertTrue(pairs) + covered = sum(length for length, _ in pairs) + self.assertGreater(covered, 0) + self.assertLessEqual(covered, size) + + def test_allocation_extents_tile_image(self): + with self._connect() as conn: + size = conn.size() + extents = conn.get_allocation_extents() + self.assertEqual(size, self.REAL_SIZE) + pos = 0 + for e in extents: + self.assertEqual(e["start"], pos) + self.assertGreater(e["length"], 0) + pos += e["length"] + self.assertEqual(pos, size) + self.assertEqual( + extents, + [ + {"start": 0, "length": 3 * self.KB64, "zero": False}, + {"start": 3 * self.KB64, "length": 5 * self.KB64, "zero": True}, + {"start": 8 * self.KB64, "length": self.KB64, "zero": False}, + {"start": 9 * self.KB64, "length": 7 * self.KB64, "zero": True}, + ], + ) + + def test_dirty_extents_match_writes(self): + with self._connect() as conn: + extents = conn.get_extents_dirty_and_zero( + f"qemu:dirty-bitmap:{self.REAL_BITMAP}" + ) + self.assertEqual( + extents, + [ + {"start": 0, "length": 2 * self.KB64, "dirty": False, "zero": False}, + {"start": 2 * self.KB64, "length": self.KB64, "dirty": True, "zero": False}, + {"start": 3 * self.KB64, "length": 5 * self.KB64, "dirty": False, "zero": True}, + {"start": 8 * self.KB64, "length": self.KB64, "dirty": True, "zero": False}, + {"start": 9 * self.KB64, "length": 7 * self.KB64, "dirty": False, "zero": True}, + ], + ) + self.assertFalse(is_fallback_dirty_response(extents)) + + +if __name__ == "__main__": + unittest.main()