Skip to content
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 15 additions & 33 deletions libcontainer/intelrdt/intelrdt.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (
"strings"
"sync"

"github.com/moby/sys/mountinfo"
"github.com/opencontainers/runc/libcontainer/configs"
)

Expand Down Expand Up @@ -241,45 +242,26 @@ func init() {

// Return the mount point path of Intel RDT "resource control" filesysem
func findIntelRdtMountpointDir() (string, error) {
f, err := os.Open("/proc/self/mountinfo")
mi, err := mountinfo.GetMounts(func(m *mountinfo.Info) (bool, bool) {
// similar to mountinfo.FstypeFilter but stops after the first match
if m.Fstype == "resctrl" {
return false, true // don't skip, stop
}
return true, false // skip, keep going
})
if err != nil {
return "", err
}
defer f.Close()

s := bufio.NewScanner(f)
for s.Scan() {
text := s.Text()
fields := strings.Split(text, " ")
// Safe as mountinfo encodes mountpoints with spaces as \040.
index := strings.Index(text, " - ")
postSeparatorFields := strings.Fields(text[index+3:])

Choose a reason for hiding this comment

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

I was working with this code because there was a possibility of accessing elements that not exist and that could cause a panic error.
Thanks, @kolyshkin for fixing it and improve code readability.

I adjusted the unit tests to your code. Could you consider using them?

kolyshkin/runc@intel-rdt...Creatone:creatone/intelrdt-moby-sys-mountinfo

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Left some comments then decided it's easier to rewrite the code. Hope it's OK with you.

Choose a reason for hiding this comment

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

I'm fine with this 👍

numPostFields := len(postSeparatorFields)

// This is an error as we can't detect if the mount is for "Intel RDT"
if numPostFields == 0 {
return "", fmt.Errorf("Found no fields post '-' in %q", text)
}

if postSeparatorFields[0] == "resctrl" {
// Check that the mount is properly formatted.
if numPostFields < 3 {
return "", fmt.Errorf("Error found less than 3 fields post '-' in %q", text)
}

// Check if MBA Software Controller is enabled through mount option "-o mba_MBps"
if strings.Contains(postSeparatorFields[2], "mba_MBps") {
isMbaScEnabled = true
}

return fields[4], nil
}
if len(mi) < 1 {
return "", NewNotFoundError("Intel RDT")
}
if err := s.Err(); err != nil {
return "", err

// Check if MBA Software Controller is enabled through mount option "-o mba_MBps"
if strings.Contains(","+mi[0].VfsOpts+",", ",mba_MBps,") {
isMbaScEnabled = true
}

return "", NewNotFoundError("Intel RDT")
return mi[0].Mountpoint, nil
}

// Gets the root path of Intel RDT "resource control" filesystem
Expand Down