Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

use grub2-install --no-nvram on PowerNV system #1742

Merged
merged 2 commits into from Feb 28, 2018

Conversation

schabrolles
Copy link
Member

@schabrolles schabrolles commented Feb 27, 2018

Relax-and-Recover (ReaR) Pull Request Template

Please fill in the following items before submitting a new pull request:

Pull Request Details:

@schabrolles schabrolles added bug The code does not do what it is meant to do special hardware or VM The issue depends on non common (virtual) hardware. labels Feb 27, 2018
@schabrolles schabrolles added this to the ReaR v2.4 milestone Feb 27, 2018
@schabrolles schabrolles self-assigned this Feb 27, 2018
@schabrolles
Copy link
Member Author

@jsmeix , @gdha I just asking for a quick review before merging. It is a pure POWER change and I know you don"t have such kind of system to validate. But you always have good feedback ;)

# Do not update nvram when system is running in PowerNV mode (BareMetal).
# grub2-install will failed if not run with the --no-nvram option on a PowerNV system.
# see https://github.com/rear/rear/pull/1742
if [[ $(awk '/platform/ {print $NF}' < /proc/cpuinfo) == PowerNV ]] ; then
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I cannot reproduce this condition on my x86_64 system (there is no 'platform' in /proc/cpuinfo)
but what I can reproduce there is e.g.:

# awk '/vendor_id/ {print $NF}' < /proc/cpuinfo
GenuineIntel
GenuineIntel
GenuineIntel
GenuineIntel
GenuineIntel
GenuineIntel
GenuineIntel
GenuineIntel

I get 8 lines because I have a 2 CPUs with 4 cores each
but when there is more than one line found
the condition does no longer result true:

# if [[ $( awk '/vendor_id/ {print $NF}' < /proc/cpuinfo ) == GenuineIntel ]] ; then echo Y ; else echo N ; fi
N

Bottom line:
The current condition is not fail-safe against multiple 'platform' lines.

Usually I use an artificial bash array to get a field at a know position
out of the first line of possibly several output lines like:

# vendor_id_lines=( $( grep '^vendor_id' /proc/cpuinfo ) )

# for element in "${vendor_id_lines[@]}" ; do echo "'$element'" ; done
'vendor_id'
':'
'GenuineIntel'
'vendor_id'
':'
'GenuineIntel'
'vendor_id'
':'
'GenuineIntel'
'vendor_id'
':'
'GenuineIntel'
'vendor_id'
':'
'GenuineIntel'
'vendor_id'
':'
'GenuineIntel'
'vendor_id'
':'
'GenuineIntel'
'vendor_id'
':'
'GenuineIntel'

# if test "${vendor_id_lines[2]}" = "GenuineIntel" ; then echo Y ; fi
Y

But this only works when the wanted value
is at a known fixed position/index in the array.
E.g. it cannot be used to test for one of the many CPU flags.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jsmeix ,

on Power, platform is printed only one time at the end:

processor	: 0
cpu		: POWER8 (raw), altivec supported
clock		: 2061.000000MHz
revision	: 2.0 (pvr 004d 0200)

[...]

processor	: 152
cpu		: POWER8 (raw), altivec supported
clock		: 2061.000000MHz
revision	: 2.0 (pvr 004d 0200)

timebase	: 512000000
platform	: PowerNV
model		: 8001-22C
machine		: PowerNV 8001-22C
firmware	: OPAL v3

We can also protect it by checking only the first line returned:

if [[ $(awk '/platform/ {print $NF}' < /proc/cpuinfo | head -n1 ) == PowerNV ]] 

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@schabrolles
feel free to implement it as you like.
When it can appear at most one time it is also o.k. as is.

Copy link
Member

@jsmeix jsmeix left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Except my generic comment where I cannot know
if it may actually matter in this case
it looks o.k. from plain looking at the code.

@schabrolles schabrolles merged commit 5638bd1 into rear:master Feb 28, 2018
@schabrolles schabrolles deleted the grub2_PowerNV branch February 28, 2018 12:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug The code does not do what it is meant to do fixed / solved / done special hardware or VM The issue depends on non common (virtual) hardware.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants