Date: Fri, 7 Dec 2018 00:47:05 +0000 (UTC) From: Conrad Meyer <cem@FreeBSD.org> To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r341672 - in head: sys/geom/mirror tests/sys/geom/class/mirror Message-ID: <201812070047.wB70l5YN084067@repo.freebsd.org>
next in thread | raw e-mail | index | archive | help
Author: cem Date: Fri Dec 7 00:47:05 2018 New Revision: 341672 URL: https://svnweb.freebsd.org/changeset/base/341672 Log: Revert r341665 due to tinderbox breakage I didn't notice that some format strings were non-portable. Will fix and re-commit later. Deleted: head/tests/sys/geom/class/mirror/component_selection.sh Modified: head/sys/geom/mirror/g_mirror.c head/sys/geom/mirror/g_mirror.h head/tests/sys/geom/class/mirror/Makefile head/tests/sys/geom/class/mirror/conf.sh Modified: head/sys/geom/mirror/g_mirror.c ============================================================================== --- head/sys/geom/mirror/g_mirror.c Fri Dec 7 00:39:34 2018 (r341671) +++ head/sys/geom/mirror/g_mirror.c Fri Dec 7 00:47:05 2018 (r341672) @@ -59,11 +59,6 @@ static SYSCTL_NODE(_kern_geom, OID_AUTO, mirror, CTLFL int g_mirror_debug = 0; SYSCTL_INT(_kern_geom_mirror, OID_AUTO, debug, CTLFLAG_RWTUN, &g_mirror_debug, 0, "Debug level"); -bool g_launch_mirror_before_timeout = true; -SYSCTL_BOOL(_kern_geom_mirror, OID_AUTO, launch_mirror_before_timeout, - CTLFLAG_RWTUN, &g_launch_mirror_before_timeout, 0, - "If false, force gmirror to wait out the full kern.geom.mirror.timeout " - "before launching mirrors"); static u_int g_mirror_timeout = 4; SYSCTL_UINT(_kern_geom_mirror, OID_AUTO, timeout, CTLFLAG_RWTUN, &g_mirror_timeout, 0, "Time to wait on all mirror components"); @@ -115,8 +110,6 @@ static int g_mirror_update_disk(struct g_mirror_disk * static void g_mirror_update_device(struct g_mirror_softc *sc, bool force); static void g_mirror_dumpconf(struct sbuf *sb, const char *indent, struct g_geom *gp, struct g_consumer *cp, struct g_provider *pp); -static int g_mirror_refresh_device(struct g_mirror_softc *sc, - const struct g_provider *pp, const struct g_mirror_metadata *md); static void g_mirror_sync_reinit(const struct g_mirror_disk *disk, struct bio *bp, off_t offset); static void g_mirror_sync_stop(struct g_mirror_disk *disk, int type); @@ -479,10 +472,6 @@ g_mirror_init_disk(struct g_mirror_softc *sc, struct g disk->d_sync.ds_update_ts = time_uptime; disk->d_genid = md->md_genid; disk->d_sync.ds_syncid = md->md_syncid; - disk->d_init_ndisks = md->md_all; - disk->d_init_slice = md->md_slice; - disk->d_init_balance = md->md_balance; - disk->d_init_mediasize = md->md_mediasize; if (errorp != NULL) *errorp = 0; return (disk); @@ -2373,74 +2362,17 @@ g_mirror_update_device(struct g_mirror_softc *sc, bool case G_MIRROR_DEVICE_STATE_STARTING: { struct g_mirror_disk *pdisk, *tdisk; - const char *mismatch; - uintmax_t found, newest; - u_int dirty, ndisks; + u_int dirty, ndisks, genid, syncid; + bool broken; - /* Pre-flight checks */ - LIST_FOREACH_SAFE(disk, &sc->sc_disks, d_next, tdisk) { - /* - * Confirm we already detected the newest genid. - */ - KASSERT(sc->sc_genid >= disk->d_genid, - ("%s: found newer genid %u (sc:%p had %u).", __func__, - disk->d_genid, sc, sc->sc_genid)); - - /* Kick out any previously tasted stale components. */ - if (disk->d_genid < sc->sc_genid) { - G_MIRROR_DEBUG(0, "Stale 'genid' field on %s " - "(device %s) (component=%u latest=%u), skipping.", - g_mirror_get_diskname(disk), sc->sc_name, - disk->d_genid, sc->sc_genid); - g_mirror_destroy_disk(disk); - sc->sc_bump_id |= G_MIRROR_BUMP_SYNCID; - continue; - } - - /* - * Confirm we already detected the newest syncid. - */ - KASSERT(sc->sc_syncid >= disk->d_sync.ds_syncid, - ("%s: found newer syncid %u (sc:%p had %u).", - __func__, disk->d_sync.ds_syncid, sc, - sc->sc_syncid)); - -#define DETECT_MISMATCH(field, name) \ - if (mismatch == NULL && \ - disk->d_init_ ## field != sc->sc_ ## field) { \ - mismatch = name; \ - found = (intmax_t)disk->d_init_ ## field; \ - newest = (intmax_t)sc->sc_ ## field; \ - } - mismatch = NULL; - DETECT_MISMATCH(ndisks, "md_all"); - DETECT_MISMATCH(balance, "md_balance"); - DETECT_MISMATCH(slice, "md_slice"); - DETECT_MISMATCH(mediasize, "md_mediasize"); -#undef DETECT_MISMATCH - if (mismatch != NULL) { - G_MIRROR_DEBUG(0, "Found a mismatching '%s' " - "field on %s (device %s) (found=%ju " - "newest=%ju).", mismatch, - g_mirror_get_diskname(disk), sc->sc_name, - found, newest); - g_mirror_destroy_disk(disk); - sc->sc_bump_id |= G_MIRROR_BUMP_SYNCID; - continue; - } - } - KASSERT(sc->sc_provider == NULL, ("Non-NULL provider in STARTING state (%s).", sc->sc_name)); /* - * Are we ready? If the timeout (force is true) has expired, and - * any disks are present, then yes. If we're permitted to launch - * before the timeout has expired and the expected number of - * current-generation mirror disks have been tasted, then yes. + * Are we ready? We are, if all disks are connected or + * if we have any disks and 'force' is true. */ ndisks = g_mirror_ndisks(sc, -1); - if ((force && ndisks > 0) || - (g_launch_mirror_before_timeout && ndisks == sc->sc_ndisks)) { + if (sc->sc_ndisks == ndisks || (force && ndisks > 0)) { ; } else if (ndisks == 0) { /* @@ -2488,6 +2420,42 @@ g_mirror_update_device(struct g_mirror_softc *sc, bool } /* + * Find the biggest genid. + */ + genid = 0; + LIST_FOREACH(disk, &sc->sc_disks, d_next) { + if (disk->d_genid > genid) + genid = disk->d_genid; + } + sc->sc_genid = genid; + /* + * Remove all disks without the biggest genid. + */ + broken = false; + LIST_FOREACH_SAFE(disk, &sc->sc_disks, d_next, tdisk) { + if (disk->d_genid < genid) { + G_MIRROR_DEBUG(0, + "Component %s (device %s) broken, skipping.", + g_mirror_get_diskname(disk), sc->sc_name); + g_mirror_destroy_disk(disk); + /* + * Bump the syncid in case we discover a healthy + * replacement disk after starting the mirror. + */ + broken = true; + } + } + + /* + * Find the biggest syncid. + */ + syncid = 0; + LIST_FOREACH(disk, &sc->sc_disks, d_next) { + if (disk->d_sync.ds_syncid > syncid) + syncid = disk->d_sync.ds_syncid; + } + + /* * Here we need to look for dirty disks and if all disks * with the biggest syncid are dirty, we have to choose * one with the biggest priority and rebuild the rest. @@ -2500,7 +2468,7 @@ g_mirror_update_device(struct g_mirror_softc *sc, bool dirty = ndisks = 0; pdisk = NULL; LIST_FOREACH(disk, &sc->sc_disks, d_next) { - if (disk->d_sync.ds_syncid != sc->sc_syncid) + if (disk->d_sync.ds_syncid != syncid) continue; if ((disk->d_flags & G_MIRROR_DISK_FLAG_SYNCHRONIZING) != 0) { @@ -2527,7 +2495,7 @@ g_mirror_update_device(struct g_mirror_softc *sc, bool "master disk for synchronization.", g_mirror_get_diskname(pdisk), sc->sc_name); LIST_FOREACH(disk, &sc->sc_disks, d_next) { - if (disk->d_sync.ds_syncid != sc->sc_syncid) + if (disk->d_sync.ds_syncid != syncid) continue; if ((disk->d_flags & G_MIRROR_DISK_FLAG_SYNCHRONIZING) != 0) { @@ -2548,7 +2516,7 @@ g_mirror_update_device(struct g_mirror_softc *sc, bool * We have some non-dirty disks. */ LIST_FOREACH(disk, &sc->sc_disks, d_next) { - if (disk->d_sync.ds_syncid != sc->sc_syncid) + if (disk->d_sync.ds_syncid != syncid) continue; if ((disk->d_flags & G_MIRROR_DISK_FLAG_SYNCHRONIZING) != 0) { @@ -2564,7 +2532,8 @@ g_mirror_update_device(struct g_mirror_softc *sc, bool /* Reset hint. */ sc->sc_hint = NULL; - if (force) { + sc->sc_syncid = syncid; + if (force || broken) { /* Remember to bump syncid on first write. */ sc->sc_bump_id |= G_MIRROR_BUMP_SYNCID; } @@ -2901,23 +2870,37 @@ g_mirror_check_metadata(struct g_mirror_softc *sc, str struct g_mirror_metadata *md) { - G_MIRROR_DEBUG(2, "%s: md_did 0x%u disk %s device %s md_all 0x%x " - "sc_ndisks 0x%x md_slice 0x%x sc_slice 0x%x md_balance 0x%x " - "sc_balance 0x%x sc_mediasize 0x%lx pp_mediasize 0x%lx " - "md_sectorsize 0x%x sc_sectorsize 0x%x md_mflags 0x%lx " - "md_dflags 0x%lx md_syncid 0x%x md_genid 0x%x md_priority 0x%x " - "sc_state 0x%x.", - __func__, md->md_did, pp->name, sc->sc_name, md->md_all, - sc->sc_ndisks, md->md_slice, sc->sc_slice, md->md_balance, - sc->sc_balance, sc->sc_mediasize, pp->mediasize, md->md_sectorsize, - sc->sc_sectorsize, md->md_mflags, md->md_dflags, md->md_syncid, - md->md_genid, md->md_priority, sc->sc_state); - if (g_mirror_id2disk(sc, md->md_did) != NULL) { G_MIRROR_DEBUG(1, "Disk %s (id=%u) already exists, skipping.", pp->name, md->md_did); return (EEXIST); } + if (md->md_all != sc->sc_ndisks) { + G_MIRROR_DEBUG(1, + "Invalid '%s' field on disk %s (device %s), skipping.", + "md_all", pp->name, sc->sc_name); + return (EINVAL); + } + if (md->md_slice != sc->sc_slice) { + G_MIRROR_DEBUG(1, + "Invalid '%s' field on disk %s (device %s), skipping.", + "md_slice", pp->name, sc->sc_name); + return (EINVAL); + } + if (md->md_balance != sc->sc_balance) { + G_MIRROR_DEBUG(1, + "Invalid '%s' field on disk %s (device %s), skipping.", + "md_balance", pp->name, sc->sc_name); + return (EINVAL); + } +#if 0 + if (md->md_mediasize != sc->sc_mediasize) { + G_MIRROR_DEBUG(1, + "Invalid '%s' field on disk %s (device %s), skipping.", + "md_mediasize", pp->name, sc->sc_name); + return (EINVAL); + } +#endif if (sc->sc_mediasize > pp->mediasize) { G_MIRROR_DEBUG(1, "Invalid size of disk %s (device %s), skipping.", pp->name, @@ -2964,21 +2947,12 @@ g_mirror_add_disk(struct g_mirror_softc *sc, struct g_ error = g_mirror_check_metadata(sc, pp, md); if (error != 0) return (error); - - if (md->md_genid < sc->sc_genid) { + if (sc->sc_state == G_MIRROR_DEVICE_STATE_RUNNING && + md->md_genid < sc->sc_genid) { G_MIRROR_DEBUG(0, "Component %s (device %s) broken, skipping.", pp->name, sc->sc_name); return (EINVAL); } - - /* - * If the component disk we're tasting has newer metadata than the - * STARTING gmirror device, refresh the device from the component. - */ - error = g_mirror_refresh_device(sc, pp, md); - if (error != 0) - return (error); - disk = g_mirror_init_disk(sc, pp, md, &error); if (disk == NULL) return (error); @@ -3055,21 +3029,6 @@ end: return (error); } -static void -g_mirror_reinit_from_metadata(struct g_mirror_softc *sc, - const struct g_mirror_metadata *md) -{ - - sc->sc_genid = md->md_genid; - sc->sc_syncid = md->md_syncid; - - sc->sc_slice = md->md_slice; - sc->sc_balance = md->md_balance; - sc->sc_mediasize = md->md_mediasize; - sc->sc_ndisks = md->md_all; - sc->sc_flags = md->md_mflags; -} - struct g_geom * g_mirror_create(struct g_class *mp, const struct g_mirror_metadata *md, u_int type) @@ -3097,8 +3056,12 @@ g_mirror_create(struct g_class *mp, const struct g_mir sc->sc_type = type; sc->sc_id = md->md_mid; - g_mirror_reinit_from_metadata(sc, md); + sc->sc_slice = md->md_slice; + sc->sc_balance = md->md_balance; + sc->sc_mediasize = md->md_mediasize; sc->sc_sectorsize = md->md_sectorsize; + sc->sc_ndisks = md->md_all; + sc->sc_flags = md->md_mflags; sc->sc_bump_id = 0; sc->sc_idle = 1; sc->sc_last_write = time_uptime; @@ -3519,52 +3482,6 @@ g_mirror_fini(struct g_class *mp) if (g_mirror_post_sync != NULL) EVENTHANDLER_DEREGISTER(shutdown_post_sync, g_mirror_post_sync); -} - -/* - * Refresh the mirror device's metadata when gmirror encounters a newer - * generation as the individual components are being added to the mirror set. - */ -static int -g_mirror_refresh_device(struct g_mirror_softc *sc, const struct g_provider *pp, - const struct g_mirror_metadata *md) -{ - - g_topology_assert_not(); - sx_assert(&sc->sc_lock, SX_XLOCKED); - - KASSERT(sc->sc_genid <= md->md_genid, - ("%s: attempted to refresh from stale component %s (device %s) " - "(%u < %u).", __func__, pp->name, sc->sc_name, md->md_genid, - sc->sc_genid)); - - if (sc->sc_genid > md->md_genid || (sc->sc_genid == md->md_genid && - sc->sc_syncid >= md->md_syncid)) - return (0); - - G_MIRROR_DEBUG(0, "Found newer version for device %s (genid: curr=%u " - "new=%u; syncid: curr=%u new=%u; ndisks: curr=%u new=%u; " - "provider=%s).", sc->sc_name, sc->sc_genid, md->md_genid, - sc->sc_syncid, md->md_syncid, sc->sc_ndisks, md->md_all, pp->name); - - if (sc->sc_state != G_MIRROR_DEVICE_STATE_STARTING) { - /* Probable data corruption detected */ - G_MIRROR_DEBUG(0, "Cannot refresh metadata in %s state " - "(device=%s genid=%u). A stale mirror device was launched.", - g_mirror_device_state2str(sc->sc_state), sc->sc_name, - sc->sc_genid); - return (EINVAL); - } - - /* Update softc */ - g_mirror_reinit_from_metadata(sc, md); - - G_MIRROR_DEBUG(1, "Refresh device %s (id=%u, state=%s) from disk %s " - "(genid=%u syncid=%u md_all=%u).", sc->sc_name, md->md_mid, - g_mirror_device_state2str(sc->sc_state), pp->name, md->md_genid, - md->md_syncid, (unsigned)md->md_all); - - return (0); } DECLARE_GEOM_CLASS(g_mirror_class, g_mirror); Modified: head/sys/geom/mirror/g_mirror.h ============================================================================== --- head/sys/geom/mirror/g_mirror.h Fri Dec 7 00:39:34 2018 (r341671) +++ head/sys/geom/mirror/g_mirror.h Fri Dec 7 00:47:05 2018 (r341672) @@ -148,10 +148,6 @@ struct g_mirror_disk { u_int d_genid; /* Disk's generation ID. */ struct g_mirror_disk_sync d_sync;/* Sync information. */ LIST_ENTRY(g_mirror_disk) d_next; - u_int d_init_ndisks; /* Initial number of mirror components */ - uint32_t d_init_slice; /* Initial slice size */ - uint8_t d_init_balance;/* Initial balance */ - uint64_t d_init_mediasize;/* Initial mediasize */ }; #define d_name d_consumer->provider->name Modified: head/tests/sys/geom/class/mirror/Makefile ============================================================================== --- head/tests/sys/geom/class/mirror/Makefile Fri Dec 7 00:39:34 2018 (r341671) +++ head/tests/sys/geom/class/mirror/Makefile Fri Dec 7 00:47:05 2018 (r341672) @@ -18,7 +18,6 @@ TAP_TESTS_SH+= 11_test TAP_TESTS_SH+= 12_test TAP_TESTS_SH+= 13_test -ATF_TESTS_SH+= component_selection ATF_TESTS_SH+= sync_error ${PACKAGE}FILES+= conf.sh Modified: head/tests/sys/geom/class/mirror/conf.sh ============================================================================== --- head/tests/sys/geom/class/mirror/conf.sh Fri Dec 7 00:39:34 2018 (r341671) +++ head/tests/sys/geom/class/mirror/conf.sh Fri Dec 7 00:47:05 2018 (r341672) @@ -19,35 +19,4 @@ syncwait() done } -consumerrefs() -{ - gclass=$1 - geom=$2 - - if [ $# -ne 2 ]; then - echo "Bad usage consumerrefs" >&2 - exit 1 - fi - - geom "${gclass}" list "${geom}" | \ - grep -A5 ^Consumers | \ - grep Mode | \ - cut -d: -f2 -} - -disconnectwait() -{ - gclass=$1 - geom=$2 - - if [ $# -ne 2 ]; then - echo "Bad usage disconnectwait" >&2 - exit 1 - fi - - while [ $(consumerrefs "$gclass" "$geom") != r0w0e0 ]; do - sleep 0.05 - done -} - . `dirname $0`/../geom_subr.sh
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201812070047.wB70l5YN084067>