Skip site navigation (1)Skip section navigation (2)
Date:      Fri, 11 Nov 2022 20:45:23 +0000
From:      bugzilla-noreply@freebsd.org
To:        testing@FreeBSD.org
Subject:   [Bug 267554] ZFS tests should avoid atf_expect_fail due to coarse integration with ATF
Message-ID:  <bug-267554-32464-pMPaMr3p4W@https.bugs.freebsd.org/bugzilla/>
In-Reply-To: <bug-267554-32464@https.bugs.freebsd.org/bugzilla/>
References:  <bug-267554-32464@https.bugs.freebsd.org/bugzilla/>

next in thread | previous in thread | raw e-mail | index | archive | help
https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=3D267554

--- Comment #2 from commit-hook@FreeBSD.org ---
A commit in branch main references this bug:

URL:
https://cgit.FreeBSD.org/src/commit/?id=3D11ed0a95bfa76791dc6428eb2d47a986c=
0c6f8a3

commit 11ed0a95bfa76791dc6428eb2d47a986c0c6f8a3
Author:     Eric van Gyzen <vangyzen@FreeBSD.org>
AuthorDate: 2022-11-03 02:42:54 +0000
Commit:     Eric van Gyzen <vangyzen@FreeBSD.org>
CommitDate: 2022-11-11 20:43:47 +0000

    zfs tests: stop writing to arbitrary devices

    TL;DR:  Three ZFS tests created ZFS pools on all unmounted devices list=
ed
    in /etc/fstab, corrupting their contents.  Stop that.

    Imagine my surprise when the ESP on my main dev/test VM would "randomly"
    become corrupted, making it unbootable.  Three tests collect various
devices
    from the system and try to add them to a test pool.  The test expects t=
his
    to fail because it _assumes_ these devices are in use and ZFS will
correctly
    reject the request.

    My /etc/fstab has two entries for devices in /dev:

        /dev/gpt/swap0  none        swap    sw,trimonce,late
        /dev/gpt/esp0   /boot/efi   msdosfs rw,noauto

    Note the `noauto` on the ESP.  In a remarkable example of irony, I chose
    this because it should keep the ESP more protected from corruption;
    in fact, mounting it would have protected it from this case.

    The tests added all of these devices to a test pool in a _single comman=
d_,
    expecting the command to fail.  The swap device was in use, so the comm=
and
    correctly failed, but the ESP was added and therefore corrupted.  Howev=
er,
    since the command correctly failed, the test didn't notice the ESP prob=
lem.
    If each device had been added with its own command, the test _might_ ha=
ve
    noticed that one of them incorrectly succeeded.  However, two of these
    tests would not have noticed:

    hotspare_create_001_neg was incorrectly specified as needing the Solaris
    dumpadm command, so it was skipped.  _Some_ of the test needs that comm=
and,
    but it checks for its presence and runs fine without it.

    Due to bug 241070, zpool_add_005_pos was marked as an expected failure.
    Due to the coarse level of integration with ATF, this test would still
    "pass" even if it failed for the wrong reason.  I wrote bug 267554 to
    reconsider the use of atf_expect_fail in these tests.

    Let's further consider the use of various devices found around the syst=
em.
    In addition to devices in /etc/fstab, the tests also used mounted devic=
es
    listed by the `mount` command.  If ZFS behaves correctly, it will refuse
    to added mounted devices and swap devices to a pool.  However, these are
    unit tests used by developers to ensure that ZFS still works after they
    modify it, so it's reasonable to expect ZFS to do the _wrong_ thing
    sometimes.  Using random host devices is unsafe.

    Fix the root problem by using only the disks provided via the "disks"
    variable in kyua.conf.  Use one to create a UFS file system and mount i=
t.
    Use another as a swap device.  Use a third as a dump device, but expect
    it to fail due to bug 241070.

    While I'm here:

    Due to commit 6b6e2954dd65, we can simply add a second dump device and
    remove it in cleanup.  We no longer need to save, replace, and restore =
the
    pre-existing dump device.

    The cleanup_devices function used `camcontrol inquiry` to distinguish d=
isks
    from other devices, such as partitions.  That works fine for SCSI, but =
not
    for ATA or VirtIO block.  Use `geom disk list` instead.

    PR:             241070
    PR:             267554
    Reviewed by:    asomers
    Sponsored by:   Dell Inc.
    Differential Revision:  https://reviews.freebsd.org/D37257

 tests/sys/cddl/zfs/include/libtest.kshlib          |  2 +-
 .../zfs/tests/cli_root/zpool_add/zpool_add.kshlib  | 68 ------------------=
----
 .../tests/cli_root/zpool_add/zpool_add_005_pos.ksh | 29 ++++++---
 .../zfs/tests/cli_root/zpool_add/zpool_add_test.sh |  3 +-
 .../zfs/tests/hotspare/hotspare_add_003_neg.ksh    | 60 +++++++++----------
 .../zfs/tests/hotspare/hotspare_create_001_neg.ksh | 60 +++++++++----------
 tests/sys/cddl/zfs/tests/hotspare/hotspare_test.sh |  4 +-
 7 files changed, 83 insertions(+), 143 deletions(-)

--=20
You are receiving this mail because:
You are the assignee for the bug.=



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?bug-267554-32464-pMPaMr3p4W>