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

only set LIBGUESTFS_HV for ppc64le on el7 #3033

Merged
merged 2 commits into from
Aug 16, 2022

Conversation

dustymabe
Copy link
Member

@dustymabe dustymabe commented Aug 11, 2022

commit 6c25ec0fd64b82dc70f31fa7262d656021cd4ccd
Author: Dusty Mabe <dusty@dustymabe.com>
Date:   Thu Aug 11 10:10:35 2022 -0400

    only set LIBGUESTFS_HV for ppc64le on el7
    
    It appears the newer P9 hardware that we are utilizing that is running
    a Fedora stack doesn't need this. Local testing seems to work fine.
    
    Let's hack around this for now while we still have el7 clusters.

commit 0eb38ebcad430c58e2e58fed5c069a638a07e73f
Author: Dusty Mabe <dusty@dustymabe.com>
Date:   Mon Aug 15 16:23:49 2022 -0400

    mantle/platform/qemu: fix seting of LIBGUESTFS env vars
    
    This setting of cmd.Env was overriding the LIBGUESTFS_BACKEND=direct
    that was being set just a few lines before. So the command was getting
    run with just LIBGUESTFS_HV being set, which happens to have no
    effect unless LIBGUESTFS_BACKEND=direct is set.
    
    Fix that by appending to cmd.Env on the second modification.

This setting of cmd.Env was overriding the LIBGUESTFS_BACKEND=direct
that was being set just a few lines before. So the command was getting
run with just LIBGUESTFS_HV being set, which happens to have no
effect unless LIBGUESTFS_BACKEND=direct is set.

Fix that by appending to cmd.Env on the second modification.
It appears the newer P9 hardware that we are utilizing that is running
a Fedora stack doesn't need this. Local testing seems to work fine.

Let's hack around this for now while we still have el7 clusters.
if err := unix.Uname(&u); err != nil {
return nil, errors.Wrapf(err, "detecting kernel information")
}
if strings.Contains(fmt.Sprintf("%s", u.Release), "el7") {
Copy link
Member

Choose a reason for hiding this comment

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

I think this would be slightly better as:

Suggested change
if strings.Contains(fmt.Sprintf("%s", u.Release), "el7") {
if strings.Contains(string(u.Release), "el7") {

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants