Skip to content

Commit

Permalink
Switch all calls to filepath.Walk to filepath.WalkDir
Browse files Browse the repository at this point in the history
WalkDir should be faster the Walk, since we often do
not need to stat files.

[NO NEW TESTS NEEDED] Existing tests should find errors.

Signed-off-by: Daniel J Walsh <[email protected]>
  • Loading branch information
rhatdan authored and mheon committed Mar 30, 2022
1 parent 713ce2a commit b362367
Show file tree
Hide file tree
Showing 11 changed files with 68 additions and 57 deletions.
11 changes: 2 additions & 9 deletions libpod/container_internal.go
Original file line number Diff line number Diff line change
Expand Up @@ -99,15 +99,8 @@ func (c *Container) rootFsSize() (int64, error) {
// rwSize gets the size of the mutable top layer of the container.
func (c *Container) rwSize() (int64, error) {
if c.config.Rootfs != "" {
var size int64
err := filepath.Walk(c.config.Rootfs, func(path string, info os.FileInfo, err error) error {
if err != nil {
return err
}
size += info.Size()
return nil
})
return size, err
size, err := util.SizeOfPath(c.config.Rootfs)
return int64(size), err
}

container, err := c.runtime.store.Container(c.ID())
Expand Down
12 changes: 2 additions & 10 deletions libpod/volume.go
Original file line number Diff line number Diff line change
@@ -1,13 +1,12 @@
package libpod

import (
"os"
"path/filepath"
"time"

"github.com/containers/podman/v4/libpod/define"
"github.com/containers/podman/v4/libpod/lock"
"github.com/containers/podman/v4/libpod/plugin"
"github.com/containers/podman/v4/pkg/util"
)

// Volume is a libpod named volume.
Expand Down Expand Up @@ -93,14 +92,7 @@ func (v *Volume) Name() string {

// Returns the size on disk of volume
func (v *Volume) Size() (uint64, error) {
var size uint64
err := filepath.Walk(v.config.MountPoint, func(path string, info os.FileInfo, err error) error {
if err == nil && !info.IsDir() {
size += (uint64)(info.Size())
}
return err
})
return size, err
return util.SizeOfPath(v.config.MountPoint)
}

// Driver retrieves the volume's driver.
Expand Down
23 changes: 18 additions & 5 deletions pkg/bindings/images/build.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"encoding/json"
"fmt"
"io"
"io/fs"
"io/ioutil"
"net/http"
"net/url"
Expand Down Expand Up @@ -557,14 +558,14 @@ func nTar(excludes []string, sources ...string) (io.ReadCloser, error) {
merr = multierror.Append(merr, err)
return
}
err = filepath.Walk(s, func(path string, info os.FileInfo, err error) error {
err = filepath.WalkDir(s, func(path string, d fs.DirEntry, err error) error {
if err != nil {
return err
}

// check if what we are given is an empty dir, if so then continue w/ it. Else return.
// if we are given a file or a symlink, we do not want to exclude it.
if info.IsDir() && s == path {
if d.IsDir() && s == path {
var p *os.File
p, err = os.Open(path)
if err != nil {
Expand All @@ -588,7 +589,11 @@ func nTar(excludes []string, sources ...string) (io.ReadCloser, error) {
return nil
}

if info.Mode().IsRegular() { // add file item
if d.Type().IsRegular() { // add file item
info, err := d.Info()
if err != nil {
return err
}
di, isHardLink := checkHardLink(info)
if err != nil {
return err
Expand Down Expand Up @@ -624,7 +629,11 @@ func nTar(excludes []string, sources ...string) (io.ReadCloser, error) {
seen[di] = name
}
return err
} else if info.Mode().IsDir() { // add folders
} else if d.IsDir() { // add folders
info, err := d.Info()
if err != nil {
return err
}
hdr, lerr := tar.FileInfoHeader(info, name)
if lerr != nil {
return lerr
Expand All @@ -634,11 +643,15 @@ func nTar(excludes []string, sources ...string) (io.ReadCloser, error) {
if lerr := tw.WriteHeader(hdr); lerr != nil {
return lerr
}
} else if info.Mode()&os.ModeSymlink != 0 { // add symlinks as it, not content
} else if d.Type()&os.ModeSymlink != 0 { // add symlinks as it, not content
link, err := os.Readlink(path)
if err != nil {
return err
}
info, err := d.Info()
if err != nil {
return err
}
hdr, lerr := tar.FileInfoHeader(info, link)
if lerr != nil {
return lerr
Expand Down
22 changes: 4 additions & 18 deletions pkg/domain/infra/abi/system.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import (
"net/url"
"os"
"os/exec"
"path/filepath"

"github.com/containers/common/pkg/cgroups"
"github.com/containers/common/pkg/config"
Expand Down Expand Up @@ -269,7 +268,7 @@ func (ic *ContainerEngine) SystemDf(ctx context.Context, options entities.System
}

dfVolumes := make([]*entities.SystemDfVolumeReport, 0, len(vols))
var reclaimableSize int64
var reclaimableSize uint64
for _, v := range vols {
var consInUse int
mountPoint, err := v.MountPoint()
Expand All @@ -282,7 +281,7 @@ func (ic *ContainerEngine) SystemDf(ctx context.Context, options entities.System
// TODO: fix this.
continue
}
volSize, err := sizeOfPath(mountPoint)
volSize, err := util.SizeOfPath(mountPoint)
if err != nil {
return nil, err
}
Expand All @@ -301,8 +300,8 @@ func (ic *ContainerEngine) SystemDf(ctx context.Context, options entities.System
report := entities.SystemDfVolumeReport{
VolumeName: v.Name(),
Links: consInUse,
Size: volSize,
ReclaimableSize: reclaimableSize,
Size: int64(volSize),
ReclaimableSize: int64(reclaimableSize),
}
dfVolumes = append(dfVolumes, &report)
}
Expand All @@ -313,19 +312,6 @@ func (ic *ContainerEngine) SystemDf(ctx context.Context, options entities.System
}, nil
}

// sizeOfPath determines the file usage of a given path. it was called volumeSize in v1
// and now is made to be generic and take a path instead of a libpod volume
func sizeOfPath(path string) (int64, error) {
var size int64
err := filepath.Walk(path, func(path string, info os.FileInfo, err error) error {
if err == nil && !info.IsDir() {
size += info.Size()
}
return err
})
return size, err
}

func (se *SystemEngine) Reset(ctx context.Context) error {
return se.Libpod.Reset(ctx)
}
Expand Down
5 changes: 3 additions & 2 deletions pkg/machine/ignition.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ package machine
import (
"encoding/json"
"fmt"
"io/fs"
"io/ioutil"
"net/url"
"os"
Expand Down Expand Up @@ -507,8 +508,8 @@ func getCerts(certsDir string, isDir bool) []File {
)

if isDir {
err := filepath.Walk(certsDir, func(path string, info os.FileInfo, err error) error {
if err == nil && !info.IsDir() {
err := filepath.WalkDir(certsDir, func(path string, d fs.DirEntry, err error) error {
if err == nil && !d.IsDir() {
certPath, err := filepath.Rel(certsDir, path)
if err != nil {
logrus.Warnf("%s", err)
Expand Down
6 changes: 3 additions & 3 deletions pkg/machine/qemu/machine.go
Original file line number Diff line number Diff line change
Expand Up @@ -891,10 +891,10 @@ func GetVMInfos() ([]*machine.ListResponse, error) {

var listed []*machine.ListResponse

if err = filepath.Walk(vmConfigDir, func(path string, info os.FileInfo, err error) error {
if err = filepath.WalkDir(vmConfigDir, func(path string, d fs.DirEntry, err error) error {
vm := new(MachineVM)
if strings.HasSuffix(info.Name(), ".json") {
fullPath := filepath.Join(vmConfigDir, info.Name())
if strings.HasSuffix(d.Name(), ".json") {
fullPath := filepath.Join(vmConfigDir, d.Name())
b, err := ioutil.ReadFile(fullPath)
if err != nil {
return err
Expand Down
7 changes: 4 additions & 3 deletions pkg/machine/wsl/machine.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"encoding/json"
"fmt"
"io"
"io/fs"
"io/ioutil"
"net/url"
"os"
Expand Down Expand Up @@ -1175,10 +1176,10 @@ func GetVMInfos() ([]*machine.ListResponse, error) {

var listed []*machine.ListResponse

if err = filepath.Walk(vmConfigDir, func(path string, info os.FileInfo, err error) error {
if err = filepath.WalkDir(vmConfigDir, func(path string, d fs.DirEntry, err error) error {
vm := new(MachineVM)
if strings.HasSuffix(info.Name(), ".json") {
fullPath := filepath.Join(vmConfigDir, info.Name())
if strings.HasSuffix(d.Name(), ".json") {
fullPath := filepath.Join(vmConfigDir, d.Name())
b, err := ioutil.ReadFile(fullPath)
if err != nil {
return err
Expand Down
5 changes: 3 additions & 2 deletions pkg/specgen/generate/config_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package generate

import (
"fmt"
"io/fs"
"io/ioutil"
"os"
"path"
Expand Down Expand Up @@ -101,8 +102,8 @@ func DevicesFromPath(g *generate.Generator, devicePath string) error {
}

// mount the internal devices recursively
if err := filepath.Walk(resolvedDevicePath, func(dpath string, f os.FileInfo, e error) error {
if f.Mode()&os.ModeDevice == os.ModeDevice {
if err := filepath.WalkDir(resolvedDevicePath, func(dpath string, d fs.DirEntry, e error) error {
if d.Type()&os.ModeDevice == os.ModeDevice {
found = true
device := fmt.Sprintf("%s:%s", dpath, filepath.Join(dest, strings.TrimPrefix(dpath, src)))
if devmode != "" {
Expand Down
18 changes: 18 additions & 0 deletions pkg/util/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package util
import (
"encoding/json"
"fmt"
"io/fs"
"math"
"os"
"os/user"
Expand Down Expand Up @@ -731,3 +732,20 @@ func LookupUser(name string) (*user.User, error) {
}
return user.Lookup(name)
}

// SizeOfPath determines the file usage of a given path. it was called volumeSize in v1
// and now is made to be generic and take a path instead of a libpod volume
func SizeOfPath(path string) (uint64, error) {
var size uint64
err := filepath.WalkDir(path, func(path string, d fs.DirEntry, err error) error {
if err == nil && !d.IsDir() {
info, err := d.Info()
if err != nil {
return err
}
size += uint64(info.Size())
}
return err
})
return size, err
}
9 changes: 7 additions & 2 deletions pkg/util/utils_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package util

import (
"fmt"
"io/fs"
"os"
"path/filepath"
"syscall"
Expand All @@ -23,17 +24,21 @@ func GetContainerPidInformationDescriptors() ([]string, error) {
// Symlinks to nodes are ignored.
func FindDeviceNodes() (map[string]string, error) {
nodes := make(map[string]string)
err := filepath.Walk("/dev", func(path string, info os.FileInfo, err error) error {
err := filepath.WalkDir("/dev", func(path string, d fs.DirEntry, err error) error {
if err != nil {
logrus.Warnf("Error descending into path %s: %v", path, err)
return filepath.SkipDir
}

// If we aren't a device node, do nothing.
if info.Mode()&(os.ModeDevice|os.ModeCharDevice) == 0 {
if d.Type()&(os.ModeDevice|os.ModeCharDevice) == 0 {
return nil
}

info, err := d.Info()
if err != nil {
return err
}
// We are a device node. Get major/minor.
sysstat, ok := info.Sys().(*syscall.Stat_t)
if !ok {
Expand Down
7 changes: 4 additions & 3 deletions test/e2e/pod_rm_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package integration

import (
"fmt"
"io/fs"
"io/ioutil"
"os"
"path/filepath"
Expand Down Expand Up @@ -46,14 +47,14 @@ var _ = Describe("Podman pod rm", func() {
Expect(result).Should(Exit(0))

// Also check that we don't leak cgroups
err := filepath.Walk("/sys/fs/cgroup", func(path string, info os.FileInfo, err error) error {
err := filepath.WalkDir("/sys/fs/cgroup", func(path string, d fs.DirEntry, err error) error {
if err != nil {
return err
}
if !info.IsDir() {
if !d.IsDir() {
Expect(err).To(BeNil())
}
if strings.Contains(info.Name(), podid) {
if strings.Contains(d.Name(), podid) {
return fmt.Errorf("leaking cgroup path %s", path)
}
return nil
Expand Down

0 comments on commit b362367

Please sign in to comment.