http://bugzilla.suse.com/show_bug.cgi?id=1126900 http://bugzilla.suse.com/show_bug.cgi?id=1126900#c4

Ignaz Forster iforster@suse.com changed:

What |Removed |Added ---------------------------------------------------------------------------- CC| |iforster@suse.com

--- Comment #4 from Ignaz Forster iforster@suse.com --- (In reply to Michael Chang from comment #3)

(In reply to Jiri Srain from comment #2)

...

It does, even though I don't really understand the rationale behind two locations of the data.

Anyway, I would like to hear Michael's opinion

I don't know what is "health-care" [...]

It's this one: https://github.com/kubic-project/health-checker / https://software.opensuse.org/package/health-checker

It is an application used to check whether a system boots successfully after an update and trigger automatic rollbacks to known good states in case of failures.

There are several stages during boot which may fail, requiring GRUB environment variables to store a flag file:

1. The kernel itself or basic initrd components are failing To detect that case health-checker is setting a GRUB variable (`save_env -f "${env_block}" health_checker_flag`). That variable will be cleared later if the boot was successful, but if that variable is set when GRUB is starting something seems to have gone wrong during a previous run and GRUB will automatically boot the previous snapshot.

2. initrd ends up in an emergency shell If the initrd should end up in an emergency shell (e.g. because of missing or broken drivers or modules or by reaching a timeout) then the default BTRFS snapshot will be reset to the previous working one and a reboot is triggered. The only GRUB related thing in this stage is that the GRUB environment block variable is cleared, so GRUB doesn't select the previous snapshot any more (`grub2-editenv - set health_checker_flag=0`).

3. Failure in the actual system Mostly the same as 2 from a GRUB perspective: If any of the health-checker checker scripts fails, then the default BTRFS snapshot will be set to the previous one, the flag is reset and the system is rebooted.

If all stages were successful the GRUB environment block variable will also be cleared.

health-checker is meant to be used on systems with a read-only root file system such as openSUSE Kubic or SUSE CaaSP

Anyway below is the comments from what I can tell.

...

This is "just" a cosmetic problem when using health-checker on a read-only root file system. The value will still be stored into the btrfs header as long as 'env_block' is set.

This looks not correct to me, it is true for env_block= instructs for chaining to other environment block, but only some specific keys like 'next_entry' would be pointed to use it. Other ordinary keys, that is only requiring read by grub, would still use /boot/grub2/grubenv.

In this case userspace and GRUB have to communicate somehow: The variable set by GRUB has to be modified from userspace, as only the system knows whether the boot was successful or not and can set the flag accordingly.

In this case, setting health_checker_flag=0 should require writing to /boot/grub2/grubenv and the error is sensible if ongoing task is on read-only file system. I really don't know anything else to expect from btrfs, since it is no difference to other file systems in this regard.

Indeed, that was something I hadn't realized until recently: While GRUB itself is *always* able to write the GRUB environment block (on supported file systems at least), it may not be possible to do so from userspace - in this case because the file system is mounted or flagged read-only. Due to this one workaround I was thinking of is to use a completely different file to store the GRUB environment block, e.g. some file below /var, as /var always has to be writeable. From what I can see the problem would be to find the correct partition or subvolume containing that file from within GRUB. I'll have to experiment with this a bit.

Regarding Jiri's question about the split into two sections: What I also don't understand is why /boot/grub2/grubenv is used on Btrfs at all. Couldn't all data be stored into the Btrfs header directly? The contents of /boot/grub2/grubenv are inconsistent on Btrfs systems anyway, as GRUB won't update that file.

(On https://www.spinics.net/lists/linux-btrfs/msg82209.html the idea to "implement its own damn file system and we give it its own partition" was brought up - this would obviously also solve the problems ;-))

http://bugzilla.suse.com/show_bug.cgi?id=1126900 http://bugzilla.suse.com/show_bug.cgi?id=1126900#c6

--- Comment #6 from Michael Chang mchang@suse.com --- (In reply to Fabian Vogt from comment #5)

(In reply to Ignaz Forster from comment #4)

...

(In reply to Michael Chang from comment #3)

...

Regarding Jiri's question about the split into two sections: What I also don't understand is why /boot/grub2/grubenv is used on Btrfs at all. Couldn't all data be stored into the Btrfs header directly? The contents of /boot/grub2/grubenv are inconsistent on Btrfs systems anyway, as GRUB won't update that file.

IMO the best option currently as it's not too complex to implement and it would also not need any migration path. Only env_block would be part of the grubenv in the file system and all other variables would be read from/written to the header.

The problem with header is that settings in it are "global" to all subvolumes, while with grubenv file each bootable subvolumes/snapshots could maintain their own copy. We have to snapshot grubenv together with grub.cfg to keep consistent environment variables to grub.cfg.

The purpose of having header section is to workaround inability of grub writing to btrfs blocks. Usually such variable do not require persistence and is used as communication channel in specific way - for eg, grub clears a flag marked by system for knowing something has been done by the system.

http://bugzilla.suse.com/show_bug.cgi?id=1126900 http://bugzilla.suse.com/show_bug.cgi?id=1126900#c8

Michael Chang mchang@suse.com changed:

What |Removed |Added ---------------------------------------------------------------------------- Flags| |needinfo?(iforster@suse.com | |)

--- Comment #8 from Michael Chang mchang@suse.com --- (In reply to Michael Chang from comment #7)

(In reply to Ignaz Forster from comment #4)

...

(In reply to Michael Chang from comment #3)

...

(In reply to Jiri Srain from comment #2)

Thanks for the elaborate explanation. I could understand why health_checker_flag expected to be in btrfs header. For that we can give it the residence permit to env_block (ie btrfs header) and also fix the error about read only file system in grub2-editenv if env_block are used. Just let me know about your thoughts then I can start to work on it.

Scratch that - It has been done already according to the changelog. I'm sorry for not keeping it up correct on my mind

Thu Mar 1 18:36:33 UTC 2018 - iforster@suse.com

• Rename grub2-btrfs-workaround-grub2-once.patch to grub2-grubenv-in-btrfs-header.patch
• Store GRUB environment variable health_checker_flag in Btrfs header

Now I am getting to understand your proclaim as this could be "just a cosmetic problem" .. The next question is do you need anything from me in order to close the ticker here ? Thanks.

http://bugzilla.suse.com/show_bug.cgi?id=1126900 http://bugzilla.suse.com/show_bug.cgi?id=1126900#c10

--- Comment #10 from Michael Chang mchang@suse.com --- (In reply to Ignaz Forster from comment #9)

(In reply to Michael Chang from comment #8)

...

Scratch that - It has been done already according to the changelog. I'm sorry for not keeping it up correct on my mind

No problem, thanks for taking care of it!

...

Now I am getting to understand your proclaim as this could be "just a cosmetic problem" .. The next question is do you need anything from me in order to close the ticker here ?

That's a good question, as I'm not sure what the best way to solve this problem would be.

My hesitation is partially caused by the fact that while playing around with our KIWI images I discovered another directly related problem: KIWI do not contain /boot/grub2/grubenv at all. Even on regular installations it is more or less a side effect that /boot/grub2/grubenv is generated at all *during installation*: In https://github.com/yast/yast-bootloader/blob/master/src/lib/bootloader/ sections.rb#L46 `/usr/sbin/grub2-set-default` is called. That script will in turn call `grubenv unset ...` towards the end, which will incidentally trigger the generation of the grubenv file. That doesn't seem to be a conscious creation of the file...

Now what's happening if the file isn't there? On rw file systems the file will just be created if one of a small set of accepted variables is set / deleted. On ro file systems the file obviously can't be created retroactively. grub2-editenv doesn't like that at all and will abort before even trying to write to the btrfs header. So with read-only root files systems we not only have that cosmetic problem, but writing the variables may fail completely.

IMHO the motivation of introducing external block is to workaround the inability of writing to btrfs filesystem's block, but if the file /boot/grub2/grubenv itself is read-only then its application is beyond the scope. In general it means all features require writing to /boot/grub2/grubenv will fail and I would treat that as a different thing - We have to seek for another location for writable environment block and btrfs header turns out to be a candidate (again) for it. And if it doesn't require any writing from grub, we can also choose to pick another location writable by the linux system.

The next question is "Does health-care require grub to write to the health_checker_flag?" Because I did not see it happening in existing grub.cfg.

I can currently see the following options to solve this:

1. Most obvious fix

• Don't print the error message in grub2-editenv if the data was written

into the btrfs meta data successfully.

• Make sure that KIWI is somehow triggering the generation of grubenv.

Pro: simple to fix in grub2-editenv

In any case, I think we have to fix the ro error message as it did not make sense when the attempt is for writable external block.

Con: Shouts "hack" loudly; KIWI's code isn't really prepared for this, as on UEFI system it's just copying the modules without even calling `grub2-install` (Note: I just realized that Yomi also added that call to `grub2-set-default`); no generic solution, but works for all *SUSE use cases

Yes. I could share their feeling but we know life isn't perfect.

2. Use different path for grubenv

• Use another location which we know will be writeable, e.g.

`${config_directory}` if it's set and `${prefix}/${grub_cpu}-${grub_platform}/grubenv` (which may translate to /boot/grub2/i386-pc/grubenv or /boot/grub2/x86_64-efi/grubenv on x86) otherwise. Con: Needs migration; avoids snapshotting of GRUB environment variables (but that may even be desired?); also a hack, there's no guarantee those directories will stay writable

3. Use different path for this one variable

• Like 2, but only save this one variable into a different file
• Package would create an additional GRUB config file

Pro: May be minimally invasive and still work?

IMHO #2 and #3 are more or less the same as we are pointing grub for another grubenv in the grub.cfg. And grub doesn't care what's inside the new grubenv because it simply loads all variables from it. Personally prefer this solution than the rest although it will require additional GRUB config file.

I am thinking if we could just drop a new /etc/grub.d/01grubenv_dir with custom path to file grubenv.

4. Store grubenv in "BIOS boot" / "EFI System" or custom GRUB partition

• The current GRUB environment block implementation has several limitations

(i.e. doesn't work with LVM, software RAIDS or some file systems); it may be worth rethinking on how it could be stored more universally.

• It may be possible to use the "BIOS boot" / "EFI System" partitions for it

if available. Pro: May provide a clean, distribution independent solution, without any hacks Con: Lots of work and discussion necessary

IMHO the "BIOS boot" is way off, as noone will take care of cleaning it up and its inadvertent residuals could have side effect. Also the bios boot is a open common disk property thus it become hard to maintain local data laid on it.

The EFI System may work, but it is specific to EFI and we still end up with the same trouble on other types of firmware.

Did I miss any options?

For LVM install, it used to have a proposition to use reserved area in LVM physical volumes, but grub2 upstream did not accept it and since the joint development was not public we have little clue for what have gone wrong.

http://bugzilla.suse.com/show_bug.cgi?id=1126900 http://bugzilla.suse.com/show_bug.cgi?id=1126900#c12

Ignaz Forster iforster@suse.com changed:

What |Removed |Added ---------------------------------------------------------------------------- Status|RESOLVED |REOPENED Resolution|WONTFIX |--- Assignee|jsrain@suse.com |iforster@suse.com

--- Comment #12 from Ignaz Forster iforster@suse.com --- I'd suggest to still solve it, I've been thinking about it for quite some time now. To do so my plan is currently as follows:

* Create a subvolume "/boot/writable" on read-only file systems (we need that anyway for Ignition to be able to store it's flag file) in "read-only-root-fs"'s posttrans script. A fixed subvolume name (in contrast to /boot/grub2/<arch> / /boot/efi as originally suggested) has the huge advantage that it doesn't need guessing where the file could life. * The package will also introduce a GRUB configuration snippet, trying to additionally read the environment block from "/boot/writable/grubenv". * health-checker would then be changed to write to that file instead of "/boot/grub/grubenv".

This doesn't solve the problem that other applications will not be able to write to the environment block / have to be explicitly made aware of the new file location, but I guess we don't want to change the default location of grubenv either, do we?

https://bugzilla.suse.com/show_bug.cgi?id=1126900 https://bugzilla.suse.com/show_bug.cgi?id=1126900#c21

Ignaz Forster iforster@suse.com changed:

What |Removed |Added ---------------------------------------------------------------------------- CC| |jalausuch@suse.com

--- Comment #21 from Ignaz Forster iforster@suse.com --- *** Bug 1191873 has been marked as a duplicate of this bug. ***