Storage Tests for qcow2 vdi image format #339

rushikeshjadhav · 2025-07-08T12:26:23Z

Incorporating image-format fixture into following SR types for storage tests

ext
lvm
xfs
zfs
largeblock

tests/storage/lvm/conftest.py

tests/storage/xfs/conftest.py

tests/storage/zfs/conftest.py

tests/storage/zfsvol/conftest.py

Wescoeur · 2025-09-30T14:13:27Z

tests/storage/zfsvol/test_zfsvol_sr.py

+        sr = host.sr_create('zfs-vol', "ZFS-local-SR-test", {
+            'device': '/dev/' + sr_disk_wiped,
+            'preferred-image-formats': image_format
+        }, verify=True)


Did you test it @rushikeshjadhav ? We don't support QCOW2 with ZFS vol driver (SMAPIv3 plugin).
So we must not add image_format for this one.

Right, although the test passes, its volume is not of qcow2 format. Will remove it and add additional checks for qcow2 VDI validation.

Added additional check for qcow2 VDI validation as well with test_vdi_image_format.

tests/storage/conftest.py

glehmann · 2025-10-01T14:39:44Z

tests/storage/ext/conftest.py

-def ext_sr(host: Host, unused_512B_disks: dict[Host, list[Host.BlockDeviceInfo]]) -> Generator[SR]:
+def ext_sr(host: Host,
+           unused_512B_disks: dict[Host, list[Host.BlockDeviceInfo]],
+           image_format) -> Generator[SR]:


Could you add the type on image_format?

Sure, added here and other possible places.

glehmann · 2025-10-02T14:27:51Z

The CI workflow is not run on the PR. We should run it before merging.

glehmann · 2025-10-02T14:44:36Z

There are a few problems to fix:

❯ mypy --install-types --non-interactive lib/ conftest.py pkgfixtures.py tests/
  pyright lib/ conftest.py pkgfixtures.py
  ruff check lib/ tests/
  flake8
tests/install/test.py:88: error: Value of type "object" is not indexable  [index]
tests/install/test.py:342: error: Value of type "object" is not indexable  [index]
Found 2 errors in 1 file (checked 191 source files)
WARNING: there is a new pyright version available (v1.1.402 -> v1.1.406).
Please install the new version or set PYRIGHT_PYTHON_FORCE_VERSION to `latest`

0 errors, 0 warnings, 0 informations
All checks passed!
./tests/storage/largeblock/conftest.py:17:19: E128 continuation line under-indented for visual indent

glehmann · 2025-10-02T14:46:14Z

@rushikeshjadhav the CI should run when using a branch directly in this repository.

stormi

Automatically installing packages that support qcow2 could have been a good idea, but here it might cause more friction than it avoids.

stormi · 2025-10-02T14:58:10Z

tests/storage/conftest.py

