Page MenuHomeFreeBSD

Restore identification of VDEVs using non-native block size.
ClosedPublic

Authored by cy on Oct 20 2020, 5:49 PM.
Tags
None
Referenced Files
Unknown Object (File)
Apr 8 2024, 9:35 AM
Unknown Object (File)
Jan 31 2024, 2:14 AM
Unknown Object (File)
Jan 23 2024, 12:49 PM
Unknown Object (File)
Dec 30 2023, 11:49 PM
Unknown Object (File)
Dec 26 2023, 6:12 AM
Unknown Object (File)
Dec 2 2023, 6:08 AM
Unknown Object (File)
Dec 2 2023, 6:08 AM
Unknown Object (File)
Dec 2 2023, 6:03 AM
Subscribers

Details

Summary

Since the import of ZFS on Linux OpenZFS, identification of VDEVs not using native block size was omitted. This is a regression.

NAME STATE READ WRITE CKSUM
dsk02 ONLINE 0 0 0

mirror-0   ONLINE       0     0     0
  ada1s4a  ONLINE       0     0     0
  ada2s4a  ONLINE       0     0     0  block size: 512B configured, 4096B native

This patch resolves the regression. The code was copied from FreeBSD 12-STABLE.

A pull request is in progress

Test Plan

Currently running here.

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Skipped
Unit
Tests Skipped
Build Status
Buildable 34284

Event Timeline

cy requested review of this revision.Oct 20 2020, 5:49 PM

The following pull request has been submitted: https://github.com/openzfs/zfs/pull/11088.

In my example, without the patch ada2s4a is not flagged using a non-native block size.

Replace 8 spaces with a tab.

@cy This all looks fine, but for anything that isn't an urgent bug I greatly prefer that it makes it way in by MFV. Thanks.

We can continue any further discussion on the PR itself. The only thing left resolve is whether or not to heed the environment variable that Brian mentioned.

I've flattened my pull request at https://github.com/openzfs/zfs/pull/11088. We can import this into vendor and MFV it if you want. I don't care. As long as the regression is fixed.

I guess the openzfs PR will add more, but so far, so good:)

This revision is now accepted and ready to land.Oct 21 2020, 5:10 AM

There are some at OpenZFS who are balking at this being too much information. My reply there is this:

I'm, with Matt on this one but if people think this is an unnecessary warning then why even print this?

status: One or more devices are configured to use a non-native block size.
Expect reduced performance.
action: Replace affected devices with devices that support the
configured block size, or migrate data to a properly configured
pool.

How am I supposed to know which devices I might want to migrate? In this case all I need to do is detach the mirror and insert a different device and problem is solved. Without the information I'm looking at backing up the pool and recreating it on two new devices. What a waste of my time that would be, not to mention the outage.

If people don't want the additional information, then simply display this in verbose mode.

To add to this, if the OpenZFS people can't agree, we should apply this to FreeBSD with a possible verbose mode test, which IMO is not necessary.

This will get merged in with the next MFV.

The upstream commit:

commit 3928ec53395fcc26be7844dd6b63df757166c281
Date: Thu Oct 22 12:15:17 2020 -0700

Restore identification of VDEVs using non-native block size

NAME         STATE     READ WRITE CKSUM
dsk02        ONLINE       0     0     0
  mirror-0   ONLINE       0     0     0
    ada1s4a  ONLINE       0     0     0
    ada2s4a  ONLINE       0     0     0  block size: 512B configured, 4096B native

Reviewed-by: Matt Macy <mmacy@FreeBSD.org>
Reviewed-by: Toomas Soome <tsoome@me.com>
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed off by: Cy Schubert <cy@FreeBSD.org>
Closes #11088

I will go ahead and commit this to our tree rather than wait for the next MFV.

This revision now requires review to proceed.Nov 17 2020, 5:54 PM
This revision was not accepted when it landed; it landed in state Needs Review.Nov 18 2020, 1:19 AM
This revision was automatically updated to reflect the committed changes.

In general if you're super eager to merge some change that for which there's no pressing reason to merge it because I haven't had time to MFV. I'd rather that you simply volunteer the time to do the MFV yourself. Thanks.

Thank you Matt. I will MFV a new tarball next time.