Skip to content
This repository was archived by the owner on Mar 9, 2022. It is now read-only.

Use device number to find uuid #335

Merged
Random-Liu merged 1 commit into from
Oct 10, 2017
Merged

Use device number to find uuid #335

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
15 changes: 5 additions & 10 deletions pkg/os/os.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ type OS interface {
Unmount(target string, flags int) error
GetMounts() ([]*mount.Info, error)
LookupMount(path string) (containerdmount.Info, error)
DeviceUUID(device string) (string, error)
DeviceUUID(device uint64) (string, error)
}

// RealOS is used to dispatch the real system level operations.
Expand Down Expand Up @@ -144,14 +144,9 @@ func blkrdev(device string) (uint64, error) {
return stat.Rdev, nil
}

// DeviceUUID gets device uuid of a device. The passed in device should be
// an absolute path of the device.
func (RealOS) DeviceUUID(device string) (string, error) {
rdev, err := blkrdev(device)
if err != nil {
return "", err
}

// DeviceUUID gets device uuid of a device. The passed in rdev should be
// linux device number.
func (RealOS) DeviceUUID(rdev uint64) (string, error) {
const uuidDir = "/dev/disk/by-uuid"
files, err := ioutil.ReadDir(uuidDir)
if err != nil {
Expand All @@ -169,5 +164,5 @@ func (RealOS) DeviceUUID(device string) (string, error) {
return file.Name(), nil
}
}
return "", fmt.Errorf("device %q not found", device)
return "", fmt.Errorf("device %d not found", rdev)
}
4 changes: 2 additions & 2 deletions pkg/os/testing/fake_os.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ type FakeOS struct {
UnmountFn func(target string, flags int) error
GetMountsFn func() ([]*mount.Info, error)
LookupMountFn func(path string) (containerdmount.Info, error)
DeviceUUIDFn func(device string) (string, error)
DeviceUUIDFn func(device uint64) (string, error)
calls []CalledDetail
errors map[string]error
}
Expand Down Expand Up @@ -258,7 +258,7 @@ func (f *FakeOS) LookupMount(path string) (containerdmount.Info, error) {
}

// DeviceUUID is a fake call that invodes DeviceUUIDFn or just return nil.
func (f *FakeOS) DeviceUUID(device string) (string, error) {
func (f *FakeOS) DeviceUUID(device uint64) (string, error) {
f.appendCalls("DeviceUUID", device)
if err := f.getError("DeviceUUID"); err != nil {
return "", err
Expand Down
7 changes: 5 additions & 2 deletions pkg/server/service.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ import (
runcapparmor "github.com/opencontainers/runc/libcontainer/apparmor"
runcseccomp "github.com/opencontainers/runc/libcontainer/seccomp"
"golang.org/x/net/context"
"golang.org/x/sys/unix"
"google.golang.org/grpc"
"k8s.io/kubernetes/pkg/kubelet/apis/cri/v1alpha1/runtime"
"k8s.io/kubernetes/pkg/kubelet/server/streaming"
Expand Down Expand Up @@ -143,6 +144,7 @@ func NewCRIContainerdService(config options.Config) (CRIContainerdService, error
if err != nil {
return nil, fmt.Errorf("failed to get imagefs uuid of %q: %v", imageFSPath, err)
}
glog.V(2).Infof("Get device uuid %q for image filesystem %q", c.imageFSUUID, imageFSPath)

c.netPlugin, err = ocicni.InitCNI(config.NetworkPluginConfDir, config.NetworkPluginBinDir)
if err != nil {
Expand Down Expand Up @@ -243,11 +245,12 @@ func (c *criContainerdService) Stop() {

// getDeviceUUID gets device uuid for a given path.
func (c *criContainerdService) getDeviceUUID(path string) (string, error) {
info, err := c.os.LookupMount(path)
mount, err := c.os.LookupMount(path)
Copy link
Contributor

@ijc ijc Oct 10, 2017

Choose a reason for hiding this comment

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

LookupMount has a comment in it:

  // Note that m.{Major, Minor} are generally unreliable for our purpose here
  // https://www.spinics.net/lists/linux-btrfs/msg58908.html

But we are now using exactly those fields below.

Copy link
Contributor

@ijc ijc Oct 10, 2017

Choose a reason for hiding this comment

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

Actually, we don't seem to rely on these any more or less than we did before, so I guess this is fine.

Copy link
Member

@mikebrow mikebrow Oct 11, 2017

Choose a reason for hiding this comment

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

@Random-Liu @ijc Interesting.. if "generally unreliable" maybe we should do it both ways and compare results then return the best result. Just thinking out loud and now worrying about what was meant by generally unreliable.

if err != nil {
return "", err
}
return c.os.DeviceUUID(info.Source)
rdev := unix.Mkdev(uint32(mount.Major), uint32(mount.Minor))
return c.os.DeviceUUID(rdev)
}

// imageFSPath returns containerd image filesystem path.
Expand Down