Skip to content

Commit a6b3128

Browse files
nvazquezabh1sar
nvazquez
and
abh1sar
authored
[Veeam] Fix for dirty context not being returned on el9
The dirty extents were not ready correctly for the block_status_64 return and as a result image server always reported the whole image range as dirty. This commit fixes by normalizing the value returned by both block_status and block_status_64. It also adds warning whenever there is an issue getting the dirty extents and the code falls back to returning the full range as dirty. Co-authored-by: Abhisar Sinha <[email protected]>
1 parent 66132f8 commit a6b3128

3 files changed

Lines changed: 228 additions & 16 deletions

File tree

‎scripts/vm/hypervisor/kvm/imageserver/backends/nbd.py‎

Lines changed: 29 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,24 @@
3232
from .base import BackendSession, ImageBackend
3333

3434

35+
def _entries_to_pairs(entries: Any) -> List[Tuple[int, int]]:
36+
"""
37+
Normalize block status callback entries to (length, flags) pairs.
38+
block_status delivers a flat list [length, flags, ...] while
39+
block_status_64 delivers a list of (length, flags) tuples.
40+
"""
41+
raw = list(entries)
42+
if not raw:
43+
return []
44+
if isinstance(raw[0], (tuple, list)):
45+
return [(int(length), int(flags)) for length, flags in raw]
46+
if isinstance(raw[0], int):
47+
return [(int(raw[i]), int(raw[i + 1])) for i in range(0, len(raw) - 1, 2)]
48+
raise TypeError(
49+
f"unrecognized block status entry type: {type(raw[0]).__name__}"
50+
)
51+
52+
3553
class NbdConnection:
3654
"""
3755
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:
190208
return 0
191209
current = off
192210
try:
193-
flat = list(entries)
194-
for i in range(0, len(flat), 2):
195-
if i + 1 >= len(flat):
196-
break
197-
length = int(flat[i])
198-
flags = int(flat[i + 1])
211+
for length, flags in _entries_to_pairs(entries):
199212
zero = (flags & (NBD_STATE_HOLE | NBD_STATE_ZERO)) != 0
200213
allocation_extents.append(
201214
{"start": current, "length": length, "zero": zero}
202215
)
203216
current += length
204-
except (TypeError, ValueError, IndexError):
205-
pass
217+
except (TypeError, ValueError, IndexError) as e:
218+
logging.warning(
219+
"get_allocation_extents: unparseable block status entries: %r", e
220+
)
206221
return 0
207222

208223
block_status_fn = getattr(
@@ -261,21 +276,19 @@ def extent_cb(*args: Any, **kwargs: Any) -> int:
261276
return 0
262277
current = off
263278
try:
264-
flat = list(entries)
265-
for i in range(0, len(flat), 2):
266-
if i + 1 >= len(flat):
267-
break
268-
length = int(flat[i])
269-
flags = int(flat[i + 1])
279+
for length, flags in _entries_to_pairs(entries):
270280
if metacontext == "base:allocation":
271281
zero = (flags & (NBD_STATE_HOLE | NBD_STATE_ZERO)) != 0
272282
allocation_extents.append((current, length, zero))
273283
elif metacontext == dirty_bitmap_context:
274284
dirty = (flags & NBD_STATE_DIRTY) != 0
275285
dirty_extents.append((current, length, dirty))
276286
current += length
277-
except (TypeError, ValueError, IndexError):
278-
pass
287+
except (TypeError, ValueError, IndexError) as e:
288+
logging.warning(
289+
"get_extents_dirty_and_zero: unparseable block status entries: %r",
290+
e,
291+
)
279292
return 0
280293

281294
block_status_fn = getattr(

‎scripts/vm/hypervisor/kvm/imageserver/handler.py‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -746,6 +746,11 @@ def _handle_get_extents_with_backend(
746746
dirty_bitmap_ctx = f"qemu:dirty-bitmap:{export_bitmap}"
747747
extents = backend.get_dirty_extents(dirty_bitmap_ctx)
748748
if is_fallback_dirty_response(extents):
749+
logging.warning(
750+
"EXTENTS image_id=%s: no usable dirty bitmap data for "
751+
"context %r; reporting all allocated extents as dirty",
752+
image_id, dirty_bitmap_ctx,
753+
)
749754
allocation = backend.get_allocation_extents()
750755
extents = [
751756
{

‎scripts/vm/hypervisor/kvm/imageserver/tests/test_nbd_entries.py‎

Lines changed: 194 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,194 @@
1+
# Licensed to the Apache Software Foundation (ASF) under one
2+
# or more contributor license agreements. See the NOTICE file
3+
# distributed with this work for additional information
4+
# regarding copyright ownership. The ASF licenses this file
5+
# to you under the Apache License, Version 2.0 (the
6+
# "License"); you may not use this file except in compliance
7+
# with the License. You may obtain a copy of the License at
8+
#
9+
# http://www.apache.org/licenses/LICENSE-2.0
10+
#
11+
# Unless required by applicable law or agreed to in writing,
12+
# software distributed under the License is distributed on an
13+
# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
14+
# KIND, either express or implied. See the License for the
15+
# specific language governing permissions and limitations
16+
# under the License.
17+
18+
"""
19+
Tests for NBD block status entry parsing.
20+
"""
21+
22+
import os
23+
import shutil
24+
import subprocess
25+
import tempfile
26+
import unittest
27+
28+
from imageserver.backends.nbd import NbdConnection, _entries_to_pairs
29+
from imageserver.util import is_fallback_dirty_response
30+
31+
class TestRealLibnbdBlockStatus(unittest.TestCase):
32+
"""
33+
Creates a 1 MiB qcow2 with a persistent dirty bitmap, dirties two
34+
64 KiB regions, exports it via qemu-nbd --bitmap, and asserts both the
35+
raw callback entry format and the parsed extents. Fails loudly if a
36+
new libnbd version changes the block status response.
37+
"""
38+
39+
REAL_BITMAP = "bm0"
40+
KB64 = 65536
41+
REAL_SIZE = 16 * KB64 # 1 MiB
42+
43+
@classmethod
44+
def setUpClass(cls):
45+
cls._tmp = tempfile.mkdtemp(prefix="nbd_entries_test_")
46+
cls._img = os.path.join(cls._tmp, "img.qcow2")
47+
cls._sock = os.path.join(cls._tmp, "nbd.sock")
48+
cls._proc = None
49+
50+
def run(*cmd):
51+
subprocess.run(cmd, check=True, capture_output=True)
52+
53+
try:
54+
run("qemu-img", "create", "-f", "qcow2", cls._img, str(cls.REAL_SIZE))
55+
# allocate 0-192 KiB before the bitmap exists (clean, allocated)
56+
run("qemu-io", "-f", "qcow2", "-c", "write -P 0xaa 0 192k", cls._img)
57+
run("qemu-img", "bitmap", "--add", cls._img, cls.REAL_BITMAP)
58+
# dirty two 64 KiB regions while the bitmap is recording
59+
run("qemu-io", "-f", "qcow2", "-c", "write -P 0xbb 128k 64k", cls._img)
60+
run("qemu-io", "-f", "qcow2", "-c", "write -P 0xcc 512k 64k", cls._img)
61+
62+
cls._proc = subprocess.Popen(
63+
[
64+
"qemu-nbd",
65+
"--socket", cls._sock,
66+
"--format", "qcow2",
67+
"--persistent",
68+
"--shared=0",
69+
"--read-only",
70+
f"--bitmap={cls.REAL_BITMAP}",
71+
cls._img,
72+
],
73+
stdout=subprocess.PIPE,
74+
stderr=subprocess.PIPE,
75+
)
76+
from .test_base import _wait_for_nbd_socket
77+
78+
_wait_for_nbd_socket(cls._sock)
79+
except BaseException:
80+
cls._teardown()
81+
raise
82+
83+
@classmethod
84+
def tearDownClass(cls):
85+
cls._teardown()
86+
87+
@classmethod
88+
def _teardown(cls):
89+
if cls._proc is not None:
90+
for pipe in (cls._proc.stdout, cls._proc.stderr):
91+
if pipe:
92+
pipe.close()
93+
cls._proc.terminate()
94+
try:
95+
cls._proc.wait(timeout=5)
96+
except subprocess.TimeoutExpired:
97+
cls._proc.kill()
98+
cls._proc.wait(timeout=5)
99+
cls._proc = None
100+
shutil.rmtree(cls._tmp, ignore_errors=True)
101+
102+
def _connect(self):
103+
return NbdConnection(
104+
self._sock,
105+
None,
106+
need_block_status=True,
107+
extra_meta_contexts=[f"qemu:dirty-bitmap:{self.REAL_BITMAP}"],
108+
)
109+
110+
def test_raw_entries_format_is_known(self):
111+
"""The raw callback entries must be flat ints or (length, flags) pairs."""
112+
with self._connect() as conn:
113+
size = conn.size()
114+
captured = []
115+
116+
def cb(*args, **kwargs):
117+
self.assertGreaterEqual(len(args), 3)
118+
captured.append((args[0], list(args[2])))
119+
return 0
120+
121+
fn = getattr(
122+
conn._nbd,
123+
"block_status_64",
124+
getattr(conn._nbd, "block_status", None),
125+
)
126+
self.assertIsNotNone(
127+
fn, "libnbd no longer exposes block_status/block_status_64"
128+
)
129+
fn(size, 0, cb)
130+
131+
self.assertTrue(captured, "block status delivered no callbacks")
132+
contexts = {ctx for ctx, _ in captured}
133+
self.assertIn("base:allocation", contexts)
134+
self.assertIn(f"qemu:dirty-bitmap:{self.REAL_BITMAP}", contexts)
135+
136+
for ctx, entries in captured:
137+
self.assertTrue(entries, f"empty entries for context {ctx}")
138+
if isinstance(entries[0], (tuple, list)):
139+
for item in entries:
140+
self.assertIsInstance(item, (tuple, list))
141+
self.assertEqual(len(item), 2)
142+
self.assertIsInstance(item[0], int)
143+
self.assertIsInstance(item[1], int)
144+
else:
145+
self.assertTrue(all(isinstance(v, int) for v in entries))
146+
self.assertEqual(len(entries) % 2, 0)
147+
# entries must parse and tile part of the queried range
148+
pairs = _entries_to_pairs(entries)
149+
self.assertTrue(pairs)
150+
covered = sum(length for length, _ in pairs)
151+
self.assertGreater(covered, 0)
152+
self.assertLessEqual(covered, size)
153+
154+
def test_allocation_extents_tile_image(self):
155+
with self._connect() as conn:
156+
size = conn.size()
157+
extents = conn.get_allocation_extents()
158+
self.assertEqual(size, self.REAL_SIZE)
159+
pos = 0
160+
for e in extents:
161+
self.assertEqual(e["start"], pos)
162+
self.assertGreater(e["length"], 0)
163+
pos += e["length"]
164+
self.assertEqual(pos, size)
165+
self.assertEqual(
166+
extents,
167+
[
168+
{"start": 0, "length": 3 * self.KB64, "zero": False},
169+
{"start": 3 * self.KB64, "length": 5 * self.KB64, "zero": True},
170+
{"start": 8 * self.KB64, "length": self.KB64, "zero": False},
171+
{"start": 9 * self.KB64, "length": 7 * self.KB64, "zero": True},
172+
],
173+
)
174+
175+
def test_dirty_extents_match_writes(self):
176+
with self._connect() as conn:
177+
extents = conn.get_extents_dirty_and_zero(
178+
f"qemu:dirty-bitmap:{self.REAL_BITMAP}"
179+
)
180+
self.assertEqual(
181+
extents,
182+
[
183+
{"start": 0, "length": 2 * self.KB64, "dirty": False, "zero": False},
184+
{"start": 2 * self.KB64, "length": self.KB64, "dirty": True, "zero": False},
185+
{"start": 3 * self.KB64, "length": 5 * self.KB64, "dirty": False, "zero": True},
186+
{"start": 8 * self.KB64, "length": self.KB64, "dirty": True, "zero": False},
187+
{"start": 9 * self.KB64, "length": 7 * self.KB64, "dirty": False, "zero": True},
188+
],
189+
)
190+
self.assertFalse(is_fallback_dirty_response(extents))
191+
192+
193+
if __name__ == "__main__":
194+
unittest.main()

0 commit comments

Comments
 (0)