+    def check_qcow2_installed(host):
+        if host.file_exists(QCOW2_REPO_FILE):
+            raise Exception(
+                f'{QCOW2_REPO_FILE} is already installed on host {host}. This should not be the case.'


I see where this comes from, but that doesn't work here. We want to keep the ability to install any packages on the test hosts, and the test must not tell us what to do here. It's different from the xostor case which is already a production feature and is tested using what's in the dedicated test repositories.

It was in an effort to keep the test standalone and work on unmodified host. We can remove it if it does not go inline with the test framework. Removing it would mean, the test would work even in case of manual preinstalled qcow2.

I discussed it with others and, yes, we prefer that the test just assumes that qcow2 is available if we ask for qcow2 tests.

This is now removed.

stormi · 2025-10-02T14:58:15Z

tests/storage/conftest.py

+    def remove_qcow2(host):
+        logging.info(f"Cleaning up qcow2 packages from host {host}...")
+        host.remove_xcpng_repo(QCOW2_REPO)
+        host.yum_downgrade(["sm", "sm-fairlock", "blktap"])


This is going to leave the hosts in a state that is not necessarily the initial state. You don't know if the contents enabled repositories match what we decided to install on the hosts to run the tests on it.

In addition to this, all this code will soon have to be deleted, once qcow2 packages are moved to the main testing repositories.

So, restoring yum saved state would be better in such case.

Once the qcow2 packages are moved to the main testing repo, we’ll still need to install them during tests or manually pre-tests, right? Or is the plan to merge the packages so that no separate installation of qcow2-related packages is needed?

As mentioned earlier, it is assumed that qcow2 is installed and no specific package management is expected from the test, so removing this as well.

rushikeshjadhav · 2025-10-03T12:40:25Z

@rushikeshjadhav the CI should run when using a branch directly in this repository.

I have pushed to https://github.com/xcp-ng/xcp-ng-tests/tree/storage-qcow2, will check on CI.

… format Signed-off-by: Rushikesh Jadhav <rushikesh7@gmail.com>

…cow2 vdi image format Signed-off-by: Rushikesh Jadhav <rushikesh7@gmail.com>

rushikeshjadhav · 2025-10-13T13:27:01Z

@rushikeshjadhav the CI should run when using a branch directly in this repository.

I have pushed to https://github.com/xcp-ng/xcp-ng-tests/tree/storage-qcow2, will check on CI.

This is found to be working. Kindly see @Wescoeur @Nambrok if it can be merged.

rushikeshjadhav marked this pull request as draft July 8, 2025 12:26

rushikeshjadhav force-pushed the storage-qcow2 branch from 0610307 to c421cc4 Compare July 9, 2025 11:14

rushikeshjadhav requested a review from Nambrok July 10, 2025 12:21

rushikeshjadhav force-pushed the storage-qcow2 branch 2 times, most recently from 1d35ab2 to da6c5d5 Compare September 18, 2025 17:35

rushikeshjadhav marked this pull request as ready for review September 18, 2025 17:41

rushikeshjadhav requested a review from Wescoeur September 18, 2025 17:42

rushikeshjadhav mentioned this pull request Sep 18, 2025

Add qcow2 parameterized tests #335

Closed

Millefeuille42 requested changes Sep 23, 2025

View reviewed changes

tests/storage/lvm/conftest.py Outdated Show resolved Hide resolved

tests/storage/xfs/conftest.py Outdated Show resolved Hide resolved

tests/storage/zfs/conftest.py Outdated Show resolved Hide resolved

tests/storage/zfsvol/conftest.py Outdated Show resolved Hide resolved

rushikeshjadhav force-pushed the storage-qcow2 branch from da6c5d5 to 5ecdfa2 Compare September 23, 2025 17:50

rushikeshjadhav requested review from Millefeuille42 and stormi September 23, 2025 17:52

Millefeuille42 approved these changes Sep 24, 2025

View reviewed changes

glehmann self-requested a review September 30, 2025 13:06

Wescoeur approved these changes Sep 30, 2025

View reviewed changes

Wescoeur requested changes Sep 30, 2025

View reviewed changes

rushikeshjadhav force-pushed the storage-qcow2 branch 3 times, most recently from 38db6d5 to 79264c0 Compare October 1, 2025 14:32

rushikeshjadhav marked this pull request as draft October 1, 2025 14:37

glehmann reviewed Oct 1, 2025

View reviewed changes

rushikeshjadhav force-pushed the storage-qcow2 branch from 79264c0 to 6360b2e Compare October 2, 2025 12:34

rushikeshjadhav marked this pull request as ready for review October 2, 2025 12:35

rushikeshjadhav force-pushed the storage-qcow2 branch from 6360b2e to 0e5f73a Compare October 2, 2025 12:58

glehmann approved these changes Oct 2, 2025

View reviewed changes

stormi reviewed Oct 2, 2025

View reviewed changes

rushikeshjadhav force-pushed the storage-qcow2 branch from 0e5f73a to 6426939 Compare October 3, 2025 12:26

rushikeshjadhav added 5 commits October 6, 2025 19:30

tests/storage/ext: Updated ext to handle both vhd and qcow2 vdi image…

2029daf

… format Signed-off-by: Rushikesh Jadhav <rushikesh7@gmail.com>

tests/storage/lvm: Updated lvm to handle both vhd and qcow2 vdi image…

a961965

… format Signed-off-by: Rushikesh Jadhav <rushikesh7@gmail.com>

tests/storage/xfs: Updated xfs to handle both vhd and qcow2 vdi image…

2ecd7a9

… format Signed-off-by: Rushikesh Jadhav <rushikesh7@gmail.com>

tests/storage/zfs: Updated zfs to handle both vhd and qcow2 vdi image…

6965c05

… format Signed-off-by: Rushikesh Jadhav <rushikesh7@gmail.com>

tests/storage/largeblock: Updated largeblock to handle both vhd and q…

fd8fa37

…cow2 vdi image format Signed-off-by: Rushikesh Jadhav <rushikesh7@gmail.com>

rushikeshjadhav force-pushed the storage-qcow2 branch from 6426939 to fd8fa37 Compare October 6, 2025 16:22

rushikeshjadhav requested a review from Wescoeur October 7, 2025 11:53

Storage Tests for qcow2 vdi image format #339

Are you sure you want to change the base?

Storage Tests for qcow2 vdi image format #339

Uh oh!

Conversation

rushikeshjadhav commented Jul 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Wescoeur Sep 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

glehmann commented Oct 2, 2025

Uh oh!

glehmann commented Oct 2, 2025

Uh oh!

glehmann commented Oct 2, 2025

Uh oh!

stormi left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

stormi Oct 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rushikeshjadhav commented Oct 3, 2025

Uh oh!

rushikeshjadhav commented Oct 13, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

rushikeshjadhav commented Jul 8, 2025 •

edited

Loading

Wescoeur Sep 30, 2025 •

edited

Loading

stormi left a comment •

edited

Loading

stormi Oct 2, 2025 •

edited

Loading