Bug 994322

Summary: bye-bye, grub_installdevice
Product: [openSUSE] openSUSE Tumbleweed Reporter: Steffen Winterfeldt <snwint>
Component: BootloaderAssignee: YaST Team <yast-internal>
Status: RESOLVED FIXED QA Contact: Jiri Srain <jsrain>
Severity: Major    
Priority: P2 - High CC: gmoro, jreidinger, mchang, mnowak, ohering, snwint
Version: Current   
Target Milestone: ---   
Hardware: Other   
OS: Other   
URL: https://trello.com/c/WmzgcCIO
Whiteboard:
Found By: Development Services Priority:
Business Priority: Blocker: ---
Marketing QA Status: --- IT Deployment: ---

Description Steffen Winterfeldt 2016-08-18 09:03:16 UTC
[follow-up from bug 979002]

plan:

Get rid of /etc/default/grub_installdevice for i386-pc target.

use-case:

To avoid complications when device names are not stable (see bug above). For example when drivers change or VMs are moved around.

Also, yast will only propose configurations where grub resides in the disk with the /boot partition - so the assumption below should ideally always be true.

how:

/usr/lib/bootloader/grub2/install replaces missing entries with defaults:

- device = disk where /boot/grub2/i386-pc/ lives
- there are other entries like 'activate' or 'generic_mbr'; they can be dropped
  as they are not used anyway by /usr/lib/bootloader/grub2/install

So, basically yast should only create grub_installdevice when grub is installed
to a non-default device.
Comment 1 Josef Reidinger 2016-08-18 10:31:36 UTC
well, purpose of grub_installdevice is that it store information where grub2 is installed as yast2 cannot detect it by itself. Reason for activate and generic_mbr is again that yast2-bootloader read these values and show it to user for current configuration and if user confirm setup, it redo it. So even if something break MBR or remove 'boot' flag from disk, yast2-bootloader will fix it. Without this file, y2-bl do not know previous setup, so cannot help with changing existing configuration.

Currently what y2-bl do is that based on hardware setup propose to write grub2 to 
1) where lives partition for /boot/grub2/i386-pc/
2) extended_partition if /boot is on logical disk
3) where lives disk for /boot/grub2/i386-pc/

It is quite complex logic where to write, depending on many parts and can be overwritten. So I worry that without any replacement y2-bl no longer will know where it will be installed.

My proposel how to handle not so stable device names ( even when using udev ) is to detect such situation in y2-bl and write it to user with option to repropose configuration from scratch.
Comment 2 Steffen Winterfeldt 2016-08-18 11:05:31 UTC
Ok, so yast also uses this file to store some settings.

Why is the file only readable by root, btw? Do we store some sensitive data here?

Anyway, isn't the idea that in future our standard grub setup is grub into mbr of the disk with boot files? It's not that complex, then (unless the user changes something).

And if we don't store defaults, the file will be gone in most cases. No?
Comment 3 Josef Reidinger 2016-08-18 11:15:39 UTC
(In reply to Steffen Winterfeldt from comment #2)
> Ok, so yast also uses this file to store some settings.
> 
> Why is the file only readable by root, btw? Do we store some sensitive data
> here?

I do not expect sensitive information there.

> 
> Anyway, isn't the idea that in future our standard grub setup is grub into
> mbr of the disk with boot files? It's not that complex, then (unless the
> user changes something).

Well, if there is plan for such change of setup, it should be properly communicate with customers, so they are aware about such change in proposal as it also have some drawbacks.

> 
> And if we don't store defaults, the file will be gone in most cases. No?

Yes, if we decide about stable defaults (stable in time), then we can ignore such file and simply use defaults if it is not there.
Comment 4 Steffen Winterfeldt 2016-08-18 13:18:21 UTC
I've hacked a proof of concept:

https://github.com/openSUSE/perl-bootloader/pull/105

This will install grub even if grub_installdevice is missing or has some invalid device name.

