2016-12-07 13:32:01

by Dmitry Monakhov

[permalink] [raw]
Subject: [PATCH 1/2] test: add f_mke2fs_baddisk

Check what mke2fs will return non zero error on broken device.

Signed-off-by: Dmitry Monakhov <[email protected]>
---
tests/f_mke2fs_baddisk/script | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)
create mode 100644 tests/f_mke2fs_baddisk/script

diff --git a/tests/f_mke2fs_baddisk/script b/tests/f_mke2fs_baddisk/script
new file mode 100644
index 0000000..a0659c7
--- /dev/null
+++ b/tests/f_mke2fs_baddisk/script
@@ -0,0 +1,19 @@
+test_description="mke2fs /dev/mapper/bad_disk should fail"
+
+OUT=$test_name.log
+
+dmsetup create -v bad_disk --table '0 1148681097 error' > $OUT 2>&1
+echo mke2fs /dev/mapper/bad_disk >> $OUT
+$MKE2FS /dev/mapper/bad_disk >> $OUT 2>&1
+status=$?
+sleep 1
+dmsetup remove --retry bad_disk >> $OUT 2>&1
+
+if [ "$status" = 0 ] ; then
+ ln -f $test_name.log $test_name.failed
+ echo "$test_name: $test_description: failed"
+else
+ echo "$test_name: $test_description: ok"
+ touch $test_name.ok
+
+fi
--
2.7.4



2016-12-07 13:31:59

by Dmitry Monakhov

[permalink] [raw]
Subject: [PATCH 2/2] ext2fs: check fsync error code

testcase: f_mke2fs_baddisk
Signed-off-by: Dmitry Monakhov <[email protected]>
---
lib/ext2fs/unix_io.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/lib/ext2fs/unix_io.c b/lib/ext2fs/unix_io.c
index 429ea24..6c54cbe 100644
--- a/lib/ext2fs/unix_io.c
+++ b/lib/ext2fs/unix_io.c
@@ -1030,7 +1030,10 @@ static errcode_t unix_flush(io_channel channel)
#ifndef NO_IO_CACHE
retval = flush_cached_blocks(channel, data, 0);
#endif
- fsync(data->dev);
+ if(fsync(data->dev)) {
+ if (!retval)
+ retval = errno;
+ }
return retval;
}

--
2.7.4


2016-12-07 14:19:44

by Theodore Ts'o

[permalink] [raw]
Subject: Re: [PATCH 1/2] test: add f_mke2fs_baddisk

On Wed, Dec 07, 2016 at 05:14:18PM +0400, Dmitry Monakhov wrote:
> Check what mke2fs will return non zero error on broken device.
>
> Signed-off-by: Dmitry Monakhov <[email protected]>

Most developers don't run "make check" as root. So at minimum the
test should check to see if it is running as root, or be able to deal
with dmsetup failing due to permissions check.

I wouldn't object if there was some environment variable, say SUDO or
DOROOT, which, if set, would prefix the commands which require root
with $DOROOT such that if the developer is willing to enable running
specific commands as root, that was allowed for the regression test.

Alternatively the long-term plan that I have is to allow pathnames to
be specified using a URL-like scheme, e.g.;

unix://path/to/filename
test://unix/path/to/filename

Which would allow us to define a proper test mock I/O manager that
would allow for these sorts of tests without requiring root.

Cheers,

- Ted

2016-12-07 14:50:14

by Dmitry Monakhov

[permalink] [raw]
Subject: Re: [PATCH 1/2] test: add f_mke2fs_baddisk

Theodore Ts'o <[email protected]> writes:

> On Wed, Dec 07, 2016 at 05:14:18PM +0400, Dmitry Monakhov wrote:
>> Check what mke2fs will return non zero error on broken device.
>>
>> Signed-off-by: Dmitry Monakhov <[email protected]>
>
> Most developers don't run "make check" as root. So at minimum the
> test should check to see if it is running as root, or be able to deal
> with dmsetup failing due to permissions check.
You right. There are number of reasons why dmsetup may fail (kernel
is too old, no kernel compiled w/o DM). I'll send updated version.
>
> I wouldn't object if there was some environment variable, say SUDO or
> DOROOT, which, if set, would prefix the commands which require root
> with $DOROOT such that if the developer is willing to enable running
> specific commands as root, that was allowed for the regression test.
>
> Alternatively the long-term plan that I have is to allow pathnames to
> be specified using a URL-like scheme, e.g.;
>
> unix://path/to/filename
> test://unix/path/to/filename
>
> Which would allow us to define a proper test mock I/O manager that
> would allow for these sorts of tests without requiring root.
>
> Cheers,
>
> - Ted