11277 Loader cannot boot from root pool with encryption enabled

Reviews
Diff

- Download Diff

Information
Submitter:	Andy Fiddaman
Repository:	illumos-gate
Branch:	master
Bugs:	11277
Depends On:
Commit:	e90dc8f...
Reviewers
Groups:	general
People:

Description

11277 Loader cannot boot from root pool with encryption enabled

Testing Done

From FreeBSD: https://svnweb.freebsd.org/base?view=revision&revision=349217

Tell loader to ignore newer features enabled on the root pool.

There are many new features in ZoF. Most, if not all, do not effect read only usage.
Encryption in particular is enabled at the pool level but used at the dataset level.
The loader obviously will not be able to boot if the boot dataset is encrypted, but
should not care if some other dataset in the root pool is encrypted.

Tested on OmniOS - used to fix a non-booting VM after creating a test encrypted dataset - rpool/enc

Issues

0
1
1
All Issues: 2

Description	From	Last Updated
Dumb question: Will features not yet in illumos but listed here cause problems? (I suspect "no", but would like to ...	Dan McDonald	June 26, 2019, 7:11 p.m.
feature@bookmark_v2 is required (and was added) with encryption. Should that be listed here?	Jerry Jelinek	June 27, 2019, 6:56 a.m.

usr/src/boot/lib/libstand/zfs/zfsimpl.c (Diff revision 1)

Show all issues

Dumb question: Will features not yet in illumos but listed here cause problems? (I suspect "no", but would like to hear it explicitly.)

Andy Fiddaman June 26, 2019, 7:11 p.m.

No, the code just does a string comparison for each enabled feature on the pool to check if they are known or not. Unknown enabled features cause a boot failure but the other way around is not a problem.

I think com.datto:resilver_defer is the only one that we don't have yet.

Ship it!

Ship It!

Ship it!

Ship It!

Ship it!

Ship It!

usr/src/boot/lib/libstand/zfs/zfsimpl.c (Diff revision 1)

Show all issues

feature@bookmark_v2 is required (and was added) with encryption. Should that be listed here?

Jerry Jelinek June 26, 2019, 8:25 p.m.

Specifically, to match the other changes here, the name is "com.datto:bookmark_v2".

Toomas Soome June 26, 2019, 8:32 p.m.

Actually yes since bookmark_v2 is not read-only compatible.

Andy Fiddaman June 26, 2019, 8:55 p.m.

If it's not read-only compatible, should it be added here?

Toomas Soome June 26, 2019, 9:05 p.m.
```
yes, that is what I meant.
```

Jerry Jelinek June 26, 2019, 9:13 p.m.

I apologize for my confusion on these responses. Could you definitively say "yes, add it" or "no, don't add it"? I'm just having a little trouble understanding the earlier responses, which might be because I don't understand the loader code at all.

Toomas Soome June 27, 2019, 5:01 a.m.

yes, add it:)

Yep, the code there is confusing; we grab ZPOOL_CONFIG_FEATURES_FOR_READ or DMU_POOL_FEATURES_FOR_READ nvlist and check if there features listed in nvlist are in our features_for_read, if no match, we stop accessing this pool.

Change Summary:

Added bookmark_v2
Changed loader version so that the bootblocks will be updated automatically.

Commit:

-	9aba5b3a9da49ee0b2457b817123c18b621c44f8
+	e90dc8f74a97ef8eb05271010a98bc9558246ce0

Diff:

Revision 2 (+10 -3)

Show changes

	usr/src/boot/Makefile.version
	usr/src/boot/lib/libstand/zfs/zfsimpl.c

Ship it!

Ship It!

Ship it!

Ship It!

Ship it!

Ship It!

You have a pending review.

Review Board 2.5.16

Screenshots

Files

Issues

Change Summary:

Commit:

Diff:

Status: Closed (submitted)