It certainly solves not all issues (e.g. grub_installdevice can have more than one device entry) - but shows the direction we could go.
Comment 5 Michael Chang 2016-08-19 09:39:00 UTC
Well my idea is bascally similar to Steffen's, but the install scripts divided by install_location.

Eg
 /usr/lib/bootloader/grub2/install_device_disk
 /usr/lib/bootloader/grub2/install_device_extended
 /usr/lib/bootloader/grub2/install_device_root
 /usr/lib/bootloader/grub2/install_device_boot

And we set install script to one of them

 ln -s /usr/lib/bootloader/grub2/install_device_mbr /usr/lib/bootloader/grub2/install_00

In case we need secondary or third we increase that counter

 ln -s /usr/lib/bootloader/grub2/install_device_mbr /usr/lib/bootloader/grub2/install_01

It's easy to get the install location by following the link, and if things go wrong like the horrendous name scheme change it's easier to adapt the script than config. :)
Comment 6 Michael Chang 2016-09-05 08:48:05 UTC
Hi Steffen,

How do you think about the workaround for bsc#979002 for SP2? I know the schedule is for SP3 to remove grub_installdevice, but it may not help for SP2 guest that require running boot loader update in which invalid or missing device won't work. we may need a fallback method here to cover this case (like what you suggested in comment#4).
Thanks.
Comment 7 Steffen Winterfeldt 2016-09-08 11:32:55 UTC
I would not do it via symlinks; that's likely to create a mess at some point
during package updates. Also, it's impractical if some tool needs to
determine the current state.

It might be a good opportunity to cut the line and move remaining config
vars out of grub_installdevice into /etc/sysconfig/bootloader.
grub_installdevice is not, as its name suggests, some file used by the
regular grub. So we can as well go the common SUSE-way of using
/etc/sysconfig.

In this case, say, bootloader::DESTINATION with values like 'disk', 'boot',
'root'.

