Bug 1180218

Summary: YaST Bootloader proposal duplicates resume parameter in some cases
Product: [openSUSE] openSUSE Tumbleweed Reporter: Ancor Gonzalez Sosa <ancor>
Component: YaST2Assignee: YaST Team <yast-internal>
Status: RESOLVED FIXED QA Contact: Jiri Srain <jsrain>
Severity: Normal    
Priority: P5 - None CC: jlopez
Version: Current   
Target Milestone: ---   
Hardware: Other   
OS: Other   
URL: https://trello.com/c/H5cMuuHj
Whiteboard:
Found By: --- Services Priority:
Business Priority: Blocker: ---
Marketing QA Status: --- IT Deployment: ---

Description Ancor Gonzalez Sosa 2020-12-18 14:16:57 UTC
During installation (with TW or any recent version of Leap/SLE):

- Follow the process until reaching the Installation Summary with whichever partitioning setup that includes a swap. At that point, the bootloader proposal adds "resume=/dev/xxx" to the Grub2 command line.
- Then go back in the installation process to the Partitioner and come up with a different partitioning in which the swap has a different name.
- Return to the installation summary again

Expectation: the "resume=" parameter as been updated with the new name of the swap device.

Result: the bootloader proposal includes two "resume=" parameters. One with the swap initially proposed and another one with the correct one.
Comment 1 Ancor Gonzalez Sosa 2020-12-18 14:18:41 UTC
Answer from Josef in IRC:

<jreidinger> yeah, that is problem with merging proposals, but feel free to
      report it (I think it is not yet reported).
<jreidinger> because reality is that kernel line proposal is like initial
      kernel options, kdump options, cpu mitigations options and bootloader
      proposal itself and bootloader just adds to existing parameters when
      proposing
Comment 2 Ancor Gonzalez Sosa 2021-01-26 12:29:15 UTC
(In reply to Ancor Gonzalez Sosa from comment #1)
> 
> <jreidinger> yeah, that is problem with merging proposals, [...]

Although it's likely true we have a problem merging proposals, that doesn't seem to be the problem here.

In theory, the bootloader proposal should be totally discarded and recreated from scratch every time the storage setup changes. But there is an error in the code. If the storage setup has indeed changed, that's not properly detected.

These are the first lines of the bootloader proposal

if Yast::BootStorage.boot_filesystem.is?(:nfs)
  return something_not_relevant_in_this_bug
end
storage_changed = Yast::BootStorage.storage_changed?
# Here everything is reset if storage_changed is true

What's the problem there? Well the call to Yast::BootStorage.boot_filesystem has an unexpected side effect. It updates the global variable used by Yast::BootStorage.storage_changed? to check whether something has changed. As a result, the call to Yast::BootStorage.storage_changed? always returns false. Just because it's performed right after a call to Yast::BootStorage.boot_filesystem.
Comment 4 Ancor Gonzalez Sosa 2021-01-27 09:46:49 UTC
Fixed by yast2-bootloader-4.3.19

See details at https://github.com/yast/yast-bootloader/pull/630

SR for Tumbleweed: https://build.opensuse.org/request/show/867112
SR for SLE-15-SP3: https://build.suse.de/request/show/234510