Skip to content

Conversation

tonyhutter
Copy link
Contributor

Motivation and Context

Add test for #17277

Description

Add a test case to reproduce issue #17277. This has actually been fixed by #17298, but add a test case for good measure. The test case verifies that we can make an xfs filesystem on a ZFS-backed loopback device.

How Has This Been Tested?

Test case added

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Performance enhancement (non-breaking change which improves efficiency)
  • Code cleanup (non-breaking change which makes code smaller or more readable)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Library ABI change (libzfs, libzfs_core, libnvpair, libuutil and libzfsbootenv)
  • Documentation (a change to man pages or other documentation)

Checklist:

@tonyhutter tonyhutter force-pushed the mountloop branch 2 times, most recently from 8415e39 to c967763 Compare May 14, 2025 23:48
@tonyhutter
Copy link
Contributor Author

@behlendorf I included your fixes in my latest push.

@robn
Copy link
Member

robn commented May 16, 2025

Two thoughts:

  • maybe a skip if you don't have mkfs.xfs and/or XFS support in your kernel (I routinely don't have those in my tiny VMs)
  • it's more work, but would we be better to identify the IO pattern that tickled the bug, and write a program to test that? If XFS or the kernel ever changes in the future, the test could end up quietly not actually testing anything useful, and not alerting us to a future break. On the other hand, there's no harm in having a straight up functional "can we even mount a loopback filesystem" test, so as long as we know what this test is, probably nbd.

@tonyhutter
Copy link
Contributor Author

@robn -

maybe a skip if you don't have mkfs.xfs and/or XFS support in your kernel (I routinely don't have those in my tiny VMs)

Thanks - I just added a check in my latest push

On the other hand, there's no harm in having a straight up functional "can we even mount a loopback filesystem" test, so as long as we know what this test is, probably nbd.

Yes, its just a basic sanity test to verify "filesystem over loopback over ZFS".

Copy link
Member

@robn robn left a comment

Choose a reason for hiding this comment

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

Nice, thanks!

@amotin
Copy link
Member

amotin commented May 19, 2025

It should probably be specified somewhere that skipped test is OK:

Tests with results other than PASS that are unexpected:
    SKIP mount/mount_loopback (expected PASS)

@amotin amotin added the Status: Revision Needed Changes are required for the PR to be accepted label May 19, 2025
@github-actions github-actions bot removed the Status: Revision Needed Changes are required for the PR to be accepted label May 30, 2025
@behlendorf behlendorf added the Status: Accepted Ready to integrate (reviewed, tested) label Jun 3, 2025
@amotin
Copy link
Member

amotin commented Jun 6, 2025

This still fails (skips) almalinux8 and debian11.

@amotin amotin added Status: Revision Needed Changes are required for the PR to be accepted and removed Status: Accepted Ready to integrate (reviewed, tested) labels Jun 6, 2025
@github-actions github-actions bot removed the Status: Revision Needed Changes are required for the PR to be accepted label Aug 7, 2025
@behlendorf behlendorf added the Status: Revision Needed Changes are required for the PR to be accepted label Aug 7, 2025
@behlendorf behlendorf requested a review from Copilot August 22, 2025 21:50
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds a new test case to verify that ZFS-backed loopback devices work correctly with other filesystems, specifically addressing issue #17277 which was previously fixed by PR #17298.

  • Adds a comprehensive test that creates both file-based and zvol-based loopback devices
  • Tests XFS filesystem creation and I/O operations on ZFS-backed loopback devices
  • Includes proper dependency checks for XFS support and required utilities

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 11 comments.

File Description
tests/zfs-tests/tests/functional/mount/mount_loopback.ksh New test script that creates loopback devices backed by ZFS and tests XFS filesystem operations
tests/zfs-tests/tests/Makefile.am Adds the new test script to the build system
tests/zfs-tests/include/commands.cfg Adds which and mkfs.xfs to available system commands for tests
tests/runfiles/linux.run Includes the new test in the Linux test suite

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Add a test case to reproduce issue openzfs#17277:

1. Make a pool
2. Write a file to the pool
3. Mount the file as a loopback device
4. Make an XFS filesystem on the loopback device
5. Mount the XFS filesystem... <hangs>

Signed-off-by: Tony Hutter <[email protected]>
@github-actions github-actions bot removed the Status: Revision Needed Changes are required for the PR to be accepted label Aug 22, 2025
@behlendorf behlendorf added the Status: Accepted Ready to integrate (reviewed, tested) label Aug 22, 2025
@behlendorf behlendorf merged commit d247538 into openzfs:master Aug 25, 2025
24 of 25 checks passed
spauka pushed a commit to spauka/zfs that referenced this pull request Aug 30, 2025
Add a test case to reproduce issue openzfs#17277:

1. Make a pool
2. Write a file to the pool
3. Mount the file as a loopback device
4. Make an XFS filesystem on the loopback device
5. Mount the XFS filesystem... <hangs>

Reviewed-by: Alexander Motin <[email protected]>
Reviewed-by: Rob Norris <[email protected]>
Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: Tony Hutter <[email protected]>
Issue openzfs#17277
Closes openzfs#17329
behlendorf pushed a commit to behlendorf/zfs that referenced this pull request Sep 10, 2025
Add a test case to reproduce issue openzfs#17277:

1. Make a pool
2. Write a file to the pool
3. Mount the file as a loopback device
4. Make an XFS filesystem on the loopback device
5. Mount the XFS filesystem... <hangs>

Reviewed-by: Alexander Motin <[email protected]>
Reviewed-by: Rob Norris <[email protected]>
Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: Tony Hutter <[email protected]>
Issue openzfs#17277
Closes openzfs#17329
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Accepted Ready to integrate (reviewed, tested)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants