zfs icon indicating copy to clipboard operation
zfs copied to clipboard

zed: Fix config_sync autoexpand flood

Open tonyhutter opened this issue 3 years ago • 1 comments

Motivation and Context

Fixes: #7132 #7366

Description

Users were seeing floods of config_sync events when autoexpand was enabled. This happened because all "disk status change" udev events invoke the autoexpand codepath, which calls zpool_relabel_disk(), which in turn cause another "disk status change" event to happen, in a feedback loop. Note that "disk status change" happens every time a user calls close() on a block device.

This commit breaks the feedback loop by only allowing an autoexpand to happen if the disk actually changed size.

How Has This Been Tested?

Tested by creating a pool with an lvm volume, expanded it, and saw zfs correctly expand the pool. Also tested sg_logs -t <vdev> to provoke a disk change event, and saw that it no longer generated a flood of config_sync events.

Types of changes

  • [x] Bug fix (non-breaking change which fixes an issue)
  • [ ] New feature (non-breaking change which adds functionality)
  • [ ] Performance enhancement (non-breaking change which improves efficiency)
  • [ ] Code cleanup (non-breaking change which makes code smaller or more readable)
  • [ ] Breaking change (fix or feature that would cause existing functionality to change)
  • [ ] Library ABI change (libzfs, libzfs_core, libnvpair, libuutil and libzfsbootenv)
  • [ ] Documentation (a change to man pages or other documentation)

Checklist:

  • [ ] My code follows the OpenZFS code style requirements.
  • [ ] I have updated the documentation accordingly.
  • [ ] I have read the contributing document.
  • [ ] I have added tests to cover my changes.
  • [ ] I have run the ZFS Test Suite with this change applied.
  • [ ] All commit messages are properly formatted and contain Signed-off-by.

tonyhutter avatar Aug 03 '22 20:08 tonyhutter

We should wait until #13743 (autoexpand test fix) is in before merging this commit

tonyhutter avatar Aug 05 '22 21:08 tonyhutter

Don't merge this yet - looks like this is failing zpool_expand_001. Let me look into it

tonyhutter avatar Aug 10 '22 18:08 tonyhutter

I've figured out what was going on. In the test case, we create a 1GB scsi_debug vdev, make a pool, expand the scsi_debug vdev to 2GB, and then expect to see the pool expand. However, when you expand the scsi_debug device (sda), it doesn't expand the partition on the device (sda1):

$ sudo ./zpool status -P
  pool: tank
 state: ONLINE
config:

	NAME         STATE     READ WRITE CKSUM
	tank         ONLINE       0     0     0
	  /dev/sda1  ONLINE       0     0     0

errors: No known data errors

$ sudo ./zpool list
NAME   SIZE  ALLOC   FREE  CKPOINT  EXPANDSZ   FRAG    CAP  DEDUP    HEALTH  ALTROOT
tank   960M   194K   960M        -         -     0%     0%  1.00x    ONLINE  -

$ sudo bash -c 'echo 2 > /sys/bus/pseudo/drivers/scsi_debug/virtual_gb'
$ lsblk
NAME   MAJ:MIN RM  SIZE RO TYPE MOUNTPOINTS
sda      8:0    0 1000M  0 disk 
├─sda1   8:1    0  990M  0 part 
└─sda9   8:9    0    8M  0 part 
                                /
$ sudo bash -c 'echo 1 > /sys/class/block/sda/device/rescan'
$ lsblk
NAME   MAJ:MIN RM  SIZE RO TYPE MOUNTPOINTS
sda      8:0    0    2G  0 disk 
├─sda1   8:1    0  990M  0 part 
└─sda9   8:9    0    8M  0 part 

So in ZED you see sda expand, but sda1 not expand. The old codepaths would blindly try to online the non-expanded sda1, which would cause it to expand, and give us the result we want. However, in this new code, zed sees that sda1 did not expand, and thus doesn't try to online it to expand it.

It would seem to me that the user should be able to expand the size of the device they used in zpool create (which would be sda), and it should "just work" to expand the pool. I'm trying to figure out what the best way to accomplish that. We might need to store the parent's size (for sda) in the config nvlist too, so that if sda1 has a change event, and sees that it did not change size, but it's parent did, then do the online to expand the pool.

tonyhutter avatar Aug 18 '22 19:08 tonyhutter

Updated PR with some fixes. Should be good to go now.

tonyhutter avatar Sep 06 '22 23:09 tonyhutter