Bug 267535 - freebsd-update does not create proper deep boot environment backups
Summary: freebsd-update does not create proper deep boot environment backups
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: bin (show other bugs)
Version: 13.1-RELEASE
Hardware: Any Any
: --- Affects Some People
Assignee: Kyle Evans
URL:
Keywords: needs-patch, needs-qa
Depends on:
Blocks:
 
Reported: 2022-11-02 23:37 UTC by cmh
Modified: 2023-11-08 12:07 UTC (History)
4 users (show)

See Also:
grahamperrin: maintainer-feedback? (kevans)
kevans: mfc-stable14+
kevans: mfc-stable13+
kevans: mfc-stable12+


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description cmh 2022-11-02 23:37:51 UTC
freebsd-update assumes that the system is using a "shallow" boot environment as created by the Auto ZFS option to bsdinstall(8). However, the zfsbe rc(8) script also supports a "deep" boot environment in which the boot environment is divided into subordinate datasets. (I am using the terms defined in bectl(8) at "Boot Environment Structures.")

In deep boot environments, The action taken by freebsd-update (bectl create) results in an empty copy of the ROOT dataset that holds the subordinate datasets. In other words, freebsd-update is backing up the wrong thing, and therefore there is no backup of the real boot environment, contrary to expectations. This is unhelpful at best, and at worst provides a false sense of security should a restore be necessary.

What should happen is that freebsd-update should determine if this is a shallow or deep boot environment. In the latter case, it should instead run "bectl create -r" to correctly back up the deep boot environment.

