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

Do not install grub on LVM physical volumes #901

Merged
merged 2 commits into from Jul 6, 2016
Merged

Conversation

stermeau
Copy link
Contributor

@stermeau stermeau commented Jul 4, 2016

No description provided.

@stermeau
Copy link
Contributor Author

stermeau commented Jul 4, 2016

Tested with Grub on RHEL 6

@gdha gdha self-assigned this Jul 5, 2016
@gdha gdha added this to the Rear v1.19 milestone Jul 5, 2016
@gdha gdha added the bug The code does not do what it is meant to do label Jul 5, 2016
is_disk_a_pv() {
disk=$1
# grep "^lvmdev .* ${disk} " $VAR_DIR/layout/disklayout.conf > /dev/null 2>&1
grep "^lvmdev .* ${disk} " $VAR_DIR/layout/disklayout.conf
Copy link
Member

Choose a reason for hiding this comment

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

please use $LAYOUT_FILE instead of $VAR_DIR/layout/disklayout.conf
and, why not using -q option to avoid displaying to the standard output?
I prefer something like:

if grep -q "^lvmdev .* ${disk} " $LAYOUT_FILE ; then
   return 1
else
   return 0
fi

@gdha
Copy link
Member

gdha commented Jul 5, 2016

@stermeau please have a look at my remarks in your pull request. Furthermore, in the meantime 21_install_grub.sh script has been updated by @jsmeix as well for another issue #895

@stermeau
Copy link
Contributor Author

stermeau commented Jul 6, 2016

using $LAYOUT_FILE instead of $VAR_DIR/layout/disklayout.conf
simplified function and tests as suggested by @gdha

@gdha gdha merged commit 91b0565 into rear:master Jul 6, 2016
@gdha
Copy link
Member

gdha commented Jul 6, 2016

@stermeau thank you - just merged it

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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants