Bug 218634 - vdev_geom only associates one vdev per consumer
Summary: vdev_geom only associates one vdev per consumer
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: kern (show other bugs)
Version: CURRENT
Hardware: Any Any
: --- Affects Some People
Assignee: Alan Somers
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-04-13 15:33 UTC by Alan Somers
Modified: 2017-05-31 14:19 UTC (History)
0 users

See Also:
asomers: mfc-stable11+
asomers: mfc-stable10+


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Alan Somers freebsd_committer 2017-04-13 15:33:33 UTC
vdev_geom.c uses the g_consumer's private field to point to a vdev_t.  That way, a GEOM event can cause a change to a ZFS vdev.  For example, when you remove a disk, the vdev's status will change to REMOVED.  However, vdev_geom will sometimes attach multiple vdevs to the same GEOM consumer.  If this happens, then geom events will only be propagated to one of the vdevs.

Steps to reproduce:
# Create two pools with a shared spare
$ sudo zpool create -f foo da0 spare da1
$ sudo zpool create -f bar da2 spare da1
# Physically remove da1
$ zpool status
  pool: bar
 state: ONLINE
  scan: none requested
config:

        NAME                    STATE     READ WRITE CKSUM
        bar                     ONLINE       0     0     0
          da2                   ONLINE       0     0     0
        spares
          13402883250515786666  REMOVED   was /dev/da1

errors: No known data errors

  pool: foo
 state: ONLINE
  scan: none requested
config:

        NAME        STATE     READ WRITE CKSUM
        foo         ONLINE       0     0     0
          da0       ONLINE       0     0     0
        spares
          da1       AVAIL

Both spares should be listed as REMOVED.  Instead, only one is.
Comment 1 commit-hook freebsd_committer 2017-05-11 16:27:47 UTC
A commit references this bug:

Author: asomers
Date: Thu May 11 16:26:56 UTC 2017
New revision: 318189
URL: https://svnweb.freebsd.org/changeset/base/318189

Log:
  vdev_geom may associate multiple vdevs per g_consumer

  vdev_geom.c currently uses the g_consumer's private field to point to a
  vdev_t. That way, a GEOM event can cause a change to a ZFS vdev. For
  example, when you remove a disk, the vdev's status will change to REMOVED.
  However, vdev_geom will sometimes attach multiple vdevs to the same GEOM
  consumer. If this happens, then geom events will only be propagated to one
  of the vdevs.

  Fix this by storing a linked list of vdevs in g_consumer's private field.

  sys/cddl/contrib/opensolaris/uts/common/fs/zfs/vdev_geom.c

  * g_consumer.private now stores a linked list of vdev pointers associated
    with the consumer instead of just a single vdev pointer.

  * Change vdev_geom_set_physpath's signature to more closely match
    vdev_geom_set_rotation_rate

  * Don't bother calling g_access in vdev_geom_set_physpath. It's guaranteed
    that we've already accessed the consumer by the time we get here.

  * Don't call vdev_geom_set_physpath in vdev_geom_attach. Instead, call it
    in vdev_geom_open, after we know that the open has succeeded.

  PR:		218634
  Reviewed by:	gibbs
  MFC after:	1 week
  Sponsored by:	Spectra Logic Corp
  Differential Revision:	https://reviews.freebsd.org/D10391

Changes:
  head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/vdev_geom.c
Comment 2 commit-hook freebsd_committer 2017-05-22 15:13:26 UTC
A commit references this bug:

Author: asomers
Date: Mon May 22 15:12:50 UTC 2017
New revision: 318648
URL: https://svnweb.freebsd.org/changeset/base/318648

Log:
  MFC r318189:

  vdev_geom may associate multiple vdevs per g_consumer

  vdev_geom.c currently uses the g_consumer's private field to point to a
  vdev_t. That way, a GEOM event can cause a change to a ZFS vdev. For
  example, when you remove a disk, the vdev's status will change to REMOVED.
  However, vdev_geom will sometimes attach multiple vdevs to the same GEOM
  consumer. If this happens, then geom events will only be propagated to one
  of the vdevs.

  Fix this by storing a linked list of vdevs in g_consumer's private field.

  sys/cddl/contrib/opensolaris/uts/common/fs/zfs/vdev_geom.c

  * g_consumer.private now stores a linked list of vdev pointers associated
    with the consumer instead of just a single vdev pointer.

  * Change vdev_geom_set_physpath's signature to more closely match
    vdev_geom_set_rotation_rate

  * Don't bother calling g_access in vdev_geom_set_physpath. It's guaranteed
    that we've already accessed the consumer by the time we get here.

  * Don't call vdev_geom_set_physpath in vdev_geom_attach. Instead, call it
    in vdev_geom_open, after we know that the open has succeeded.

  PR:		218634
  Reviewed by:	gibbs
  Sponsored by:	Spectra Logic Corp
  Differential Revision:	https://reviews.freebsd.org/D10391

Changes:
_U  stable/11/
  stable/11/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/vdev_geom.c
Comment 3 commit-hook freebsd_committer 2017-05-30 22:55:18 UTC
A commit references this bug:

Author: asomers
Date: Tue May 30 22:54:53 UTC 2017
New revision: 319268
URL: https://svnweb.freebsd.org/changeset/base/319268

Log:
  MFC r318189:

  vdev_geom may associate multiple vdevs per g_consumer

  vdev_geom.c currently uses the g_consumer's private field to point to a
  vdev_t. That way, a GEOM event can cause a change to a ZFS vdev. For
  example, when you remove a disk, the vdev's status will change to REMOVED.
  However, vdev_geom will sometimes attach multiple vdevs to the same GEOM
  consumer. If this happens, then geom events will only be propagated to one
  of the vdevs.

  Fix this by storing a linked list of vdevs in g_consumer's private field.

  sys/cddl/contrib/opensolaris/uts/common/fs/zfs/vdev_geom.c

  * g_consumer.private now stores a linked list of vdev pointers associated
    with the consumer instead of just a single vdev pointer.

  * Change vdev_geom_set_physpath's signature to more closely match
    vdev_geom_set_rotation_rate

  * Don't bother calling g_access in vdev_geom_set_physpath. It's guaranteed
    that we've already accessed the consumer by the time we get here.

  * Don't call vdev_geom_set_physpath in vdev_geom_attach. Instead, call it
    in vdev_geom_open, after we know that the open has succeeded.

  PR:		218634
  Reviewed by:	gibbs
  Sponsored by:	Spectra Logic Corp
  Differential Revision:	https://reviews.freebsd.org/D10391

Changes:
_U  stable/10/
  stable/10/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/vdev_geom.c