Thank you for a useful utility.
Comment 1 cmh 2022-11-02 23:49:19 UTC
(In reply to cmh from comment #0)

Correction to paragraph 2 of original report (apologies hit send too quickly):

In deep boot environments, the action taken by freebsd-update (bectl create) results in >>>>a copy of the ROOT dataset without the subordinate datasets. In other words, freebsd-update is creating a partial backup, and there is no full backup of the boot environment, contrary to expectations.<<<<< This is unhelpful at best, and at worst provides a false sense of security should a restore be necessary.
Comment 2 tamaru 2023-10-12 02:47:48 UTC
I am suffering from this issue as well.

I have extra issue on bectl side due to "sparse" deep boot environment I have:

"bectl mount does not recognize intermediate canmount=off partionss for deep boot environment"
https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=274421

but, I see that both problems can be solved independently.

Moreover, if the behavior of bectl is updated as the man page says at the very end,
I think freebsd-update side can be left untouched; am I correct?

---------
     bectl subcommands that have their own -r operate on this second, deep
     style of boot environment, when the -r flag is set.  A future version of
     bectl may default to handling both styles and deprecate the various -r
     flags.
---------
Comment 3 Kyle Evans freebsd_committer freebsd_triage 2023-10-12 02:53:52 UTC
*insert gripe about bugzilla here*
Comment 4 commit-hook freebsd_committer freebsd_triage 2023-10-12 02:54:21 UTC
A commit in branch main references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=989c5f6da99081b1f2b76ec09e91078e531e1250

commit 989c5f6da99081b1f2b76ec09e91078e531e1250
Author:     Kyle Evans <kevans@FreeBSD.org>
AuthorDate: 2023-10-12 02:51:07 +0000
Commit:     Kyle Evans <kevans@FreeBSD.org>
CommitDate: 2023-10-12 02:54:03 +0000

    freebsd-update: create deep BEs by default

    The -r flag to bectl needs to go away, and we need to just do the right
    thing.  In the meantime, we can apply an -r in freebsd-update as a
    minimal fix to stop creating partial backups in these (non-default) deep
    BE setups.

    PR:             267535

 usr.sbin/freebsd-update/freebsd-update.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Comment 5 commit-hook freebsd_committer freebsd_triage 2023-10-24 00:05:43 UTC
A commit in branch stable/14 references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=5c2a559876d123ba386612319bf42e7b32dee590

commit 5c2a559876d123ba386612319bf42e7b32dee590
Author:     Kyle Evans <kevans@FreeBSD.org>
AuthorDate: 2023-10-12 02:51:07 +0000
Commit:     Kyle Evans <kevans@FreeBSD.org>
CommitDate: 2023-10-24 00:04:14 +0000

    freebsd-update: create deep BEs by default

    The -r flag to bectl needs to go away, and we need to just do the right
    thing.  In the meantime, we can apply an -r in freebsd-update as a
    minimal fix to stop creating partial backups in these (non-default) deep
    BE setups.

    PR:             267535
    (cherry picked from commit 989c5f6da99081b1f2b76ec09e91078e531e1250)

 usr.sbin/freebsd-update/freebsd-update.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Comment 6 commit-hook freebsd_committer freebsd_triage 2023-10-24 00:05:46 UTC
A commit in branch stable/12 references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=c1c0873c9d3f6c05168303d0b95f9363c1e6c371

commit c1c0873c9d3f6c05168303d0b95f9363c1e6c371
Author:     Kyle Evans <kevans@FreeBSD.org>
AuthorDate: 2023-10-12 02:51:07 +0000
Commit:     Kyle Evans <kevans@FreeBSD.org>
CommitDate: 2023-10-24 00:04:42 +0000

    freebsd-update: create deep BEs by default

    The -r flag to bectl needs to go away, and we need to just do the right
    thing.  In the meantime, we can apply an -r in freebsd-update as a
    minimal fix to stop creating partial backups in these (non-default) deep
    BE setups.

    PR:             267535
    (cherry picked from commit 989c5f6da99081b1f2b76ec09e91078e531e1250)

 usr.sbin/freebsd-update/freebsd-update.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Comment 7 commit-hook freebsd_committer freebsd_triage 2023-10-24 00:05:47 UTC
A commit in branch stable/13 references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=80f747781f125576de40ab8b3d8d70b351ef0518

commit 80f747781f125576de40ab8b3d8d70b351ef0518
Author:     Kyle Evans <kevans@FreeBSD.org>
AuthorDate: 2023-10-12 02:51:07 +0000
Commit:     Kyle Evans <kevans@FreeBSD.org>
CommitDate: 2023-10-24 00:04:18 +0000

    freebsd-update: create deep BEs by default

    The -r flag to bectl needs to go away, and we need to just do the right
    thing.  In the meantime, we can apply an -r in freebsd-update as a
    minimal fix to stop creating partial backups in these (non-default) deep
    BE setups.

    PR:             267535
    (cherry picked from commit 989c5f6da99081b1f2b76ec09e91078e531e1250)

 usr.sbin/freebsd-update/freebsd-update.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Comment 8 Kyle Evans freebsd_committer freebsd_triage 2023-10-24 00:06:41 UTC
Will EN. Sorry for the hassle. Thanks!
Comment 9 commit-hook freebsd_committer freebsd_triage 2023-10-24 16:12:48 UTC
A commit in branch releng/14.0 references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=e34fdb7c119e2913d0910033e6a26fd6c08b9503

commit e34fdb7c119e2913d0910033e6a26fd6c08b9503
Author:     Kyle Evans <kevans@FreeBSD.org>
AuthorDate: 2023-10-12 02:51:07 +0000
Commit:     Kyle Evans <kevans@FreeBSD.org>
CommitDate: 2023-10-24 16:12:01 +0000

    freebsd-update: create deep BEs by default

    The -r flag to bectl needs to go away, and we need to just do the right
    thing.  In the meantime, we can apply an -r in freebsd-update as a
    minimal fix to stop creating partial backups in these (non-default) deep
    BE setups.

    PR:             267535
    Approved by:    re (gjb)

    (cherry picked from commit 989c5f6da99081b1f2b76ec09e91078e531e1250)
    (cherry picked from commit 5c2a559876d123ba386612319bf42e7b32dee590)

 usr.sbin/freebsd-update/freebsd-update.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Comment 10 tamaru 2023-11-02 11:38:04 UTC
Thank you for pushing this into FreeBSD 14.0-RELEASE!
It is very helpful. I've tested it with RC3.
Comment 11 commit-hook freebsd_committer freebsd_triage 2023-11-08 01:01:11 UTC
A commit in branch releng/13.2 references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=e79edfaf68c542c8545670e911ae83ca0e3493b5

commit e79edfaf68c542c8545670e911ae83ca0e3493b5
Author:     Kyle Evans <kevans@FreeBSD.org>
AuthorDate: 2023-10-12 02:51:07 +0000
Commit:     Ed Maste <emaste@FreeBSD.org>
CommitDate: 2023-11-08 00:59:45 +0000

    freebsd-update: create deep BEs by default

    The -r flag to bectl needs to go away, and we need to just do the right
    thing.  In the meantime, we can apply an -r in freebsd-update as a
    minimal fix to stop creating partial backups in these (non-default) deep
    BE setups.

    PR:             267535
    (cherry picked from commit 989c5f6da99081b1f2b76ec09e91078e531e1250)
    (cherry picked from commit 80f747781f125576de40ab8b3d8d70b351ef0518)

    Approved by:    so
    Security:       FreeBSD-EN-23:13

 usr.sbin/freebsd-update/freebsd-update.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Comment 12 commit-hook freebsd_committer freebsd_triage 2023-11-08 01:04:17 UTC
A commit in branch releng/12.4 references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=e648a628a66a09973154b442ac1266316be20506

commit e648a628a66a09973154b442ac1266316be20506
Author:     Kyle Evans <kevans@FreeBSD.org>
AuthorDate: 2023-10-12 02:51:07 +0000
Commit:     Ed Maste <emaste@FreeBSD.org>
CommitDate: 2023-11-08 01:01:37 +0000

    freebsd-update: create deep BEs by default

    The -r flag to bectl needs to go away, and we need to just do the right
    thing.  In the meantime, we can apply an -r in freebsd-update as a
    minimal fix to stop creating partial backups in these (non-default) deep
    BE setups.

    PR:             267535
    (cherry picked from commit 989c5f6da99081b1f2b76ec09e91078e531e1250)
    (cherry picked from commit 5c2a559876d123ba386612319bf42e7b32dee590)

    Approved by:    so
    Security:       FreeBSD-EN-23:13

 usr.sbin/freebsd-update/freebsd-update.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Comment 13 Kyle Evans freebsd_committer freebsd_triage 2023-11-08 04:30:09 UTC
(In reply to tamaru from comment #10)

Excellent, thanks for testing! As noted by the last comments, secteam was kind enough to take it all the way back to 12.4; so while we'll still create broken backups for people up until they're at the tip of the branch, at least they have a chance to get to the tip of the branch (which likely won't be that risky of an update).
Comment 14 cmh 2023-11-08 12:07:51 UTC
Thanks Kyle for this fix and Tamaru for the testing. Was great to see this in the update emails today. Most successful bug report ever! Looking forward to smoother updates.