diff mbox series

[RFC,7/9] selftests: block_seek_hole: add dm-linear test

Message ID 20240328203910.2370087-8-stefanha@redhat.com (mailing list archive)
State Deferred, archived
Delegated to: Mike Snitzer
Headers show
Series block: add llseek(SEEK_HOLE/SEEK_DATA) support | expand

Commit Message

Stefan Hajnoczi March 28, 2024, 8:39 p.m. UTC
The dm-linear linear target passes through SEEK_HOLE/SEEK_DATA. Extend
the test case to check that the same holes/data are reported as for the
underlying file.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 tools/testing/selftests/block_seek_hole/test.py | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

Comments

Eric Blake March 29, 2024, 12:59 a.m. UTC | #1
On Thu, Mar 28, 2024 at 04:39:08PM -0400, Stefan Hajnoczi wrote:
> The dm-linear linear target passes through SEEK_HOLE/SEEK_DATA. Extend
> the test case to check that the same holes/data are reported as for the
> underlying file.
> 
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
>  tools/testing/selftests/block_seek_hole/test.py | 16 +++++++++++++++-
>  1 file changed, 15 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/testing/selftests/block_seek_hole/test.py b/tools/testing/selftests/block_seek_hole/test.py
> index 4f7c2d01ab3d3..6360b72aee338 100755
> --- a/tools/testing/selftests/block_seek_hole/test.py
> +++ b/tools/testing/selftests/block_seek_hole/test.py
> @@ -45,6 +45,20 @@ def loop_device(file_path):
>      finally:
>          run(['losetup', '-d', loop_path])
>  
> +@contextmanager
> +def dm_linear(file_path):
> +    file_size = os.path.getsize(file_path)
> +
> +    with loop_device(file_path) as loop_path:
> +        dm_name = f'test-{os.getpid()}'
> +        run(['dmsetup', 'create', dm_name, '--table',
> +             f'0 {file_size // 512} linear {loop_path} 0'])

Would it be worth tryiing to create the dm with two copies of
loop_path concatenated one after the other?  You'd have to do more
work on expected output (coalescing adjacent data or holes between the
tail of the first copy and the head of the second), but without that
in place, I worry that you are missing logic bugs for when there is
more than one table in the overall dm (as evidenced by my review in
4/9).
Stefan Hajnoczi April 3, 2024, 2:23 p.m. UTC | #2
On Thu, Mar 28, 2024 at 07:59:14PM -0500, Eric Blake wrote:
> On Thu, Mar 28, 2024 at 04:39:08PM -0400, Stefan Hajnoczi wrote:
> > The dm-linear linear target passes through SEEK_HOLE/SEEK_DATA. Extend
> > the test case to check that the same holes/data are reported as for the
> > underlying file.
> > 
> > Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> > ---
> >  tools/testing/selftests/block_seek_hole/test.py | 16 +++++++++++++++-
> >  1 file changed, 15 insertions(+), 1 deletion(-)
> > 
> > diff --git a/tools/testing/selftests/block_seek_hole/test.py b/tools/testing/selftests/block_seek_hole/test.py
> > index 4f7c2d01ab3d3..6360b72aee338 100755
> > --- a/tools/testing/selftests/block_seek_hole/test.py
> > +++ b/tools/testing/selftests/block_seek_hole/test.py
> > @@ -45,6 +45,20 @@ def loop_device(file_path):
> >      finally:
> >          run(['losetup', '-d', loop_path])
> >  
> > +@contextmanager
> > +def dm_linear(file_path):
> > +    file_size = os.path.getsize(file_path)
> > +
> > +    with loop_device(file_path) as loop_path:
> > +        dm_name = f'test-{os.getpid()}'
> > +        run(['dmsetup', 'create', dm_name, '--table',
> > +             f'0 {file_size // 512} linear {loop_path} 0'])
> 
> Would it be worth tryiing to create the dm with two copies of
> loop_path concatenated one after the other?  You'd have to do more
> work on expected output (coalescing adjacent data or holes between the
> tail of the first copy and the head of the second), but without that
> in place, I worry that you are missing logic bugs for when there is
> more than one table in the overall dm (as evidenced by my review in
> 4/9).

Yes, I agree that more tests are needed to cover transitions between
adjacent targets.

Stefan
diff mbox series

Patch

diff --git a/tools/testing/selftests/block_seek_hole/test.py b/tools/testing/selftests/block_seek_hole/test.py
index 4f7c2d01ab3d3..6360b72aee338 100755
--- a/tools/testing/selftests/block_seek_hole/test.py
+++ b/tools/testing/selftests/block_seek_hole/test.py
@@ -45,6 +45,20 @@  def loop_device(file_path):
     finally:
         run(['losetup', '-d', loop_path])
 
+@contextmanager
+def dm_linear(file_path):
+    file_size = os.path.getsize(file_path)
+
+    with loop_device(file_path) as loop_path:
+        dm_name = f'test-{os.getpid()}'
+        run(['dmsetup', 'create', dm_name, '--table',
+             f'0 {file_size // 512} linear {loop_path} 0'])
+
+        try:
+            yield f'/dev/mapper/{dm_name}'
+        finally:
+            run(['dmsetup', 'remove', dm_name])
+
 def test(layout, dev_context_manager):
     with test_file(layout) as file_path, dev_context_manager(file_path) as dev_path:
         cmd = run(['./map_holes.py', file_path])
@@ -99,5 +113,5 @@  if __name__ == '__main__':
                holes_at_beginning_and_end,
                no_holes,
                empty_file]
-    dev_context_managers = [loop_device]
+    dev_context_managers = [loop_device, dm_linear]
     test_all(layouts, dev_context_managers)