"zpool add" safety checks are skipped if a pool was created with "-f"

[Graham Todd]

On Tue, 13 Nov 2012, nikitosiusis wrote:

> Good day everyone.
> I managed to add a single device to a raidz2 pool by a typo(actually I
> wanted to add a cache device) without "-f" flag. This can be
> reproduced with md devices.
> # for a in {1..8}; do dd if=/dev/zero of=$a bs=1M count=96;done
> # dd if=/dev/zero of=9 bs=1M count=128
> # ls -la
> -rw-r--r--  1 root  wheel  100663296 Nov 12 21:04 1
> -rw-r--r--  1 root  wheel  100663296 Nov 12 21:04 2
> -rw-r--r--  1 root  wheel  100663296 Nov 12 21:04 3
> -rw-r--r--  1 root  wheel  100663296 Nov 12 21:04 4
> -rw-r--r--  1 root  wheel  100663296 Nov 12 21:04 5
> -rw-r--r--  1 root  wheel  100663296 Nov 12 21:04 6
> -rw-r--r--  1 root  wheel  100663296 Nov 12 21:04 7
> -rw-r--r--  1 root  wheel  100663296 Nov 12 21:04 8
> -rw-r--r--  1 root  wheel  134217728 Nov 12 21:04 9
> # for a in {1..9}; do mdconfig -f ~/tmp/$a; done
>
> Now we create a raidz pool with 8 identical drives.
>
> # zpool create testpool raidz2 md{1..8}
> # zpool add testpool md9
> invalid vdev specification
> use '-f' to override the following errors:
> mismatched replication level: pool uses raidz and new vdev is disk
>
> It's ok. It doesn't allow to add a device.
> Now we create a new pool, but with devices of different size(I don't
> know what is the difference in size allowed, I used 32mb).
>
> # zpool create testpool raidz2 md{1..7} md9
> invalid vdev specification
> use '-f' to override the following errors:
> raidz contains devices of different sizes
> # zpool create -f testpool raidz2 md{1..7} md9
> # zpool add testpool md8
> And we have no error here - single drive is added to the pool. Since
> it is an undoable action - it can ruin your pool and should be
> considered as a bug imho. Here is my result in production.
>    NAME         SIZE  ALLOC   FREE    CAP  DEDUP  HEALTH  ALTROOT
>    root        3.63T  1.06T  2.57T    29%  1.00x  ONLINE  -
>      raidz2    3.62T  1.06T  2.57T         -
>        ada0        -      -      -         -
>        ada1        -      -      -         -
>        ada3        -      -      -         -
>        ada4        -      -      -         -
>        ada5        -      -      -         -
>        ada6        -      -      -         -
>        ada7        -      -      -         -
>        ada8        -      -      -         -
>      da0p1     3.75G  76.5K  3.75G         -
>
> # uname -rv
> 9.1-PRERELEASE FreeBSD 9.1-PRERELEASE #2: Thu Nov  8 13:50:55 UTC 2012
>    root at ex.a.nikitos.name:/usr/obj/usr/src/sys/GENERIC

I haven't done it lately, but it was possible to make a similar error with 
a simple mirror vdev if you were trying to add a second pool or a second 
separate vdev in an existing pool and "added" instead of "attached", or 
mistyped the pool name (sometimes pools have similar names). I agree that 
if this risks creating an undoable mistake in a pool the lack of checks or 
verification should be considered a "bug".

> # zpool get version testpool
> NAME      PROPERTY  VALUE    SOURCE
> testpool  version   28       default
> # zfs get version testpool
> NAME      PROPERTY  VALUE    SOURCE
> testpool  version   5        -
>
> btw is there a chance to remove this device now?

I'm not sure if some combination of split, detach, remove, destroy would 
have helped in the simpler case of a mirror - the split feature was not 
available when this happened.  We had root/boot on a separate device, not 
much data and extra disks, so we could zfs send/recv things to a new pool 
and rebuild things that way. Better checks at creation time would have 
prevented any problems.

Is there a PR for these sort of issues at freebsd.org or illumos or 
elsewhere?

cheers