svn commit: r216230 - head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs

[Ivan Voras]

On 7 December 2010 12:31, Pawel Jakub Dawidek <pjd at freebsd.org> wrote:
> On Tue, Dec 07, 2010 at 12:25:28PM +0100, Ivan Voras wrote:
>> On 7 December 2010 11:21, Pawel Jakub Dawidek <pjd at freebsd.org> wrote:
>>
>> > PS. Do you know your change breaks all current ZFS installation if
>> > stripesize is defined for a provider?
>> >
>> >        # zpool create tank ada0
>> >        (upgrade FreeBSD so that ada0 now reports 4kB stripesize)
>> >        # zpool import tank
>> >        cannot import 'tank': invalid vdev configuration
>>
>> Actually I did test the patch and, similar to the Solaris people,
>> found that ZFS records ashift in metadata and uses the recorded one
>> instead of hardcoded / detected one. What you found here really
>> shouldn't happen. Are you sure only stripesize was increased in your
>> case, not sectorsize?
>
>        if (pp->stripesize > pp->sectorsize)
>                *ashift = highbit(MIN(pp->stripesize, SPA_MAXBLOCKSIZE)) - 1;
>        else
>                *ashift = highbit(MAX(pp->sectorsize, SPA_MINBLOCKSIZE)) - 1;
>
> If stripesize will start to be 4096, the ashift will change.
>
> Not sure what you test was, but it only works the other way around -
> create pool with 4kB ashift and import it when ashift is 512 bytes.

My test was flawed, it made me conclude that ashift from metadata
would be used in this case (this case is actually one of the more
trivial ones - vdev created with ashift=9 used on device which is
capable of ashift=9 but advertises ashift=12).

In case it would be useful in the future, this is blocked in vdev_open():

http://fxr.watson.org/fxr/source/cddl/contrib/opensolaris/uts/common/fs/zfs/vdev.c#L1075

I think that to solve this edge case properly, ZFS would have to know
that both ashift values are "valid" for the device, i.e. basically it
would at this point need to somehow know about our sectorsize and
stripesize, not just ashift.

OpenSolaris lists mention that pools can be created from devices with
different ashift values, which is good news.