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
Add tool to install modules in lcow and plumb through shim #1195
Merged
katiewasnothere
merged 3 commits into
microsoft:master
from
katiewasnothere:lcow_install_modules
Nov 2, 2021
Merged
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,5 @@ | ||
// +build linux | ||
|
||
package main | ||
|
||
import ( | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,96 @@ | ||
// +build linux | ||
|
||
package main | ||
|
||
import ( | ||
"context" | ||
"fmt" | ||
"os" | ||
"os/exec" | ||
"path/filepath" | ||
"strings" | ||
|
||
"github.com/Microsoft/hcsshim/internal/guest/storage/overlay" | ||
"github.com/google/uuid" | ||
"github.com/pkg/errors" | ||
"github.com/sirupsen/logrus" | ||
) | ||
|
||
const ( | ||
lcowGlobalDriversFormat = "/run/drivers/%s" | ||
|
||
moduleExtension = ".ko" | ||
) | ||
|
||
func install(ctx context.Context) error { | ||
args := []string(os.Args[1:]) | ||
|
||
if len(args) == 0 { | ||
return errors.New("no driver paths provided for install") | ||
} | ||
|
||
for _, driver := range args { | ||
modules := []string{} | ||
|
||
driverGUID, err := uuid.NewRandom() | ||
if err != nil { | ||
return err | ||
} | ||
|
||
// create an overlay mount from the driver's UVM path so we can write to the | ||
// mount path in the UVM despite having mounted in the driver originally as | ||
// readonly | ||
runDriverPath := fmt.Sprintf(lcowGlobalDriversFormat, driverGUID.String()) | ||
upperPath := filepath.Join(runDriverPath, "upper") | ||
workPath := filepath.Join(runDriverPath, "work") | ||
rootPath := filepath.Join(runDriverPath, "content") | ||
if err := overlay.Mount(ctx, []string{driver}, upperPath, workPath, rootPath, false); err != nil { | ||
return err | ||
} | ||
|
||
// find all module files, which end with ".ko" extension, and remove extension | ||
// for use when calling `modprobe` below. | ||
if walkErr := filepath.Walk(rootPath, func(path string, info os.FileInfo, err error) error { | ||
if err != nil { | ||
return errors.Wrap(err, "failed to read directory while walking dir") | ||
} | ||
if !info.IsDir() && filepath.Ext(info.Name()) == moduleExtension { | ||
moduleName := strings.TrimSuffix(info.Name(), moduleExtension) | ||
modules = append(modules, moduleName) | ||
} | ||
return nil | ||
}); walkErr != nil { | ||
return walkErr | ||
} | ||
|
||
// create a new module dependency map database for the driver | ||
depmodArgs := []string{"-b", rootPath} | ||
cmd := exec.Command("depmod", depmodArgs...) | ||
out, err := cmd.CombinedOutput() | ||
if err != nil { | ||
return errors.Wrapf(err, "failed to run depmod with args %v: %s", depmodArgs, out) | ||
} | ||
|
||
// run modprobe for every module name found | ||
modprobeArgs := append([]string{"-d", rootPath, "-a"}, modules...) | ||
cmd = exec.Command( | ||
"modprobe", | ||
modprobeArgs..., | ||
) | ||
|
||
out, err = cmd.CombinedOutput() | ||
if err != nil { | ||
return errors.Wrapf(err, "failed to run modporbe with args %v: %s", modprobeArgs, out) | ||
} | ||
} | ||
|
||
return nil | ||
} | ||
|
||
func installDriversMain() { | ||
ctx := context.Background() | ||
logrus.SetOutput(os.Stderr) | ||
if err := install(ctx); err != nil { | ||
logrus.Fatalf("error in install drivers: %s", err) | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,6 +5,9 @@ package devices | |
import ( | ||
"context" | ||
"fmt" | ||
"io/ioutil" | ||
"net" | ||
"strings" | ||
|
||
"github.com/Microsoft/hcsshim/internal/cmd" | ||
"github.com/Microsoft/hcsshim/internal/log" | ||
|
@@ -22,6 +25,8 @@ const ( | |
is an expected race and can be ignored.` | ||
) | ||
|
||
var noExecOutputErr = errors.New("failed to get any pipe output") | ||
|
||
// createPnPInstallDriverCommand creates a pnputil command to add and install drivers | ||
// present in `driverUVMPath` and all subdirectories. | ||
func createPnPInstallDriverCommand(driverUVMPath string) []string { | ||
|
@@ -61,3 +66,72 @@ func execPnPInstallDriver(ctx context.Context, vm *uvm.UtilityVM, driverDir stri | |
log.G(ctx).WithField("added drivers", driverDir).Debug("installed drivers") | ||
return nil | ||
} | ||
|
||
func execModprobeInstallDriver(ctx context.Context, vm *uvm.UtilityVM, driverDir string) error { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Move LCOW install support routines out of pnp.go There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done |
||
p, l, err := cmd.CreateNamedPipeListener() | ||
if err != nil { | ||
return err | ||
} | ||
defer l.Close() | ||
|
||
var pipeResults []string | ||
errChan := make(chan error) | ||
|
||
go readCsPipeOutput(l, errChan, &pipeResults) | ||
|
||
args := []string{ | ||
"/bin/install-drivers", | ||
driverDir, | ||
} | ||
req := &cmd.CmdProcessRequest{ | ||
Args: args, | ||
Stderr: p, | ||
} | ||
|
||
exitCode, err := cmd.ExecInUvm(ctx, vm, req) | ||
if err != nil && err != noExecOutputErr { | ||
return errors.Wrapf(err, "failed to install driver %s in uvm with exit code %d", driverDir, exitCode) | ||
} | ||
|
||
// wait to finish parsing stdout results | ||
select { | ||
case err := <-errChan: | ||
if err != nil { | ||
return err | ||
} | ||
case <-ctx.Done(): | ||
return ctx.Err() | ||
} | ||
|
||
log.G(ctx).WithField("added drivers", driverDir).Debug("installed drivers") | ||
return nil | ||
} | ||
|
||
// readCsPipeOutput is a helper function that connects to a listener and reads | ||
// the connection's comma separated output until done. resulting comma separated | ||
// values are returned in the `result` param. The `errChan` param is used to | ||
// propagate an errors to the calling function. | ||
func readCsPipeOutput(l net.Listener, errChan chan<- error, result *[]string) { | ||
defer close(errChan) | ||
c, err := l.Accept() | ||
if err != nil { | ||
errChan <- errors.Wrapf(err, "failed to accept named pipe") | ||
return | ||
} | ||
bytes, err := ioutil.ReadAll(c) | ||
if err != nil { | ||
errChan <- err | ||
return | ||
} | ||
|
||
elementsAsString := strings.TrimSuffix(string(bytes), "\n") | ||
elements := strings.Split(elementsAsString, ",") | ||
*result = append(*result, elements...) | ||
|
||
if len(*result) == 0 { | ||
errChan <- noExecOutputErr | ||
return | ||
} | ||
|
||
errChan <- nil | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this new dep only because go-winio/pkg/guid doesn't build on Linux? 😔 microsoft/go-winio#169 We should move this forward, I'll approve and we should get a new release of go-winio out with this, didn't realize this (or know about that pr) until now. We shouldn't have to bring in a new dep for this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok now that that's in, we'll just need to cut a release and revendor here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did we come to a conclusion on cutting a new tag?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think so, but since this is an official google package I'd think the risk is probably minimal either way.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not worried about risk, more that we're introducing a new dep just to generate an ID when a project we maintain has the same functionality (if we cut a new tag) haha
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's just do a follow up pr to remove the dep with a new tag of winio so we can move this forward