svn commit: r292066 - head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs
Alan Somers
asomers at FreeBSD.org
Thu Dec 10 21:46:22 UTC 2015
Author: asomers
Date: Thu Dec 10 21:46:21 2015
New Revision: 292066
URL: https://svnweb.freebsd.org/changeset/base/292066
Log:
During vdev_geom_open, require that the vdev guids match the device's label
except during split, add, or create operations. This fixes a bug where the
wrong disk could be returned, and higher layers of ZFS would immediately
eject it again.
sys/cddl/contrib/opensolaris/uts/common/fs/zfs/vdev_geom.c:
o When opening by GUID, require both the pool and vdev GUIDs to
match. While it is highly unlikely for two vdevs to have the same
vdev GUIDs, the ZFS storage pool allocator only guarantees they
are unique within a pool.
o Modify the open behavior to:
- If we are opening a vdev that hasn't previously been opened,
open by path without checking GUIDs.
- Otherwise, open by path and verify GUIDs.
- If that fails, search all geom providers for a device with
matching GUIDs.
- If that fails, return ENOENT.
Submitted by: gibbs, asomers
Reviewed by: smh
MFC after: 4 weeks
Sponsored by: Spectra Logic Corp
Differential Revision: https://reviews.freebsd.org/D4486
Modified:
head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/vdev_geom.c
Modified: head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/vdev_geom.c
==============================================================================
--- head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/vdev_geom.c Thu Dec 10 16:44:40 2015 (r292065)
+++ head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/vdev_geom.c Thu Dec 10 21:46:21 2015 (r292066)
@@ -206,14 +206,12 @@ vdev_geom_detach(void *arg, int flag __u
}
}
-static uint64_t
-nvlist_get_guid(nvlist_t *list)
+static void
+nvlist_get_guids(nvlist_t *list, uint64_t *pguid, uint64_t *vguid)
{
- uint64_t value;
- value = 0;
- nvlist_lookup_uint64(list, ZPOOL_CONFIG_GUID, &value);
- return (value);
+ nvlist_lookup_uint64(list, ZPOOL_CONFIG_GUID, vguid);
+ nvlist_lookup_uint64(list, ZPOOL_CONFIG_POOL_GUID, pguid);
}
static int
@@ -268,7 +266,7 @@ vdev_geom_read_config(struct g_consumer
size_t buflen;
uint64_t psize;
off_t offset, size;
- uint64_t guid, state, txg;
+ uint64_t state, txg;
int error, l, len;
g_topology_assert_not();
@@ -282,7 +280,6 @@ vdev_geom_read_config(struct g_consumer
size = sizeof(*label) + pp->sectorsize -
((sizeof(*label) - 1) % pp->sectorsize) - 1;
- guid = 0;
label = kmem_alloc(size, KM_SLEEP);
buflen = sizeof(label->vl_vdev_phys.vp_nvlist);
@@ -477,30 +474,29 @@ vdev_geom_read_pool_label(const char *na
return (*count > 0 ? 0 : ENOENT);
}
-static uint64_t
-vdev_geom_read_guid(struct g_consumer *cp)
+static void
+vdev_geom_read_guids(struct g_consumer *cp, uint64_t *pguid, uint64_t *vguid)
{
nvlist_t *config;
- uint64_t guid;
g_topology_assert_not();
- guid = 0;
+ *pguid = 0;
+ *vguid = 0;
if (vdev_geom_read_config(cp, &config) == 0) {
- guid = nvlist_get_guid(config);
+ nvlist_get_guids(config, pguid, vguid);
nvlist_free(config);
}
- return (guid);
}
static struct g_consumer *
-vdev_geom_attach_by_guid(uint64_t guid)
+vdev_geom_attach_by_guids(uint64_t pool_guid, uint64_t vdev_guid)
{
struct g_class *mp;
struct g_geom *gp, *zgp;
struct g_provider *pp;
struct g_consumer *cp, *zcp;
- uint64_t pguid;
+ uint64_t pguid, vguid;
g_topology_assert();
@@ -520,15 +516,15 @@ vdev_geom_attach_by_guid(uint64_t guid)
if (vdev_geom_attach_taster(zcp, pp) != 0)
continue;
g_topology_unlock();
- pguid = vdev_geom_read_guid(zcp);
+ vdev_geom_read_guids(zcp, &pguid, &vguid);
g_topology_lock();
vdev_geom_detach_taster(zcp);
- if (pguid != guid)
+ if (pguid != pool_guid || vguid != vdev_guid)
continue;
cp = vdev_geom_attach(pp);
if (cp == NULL) {
- printf("ZFS WARNING: Unable to attach to %s.\n",
- pp->name);
+ ZFS_LOG(1, "ZFS WARNING: Unable to "
+ "attach to %s.\n", pp->name);
continue;
}
break;
@@ -546,7 +542,7 @@ end:
}
static struct g_consumer *
-vdev_geom_open_by_guid(vdev_t *vd)
+vdev_geom_open_by_guids(vdev_t *vd)
{
struct g_consumer *cp;
char *buf;
@@ -555,7 +551,7 @@ vdev_geom_open_by_guid(vdev_t *vd)
g_topology_assert();
ZFS_LOG(1, "Searching by guid [%ju].", (uintmax_t)vd->vdev_guid);
- cp = vdev_geom_attach_by_guid(vd->vdev_guid);
+ cp = vdev_geom_attach_by_guids(spa_guid(vd->vdev_spa), vd->vdev_guid);
if (cp != NULL) {
len = strlen(cp->provider->name) + strlen("/dev/") + 1;
buf = kmem_alloc(len, KM_SLEEP);
@@ -564,10 +560,12 @@ vdev_geom_open_by_guid(vdev_t *vd)
spa_strfree(vd->vdev_path);
vd->vdev_path = buf;
- ZFS_LOG(1, "Attach by guid [%ju] succeeded, provider %s.",
+ ZFS_LOG(1, "Attach by guid [%ju:%ju] succeeded, provider %s.",
+ (uintmax_t)spa_guid(vd->vdev_spa),
(uintmax_t)vd->vdev_guid, vd->vdev_path);
} else {
- ZFS_LOG(1, "Search by guid [%ju] failed.",
+ ZFS_LOG(1, "Search by guid [%ju:%ju] failed.",
+ (uintmax_t)spa_guid(vd->vdev_spa),
(uintmax_t)vd->vdev_guid);
}
@@ -579,7 +577,7 @@ vdev_geom_open_by_path(vdev_t *vd, int c
{
struct g_provider *pp;
struct g_consumer *cp;
- uint64_t guid;
+ uint64_t pguid, vguid;
g_topology_assert();
@@ -591,14 +589,17 @@ vdev_geom_open_by_path(vdev_t *vd, int c
if (cp != NULL && check_guid && ISP2(pp->sectorsize) &&
pp->sectorsize <= VDEV_PAD_SIZE) {
g_topology_unlock();
- guid = vdev_geom_read_guid(cp);
+ vdev_geom_read_guids(cp, &pguid, &vguid);
g_topology_lock();
- if (guid != vd->vdev_guid) {
+ if (pguid != spa_guid(vd->vdev_spa) ||
+ vguid != vd->vdev_guid) {
vdev_geom_detach(cp, 0);
cp = NULL;
ZFS_LOG(1, "guid mismatch for provider %s: "
- "%ju != %ju.", vd->vdev_path,
- (uintmax_t)vd->vdev_guid, (uintmax_t)guid);
+ "%ju:%ju != %ju:%ju.", vd->vdev_path,
+ (uintmax_t)spa_guid(vd->vdev_spa),
+ (uintmax_t)vd->vdev_guid,
+ (uintmax_t)pguid, (uintmax_t)vguid);
} else {
ZFS_LOG(1, "guid match for provider %s.",
vd->vdev_path);
@@ -632,23 +633,38 @@ vdev_geom_open(vdev_t *vd, uint64_t *psi
g_topology_lock();
error = 0;
- /*
- * If we're creating or splitting a pool, just find the GEOM provider
- * by its name and ignore GUID mismatches.
- */
- if (vd->vdev_spa->spa_load_state == SPA_LOAD_NONE ||
- vd->vdev_spa->spa_splitting_newspa == B_TRUE)
+ if (vd->vdev_spa->spa_splitting_newspa ||
+ (vd->vdev_prevstate == VDEV_STATE_UNKNOWN &&
+ vd->vdev_spa->spa_load_state == SPA_LOAD_NONE)) {
+ /*
+ * We are dealing with a vdev that hasn't been previously
+ * opened (since boot), and we are not loading an
+ * existing pool configuration. This looks like a
+ * vdev add operation to a new or existing pool.
+ * Assume the user knows what he/she is doing and find
+ * GEOM provider by its name, ignoring GUID mismatches.
+ *
+ * XXPOLICY: It would be safer to only allow a device
+ * that is unlabeled or labeled but missing
+ * GUID information to be opened in this fashion,
+ * unless we are doing a split, in which case we
+ * should allow any guid.
+ */
cp = vdev_geom_open_by_path(vd, 0);
- else {
+ } else {
+ /*
+ * Try using the recorded path for this device, but only
+ * accept it if its label data contains the expected GUIDs.
+ */
cp = vdev_geom_open_by_path(vd, 1);
if (cp == NULL) {
/*
* The device at vd->vdev_path doesn't have the
- * expected guid. The disks might have merely
+ * expected GUIDs. The disks might have merely
* moved around so try all other GEOM providers
- * to find one with the right guid.
+ * to find one with the right GUIDs.
*/
- cp = vdev_geom_open_by_guid(vd);
+ cp = vdev_geom_open_by_guids(vd);
}
}
More information about the svn-src-head
mailing list