]> granicus.if.org Git - zfs/commitdiff
Use stored whole_disk property when opening a vdev
authorNed Bass <bass6@llnl.gov>
Mon, 4 Oct 2010 20:08:24 +0000 (13:08 -0700)
committerBrian Behlendorf <behlendorf1@llnl.gov>
Mon, 4 Oct 2010 20:53:18 +0000 (13:53 -0700)
This commit fixes a bug in vdev_disk_open() in which the whole_disk property
was getting set to 0 for disk devices, even when it was stored as a 1 when the
zpool was created.  The whole_disk property lets us detect when the partition
suffix should be stripped from the device name in CLI output.  It is also used
to determine how writeback cache should be set for a device.

When an existing zpool is imported its configuration is read from the vdev
label by user space in zpool_read_label().  The whole_disk property is saved in
the nvlist which gets passed into the kernel, where it in turn gets saved in
the vdev struct in vdev_alloc().  Therefore, this value is available in
vdev_disk_open() and should not be overridden by checking the provided device
path, since that path will likely point to a partition and the check will
return the wrong result.

We also add an ASSERT that the whole_disk property is set.  We are not aware of
any cases where vdev_disk_open() should be called with a config that doesn't
have this property set.  The ASSERT is there so that when debugging is enabled
we can identify any legitimate cases that we are missing.  If we never hit the
ASSERT, we can at some point remove it along with the conditional whole_disk
check.

Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov>
module/zfs/vdev_disk.c

index dbf9b08e763aee6ddf0949b3ac21ad8b9f01a993..6b410b1b6de3b561dab81b1360fa985ab5d02696 100644 (file)
@@ -133,9 +133,19 @@ vdev_disk_open(vdev_t *v, uint64_t *psize, uint64_t *ashift)
        vd->vd_bdev = bdev;
        block_size =  vdev_bdev_block_size(bdev);
 
-       /* Check if this is a whole device.  When bdev->bd_contains ==
-        * bdev we have a whole device and not simply a partition. */
-       v->vdev_wholedisk = !!(bdev->bd_contains == bdev);
+       /* We think the wholedisk property should always be set when this
+        * function is called.  ASSERT here so if any legitimate cases exist
+        * where it's not set, we'll find them during debugging.  If we never
+        * hit the ASSERT, this and the following conditional statement can be
+        * removed. */
+       ASSERT3S(v->vdev_wholedisk, !=, -1ULL);
+
+       /* The wholedisk property was initialized to -1 in vdev_alloc() if it
+        * was unspecified.  In that case, check if this is a whole device.
+        * When bdev->bd_contains == bdev we have a whole device and not simply
+        * a partition. */
+       if (v->vdev_wholedisk == -1ULL)
+               v->vdev_wholedisk = (bdev->bd_contains == bdev);
 
        /* Clear the nowritecache bit, causes vdev_reopen() to try again. */
        v->vdev_nowritecache = B_FALSE;