We might put comment 4 into sp2 as it doesn't change the way things work
currently and only steps in if grub_installdevice is obviously invalid. But
for that I'd like some feedback also from Josef and particularly Olaf as to
whether this actually helps.
Comment 8 Olaf Hering 2016-09-13 07:18:59 UTC
(In reply to Steffen Winterfeldt from comment #4)
> I've hacked a proof of concept:
> 
> https://github.com/openSUSE/perl-bootloader/pull/105

This seems to rely on df output. Does df return an usable device name, never things like "by-*/*-partN"? Would a stat + mknod be more reliable?

Looks like install() will always set installed even if grub2-install fails.
Comment 9 Josef Reidinger 2016-09-13 07:27:43 UTC
(In reply to Steffen Winterfeldt from comment #7)
> I would not do it via symlinks; that's likely to create a mess at some point
> during package updates. Also, it's impractical if some tool needs to
> determine the current state.
> 
> It might be a good opportunity to cut the line and move remaining config
> vars out of grub_installdevice into /etc/sysconfig/bootloader.
> grub_installdevice is not, as its name suggests, some file used by the
> regular grub. So we can as well go the common SUSE-way of using
> /etc/sysconfig.
> 
> In this case, say, bootloader::DESTINATION with values like 'disk', 'boot',
> 'root'.
> 
> We might put comment 4 into sp2 as it doesn't change the way things work
> currently and only steps in if grub_installdevice is obviously invalid. But
> for that I'd like some feedback also from Josef and particularly Olaf as to
> whether this actually helps.

Well, in past we try to reduce content of /etc/sysconfig/bootloader because it confuse users of other distribution which have everything in upstream files. So even for this case I suggest to check how do it other distributions? What they do if there is update in grub2 stage1 code? how they recognize in which location it should install it? I am sure RHEL, ubuntu and others have to have way how to do it, so if they use some common place, we should also use it to not confuse our users.
Comment 10 Olaf Hering 2016-09-13 07:34:07 UTC
(In reply to Josef Reidinger from comment #9)
> Well, in past we try to reduce content of /etc/sysconfig/bootloader because
> it confuse users of other distribution which have everything in upstream
> files.

Its probably worth a check. But in the end using upstream "solutions" is always good for a laught. And it confuses OUR users already today (why would SUSE knowingly break my system?), see the localized grub.cfg for a prominent example...
Comment 11 Steffen Winterfeldt 2016-09-13 08:21:00 UTC
Well, grub_installdevice isn't upstream either. So imho, if upstream doesn't have a solution and we have to supplement upstream functionality we should do it the SUSE way. And that would be sysconfig.

That doesn't mean we have to be ignorant about what other distros do but we don't have to blindly follow either.
Comment 12 Steffen Winterfeldt 2016-09-13 11:21:03 UTC
> This seems to rely on df output. Does df return an usable device name, never
> things like "by-*/*-partN"? Would a stat + mknod be more reliable?

'df' is some classical unix tool and works quite fine. I don't think we improve
by re-implementing finding the device ourselves.

It returns the kernel names and even if it would not it wouldn't matter as long
as the device node it reports actually exists at that time. And I'd trust it
that far.

> Looks like install() will always set installed even if grub2-install fails.

'install_tried' might be a semantically more appropriate name, yes.

My idea was to find a valid device, then install. I don't think it's the task
of the script to handle the situation 'device exists - but grub can't be
installed there'. That would be a real config error and might as well be
visible as such.
Comment 13 Steffen Winterfeldt 2017-03-09 09:00:05 UTC
Here's POC #2

https://github.com/openSUSE/perl-bootloader/pull/109

This goes a bit further and tries to resolve raid/lvm and crypto setups.
Comment 14 Dirk Mueller 2018-10-30 12:29:38 UTC
superseded by https://github.com/openSUSE/perl-bootloader/pull/117 which got merged. is this done?
Comment 15 Steffen Winterfeldt 2018-10-30 12:59:39 UTC
Yes, can be closed; thanks.
Comment 17 Swamp Workflow Management 2018-12-12 08:50:16 UTC
SUSE-RU-2018:4085-1: An update that has three recommended fixes can now be installed.

Category: recommended (moderate)
Bug References: 1079321,1108777,994322
CVE References: 
Sources used:
SUSE Linux Enterprise Server 12-SP3 (src):    perl-Bootloader-0.923-3.9.1
SUSE Linux Enterprise Desktop 12-SP3 (src):    perl-Bootloader-0.923-3.9.1
SUSE CaaS Platform ALL (src):    perl-Bootloader-0.923-3.9.1
SUSE CaaS Platform 3.0 (src):    perl-Bootloader-0.923-3.9.1
Comment 18 Swamp Workflow Management 2018-12-13 02:11:23 UTC
openSUSE-RU-2018:4102-1: An update that has three recommended fixes can now be installed.

Category: recommended (moderate)
Bug References: 1079321,1108777,994322
CVE References: 
Sources used:
openSUSE Leap 42.3 (src):    perl-Bootloader-0.923-2.6.1
Comment 19 Swamp Workflow Management 2018-12-17 23:11:16 UTC
SUSE-RU-2018:4161-1: An update that has three recommended fixes can now be installed.

Category: recommended (low)
Bug References: 1079321,1108777,994322
CVE References: 
Sources used:
SUSE Linux Enterprise Module for Development Tools 15 (src):    perl-Bootloader-0.923-4.6.1
SUSE Linux Enterprise Module for Basesystem 15 (src):    perl-Bootloader-0.923-4.6.1
Comment 20 Swamp Workflow Management 2018-12-22 17:09:22 UTC
openSUSE-RU-2018:4239-1: An update that has three recommended fixes can now be installed.

Category: recommended (low)
Bug References: 1079321,1108777,994322
CVE References: 
Sources used:
openSUSE Leap 15.0 (src):    perl-Bootloader-0.923-lp150.3.6.1