Comment 11 for bug 2040181

Revision history for this message
Mauricio Faria de Oliveira (mfo) wrote :

Hi Dimitri,

Thanks for the details of the test plan execution!

That covers the kernel module/filesystem code quite well, thanks,
and there's the userspace tooling changes left, as you mentioned.

> In practice this SRU is already released to most users via LXD and via kernel upgrades.
> The change this sru release will introduce is userspace tooling update, which was minor in this update.

I looked up the changes to userspace commands (in subdir `zfs/cmd/`),
and would agree they are (mostly) minor, with a few notes to have
documented (and for others to review), but IMHO, none are blocking
for the release of this SRU.

cheers,
Mauricio

Related commits:

 $ git log --oneline zfs-2.2.0-rc3..zfs-2.2.0 -- cmd/
 33d7c2d165c2 import: require force when cachefile hostid doesn't match on-disk
 8015e2ea66b4 Add '-u' - nomount flag for zfs set
 c53bc3837cb6 Improve the handling of sharesmb,sharenfs properties
 e9dc31c74e7b Update the behavior of mountpoint property
 608741d062fe Report ashift of L2ARC devices in zdb
 0ce1b2ca1930 Invoke zdb by guid to avoid import errors
 0aabd6b48228 ZIL: Avoid dbuf_read() in ztest_get_data()
 3af63683fe07 cmd: add 'help' subcommand to zpool and zfs
 9aa1a2878ea9 Fix incorrect expected error in ztest
 54c6fbd378ea zed: Allow autoreplace and fault LEDs for removed vdevs
 32949f2560bf Relax error reporting in zpool import and zpool split
 63159e5bda1c checkstyle: fix action failures
 e99e684b337b zed: update zed.d/statechange-slot_off.sh
 d19304ffeec5 zed: Add zedlet to power off slot when drive is faulted
 df8c9f351dab ZIL: Second attempt to reduce scope of zl_issuer_lock.
 12f2b1f65e91 zdb: include cloned blocks in block statistics

Spotted changes/commits:

1) Config option with typo

commit d19304ffeec50ebc02cf4496c14e8945c74fb76a
Author: Tony Hutter <email address hidden>
Date: Thu Aug 24 11:59:03 2023 -0700

    zed: Add zedlet to power off slot when drive is faulted

    If ZED_POWER_OFF_ENCLOUSRE_SLOT_ON_FAULT is enabled in zed.rc, then
    power off the drive's slot in the enclosure if it becomes FAULTED.
    This can help silence misbehaving drives. This assumes your drive
    enclosure fully supports slot power control via sysfs.

This new option is disabled by default, good.
However, the option has a typo, so I submitted patch [1] upstream,
and it would be nice if we pick it up later on.

[1] https://github.com/openzfs/zfs/pull/15651

...

2) zpool import/split no longer reflect errors to mount/share

commit 32949f2560bf35ec86dfa5d984514908e0eb3ecc
Author: Umer Saleem <email address hidden>
Date: Sat Sep 2 05:25:11 2023 +0500

    Relax error reporting in zpool import and zpool split

    For zpool import and zpool split, zpool_enable_datasets is called
    to mount and share all datasets in a pool. If there is an error
    while mounting or sharing any dataset in the pool, the status of
    import or split is reported as failure. However, the changes do
    show up in zpool list.

    This commit updates the error reporting in zpool import and zpool
    split path. More descriptive messages are shown to user in case
    there is an error during mount or share. Errors in mount or share
    do not effect the overall status of zpool import and zpool split.

...

3) zed can now autoreplace vdevs marked as REMOVED

This change is effective by default, since zed/config option are.

commit 54c6fbd378eaa402eff34acf6a91c02d6cf9da11
Author: Tony Hutter <email address hidden>
Date: Mon Sep 18 16:25:58 2023 -0700

    zed: Allow autoreplace and fault LEDs for removed vdevs

    Allow zed to autoreplace vdevs marked as REMOVED. Also update
    statechange-led zedlet to toggle fault LEDs for REMOVED vdevs.

    Reviewed-by: Brian Behlendorf <email address hidden>
    Signed-off-by: Tony Hutter <email address hidden>
    Closes #15281

See,

    $ git show 54c6fbd378eaa402eff34acf6a91c02d6cf9da11:cmd/zed/zed.d/statechange-led.sh | grep 'if .*ZED'
    if [ "${ZED_USE_ENCLOSURE_LEDS}" != "1" ] ; then

    $ sudo apt update && sudo apt install -y zfsutils-linux

    $ systemctl status zfs-zed.service | grep Active:
         Active: active (running) ...

    $ grep ZED_USE_ENCLOSURE_LEDS /etc/zfs/zed.d/zed.rc
    ZED_USE_ENCLOSURE_LEDS=1

...

4) Changes to mountpoint property handling / remounting

This is a behavior change, but it looks like a good/consistent one,
adopted by upstream between RC and Release time, so it seems worse
to diverge from upstream when we do a similar RC to Release update.

And in case of impact to particular users/scripts/scenarios, there
is a new option to fall back to previous behavior.

commit e9dc31c74e7b28a0cb2a321bc220074f6461d231
Author: Umer Saleem <email address hidden>
Date: Tue Sep 5 13:27:53 2023 +0500

    Update the behavior of mountpoint property

    ...

    To make the behavior consistent in case dataset is mounted or
    unmounted, we should try to mount the dataset whenever mountpoint
    property is updated. This would result in mounting the datasets
    if canmount property is set to on, regardless if the dataset was
    previously unmounted.

    The failure in mount operation while setting the mountpoint
    property should not be treated as failure, since the property is
    actually set now to user requested value.

commit 8015e2ea66b4f6233877fef29a8a35594f33558d
Author: Umer Saleem <email address hidden>
Date: Tue Oct 3 04:58:54 2023 +0500

    Add '-u' - nomount flag for zfs set

    ...

    Previously, if dataset was unmounted, and mountpoint property was
    updated, dataset was not mounted after the update. This behavior
    is changed in #15240. We mount the dataset whenever mountpoint
    property is updated, regardless if it's mounted or not.

    To provide the user with option to keep the dataset unmounted and
    still update the mountpoint without mounting the dataset, '-u'
    flag can be used.

    ...