Bug 1126900 - health-checker: grub2-editenv: error: cannot open `/boot/grub2/grubenv': Read-only file system.
health-checker: grub2-editenv: error: cannot open `/boot/grub2/grubenv': Read...
Status: REOPENED
: 1191873 (view as bug list)
Classification: openSUSE
Product: openSUSE Tumbleweed
Classification: openSUSE
Component: Bootloader
Current
Other Other
: P5 - None : Normal (vote)
: ---
Assigned To: Ignaz Forster
Jiri Srain
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2019-02-25 19:45 UTC by Thorsten Kukuk
Modified: 2021-10-22 16:57 UTC (History)
5 users (show)

See Also:
Found By: ---
Services Priority:
Business Priority:
Blocker: ---
Marketing QA Status: ---
IT Deployment: ---
iforster: needinfo? (mchang)


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Thorsten Kukuk 2019-02-25 19:45:14 UTC
From a health-checker run:

Clearing GRUB flag
+ grub2-editenv - set health_checker_flag=0
grub2-editenv: error: cannot open `/boot/grub2/grubenv': Read-only file system.
Starting health check
Comment 1 Ignaz Forster 2019-04-18 17:14:40 UTC
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.

One possible solution I was thinking of would be to just ignore write errors to /boot/grub/grubenv on btrfs.
Reasoning: For technical reasons on btrfs /boot/grub/grubenv is not the only source to store the GRUB environment variables, they may also be stored as btrfs meta data. When reading a value (with grub2-editenv or within GRUB itself) these two sources will be merged to get the actual variable value.
As none of these sources is valid in itself anyway it doesn't matter whether the data is visible in /boot/grub/grubenv. That's how it behaves nowadays anyway, it's just that the error message wouldn't appear any more...

Does this sound reasonable?
Comment 2 Jiri Srain 2019-04-24 06:31:31 UTC
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
Comment 3 Michael Chang 2019-04-25 04:44:37 UTC
(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", and googling it likely to be this project? 

> http://lnxhc.sourceforge.net/

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, 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.
Comment 4 Ignaz Forster 2019-04-29 14:52:50 UTC
(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 ;-))
Comment 5 Fabian Vogt 2019-04-29 15:26:54 UTC
(In reply to Ignaz Forster from comment #4)
> (In reply to Michael Chang from comment #3)
> > 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.

/var might not exist, so I suggest a subvolume with a fixed path below /boot/grub2. It would always be on the same partition as /, which is not necessarily true for /var and can also be found using just the knowledge of the filesystem grub already has.

> 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.
Comment 6 Michael Chang 2019-04-30 04:40:58 UTC
(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.
Comment 7 Michael Chang 2019-04-30 06:14:48 UTC
(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.
Comment 8 Michael Chang 2019-05-06 09:13:24 UTC
(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.
Comment 9 Ignaz Forster 2019-05-06 17:08:16 UTC
(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.


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

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?

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

Did I miss any options?
Comment 10 Michael Chang 2019-05-07 07:12:02 UTC
(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.
Comment 11 Michael Chang 2019-06-14 07:40:54 UTC
To be honest I didn't know still having any plan to fix this, or are we perusing other possibility than tweaking with the grubenv. Anyway feel free to reopen and update the status if you need anything from me. Thanks.
Comment 12 Ignaz Forster 2019-06-14 09:11:23 UTC
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?
Comment 13 Ignaz Forster 2019-07-29 15:54:01 UTC
Implementation (part 1):

read-only-root-fs was changed to additionally load its variables from a GRUB environment block in /boot/writable.

The post scripts will create the /boot/writable subvolume using mksubvol from the snapper package. However, this is potentially breaking rollback functionality, so https://github.com/openSUSE/snapper/issues/236 will have to be solved before releasing the update.
Comment 14 Ignaz Forster 2019-12-06 14:48:58 UTC
Both snapper and read-only-root-fs have been released for a while, so I finally resumed work on this ticket.

However I realized that just using a writable place for grubenv alone doesn't help: The Btrfs header section is *only* written when using the default file name (i.e. '-' or '/boot/grub2/grubenv'). When using another file name the variables will end up in that file instead, making an interaction between GRUB and the system impossible (the file's values would always win).

This seems easy to resolve though. My idea would be to link '/boot/grub2/grubenv' to '/boot/writable/grubenv' on read-only root file systems instead.

The package 'read-only-root-fs' would have to create an empty grubenv file using `grub2-editenv create`, move it to '/boot/writable' and create the link in its %post script. If this works it would also mostly solve bug 1156441, as the file *is* writable and the 'env_block' can be written on first use then.
Comment 15 Ignaz Forster 2019-12-19 15:58:11 UTC
Several people suggested to use a subvolume called "/boot/var" instead of "/boot/writable", and changing the grub2 package to always put the grubenv file into that directory.

Advantages:
* consistent path on both read-write and read-only systems
* read-only-root-fs does not have to do any hacks to read from the correct location (as grub does not know how to handle symlinks)
* `grub2-editenv create` would work (which doesn't if it is a symlink on a read-only root file system)

Would this approach be acceptable for you?
Comment 16 Ignaz Forster 2020-01-03 11:04:21 UTC
I'm thinking of something like the following patch:
https://build.opensuse.org/package/rdiff/home:fos:branches:Base:System/grub2?linkrev=base&rev=2

This adds a new configure option `--with-statedir`, which will be the name of the directory containing the grubenv file below /boot.

This is a prototype, for a final version open_envblk_file in grub-core/commands/loadenv.c may also need adjustments to load the new default file, and the TODOs should be removed.
Comment 17 Ignaz Forster 2020-01-09 15:49:00 UTC
@Michael: Did you happen to have a chance to look at the proposed grub2 change already?
Comment 18 Michael Chang 2020-01-10 07:19:24 UTC
(In reply to Ignaz Forster from comment #17)
> @Michael: Did you happen to have a chance to look at the proposed grub2
> change already?

Sorry for the delay in replying. I hope you have enjoyed your vacation. :) 

Honestly I didn't really like it, maybe because it's prototype so still much to improve. And also I don't think the idea of --with-statedir itself a good one.

1. The default grubenv needs to be always inside /boot directory or subvolume, as /boot is guaranteed to be accessible by grub2 at all times. Moving it elsewhere out of this boundary may bring regression.

2. The default grubenv needs to be in the same directory of grub.cfg, that is $prefix (/boot/grub2) where grub.cfg knows to look it up. Moreover grub2 allows to chainload image[1] AND config[2] so having it commonly known helps to not loose when interfacing different distribtions.

3. I think the idea of having /boot/var is only useful for btrfs readonly snapshot, but the configure option --with-statedir enables it globally that I think the unnessary detoure is too intrusive.

[1] https://www.gnu.org/software/grub/manual/grub/grub.html#chainloader
[2] https://www.gnu.org/software/grub/manual/grub/grub.html#configfile
Comment 19 Ignaz Forster 2020-01-10 19:35:00 UTC
(In reply to Michael Chang from comment #18)
> (In reply to Ignaz Forster from comment #17)
> > @Michael: Did you happen to have a chance to look at the proposed grub2
> > change already?
> 
> Sorry for the delay in replying. I hope you have enjoyed your vacation. :) 

Thanks a lot, vacation was great :-) No worries, I just thought I'd ask - thanks for taking the time to review!

> Honestly I didn't really like it, maybe because it's prototype so still much
> to improve.

The prototype essentially does what it is supposed to do, so I fear it's the concept ;-)

> And also I don't think the idea of --with-statedir itself a good
> one.
> 
> 1. The default grubenv needs to be always inside /boot directory or
> subvolume, as /boot is guaranteed to be accessible by grub2 at all times.
> Moving it elsewhere out of this boundary may bring regression.

I'm wondering: /boot/grub2/i386-pc, /boot/grub2/x86_64-efi etc, ... are already located on a separate subvolume - so how does it work that these directories are available from within grub2? Couldn't we use the same mechanism to also provide access to something like /boot/var?

> 2. The default grubenv needs to be in the same directory of grub.cfg, that
> is $prefix (/boot/grub2) where grub.cfg knows to look it up. Moreover grub2
> allows to chainload image[1] AND config[2] so having it commonly known helps
> to not loose when interfacing different distribtions.

I don't think grubenv and grub.cfg have to be (or even should be) located in the same directory.
I do *not* want to change the path of grub.cfg - not only because of chainloading, but because on Btrfs this file conceptually belongs to a certain snapshot: If we would put grub.cfg into a common area we wouldn't be able to roll back in case of a broken configuration file, partially killing our own rollback mechanism.
grubenv on the other hand is supposed to store information that should survive between reboots and rollbacks, so it should not be snapshot specific (but currently is).

I agree that grub.cfg *could* be seen as variable data, as the file can be generated from /etc/grub.d snippets any time, but I don't think this has to be considered in this context.

> 3. I think the idea of having /boot/var is only useful for btrfs readonly
> snapshot, but the configure option --with-statedir enables it globally that
> I think the unnessary detoure is too intrusive.

My original approach was to implement this only for read-only systems, but apart from the fact that KIWI currently doesn't like it (see https://bugzilla.opensuse.org/show_bug.cgi?id=1156441#c20), it also resulted in really messy workarounds, e.g. temporarily modifying the $config_directory variable.

Personally I'm not (yet) opposed to leaving this a read-only-root-fs specific patch, so I'll check which workarounds I may be able to avoid by moving some handling the grub2 package itself next week.
Comment 20 Michael Chang 2020-01-13 09:00:15 UTC
(In reply to Ignaz Forster from comment #19)
> (In reply to Michael Chang from comment #18)
> > (In reply to Ignaz Forster from comment #17)
> > > @Michael: Did you happen to have a chance to look at the proposed grub2
> > > change already?
> > 
> > Sorry for the delay in replying. I hope you have enjoyed your vacation. :) 
> 
> Thanks a lot, vacation was great :-) No worries, I just thought I'd ask -
> thanks for taking the time to review!

Glad to hear that, too. :) 

> 
> > Honestly I didn't really like it, maybe because it's prototype so still much
> > to improve.
> 
> The prototype essentially does what it is supposed to do, so I fear it's the
> concept ;-)
> 
> > And also I don't think the idea of --with-statedir itself a good
> > one.
> > 
> > 1. The default grubenv needs to be always inside /boot directory or
> > subvolume, as /boot is guaranteed to be accessible by grub2 at all times.
> > Moving it elsewhere out of this boundary may bring regression.
> 
> I'm wondering: /boot/grub2/i386-pc, /boot/grub2/x86_64-efi etc, ... are
> already located on a separate subvolume - so how does it work that these
> directories are available from within grub2?

The command `btrfs-mount-subvol` does the trick to mount the subvol to a directory for grub2.

> Couldn't we use the same
> mechanism to also provide access to something like /boot/var?

Yes, we could. But I want to avoid doing so unless absolutely necessary, as we know such request are very specific.

> 
> > 2. The default grubenv needs to be in the same directory of grub.cfg, that
> > is $prefix (/boot/grub2) where grub.cfg knows to look it up. Moreover grub2
> > allows to chainload image[1] AND config[2] so having it commonly known helps
> > to not loose when interfacing different distribtions.
> 
> I don't think grubenv and grub.cfg have to be (or even should be) located in
> the same directory.
> I do *not* want to change the path of grub.cfg - not only because of
> chainloading, but because on Btrfs this file conceptually belongs to a
> certain snapshot: If we would put grub.cfg into a common area we wouldn't be
> able to roll back in case of a broken configuration file, partially killing
> our own rollback mechanism.

Agreed. We shouldn't change path of grub.cfg. I was not asking for it but to highlight the fact that it is not a good idea to have default grubenv and grub.cfg on separate folder.

> grubenv on the other hand is supposed to store information that should
> survive between reboots and rollbacks, so it should not be snapshot specific
> (but currently is).

No, I think really it has to depend on the variable/information to keep. For instance, the commonly used $saved_entry for setting default entry of grub.cfg which is only specific to the grub.cfg at that given point of time. It may not find the matching entry from other grub.cfg if making it persist to rollback.

> I agree that grub.cfg *could* be seen as variable data, as the file can be
> generated from /etc/grub.d snippets any time, but I don't think this has to
> be considered in this context.

Agreed (See also above).

> 
> > 3. I think the idea of having /boot/var is only useful for btrfs readonly
> > snapshot, but the configure option --with-statedir enables it globally that
> > I think the unnessary detoure is too intrusive.
> 
> My original approach was to implement this only for read-only systems, but
> apart from the fact that KIWI currently doesn't like it (see
> https://bugzilla.opensuse.org/show_bug.cgi?id=1156441#c20), it also resulted
> in really messy workarounds, e.g. temporarily modifying the
> $config_directory variable.
> 
> Personally I'm not (yet) opposed to leaving this a read-only-root-fs
> specific patch, so I'll check which workarounds I may be able to avoid by
> moving some handling the grub2 package itself next week.

I'm still wondering why this can't be handled in separate grub scripts, which loads grubenv from other paths. By doing so we can also make it a separate package to grub2 and belongs to the package which needs the /boot/var. Anyway I'll try to come up with POC according to my understanding and see ...

Thanks.
Comment 21 Ignaz Forster 2021-10-22 16:57:27 UTC
*** Bug 1191873 has been marked as a duplicate of this bug. ***