Bug 147444

Comment 1 Mark Linimon 2010-06-04 02:11:01 UTC

Responsible Changed
From-To: freebsd-bugs->freebsd-rc

Over to maintainer(s).

Comment 2 Eitan Adler 2017-12-31 07:59:57 UTC

For bugs matching the following criteria:

Status: In Progress Changed: (is less than) 2014-06-01

Reset to default assignee and clear in-progress tags.

Mail being skipped

Created attachment 215840 [details]
Add 'shutdown' keyword to /etc/rc.d/zfs

It's amazing that after more than 10 years a single line could not be added.

Please find attached the diff to facilitate that change. If there is a reason why /etc/rc.d/zfs should not be called by rc.shutdown, please let us know.


Patch:

rc.shutdown calls rcorder(8) with the '-k shutdown' option. rcorder(8) will therefore only return files declaring the 'shutdown' keyword:
# KEYWORD: shutdown

As of 12.1, /etc/rc.d/zfs does not declare any keywords. This patch adds the 'shutdown' keyword.


Use case: Providing ZFS datasets to a jail


Problem:

Without calling /etc/rc.d/zfs on shutdown, resources associated with ZFS mounts are not freed and the jail will remain in dying state. In addition, the dataset is now in a dangling state, as the jail it is attached to is dying.


Workaround:

In /etc/jail.conf, make sure to run 'service zfs stop' when the jail is stopped:

    exec.stop = "/bin/sh /etc/rc.shutdown";
    exec.stop += "/usr/sbin/service zfs stop";


Desaster recovery:

With the jail in dying state, issue:

# After the following statements, the dataset will be unmounted and the jail
# will finally be gone
zfs set jailed=off tank/jaildata
zfs unmount tank/jaildata

# Don't forget to set jailed=on before starting the jail again
zfs set jailed=on tank/jaildata


How to reproduce:

# jail.conf
test {
        path = "/jails/test";
        exec.clean;
        exec.start = "/bin/sh /etc/rc";
        exec.stop = "/bin/sh /etc/rc.shutdown";

        # Make sure to unmount all ZFS datasets before stopping the jail
        # Required unless the jails /etc/rc.d/zfs contains '# KEYWORD: shutdown'
        exec.stop += "/usr/sbin/service zfs stop";

        # Mandatory to use ZFS in jail
        allow.mount;
        allow.mount.zfs;
        enforce_statfs = 1;     # must be less than 2

        # Attach ZFS dataset to jail
        exec.created = "/sbin/zfs jail test tank/jaildata";

        # Make sure the /dev/zfs device is included (it is with the default
        # devfs_ruleset = 4)
        mount.devfs;
}

# Create dataset
zfs create -o jailed=on -o mountpoint=/data tank/jaildata
mkdir -p /jails/test/data
sysrc -f /jails/test/etc/rc.conf zfs_enable=YES

# Start the jail
jail -c test

# List ZFS mounts
zfs mount | grep jaildata
tank/jaildata                   /jails/test/data

# Stop the jail
jail -r test

# List ZFS mounts (mount is still there)
zfs mount | grep jaildata
tank/jaildata                   /jails/test/data


Additional Remarks:

While the workaround seems to be okay-ish for the jail situation, it is still unclean. However, for physical hosts this may wreak havoc with the pool if shared spares are used, as 'zfs unshare' is never invoked on shutdown.

Correction for the workaround proposed in my previous post:

Instead of

    exec.stop += "/usr/sbin/service zfs stop";

the second line must read:

    exec.stop += "/usr/sbin/service zfs stop || /usr/bin/true";

This is necessary because '/usr/sbin/service zfs stop' will always return false if the root filesystem of the jail happens to be a ZFS dataset, which is very likely to be the case.

Comment 6 Mateusz Piotrowski 2020-11-01 11:13:50 UTC

*** Bug 249278 has been marked as a duplicate of this bug. ***

Comment 7 commit-hook 2020-11-03 10:02:57 UTC

A commit references this bug:

Author: 0mp
Date: Tue Nov  3 10:02:53 UTC 2020
New revision: 367291
URL: https://svnweb.freebsd.org/changeset/base/367291

Log:
  rc.d/zfs: Add shutdown to KEYWORDS

  The problem is that Without walling /etc/rc.d/zfs on shutdown, resources
  associated with ZFS mounts are not freed and the jails will remain in dying
  state. In addition, the dataset is now in a dangling state, as the jail it
  is attached to is dying.

  A known workaround for jails was to add the following lines
  to /etc/jail.conf, to make sure that "service zfs stop" is run
  when the jail is stopped:

      exec.stop = "/bin/sh /etc/rc.shutdown";
      exec.stop += "/usr/sbin/service zfs stop || /usr/bin/true";

  While the workaround seems to be okay-ish for the jail situation, it is
  still unclean. However, for physical hosts this may wreak havoc with the
  pool if shared spares are used, as "zfs unshare" is never invoked on
  shutdown.

  PR:		147444
  Submitted by:	Markus Stoff <markus__stoffdv_at>
  Reported by:	Mykah <mburkhardt__exavault_com>
  Reviewed by:	cy
  Approved by:	cy (src)
  MFC after:	1 week
  Relnotes:	yes
  Differential Revision:	https://reviews.freebsd.org/D27039

Changes:
  head/libexec/rc/rc.d/zfs

Comment 8 Mateusz Piotrowski 2020-11-03 10:04:11 UTC

It took a while but now it's finally in the base. Thanks a lot!

Comment 9 commit-hook 2020-11-10 07:47:47 UTC

A commit references this bug:

Author: 0mp
Date: Tue Nov 10 07:47:25 UTC 2020
New revision: 367546
URL: https://svnweb.freebsd.org/changeset/base/367546

Log:
  MFC r367291:

  rc.d/zfs: Add shutdown to KEYWORDS

  The problem is that without walling /etc/rc.d/zfs on shutdown, resources
  associated with ZFS mounts are not freed and the jails will remain in dying
  state. In addition, the dataset is now in a dangling state, as the jail it
  is attached to is dying.

  A known workaround for jails was to add the following lines
  to /etc/jail.conf, to make sure that "service zfs stop" is run
  when the jail is stopped:

      exec.stop = "/bin/sh /etc/rc.shutdown";
      exec.stop += "/usr/sbin/service zfs stop || /usr/bin/true";

  While the workaround seems to be okay-ish for the jail situation, it is
  still unclean. However, for physical hosts this may wreak havoc with the
  pool if shared spares are used, as "zfs unshare" is never invoked on
  shutdown.

  PR:		147444
  Submitted by:	Markus Stoff <markus__stoffdv_at>
  Reported by:	Mykah <mburkhardt__exavault_com>
  Reviewed by:	cy
  Approved by:	cy (src)
  Relnotes:	yes

Changes:
_U  stable/12/
  stable/12/libexec/rc/rc.d/zfs