Add CBT (Changed Block Tracking) test suite - #541
Conversation
ee415bc to
71b7668
Compare
|
@Lankou66 could you rebase on |
|
|
||
| ImageFormat = Literal['qcow2', 'raw', 'vhd'] | ||
|
|
||
|
|
stormi
left a comment
There was a problem hiding this comment.
I had a quick look but stopped due to:
- whitespace changes that are not relevant to this PR
- missing commit messages and commit titles that don't fully follow the conventions: https://docs.xcp-ng.org/project/development-process/commit-message-conventions/
As time is precious, I'll consider reviewing once this is fixed.
Signed-off-by: Goulven Riou <goulven.riou@vates.tech>
Signed-off-by: Goulven Riou <goulven.riou@vates.tech>
Signed-off-by: Goulven Riou <goulven.riou@vates.tech>
Signed-off-by: Goulven Riou <goulven.riou@vates.tech>
Signed-off-by: Goulven Riou <goulven.riou@vates.tech>
Signed-off-by: Goulven Riou <goulven.riou@vates.tech>
Signed-off-by: Goulven Riou <goulven.riou@vates.tech>
Signed-off-by: Goulven Riou <goulven.riou@vates.tech>
glehmann
left a comment
There was a problem hiding this comment.
On the commit organization, I would have preferred, to ease the review, that each commit introduce a test or a set of related tests, with the generic method in storage.py, the implementation for all the SR types, and an explanation of what it does in the commit description.
| result = host.ssh(f'test -f {log_path}', check=False, simple_output=False) | ||
| assert result.returncode == 0, f"CBT log not found at {log_path}" |
There was a problem hiding this comment.
| result = host.ssh(f'test -f {log_path}', check=False, simple_output=False) | |
| assert result.returncode == 0, f"CBT log not found at {log_path}" | |
| assert host.file_exists(log_path), f"CBT log not found at {log_path}" |
| result = host.ssh(f'stat -c %s {log_path}', simple_output=False) | ||
| log_size = int(result.stdout.strip()) |
There was a problem hiding this comment.
| result = host.ssh(f'stat -c %s {log_path}', simple_output=False) | |
| log_size = int(result.stdout.strip()) | |
| log_size = int(host.ssh(f'stat -c %s {log_path}').strip()) |
| result = host.ssh(f'lvs --noheadings -o lv_name {vg_name}', simple_output=False) | ||
| assert cbt_log_name in result.stdout, \ | ||
| f"CBT log LV {cbt_log_name} not found in VG {vg_name}" |
There was a problem hiding this comment.
| result = host.ssh(f'lvs --noheadings -o lv_name {vg_name}', simple_output=False) | |
| assert cbt_log_name in result.stdout, \ | |
| f"CBT log LV {cbt_log_name} not found in VG {vg_name}" | |
| result = host.ssh(f'lvs --noheadings -o lv_name {vg_name}') | |
| assert cbt_log_name in result, f"CBT log LV {cbt_log_name} not found in VG {vg_name}" |
| result = host.ssh(f'test -f {log_path}', check=False, simple_output=False) | ||
| assert result.returncode != 0, f"CBT log should not exist at {log_path}" |
There was a problem hiding this comment.
| result = host.ssh(f'test -f {log_path}', check=False, simple_output=False) | |
| assert result.returncode != 0, f"CBT log should not exist at {log_path}" | |
| assert host.file_exists(log_path), f"CBT log should not exist at {log_path}" |
| result = host.ssh(f'lvs --noheadings -o lv_name {vg_name}', simple_output=False) | ||
| assert cbt_log_name not in result.stdout, \ | ||
| f"CBT log LV {cbt_log_name} should not exist in VG {vg_name}" |
There was a problem hiding this comment.
| result = host.ssh(f'lvs --noheadings -o lv_name {vg_name}', simple_output=False) | |
| assert cbt_log_name not in result.stdout, \ | |
| f"CBT log LV {cbt_log_name} should not exist in VG {vg_name}" | |
| result = host.ssh(f'lvs --noheadings -o lv_name {vg_name}') | |
| assert cbt_log_name not in result, f"CBT log LV {cbt_log_name} should not exist in VG {vg_name}" |
|
|
||
|
|
||
| @contextmanager | ||
| def cbt_enabled(vdi: VDI) -> Generator[VDI, None, None]: |
There was a problem hiding this comment.
Shouldn't this rather be a fixture?
Fixtures has a great advantage over context manager to deal with resources: when a test fails, the debugger is started before the resources are destroyed. The teardown is executed once you exit the debugger. It helps a lot to investigate in case of failure.
This is probably also true—at least I would ask the same question—for the other context managers.
| vm.disconnect_vdi(vdi) | ||
| vm.shutdown(verify=True) | ||
| second = vdi.snapshot() | ||
| stack.callback(second.destroy) |
There was a problem hiding this comment.
The same idea as for the context manager applies here.
We have a defer fixture which does the same thing, but ensures that the resources are preserved when entering the debugger.
| for i in range(len(snapshots) - 1): | ||
| assert verify_changed_blocks_detected(snapshots[i], snapshots[i + 1]) | ||
| logging.info(f"Changes detected: snap{i} -> snap{i + 1}") | ||
| finally: |
There was a problem hiding this comment.
Same for the try/finally blocks, defer helps a lot.
Add CBT (Changed Block Tracking) test suite
This PR implements a test suite for the CBT feature across following SR types: