Fix import can't find spare/l2cache when path change #6158
Conversation
|
@tuxoko, thanks for your PR! By analyzing the history of the files in this pull request, we identified @behlendorf, @don-brady and @tonyhutter to be potential reviewers. |
|
@tonyhutter |
lib/libzfs/libzfs_import.c
Outdated
| /* | ||
| * Check if it's a spare or l2cache device. If it is, | ||
| * we need to skip the name and guid check since they | ||
| * don't exists on aux devcie label. |
There was a problem hiding this comment.
typo "device"
otherwise LGTM
|
@tuxoko Looks like the buildbot may not have seen the change. Would you mind pushing it again? |
When spare or l2cache device path changes, zpool import will not fix up their paths like normal vdev. The issue is that when you supply a pool name argument to zpool import, it will use it to filter out device which doesn't have the pool name in the label. Since spare and l2cache device never have that in the label, they'll always get filtered out. We fix this by making sure we never filter out a spare or l2cache device. Signed-off-by: Chunwei Chen <david.chen@osnexus.com>
|
Does this commit address the problem when the device is there, but is not the log/cache device expected? In OsX we have to find a solution for that as OpenZFS will just open any device it finds, if it happens to have the name of another pool's log/cache device, then write all over it. Destroying any data already on the device, be it another zpool, or hfs/fat/ext3. Which is true if you use /dev/disk (/dev/sdX) path names. (Luckily, most users use by-id these days). |
|
@lundman |
|
Righto, I'll leave it on the todo for us.. What OsX really need is for log/cache devices to be labelled :) |
If, for example, your aux device was /dev/sdc, but now the aux device is
removed and /dev/sdc points to other device. zpool import will still
use that device and corrupt it.
The problem is that the spa_validate_aux in spa_import, rather than
validate the on-disk label, it would actually write label to disk. We
remove them since spa_load_{spares,l2cache} seems to do everything we
need and they would actually validate on-disk label.
Signed-off-by: Chunwei Chen <david.chen@osnexus.com>
|
Add a fix for import using wrong spare/l2 device. |
|
@tuxoko I'll give this a test on my VM |
|
I was able to verify that this does fix the case of a spare moving from I also tried corrupting a device (#6158 (comment)) by creating it as a spare or cache in a pool, exporting the pool, zeroing the device, and then importing the pool again. I was expecting to see the zero'd device get corrupted with a new label, but instead zpool just listed it as UNAVAIL, and the device remained zeroed. @tuxoko were you able to corrupt a device in your testing? Or is it just a OSX issue? |
|
@tonyhutter |
|
@tuxoko can your describe your test setup for corrupting the disk? I'd like to try it on my VM. |
|
@tonyhutter |
| VDEV_ALLOC_SPARE); | ||
| if (error == 0) | ||
| error = spa_validate_aux(spa, nvroot, -1ULL, | ||
| VDEV_ALLOC_L2CACHE); |
There was a problem hiding this comment.
Dropping these checks will result in the shared aux device tests being disabled during import. That shouldn't be a problem since these checks only make sense during zpool create and zpool add. At import time we shouldn't be re-initializing any devices and instead only verifying that the required devices all have valid labels,
If, for example, your aux device was /dev/sdc, but now the aux device is
removed and /dev/sdc points to other device. zpool import will still
use that device and corrupt it.
The problem is that the spa_validate_aux in spa_import, rather than
validate the on-disk label, it would actually write label to disk. We
remove them since spa_load_{spares,l2cache} seems to do everything we
need and they would actually validate on-disk label.
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Chunwei Chen <david.chen@osnexus.com>
Closes #6158
When spare or l2cache device path changes, zpool import will not fix up their paths like normal vdev. The issue is that when you supply a pool name argument to zpool import, it will use it to filter out device which doesn't have the pool name in the label. Since spare and l2cache device never have that in the label, they'll always get filtered out. We fix this by making sure we never filter out a spare or l2cache device. Reviewed by: Richard Elling <Richard.Elling@RichardElling.com> Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Signed-off-by: Chunwei Chen <david.chen@osnexus.com> Closes openzfs#6158
If, for example, your aux device was /dev/sdc, but now the aux device is
removed and /dev/sdc points to other device. zpool import will still
use that device and corrupt it.
The problem is that the spa_validate_aux in spa_import, rather than
validate the on-disk label, it would actually write label to disk. We
remove them since spa_load_{spares,l2cache} seems to do everything we
need and they would actually validate on-disk label.
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Chunwei Chen <david.chen@osnexus.com>
Closes openzfs#6158
When spare or l2cache device path changes, zpool import will not fix up their paths like normal vdev. The issue is that when you supply a pool name argument to zpool import, it will use it to filter out device which doesn't have the pool name in the label. Since spare and l2cache device never have that in the label, they'll always get filtered out. We fix this by making sure we never filter out a spare or l2cache device. Reviewed by: Richard Elling <Richard.Elling@RichardElling.com> Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Signed-off-by: Chunwei Chen <david.chen@osnexus.com> Closes openzfs#6158
If, for example, your aux device was /dev/sdc, but now the aux device is
removed and /dev/sdc points to other device. zpool import will still
use that device and corrupt it.
The problem is that the spa_validate_aux in spa_import, rather than
validate the on-disk label, it would actually write label to disk. We
remove them since spa_load_{spares,l2cache} seems to do everything we
need and they would actually validate on-disk label.
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Chunwei Chen <david.chen@osnexus.com>
Closes openzfs#6158
When spare or l2cache device path changes, zpool import will not fix up their paths like normal vdev. The issue is that when you supply a pool name argument to zpool import, it will use it to filter out device which doesn't have the pool name in the label. Since spare and l2cache device never have that in the label, they'll always get filtered out. We fix this by making sure we never filter out a spare or l2cache device. Reviewed by: Richard Elling <Richard.Elling@RichardElling.com> Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Signed-off-by: Chunwei Chen <david.chen@osnexus.com> Closes #6158
If, for example, your aux device was /dev/sdc, but now the aux device is
removed and /dev/sdc points to other device. zpool import will still
use that device and corrupt it.
The problem is that the spa_validate_aux in spa_import, rather than
validate the on-disk label, it would actually write label to disk. We
remove them since spa_load_{spares,l2cache} seems to do everything we
need and they would actually validate on-disk label.
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Chunwei Chen <david.chen@osnexus.com>
Closes #6158
When spare or l2cache device path changes, zpool import will not fix up
their paths like normal vdev. The issue is that when you supply a pool
name argument to zpool import, it will use it to filter out device which
doesn't have the pool name in the label. Since spare and l2cache device
never have that in the label, they'll always get filtered out.
We fix this by making sure we never filter out a spare or l2cache
device.
If, for example, your aux device was /dev/sdc, but now the aux device is
removed and /dev/sdc points to other device. zpool import will still
use that device and corrupt it.
The problem is that the spa_validate_aux in spa_import, rather than
validate the on-disk label, it would actually write label to disk. We
remove them since spa_load_{spares,l2cache} seems to do everything we
need and they would actually validate on-disk label.
Description
Motivation and Context
How Has This Been Tested?
Types of changes
Checklist:
Signed-off-by.