Re: [PATCH blktests] block/013: Add test for BLKRRPART ioctl

2017-12-20 Thread Johannes Thumshirn
Omar Sandoval  writes:

> On Tue, Dec 19, 2017 at 11:47:09AM +0100, Johannes Thumshirn wrote:
>> xiao yang  writes:
>> 
>> > +requires() {
>> > +  _have_program mkfs.ext3
>> > +}
>> [...]
>> > +  # Format
>> > +  mkfs.ext3 -F "$TEST_DEV" >> "$FULL" 2>&1
>> 
>> What's the reason to limit the test case to ext3 only? Can you switch it
>> to the generic 'mkfs' command? IIRC we require 'util-linux' to be
>> present for blktests but mkfs.ext3 is (at least in SUSE based distros)
>> in the e2fsprogs package.
>
> I'm fine with only testing one specific filesystem, since we're really
> testing generic functionality and not any filesystem code. And according
> to the manpage, bare mkfs is deprecated. I'm fine with requiring
> e2fsprogs for this test, and xiao yang added the _have_program there for
> it.

what about something like (totally untested):

if _have_program mkfs.ext3; then
   mkfs_prog="mkfs.ext3"
   return 1
elif _have_program mkfs.xfs; then
   mkfs_prog="mkfs.xfs"
   return 1
elif _have_program mkfs.brtfs; then
   mkfs_prog="mkfs.brtfs"
   return 1
else
   retrun 0
fi


-- 
Johannes Thumshirn  Storage
jthumsh...@suse.de+49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850


Re: [PATCH blktests] block/013: Add test for BLKRRPART ioctl

2017-12-19 Thread Omar Sandoval
On Tue, Dec 19, 2017 at 06:30:23PM +0800, xiao yang wrote:
> If the entire block device is formatted with a filesystem and
> mounted, running "blockdev --rereadpt" should fail and return
> EBUSY instead of pass.
> 
> Signed-off-by: xiao yang 

Thanks, applied (with some slight modifications to make it easier to
diagnose a failure).


Re: [PATCH blktests] block/013: Add test for BLKRRPART ioctl

2017-12-19 Thread Omar Sandoval
On Tue, Dec 19, 2017 at 11:47:09AM +0100, Johannes Thumshirn wrote:
> xiao yang  writes:
> 
> > +requires() {
> > +   _have_program mkfs.ext3
> > +}
> [...]
> > +   # Format
> > +   mkfs.ext3 -F "$TEST_DEV" >> "$FULL" 2>&1
> 
> What's the reason to limit the test case to ext3 only? Can you switch it
> to the generic 'mkfs' command? IIRC we require 'util-linux' to be
> present for blktests but mkfs.ext3 is (at least in SUSE based distros)
> in the e2fsprogs package.

I'm fine with only testing one specific filesystem, since we're really
testing generic functionality and not any filesystem code. And according
to the manpage, bare mkfs is deprecated. I'm fine with requiring
e2fsprogs for this test, and xiao yang added the _have_program there for
it.

> apart from that:
> Reviewed-by: Johannes Thumshirn 


[PATCH blktests] block/013: Add test for BLKRRPART ioctl

2017-12-19 Thread xiao yang
If the entire block device is formatted with a filesystem and
mounted, running "blockdev --rereadpt" should fail and return
EBUSY instead of pass.

Signed-off-by: xiao yang 
---
 tests/block/013 | 62 +
 tests/block/013.out |  3 +++
 2 files changed, 65 insertions(+)
 create mode 100755 tests/block/013
 create mode 100644 tests/block/013.out

diff --git a/tests/block/013 b/tests/block/013
new file mode 100755
index 000..77ae55c
--- /dev/null
+++ b/tests/block/013
@@ -0,0 +1,62 @@
+#!/bin/bash
+#
+# If the entire block device is formatted with a filesystem and
+# mounted, running "blockdev --rereadpt" should fail and return
+# EBUSY.  On buggy kernel, it passes unexpectedly.
+#
+# Regression test for commit 77032ca66f86 ("Return EBUSY from
+# BLKRRPART for mounted whole-dev fs").
+#
+# Copyright (c) 2017 FUJITSU LIMITED. All rights reserved.
+# Author: Xiao Yang 
+#
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see .
+
+DESCRIPTION="test return EBUSY from BLKRRPART for mounted whole-dev"
+QUICK=1
+
+requires() {
+   _have_program mkfs.ext3
+}
+
+test_device() {
+   echo "Running ${TEST_NAME}"
+
+   rm -f "$FULL"
+   mkdir "$TMPDIR/mntpoint"
+
+   # Format
+   mkfs.ext3 -F "$TEST_DEV" >> "$FULL" 2>&1
+
+   # Mount
+   mount "$TEST_DEV" "$TMPDIR/mntpoint"
+
+   # Check mounted whole-dev
+   local out=$(blockdev --rereadpt "$TEST_DEV" 2>&1)
+   echo $out | grep -q "Device or resource busy"
+   if [ $? -eq 0 ]; then
+   echo "Return EBUSY for mounted whole-dev"
+   else
+   echo "Don't return EBUSY for mounted whole-dev"
+   fi
+
+   echo $out >> "$FULL"
+
+   # Umount
+   umount "$TMPDIR/mntpoint"
+
+   rm -rf "$TMPDIR/mntpoint"
+
+   echo "Test complete"
+}
diff --git a/tests/block/013.out b/tests/block/013.out
new file mode 100644
index 000..74d1d08
--- /dev/null
+++ b/tests/block/013.out
@@ -0,0 +1,3 @@
+Running block/013
+Return EBUSY for mounted whole-dev
+Test complete
-- 
1.8.3.1