Bug 233113 - bectl activate fails to set canmount=noauto for old boot environment
Summary: bectl activate fails to set canmount=noauto for old boot environment
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: bin (show other bugs)
Version: 12.0-STABLE
Hardware: Any Any
: --- Affects Many People
Assignee: Kyle Evans
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2018-11-10 18:40 UTC by Alan Somers
Modified: 2018-11-15 16:04 UTC (History)
1 user (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Alan Somers freebsd_committer freebsd_triage 2018-11-10 18:40:37 UTC
"bectl activate" is supposed to set canmount=auto for the old boot environment and set the pool's bootfs property to the new boot environment.  That's what beadm does.  However, bectl fails to clear canmount for the old boot environment.  Observed on 12.0-BETA3

Example:

somers@threonine /> bectl list
BE                              Active Mountpoint Space Created
12.0-BETA4                      NR     /          9.45G 2018-11-10 10:09
12.0-BETA3                      -      -          1.52M 2018-11-08 09:35
12.0-CURRENT-up-20180401_231457 -      -          38.0G 2018-04-01 23:13
12.0-CURRENT-up-20180327_233010 -      -          13.2G 2018-03-27 23:28
somers@threonine /> zfs list -o name,mountpoint,canmount -r tank/ROOT
NAME                                       MOUNTPOINT  CANMOUNT
tank/ROOT                                  none              on
tank/ROOT/12.0-BETA3                       /             noauto
tank/ROOT/12.0-BETA4                       /                 on
tank/ROOT/12.0-CURRENT-up-20180327_233010  /             noauto
tank/ROOT/12.0-CURRENT-up-20180401_231457  /             noauto
somers@threonine /> sudo bectl create test
somers@threonine /> sudo bectl activate test
successfully activated boot environment test
somers@threonine /> zpool get bootfs tank
NAME  PROPERTY  VALUE           SOURCE
tank  bootfs    tank/ROOT/test  local
somers@threonine /> zfs list -o name,mountpoint,canmount -r tank/ROOT
NAME                                       MOUNTPOINT  CANMOUNT
tank/ROOT                                  none              on
tank/ROOT/12.0-BETA3                       /             noauto
tank/ROOT/12.0-BETA4                       /                 on
tank/ROOT/12.0-CURRENT-up-20180327_233010  /             noauto
tank/ROOT/12.0-CURRENT-up-20180401_231457  /             noauto
tank/ROOT/test                             /             noauto

!!!! 12.0-BETA4 still has CANMOUNT set to on !!!!

But beadm has no problem here:

somers@threonine /> sudo beadm activate 12.0-BETA4
Activated successfully
somers@threonine /> sudo beadm destroy test
Are you sure you want to destroy 'test'?
This action cannot be undone (y/[n]): y
Destroyed successfully
somers@threonine /> zfs list -o name,mountpoint,canmount -r tank/ROOT
NAME                                       MOUNTPOINT  CANMOUNT
tank/ROOT                                  none              on
tank/ROOT/12.0-BETA3                       /             noauto
tank/ROOT/12.0-BETA4                       /                 on
tank/ROOT/12.0-CURRENT-up-20180327_233010  /             noauto
tank/ROOT/12.0-CURRENT-up-20180401_231457  /             noauto
somers@threonine /> sudo beadm create test
Created successfully
somers@threonine /> sudo beadm activate test
Activated successfully
somers@threonine /> zpool get bootfs tank
NAME  PROPERTY  VALUE           SOURCE
tank  bootfs    tank/ROOT/test  local
somers@threonine /> zfs list -o name,mountpoint,canmount -r tank/ROOT
NAME                                       MOUNTPOINT  CANMOUNT
tank/ROOT                                  none              on
tank/ROOT/12.0-BETA3                       /             noauto
tank/ROOT/12.0-BETA4                       /             noauto
tank/ROOT/12.0-CURRENT-up-20180327_233010  /             noauto
tank/ROOT/12.0-CURRENT-up-20180401_231457  /             noauto
tank/ROOT/test                             /             noauto


This bug renders the new boot environment unbootable, because two root filesystems get mounted, making devfs invisible.  So it's a pretty critical bug.
Comment 1 Kyle Evans freebsd_committer freebsd_triage 2018-11-10 19:54:36 UTC
(In reply to Alan Somers from comment #0)

Hmm... libbe currently only sets bootfs on the pool and promotes the newly-activated dataset, leaving canmount alone completely.

I vaguely recall discussing this with Allan a week or two ago, though- bectl should always set canmount=noauto for the now-deactivated BE, and *could* get away with not setting canmount=on (leaving it at noauto) because of the zfsbe rc script, right? Or is it better to just set canmount=on for the new BE anyways?
Comment 2 Alan Somers freebsd_committer freebsd_triage 2018-11-10 20:25:28 UTC
(In reply to Kyle Evans from comment #1)

In my testing, the value of canmount for the new BE doesn't seem to matter.  But for the old BE it must be noauto.
Comment 3 commit-hook freebsd_committer freebsd_triage 2018-11-10 20:42:52 UTC
A commit references this bug:

Author: kevans
Date: Sat Nov 10 20:42:30 UTC 2018
New revision: 340334
URL: https://svnweb.freebsd.org/changeset/base/340334

Log:
  libbe(3): Set canmount properly when activating a new BE

  The previously activated BE should have canmount=noauto set on it upon
  activation of the new BE, but we previously did not touch canmount on either
  old or new BE.

  PR:		233113
  MFC after:	3 days

Changes:
  head/lib/libbe/be.c
Comment 4 Kyle Evans freebsd_committer freebsd_triage 2018-11-11 03:23:54 UTC
(In reply to Alan Somers from comment #2)

Good enough for me. =) Thanks!
Comment 5 commit-hook freebsd_committer freebsd_triage 2018-11-15 16:04:44 UTC
A commit references this bug:

Author: kevans
Date: Thu Nov 15 16:03:52 UTC 2018
New revision: 340454
URL: https://svnweb.freebsd.org/changeset/base/340454

Log:
  MFC r340334: libbe(3): Set canmount properly when activating a new BE

  The previously activated BE should have canmount=noauto set on it upon
  activation of the new BE, but we previously did not touch canmount on either
  old or new BE.

  PR:		233113
  Approved by:	re (gjb)

Changes:
_U  stable/12/
  stable/12/lib/libbe/be.c