From ce19b922e6b258d547fb4fea6c309ab85e1b5120 Mon Sep 17 00:00:00 2001 From: Abhisar Sinha <63767682+abh1sar@users.noreply.github.com> Date: Thu, 26 Mar 2026 11:55:42 +0530 Subject: [PATCH] coalesce similar extents --- .../kvm/imageserver/backends/nbd.py | 4 +- .../hypervisor/kvm/imageserver/concurrency.py | 4 +- .../vm/hypervisor/kvm/imageserver/server.py | 2 +- .../kvm/imageserver/tests/test_util.py | 122 ++++++++++++++++++ scripts/vm/hypervisor/kvm/imageserver/util.py | 48 ++++++- 5 files changed, 174 insertions(+), 6 deletions(-) create mode 100644 scripts/vm/hypervisor/kvm/imageserver/tests/test_util.py diff --git a/scripts/vm/hypervisor/kvm/imageserver/backends/nbd.py b/scripts/vm/hypervisor/kvm/imageserver/backends/nbd.py index 48ba1d9fe90..aa247be29f2 100644 --- a/scripts/vm/hypervisor/kvm/imageserver/backends/nbd.py +++ b/scripts/vm/hypervisor/kvm/imageserver/backends/nbd.py @@ -28,7 +28,7 @@ from ..constants import ( NBD_STATE_HOLE, NBD_STATE_ZERO, ) -from ..util import merge_dirty_zero_extents +from ..util import coalesce_allocation_extents, merge_dirty_zero_extents from .base import BackendSession, ImageBackend @@ -225,7 +225,7 @@ class NbdConnection: return [{"start": 0, "length": size, "zero": False}] if not allocation_extents: return [{"start": 0, "length": size, "zero": False}] - return allocation_extents + return coalesce_allocation_extents(allocation_extents) def get_extents_dirty_and_zero( self, dirty_bitmap_context: str diff --git a/scripts/vm/hypervisor/kvm/imageserver/concurrency.py b/scripts/vm/hypervisor/kvm/imageserver/concurrency.py index a446786224d..7d91aea6013 100644 --- a/scripts/vm/hypervisor/kvm/imageserver/concurrency.py +++ b/scripts/vm/hypervisor/kvm/imageserver/concurrency.py @@ -29,8 +29,8 @@ class ConcurrencyManager: """ Manages per-image read/write semaphores and per-image mutual-exclusion locks. - Each image_id gets its own independent pool of read slots (default 8) - and write slots (default 1), so concurrent transfers to different images + Each image_id gets its own independent pool of read slots (default MAX_PARALLEL_READS) + and write slots (default MAX_PARALLEL_WRITES), so concurrent transfers to different images do not contend with each other. The per-image lock serialises operations that must not overlap on the diff --git a/scripts/vm/hypervisor/kvm/imageserver/server.py b/scripts/vm/hypervisor/kvm/imageserver/server.py index 53d4383b96f..6d6648030d1 100644 --- a/scripts/vm/hypervisor/kvm/imageserver/server.py +++ b/scripts/vm/hypervisor/kvm/imageserver/server.py @@ -54,7 +54,7 @@ def make_handler( Create a Handler subclass with injected dependencies. BaseHTTPRequestHandler is instantiated per-request by the server, so we - cannot pass constructor args. Instead we set class-level attributes. + cannot pass constructor args. Instead, we set class-level attributes. """ class ConfiguredHandler(Handler): diff --git a/scripts/vm/hypervisor/kvm/imageserver/tests/test_util.py b/scripts/vm/hypervisor/kvm/imageserver/tests/test_util.py new file mode 100644 index 00000000000..159dff30a92 --- /dev/null +++ b/scripts/vm/hypervisor/kvm/imageserver/tests/test_util.py @@ -0,0 +1,122 @@ +# 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. + +"""Unit tests for imageserver.util extent coalescing helpers.""" + +import unittest + +from imageserver.util import ( + coalesce_allocation_extents, + coalesce_dirty_zero_extents, + merge_dirty_zero_extents, +) + + +class TestCoalesceAllocationExtents(unittest.TestCase): + def test_empty(self): + self.assertEqual(coalesce_allocation_extents([]), []) + + def test_single(self): + inp = [{"start": 0, "length": 4096, "zero": False}] + out = coalesce_allocation_extents(inp) + self.assertEqual(out, [{"start": 0, "length": 4096, "zero": False}]) + self.assertIsNot(out[0], inp[0]) + + def test_merges_contiguous_same_zero(self): + inp = [ + {"start": 0, "length": 10, "zero": False}, + {"start": 10, "length": 5, "zero": False}, + {"start": 15, "length": 100, "zero": False}, + ] + self.assertEqual( + coalesce_allocation_extents(inp), + [{"start": 0, "length": 115, "zero": False}], + ) + + def test_does_not_merge_different_zero(self): + inp = [ + {"start": 0, "length": 64, "zero": False}, + {"start": 64, "length": 64, "zero": True}, + {"start": 128, "length": 64, "zero": False}, + ] + self.assertEqual(coalesce_allocation_extents(inp), inp) + + def test_does_not_merge_gap(self): + inp = [ + {"start": 0, "length": 100, "zero": False}, + {"start": 200, "length": 50, "zero": False}, + ] + self.assertEqual(coalesce_allocation_extents(inp), inp) + + def test_does_not_merge_same_zero_with_gap(self): + inp = [ + {"start": 0, "length": 10, "zero": True}, + {"start": 20, "length": 10, "zero": True}, + ] + self.assertEqual(coalesce_allocation_extents(inp), inp) + + +class TestCoalesceDirtyZeroExtents(unittest.TestCase): + def test_empty(self): + self.assertEqual(coalesce_dirty_zero_extents([]), []) + + def test_single(self): + inp = [{"start": 0, "length": 8192, "dirty": True, "zero": False}] + out = coalesce_dirty_zero_extents(inp) + self.assertEqual( + out, [{"start": 0, "length": 8192, "dirty": True, "zero": False}] + ) + + def test_merges_contiguous_same_flags(self): + inp = [ + {"start": 0, "length": 50, "dirty": True, "zero": False}, + {"start": 50, "length": 50, "dirty": True, "zero": False}, + ] + self.assertEqual( + coalesce_dirty_zero_extents(inp), + [{"start": 0, "length": 100, "dirty": True, "zero": False}], + ) + + def test_does_not_merge_differing_dirty(self): + inp = [ + {"start": 0, "length": 32, "dirty": False, "zero": False}, + {"start": 32, "length": 32, "dirty": True, "zero": False}, + ] + self.assertEqual(coalesce_dirty_zero_extents(inp), inp) + + def test_does_not_merge_differing_zero(self): + inp = [ + {"start": 0, "length": 16, "dirty": False, "zero": False}, + {"start": 16, "length": 16, "dirty": False, "zero": True}, + ] + self.assertEqual(coalesce_dirty_zero_extents(inp), inp) + + +class TestMergeDirtyZeroExtentsCoalescing(unittest.TestCase): + def test_coalesces_adjacent_identical_flags_after_boundary_merge(self): + """Boundary grid can split one logical run; coalesce should reunite.""" + allocation = [(0, 200, False)] + dirty = [(0, 100, False), (100, 100, False)] + merged = merge_dirty_zero_extents(allocation, dirty, 200) + self.assertEqual( + merged, + [{"start": 0, "length": 200, "dirty": False, "zero": False}], + ) + + +if __name__ == "__main__": + unittest.main() diff --git a/scripts/vm/hypervisor/kvm/imageserver/util.py b/scripts/vm/hypervisor/kvm/imageserver/util.py index 71e51cec65a..473f58a50c0 100644 --- a/scripts/vm/hypervisor/kvm/imageserver/util.py +++ b/scripts/vm/hypervisor/kvm/imageserver/util.py @@ -20,6 +20,52 @@ import time from typing import Any, Dict, List, Set, Tuple +def coalesce_allocation_extents( + extents: List[Dict[str, Any]], +) -> List[Dict[str, Any]]: + """Merge contiguous extents that share the same ``zero`` flag.""" + if not extents: + return [] + out: List[Dict[str, Any]] = [dict(extents[0])] + for e in extents[1:]: + prev = out[-1] + if ( + prev["start"] + prev["length"] == e["start"] + and prev["zero"] == e["zero"] + ): + prev["length"] += e["length"] + else: + out.append({"start": e["start"], "length": e["length"], "zero": e["zero"]}) + return out + + +def coalesce_dirty_zero_extents( + extents: List[Dict[str, Any]], +) -> List[Dict[str, Any]]: + """Merge contiguous extents that share the same ``dirty`` and ``zero`` flags.""" + if not extents: + return [] + out: List[Dict[str, Any]] = [dict(extents[0])] + for e in extents[1:]: + prev = out[-1] + if ( + prev["start"] + prev["length"] == e["start"] + and prev["dirty"] == e["dirty"] + and prev["zero"] == e["zero"] + ): + prev["length"] += e["length"] + else: + out.append( + { + "start": e["start"], + "length": e["length"], + "dirty": e["dirty"], + "zero": e["zero"], + } + ) + return out + + def json_bytes(obj: Any) -> bytes: return json.dumps(obj, separators=(",", ":"), ensure_ascii=False).encode("utf-8") @@ -63,7 +109,7 @@ def merge_dirty_zero_extents( "zero": lookup(allocation_extents, a, False), } ) - return result + return coalesce_dirty_zero_extents(result) def is_fallback_dirty_response(extents: List[Dict[str, Any]]) -> bool: