From 3a7381e531f55929f894fa2e81b79e1980de6f22 Mon Sep 17 00:00:00 2001 From: Ned Bass Date: Mon, 4 Oct 2010 13:08:24 -0700 Subject: Use stored whole_disk property when opening a vdev 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 --- module/zfs/vdev_disk.c | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) (limited to 'module') diff --git a/module/zfs/vdev_disk.c b/module/zfs/vdev_disk.c index dbf9b08e7..6b410b1b6 100644 --- a/module/zfs/vdev_disk.c +++ b/module/zfs/vdev_disk.c @@ -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; -- cgit v1.2.3