Skip to content

Conversation

@glehmann
Copy link
Member

No description provided.

@glehmann glehmann requested a review from a team as a code owner November 27, 2025 20:03
@glehmann glehmann changed the base branch from gln/coalesce to gln/migration-integrity November 27, 2025 20:04
vm.export(xva_path, compression)
# check that the zero blocks are not part of the result. Most of the data is from the random stream, so
# compression has little effect. We just check the result is between 500 and 700 MiB
size_mb = int(vm.host.ssh(f'du -sm {xva_path}').split()[0])
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

du may not reflect the actual size , or maybe you need to use --apparent-size

# check that the zero blocks are not part of the result. Most of the data is from the random stream, so
# compression has little effect. We just check the result is between 500 and 700 MiB
size_mb = int(vm.host.ssh(f'du -sm {xva_path}').split()[0])
assert 500 < size_mb < 700, f"unexpected xva size: {size_mb}"
Copy link

@rzr rzr Nov 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this looks empirical to me, there is no guarantee is there ?

well since the random is controlled, it should be ok

imported_vm = None
try:
vm.export(xva_path, compression)
# check that the zero blocks are not part of the result. Most of the data is from the random stream, so
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
# check that the zero blocks are not part of the result. Most of the data is from the random stream, so
# check that the zero blocks are not part of the result.
# Most of the data is from the pseudo-random stream, so

imported_vm = vm.host.import_vm(xva_path, vm.vdis[0].sr.uuid)
imported_vm.start()
imported_vm.wait_for_vm_running_and_ssh_up()
imported_vm.ssh("randstream validate -v --expected-checksum 24e905d6 /root/data")
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

would make a constant for 24e905d6 and 500MiB


@pytest.mark.small_vm
@pytest.mark.parametrize("compression", ["none", "gzip", "zstd"])
def test_xva_export_import(self, vm_on_lvm_sr: VM, compression):
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't this factorized somewhere ?


@pytest.fixture(scope='package')
def nfs4_sr(host, nfs4_device_config):
def nfs4_sr(host, image_format: str, nfs4_device_config):
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i would put this extra param at the end and set a default value

Copy link

@rzr rzr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm but I see lot of duplicated code, but Is the original intend to make tests "self sufficient" ?

@rzr rzr requested a review from a team November 28, 2025 11:08
@stormi stormi requested a review from a team November 28, 2025 16:01
Signed-off-by: Gaëtan Lehmann <gaetan.lehmann@vates.tech>
Signed-off-by: Gaëtan Lehmann <gaetan.lehmann@vates.tech>
Signed-off-by: Gaëtan Lehmann <gaetan.lehmann@vates.tech>
Signed-off-by: Gaëtan Lehmann <gaetan.lehmann@vates.tech>
Signed-off-by: Gaëtan Lehmann <gaetan.lehmann@vates.tech>
Signed-off-by: Gaëtan Lehmann <gaetan.lehmann@vates.tech>
Signed-off-by: Gaëtan Lehmann <gaetan.lehmann@vates.tech>
Signed-off-by: Gaëtan Lehmann <gaetan.lehmann@vates.tech>
@glehmann glehmann force-pushed the gln/xva-export-integrity branch from 04dc842 to 40b31bf Compare November 29, 2025 22:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants