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

On Nov 7, 2022, at 11:34, Ravi Pokala <rpokala@freebsd.org> wrote:

> -----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@)

Reviewing the zpool features, there is at least one for which:

"This feature becomes active as soon as it is enabled and will
never return to being enabled."

(or analogous). There are ones for which:

"READ-ONLY COMPATIBLE  no"

Some have both statuses, for example, head_errlog.

However, the loader may do so little that its stage avoids
the general "READ-ONLY COMPATIBLE  no" status for some
features.

===
Mark Millard
marklmi at yahoo.com