Pool can be created on top of a ext2 device

[FransUrbo]

Way to reproduce:

#!/bin/sh                                                                                                                                                     

rm -f /var/tmp/zfs_test-1
truncate -s 25000m /var/tmp/zfs_test-1
zpool create test /var/tmp/zfs_test-1

parted -s /var/tmp/zfs_test-1 mklabel msdos
parted -s /var/tmp/zfs_test-1 mkpart primary 1 205

set -- `kpartx -asfv /var/tmp/zfs_test-1`
loop_part=/dev/mapper/${8##*/}p1
mke2fs ${loop_part} > /dev/null

zpool create test2 ${loop_part}
res=$?
echo "zpool create test2: $res"

[ "$res" -eq "0" ] && zpool destroy test2
kpartx -d /var/tmp/zfs_test-1

Results in:

# /tmp/testme.sh
mke2fs 1.42.5 (29-Jul-2012)
zpool create test2: 0
loop deleted : /dev/loop0

[behlendorf]

I'm not sure what can be done about this. The best we could hope for is to have ZFS open the file O_EXCL and then hope the various other utilities honor that.

[FransUrbo]

I'm not sure what can be done about this. The best we could hope for is to have ZFS open the file O_EXCL and then hope the various other utilities honor that.

The other way around... First ext2 and then ZFS - we need to verify what type of device we're "sending" (using) to zpool and ignore (or at least require "-f") if it's of a known type (extX, xfs, jfs, md etc, etc).

[ryao]

@FransUrbo What you want to accomplish is doable by hooking into libblkid.

That being said, I am think this is on the borderline of being a bug and not being a bug. My reasoning is that mkfs and zpool create are destructive operations like dd and should be treated as such. While there have been users who lost data misusing the dd tool, no one has proposed modifying dd to be safe. At the same time, I believe that the ext4 developers rely on libblkid to check and it would be good to reciprocate.

[FransUrbo]

That being said, I am think this is on the borderline of being a bug and not being a bug.

I'm borderline agreeing with you :).

HOWEVER, this is a bug in the Illumos code. If it's a bug there, it should be
one here as well - "all ZFS should behave similarly"...

My reasoning is that mkfs and zpool create are destructive operations like dd and should be treated as such.

Yes, but not without a 'force' option. Accepting such a device, without any
warning what so ever is simply wrong. It should warn, and force the admin/user
to use '-f'...
While there have been users who lost data misusing the dd tool, no one has proposed modifying dd to be safe.

Fair enough...

[behlendorf]

@FransUrbo I see what you're saying now. Actually this case should be already handled assuming you've built with libblkid support. The current code makes this functionality optional but perhaps it should be mandatory. Check if your build has HAVE_LIBBLKID enabled. The function check_slice() will be called during by zpool create to check if each device is safe to use. Assuming libblkid is available it should fail with the following warning if there is a fileystem on the partition.

/dev/loop1 contains a filesystem of type 'ext4'

[FransUrbo]

The current code makes this functionality optional but perhaps it should be mandatory.

I think it's such an important (and expected - on Linux?) feature that it should be mandatory. "Everyone else" does it :)
Check if your build has HAVE_LIBBLKID enabled.

It doesn't. I'll try to remember to make sure it does in the future... Thanx.

Close or relabel as "Linking with libblkid should be mandatory"?

[behlendorf]

@FransUrbo Better yet, let's just enable it by default in the packaging. I believe @ryao already does this in the Gentoo packaging.

http://www.rpm.org/wiki/PackagerDocs/ConditionalBuilds
#2502

With the above patch applied I get the correct behavior with your test script.

Warning: The resulting partition is not properly aligned for best performance.
mke2fs 1.41.12 (17-May-2010)
invalid vdev specification
use '-f' to override the following errors:
/dev/mapper/loop0p1 contains a filesystem of type 'ext2'
zpool create test2: 1
loop deleted : /dev/loop0

[ryao]

@FransUrbo Better yet, let's just enable it by default in the packaging. I believe @ryao already does this in the Gentoo packaging.

That is correct.

[FransUrbo]

@FransUrbo Better yet, let's just enable it by default in the packaging.

So your saying that a source install shouldn't have this by default? Only packages?

[behlendorf]

No... I'm just saying that is slightly more work. A source install will enable this by default if the libblkid-devel package is installed. The current behavior is to auto-detect the library. Arguably your right we should make the default to require it but allow it to be conditionally disabled.

[FransUrbo]

Pull request #2512 includes your fix, with additional changes to config/user-libblkid.m4 to make sure that libblkid is included in build (i.e. make it default/required). To disable this behaviour, use --without-blkid.

[behlendorf]

@FransUrbo I'm all for changing the default as you've done in #2512. I've posted a few review comments on the patch so we can conform to the standard guidelines for this kind of check.

[FransUrbo]

Ok, force updated with your suggestions...