)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"4caf82df3116392e004a33763a6dcd7f80ccb7b8","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"04dae59d_8b067e29","updated":"2026-07-02 06:01:02.000000000","message":"thanks for adding this, this will be very helpful in future testing of CBT.not done yet, will get back to it again.\n\noverall looks good, I have few nits and questions just to improve my understanding","commit_id":"1df7340cb46cc47cc798248d33143dc764388cce"},{"author":{"_account_id":13425,"name":"Simon Dodsley","email":"simon@purestorage.com","username":"sdodsley"},"change_message_id":"1def29255aaae49a1d7c9420f106f889ce2fc5e6","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"b8ab68d3_1cf4722a","updated":"2026-07-02 18:52:59.000000000","message":"recheck","commit_id":"76de4cc218f5a9c89c4e1bd0b6398c377b7f19d5"}],"cinder/tests/fake_cbt_driver.py":[{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"4caf82df3116392e004a33763a6dcd7f80ccb7b8","unresolved":true,"context_lines":[{"line_number":1,"context_line":"#    Copyright 2026 OpenStack Foundation"},{"line_number":2,"context_line":"#"},{"line_number":3,"context_line":"#    Licensed under the Apache License, Version 2.0 (the \"License\"); you may"},{"line_number":4,"context_line":"#    not use this file except in compliance with the License. You may obtain"}],"source_content_type":"text/x-python","patch_set":2,"id":"1f989627_1af3cfc3","line":1,"range":{"start_line":1,"start_character":1,"end_line":1,"end_character":5},"updated":"2026-07-02 06:01:02.000000000","message":"nit: extra space","commit_id":"1df7340cb46cc47cc798248d33143dc764388cce"},{"author":{"_account_id":13425,"name":"Simon Dodsley","email":"simon@purestorage.com","username":"sdodsley"},"change_message_id":"9a448a85777fa1120391c31c19913ba4b311da63","unresolved":false,"context_lines":[{"line_number":1,"context_line":"#    Copyright 2026 OpenStack Foundation"},{"line_number":2,"context_line":"#"},{"line_number":3,"context_line":"#    Licensed under the Apache License, Version 2.0 (the \"License\"); you may"},{"line_number":4,"context_line":"#    not use this file except in compliance with the License. You may obtain"}],"source_content_type":"text/x-python","patch_set":2,"id":"bceb2077_afaccc5b","line":1,"range":{"start_line":1,"start_character":1,"end_line":1,"end_character":5},"in_reply_to":"1f989627_1af3cfc3","updated":"2026-07-02 12:01:01.000000000","message":"Done","commit_id":"1df7340cb46cc47cc798248d33143dc764388cce"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"4caf82df3116392e004a33763a6dcd7f80ccb7b8","unresolved":true,"context_lines":[{"line_number":46,"context_line":"_ZERO_BLOCK \u003d bytes(BLOCK_SIZE)"},{"line_number":47,"context_line":""},{"line_number":48,"context_line":""},{"line_number":49,"context_line":"class FakeCBTVolumeDriver(fake_driver.FakeLoggingVolumeDriver):"},{"line_number":50,"context_line":"    \"\"\"A fake volume driver that implements Changed Block Tracking."},{"line_number":51,"context_line":""},{"line_number":52,"context_line":"    Overriding :meth:`get_cbt_extents` is, by itself, what advertises CBT"}],"source_content_type":"text/x-python","patch_set":2,"id":"7b95acd1_6f830fd2","line":49,"range":{"start_line":49,"start_character":6,"end_line":49,"end_character":25},"updated":"2026-07-02 06:01:02.000000000","message":"CBT is not really a driver like RBD or iSCSI but a technique for incremental backup, I think name of class and file can be updated to simething else!","commit_id":"1df7340cb46cc47cc798248d33143dc764388cce"},{"author":{"_account_id":13425,"name":"Simon Dodsley","email":"simon@purestorage.com","username":"sdodsley"},"change_message_id":"9a448a85777fa1120391c31c19913ba4b311da63","unresolved":true,"context_lines":[{"line_number":46,"context_line":"_ZERO_BLOCK \u003d bytes(BLOCK_SIZE)"},{"line_number":47,"context_line":""},{"line_number":48,"context_line":""},{"line_number":49,"context_line":"class FakeCBTVolumeDriver(fake_driver.FakeLoggingVolumeDriver):"},{"line_number":50,"context_line":"    \"\"\"A fake volume driver that implements Changed Block Tracking."},{"line_number":51,"context_line":""},{"line_number":52,"context_line":"    Overriding :meth:`get_cbt_extents` is, by itself, what advertises CBT"}],"source_content_type":"text/x-python","patch_set":2,"id":"5d98f3ba_552dec28","line":49,"range":{"start_line":49,"start_character":6,"end_line":49,"end_character":25},"in_reply_to":"7b95acd1_6f830fd2","updated":"2026-07-02 12:01:01.000000000","message":"I\u0027d like to keep the name if that\u0027s OK. It genuinely is a fake volume driver — it subclasses FakeLoggingVolumeDriver and CBT support is expressed as volume-driver methods (get_cbt_extents/create_cbt_snapshot/delete_cbt_snapshot), which is literally what supports_cbt keys off of. So FakeCBTVolumeDriver reads as \"a fake volume driver that implements CBT,\" matching the existing FakeLoggingVolumeDriver/FakeGateDriver naming in this same module. Alternatives like FakeCBTBackupDriver would be more misleading, since it\u0027s not a backup driver. Happy to revisit if you feel strongly.","commit_id":"1df7340cb46cc47cc798248d33143dc764388cce"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"4caf82df3116392e004a33763a6dcd7f80ccb7b8","unresolved":true,"context_lines":[{"line_number":141,"context_line":"            # Allocation map: every block that was ever written, regardless"},{"line_number":142,"context_line":"            # of its content (an explicitly zero-written block is allocated"},{"line_number":143,"context_line":"            # and still shipped so restore reproduces it exactly)."},{"line_number":144,"context_line":"            changed \u003d sorted(target)"},{"line_number":145,"context_line":"        else:"},{"line_number":146,"context_line":"            base \u003d self._snapshots.get(from_snapshot)"},{"line_number":147,"context_line":"            if base is None:"}],"source_content_type":"text/x-python","patch_set":2,"id":"21ae863d_bba45199","line":144,"updated":"2026-07-02 06:01:02.000000000","message":"is this valid ?\nso I understand this mean we did not found original or first-snapshot,\nso we are returning all indexes of target to backup further - so full backup instead of CBT ?","commit_id":"1df7340cb46cc47cc798248d33143dc764388cce"},{"author":{"_account_id":13425,"name":"Simon Dodsley","email":"simon@purestorage.com","username":"sdodsley"},"change_message_id":"9a448a85777fa1120391c31c19913ba4b311da63","unresolved":true,"context_lines":[{"line_number":141,"context_line":"            # Allocation map: every block that was ever written, regardless"},{"line_number":142,"context_line":"            # of its content (an explicitly zero-written block is allocated"},{"line_number":143,"context_line":"            # and still shipped so restore reproduces it exactly)."},{"line_number":144,"context_line":"            changed \u003d sorted(target)"},{"line_number":145,"context_line":"        else:"},{"line_number":146,"context_line":"            base \u003d self._snapshots.get(from_snapshot)"},{"line_number":147,"context_line":"            if base is None:"}],"source_content_type":"text/x-python","patch_set":2,"id":"6f5c00c4_7f868500","line":144,"in_reply_to":"21ae863d_bba45199","updated":"2026-07-02 12:01:01.000000000","message":"Good question — this is intentional, not a \"base snapshot not found\" fallback. from_snapshot\u003dNone is part of the driver contract: the framework passes it for the first/full backup to request the volume\u0027s allocation map, so even the full backup is sparse (only written blocks shipped). A base id that was passed but doesn\u0027t exist is a real failure and returns None (the base \u003d self._snapshots.get(...); if base is None: return None branch just below). I\u0027ve clarified this in the docstring and the inline comment in the new patchset.","commit_id":"1df7340cb46cc47cc798248d33143dc764388cce"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"4caf82df3116392e004a33763a6dcd7f80ccb7b8","unresolved":true,"context_lines":[{"line_number":150,"context_line":"            # a zero block for absent indices means both a fresh write and a"},{"line_number":151,"context_line":"            # data-\u003ezero change are correctly reported as must-ship."},{"line_number":152,"context_line":"            changed \u003d []"},{"line_number":153,"context_line":"            for index in sorted(set(target) | set(base)):"},{"line_number":154,"context_line":"                target_block \u003d target.get(index, _ZERO_BLOCK)"},{"line_number":155,"context_line":"                base_block \u003d base.get(index, _ZERO_BLOCK)"},{"line_number":156,"context_line":"                if target_block !\u003d base_block:"},{"line_number":157,"context_line":"                    changed.append(index)"},{"line_number":158,"context_line":""},{"line_number":159,"context_line":"        return self._consolidate("},{"line_number":160,"context_line":"            [{\u0027offset\u0027: index * BLOCK_SIZE, \u0027length\u0027: BLOCK_SIZE,"},{"line_number":161,"context_line":"              \u0027exists\u0027: True} for index in changed])"}],"source_content_type":"text/x-python","patch_set":2,"id":"44bcf863_ff946932","line":158,"range":{"start_line":153,"start_character":0,"end_line":158,"end_character":0},"updated":"2026-07-02 06:01:02.000000000","message":"I am having hard time imaging and track these objs (even good number of tests are added).\nas there is already many comments, can you please provide an example objects (from_snap, to_snap, target, changed) may be in doc_string","commit_id":"1df7340cb46cc47cc798248d33143dc764388cce"},{"author":{"_account_id":13425,"name":"Simon Dodsley","email":"simon@purestorage.com","username":"sdodsley"},"change_message_id":"9a448a85777fa1120391c31c19913ba4b311da63","unresolved":false,"context_lines":[{"line_number":150,"context_line":"            # a zero block for absent indices means both a fresh write and a"},{"line_number":151,"context_line":"            # data-\u003ezero change are correctly reported as must-ship."},{"line_number":152,"context_line":"            changed \u003d []"},{"line_number":153,"context_line":"            for index in sorted(set(target) | set(base)):"},{"line_number":154,"context_line":"                target_block \u003d target.get(index, _ZERO_BLOCK)"},{"line_number":155,"context_line":"                base_block \u003d base.get(index, _ZERO_BLOCK)"},{"line_number":156,"context_line":"                if target_block !\u003d base_block:"},{"line_number":157,"context_line":"                    changed.append(index)"},{"line_number":158,"context_line":""},{"line_number":159,"context_line":"        return self._consolidate("},{"line_number":160,"context_line":"            [{\u0027offset\u0027: index * BLOCK_SIZE, \u0027length\u0027: BLOCK_SIZE,"},{"line_number":161,"context_line":"              \u0027exists\u0027: True} for index in changed])"}],"source_content_type":"text/x-python","patch_set":2,"id":"2eb446bd_b387b184","line":158,"range":{"start_line":153,"start_character":0,"end_line":158,"end_character":0},"in_reply_to":"44bcf863_ff946932","updated":"2026-07-02 12:01:01.000000000","message":"Done","commit_id":"1df7340cb46cc47cc798248d33143dc764388cce"}],"cinder/tests/unit/volume/drivers/test_fake_cbt_driver.py":[{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"4caf82df3116392e004a33763a6dcd7f80ccb7b8","unresolved":true,"context_lines":[{"line_number":45,"context_line":"    def test_create_snapshot_returns_reference_id(self):"},{"line_number":46,"context_line":"        self.driver.simulate_write(self.volume, 0, b\u0027x\u0027 * BLOCK)"},{"line_number":47,"context_line":"        self.assertEqual(\u0027snap1\u0027,"},{"line_number":48,"context_line":"                         self.driver.create_cbt_snapshot(self.volume, \u0027snap1\u0027))"},{"line_number":49,"context_line":""},{"line_number":50,"context_line":"    def test_create_snapshot_unknown_volume_returns_none(self):"},{"line_number":51,"context_line":"        # Nothing was ever written to this volume -\u003e no backend object to"}],"source_content_type":"text/x-python","patch_set":2,"id":"b602c371_8ee7c495","line":48,"updated":"2026-07-02 06:01:02.000000000","message":"this test does not really verify if snapshot is created, it just test same name is returned what was passed.\n\nwhat I mean is if tomorrow someone changed the logic of saving snapshot with copy into something wrong, this test wont catch that.\nthe next test at L53 works.","commit_id":"1df7340cb46cc47cc798248d33143dc764388cce"},{"author":{"_account_id":13425,"name":"Simon Dodsley","email":"simon@purestorage.com","username":"sdodsley"},"change_message_id":"9a448a85777fa1120391c31c19913ba4b311da63","unresolved":false,"context_lines":[{"line_number":45,"context_line":"    def test_create_snapshot_returns_reference_id(self):"},{"line_number":46,"context_line":"        self.driver.simulate_write(self.volume, 0, b\u0027x\u0027 * BLOCK)"},{"line_number":47,"context_line":"        self.assertEqual(\u0027snap1\u0027,"},{"line_number":48,"context_line":"                         self.driver.create_cbt_snapshot(self.volume, \u0027snap1\u0027))"},{"line_number":49,"context_line":""},{"line_number":50,"context_line":"    def test_create_snapshot_unknown_volume_returns_none(self):"},{"line_number":51,"context_line":"        # Nothing was ever written to this volume -\u003e no backend object to"}],"source_content_type":"text/x-python","patch_set":2,"id":"44f9baa2_f0db9ba7","line":48,"in_reply_to":"b602c371_8ee7c495","updated":"2026-07-02 12:01:01.000000000","message":"Agreed — good catch. Reworked it into test_create_snapshot_freezes_a_copy: it now takes the snapshot, then writes another block to the volume, and asserts the snapshot\u0027s allocation map still shows only the original block. If create_cbt_snapshot stored a reference instead of a deep copy, block 5 would leak in and the test would fail — which is exactly the regression you described.","commit_id":"1df7340cb46cc47cc798248d33143dc764388cce"}]}