Relax MBR partition scanning requirement

When checking a whole disk to see if it can be safely added to
the pool a variety of checks are done.  One of those checks is
to attempt to determine the partition information and scan all
the partitions for existing filesystems.

Since ZoL contains a EFI library this partition scanning is
easy to do for GPT partitioned disks.  However, for non-GPT
partitioned disks (MBR/EBR) things are a bit harder.  The lack of
a convenient library means non-GPT partitioned disks will not
have all their partitions checked.  For this reason, the default
behavior was to require the force option.  For example:

invalid vdev specification
use '-f' to override the following errors:
/dev/vdb does not contain an GPT label but it may contain partition
information in the MBR.

However in practice requiring the force option for this case is
counter-intuitively less safe.  The reason is because only the first
error is returned.  By passing the force option it will suppress
this first warning and potentially others you were not aware of.

Therefore this patch inverts the default behavior for non-GPT
formated disks (unformatted, MBR/EBR, etc).  If no GPT table is
detected and there is no file system detected on the provided
block device.  Then it will be assumed that block device is safe
to use.

Longer term it would be nice to see MBR/EBR scanning added to
the utilities.  This should be fairly straight forward to do.
However these days it's somewhat less critical because Linux
defaults to GPT partition tables for devices 2TB or larger.

Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov>
Closes #2660
Closes #2274

diff --git a/cmd/zpool/zpool_vdev.c b/cmd/zpool/zpool_vdev.c
index 54ca1b261959b022b9414dfe6262788621a4649e..6469de1ce02db6f2668e3bf7083a37ea442a815d 100644 (file)
--- a/cmd/zpool/zpool_vdev.c
+++ b/cmd/zpool/zpool_vdev.c
@@ -399,8 +399,16 @@ check_slice(const char *path, blkid_cache cache, int force, boolean_t isspare)
 }
 
 /*
- * Validate a whole disk.  Iterate over all slices on the disk and make sure
- * that none is in use by calling check_slice().
+ * Validate that a disk including all partitions are safe to use.
+ *
+ * For EFI labeled disks this can done relatively easily with the libefi
+ * library.  The partition numbers are extracted from the label and used
+ * to generate the expected /dev/ paths.  Each partition can then be
+ * checked for conflicts.
+ *
+ * For non-EFI labeled disks (MBR/EBR/etc) the same process is possible
+ * but due to the lack of a readily available libraries this scanning is
+ * not implemented.  Instead only the device path as given is checked.
  */
 static int
 check_disk(const char *path, blkid_cache cache, int force,
@@ -411,34 +419,22 @@ check_disk(const char *path, blkid_cache cache, int force,
        int err = 0;
        int fd, i;
 
-       /* This is not a wholedisk we only check the given partition */
        if (!iswholedisk)
                return (check_slice(path, cache, force, isspare));
 
-       /*
-        * When the device is a whole disk try to read the efi partition
-        * label.  If this is successful we safely check the all of the
-        * partitions.  However, when it fails it may simply be because
-        * the disk is partitioned via the MBR.  Since we currently can
-        * not easily decode the MBR return a failure and prompt to the
-        * user to use force option since we cannot check the partitions.
-        */
        if ((fd = open(path, O_RDONLY|O_DIRECT)) < 0) {
                check_error(errno);
                return (-1);
        }
 
-       if ((err = efi_alloc_and_read(fd, &vtoc)) != 0) {
+       /*
+        * Expected to fail for non-EFI labled disks.  Just check the device
+        * as given and do not attempt to detect and scan partitions.
+        */
+       err = efi_alloc_and_read(fd, &vtoc);
+       if (err) {
                (void) close(fd);
-
-               if (force) {
-                       return (0);
-               } else {
-                       vdev_error(gettext("%s does not contain an EFI "
-                           "label but it may contain partition\n"
-                           "information in the MBR.\n"), path);
-                       return (-1);
-               }
+               return (check_slice(path, cache, force, isspare));
        }
 
        /*