Re: 3bf53c4c8f53 - main - release(7): Enable zpoolupgrade rc script in ZFS based VM images

-----Original Message-----
From: Li-Wen Hsu <lwhsu@freebsd.org>
Date: 2022-11-06, Sunday at 22:48
To: Ravi Pokala <rpokala@freebsd.org>
Cc: <src-committers@freebsd.org>, <dev-commits-src-all@freebsd.org>, <dev-commits-src-main@freebsd.org>
Subject: Re: 3bf53c4c8f53 - main - release(7): Enable zpoolupgrade rc script in ZFS based VM images

    On Mon, Nov 7, 2022 at 1:33 PM Ravi Pokala <rpokala@freebsd.org> wrote:
    >
    > Hi Li-Wen,
    >
    > If I'm reading this (and 72a1cb05cd23) correctly, this will run `zpool upgrade' on the "zroot" pool on every boot. That's fine for the first time a VM image is used, since presumably the root pool and the bootloader were generated from the same sources. But if the root pool is subsequently upgraded by the running VM, don't we need to make sure the bootloader is also upgraded? Otherwise, don't we run into the possibility of this new `zpoolupgrade' script enabling features which are not supported by the bootloader?
    >
    > There should be some mechanism for upgrading the bootloader, or else something else that runs on the first boot from the VM image should disable `zpoolupgrade' so it is only run the first time.
    >
    > Thanks,
    >
    > Ravi (rpokala@)

    The zpoolupgrade rc script has "KEYWORD: firstboot" so it is only
    executed when the ${firstboot_sentinel} file exists, it works in the
    same way as growfs and zpoolreguid rc scripts. I've been thinking
    renaming these to firstboot_* as others provided by
    sysutils/firstboot-* from ports, but I think it's also fine to keep
    the consistency for now.

Ah! I completely missed the 'firstboot' keyword. That sounds much better!

That said, Mark Millard brought up a good point about only enabling features supported by RELEASE bootloaders.

Thanks,

Ravi (rpokala@)



    Best,
    Li-Wen

    >
    > -----Original Message-----
    > From: <owner-src-committers@freebsd.org> on behalf of Li-Wen Hsu <lwhsu@FreeBSD.org>
    > Date: 2022-11-06, Sunday at 19:50
    > To: <src-committers@FreeBSD.org>, <dev-commits-src-all@FreeBSD.org>, <dev-commits-src-main@FreeBSD.org>
    > Subject: git: 3bf53c4c8f53 - main - release(7): Enable zpoolupgrade rc script in ZFS based VM images
    >
    >     The branch main has been updated by lwhsu:
    >
    >     URL: https://cgit.FreeBSD.org/src/commit/?id=3bf53c4c8f53b1f19313e9c31415c7eee830cdc0
    >
    >     commit 3bf53c4c8f53b1f19313e9c31415c7eee830cdc0
    >     Author:     Li-Wen Hsu <lwhsu@FreeBSD.org>
    >     AuthorDate: 2022-11-07 03:47:33 +0000
    >     Commit:     Li-Wen Hsu <lwhsu@FreeBSD.org>
    >     CommitDate: 2022-11-07 03:47:33 +0000
    >
    >         release(7): Enable zpoolupgrade rc script in ZFS based VM images
    >
    >         This will enable VM access to all ZFS feature automatically, only on a
    >         newly installed or provisioned VM or cloud instance.
    >
    >         Reviewed by:    markj
    >         Sponsored by:   The FreeBSD Foundation
    >         Differential Revision: https://reviews.freebsd.org/D37283
    >     ---
    >      release/tools/vmimage.subr | 1 +
    >      1 file changed, 1 insertion(+)
    >
    >     diff --git a/release/tools/vmimage.subr b/release/tools/vmimage.subr
    >     index 8982e768527a..a65ec4f1a1f9 100644
    >     --- a/release/tools/vmimage.subr
    >     +++ b/release/tools/vmimage.subr
    >     @@ -83,6 +83,7 @@ vm_install_base() {
    >         if [ "${VMFS}" = zfs ]; then
    >                 echo "zfs_enable=\"YES\"" >> ${DESTDIR}/etc/rc.conf
    >                 echo "zpool_reguid=\"zroot\"" >> ${DESTDIR}/etc/rc.conf
    >     +           echo "zpool_upgrade=\"zroot\"" >> ${DESTDIR}/etc/rc.conf
    >         fi
    >
    >         if ! [ -z "${QEMUSTATIC}" ]; then
    >
    >