Skip to content

Commit c5fa029

Browse files
committed
Address loop dev PR comments #4178
Signed-off-by: Maksym Pavlenko <[email protected]>
1 parent b702623 commit c5fa029

3 files changed

Lines changed: 54 additions & 95 deletions

File tree

mount/losetup_linux.go

Lines changed: 49 additions & 83 deletions
Original file line numberDiff line numberDiff line change
@@ -26,28 +26,17 @@ import (
2626
"unsafe"
2727

2828
"github.com/pkg/errors"
29+
"golang.org/x/sys/unix"
2930
)
3031

3132
const (
3233
loopControlPath = "/dev/loop-control"
3334
loopDevFormat = "/dev/loop%d"
3435

35-
// According to util-linux/include/loopdev.h
36-
ioctlSetFd = 0x4C00
37-
ioctlClrFd = 0x4C01
38-
ioctlSetStatus64 = 0x4C04
39-
ioctlGetFree = 0x4C82
40-
41-
loFlagsReadonly = 1
42-
//loFlagsUseAops = 2
43-
loFlagsAutoclear = 4
44-
//loFlagsPartScan = 8
45-
loFlagsDirectIO = 16
46-
4736
ebusyString = "device or resource busy"
4837
)
4938

50-
// parameters to control loop device setup
39+
// LoopParams parameters to control loop device setup
5140
type LoopParams struct {
5241
// Loop device should forbid write
5342
Readonly bool
@@ -58,29 +47,6 @@ type LoopParams struct {
5847
Direct bool
5948
}
6049

61-
// struct loop_info64 in util-linux/include/loopdev.h
62-
type loopInfo struct {
63-
/*
64-
device uint64
65-
inode uint64
66-
rdevice uint64
67-
offset uint64
68-
sizelimit uint64
69-
number uint32
70-
encryptType uint32
71-
encryptKeySize uint32
72-
*/
73-
_ [13]uint32
74-
flags uint32
75-
fileName [64]byte
76-
/*
77-
cryptName [64]byte
78-
encryptKey [32]byte
79-
init [2]uint64
80-
*/
81-
_ [112]byte
82-
}
83-
8450
func ioctl(fd, req, args uintptr) (uintptr, uintptr, error) {
8551
r1, r2, errno := syscall.Syscall(syscall.SYS_IOCTL, fd, req, args)
8652
if errno != 0 {
@@ -96,63 +62,70 @@ func getFreeLoopDev() (uint32, error) {
9662
return 0, errors.Errorf("could not open %v: %v", loopControlPath, err)
9763
}
9864
defer ctrl.Close()
99-
num, _, err := ioctl(ctrl.Fd(), ioctlGetFree, 0)
65+
num, _, err := ioctl(ctrl.Fd(), unix.LOOP_CTL_GET_FREE, 0)
10066
if err != nil {
10167
return 0, errors.Wrap(err, "could not get free loop device")
10268
}
10369
return uint32(num), nil
10470
}
10571

106-
func setupLoopDev(backingFile, loopDev string, param LoopParams) (devFile *os.File, err error) {
72+
func setupLoopDev(backingFile, loopDev string, param LoopParams) error {
10773
// 1. Open backing file and loop device
108-
oflags := os.O_RDWR
74+
flags := os.O_RDWR
10975
if param.Readonly {
110-
oflags = os.O_RDONLY
76+
flags = os.O_RDONLY
11177
}
112-
back, err := os.OpenFile(backingFile, oflags, 0)
78+
79+
back, err := os.OpenFile(backingFile, flags, 0)
11380
if err != nil {
114-
return nil, errors.Errorf("could not open backing file: %v", err)
81+
return errors.Wrapf(err, "could not open backing file: %s", backingFile)
11582
}
11683
defer back.Close()
11784

118-
loopFile, err := os.OpenFile(loopDev, oflags, 0)
85+
loop, err := os.OpenFile(loopDev, flags, 0)
11986
if err != nil {
120-
return nil, errors.Errorf("could not open loop device: %v", err)
87+
return errors.Wrapf(err, "could not open loop device: %s", loopDev)
12188
}
122-
defer func() {
123-
if err != nil {
124-
loopFile.Close()
125-
}
126-
}()
89+
defer loop.Close()
12790

12891
// 2. Set FD
129-
if _, _, err = ioctl(loopFile.Fd(), ioctlSetFd, back.Fd()); err != nil {
130-
return nil, errors.Errorf("could not set loop fd: %v", err)
92+
if _, _, err = ioctl(loop.Fd(), unix.LOOP_SET_FD, back.Fd()); err != nil {
93+
return errors.Wrapf(err, "could not set loop fd for device: %s", loopDev)
13194
}
13295

13396
// 3. Set Info
134-
info := loopInfo{}
135-
copy(info.fileName[:], []byte(backingFile))
97+
info := unix.LoopInfo64{}
98+
copy(info.File_name[:], backingFile)
13699
if param.Readonly {
137-
info.flags |= loFlagsReadonly
100+
info.Flags |= unix.LO_FLAGS_READ_ONLY
138101
}
102+
139103
if param.Autoclear {
140-
info.flags |= loFlagsAutoclear
104+
info.Flags |= unix.LO_FLAGS_AUTOCLEAR
141105
}
106+
142107
if param.Direct {
143-
info.flags |= loFlagsAutoclear
108+
info.Flags |= unix.LO_FLAGS_DIRECT_IO
109+
}
110+
111+
_, _, err = ioctl(loop.Fd(), unix.LOOP_SET_STATUS64, uintptr(unsafe.Pointer(&info)))
112+
if err == nil {
113+
return nil
144114
}
145-
if _, _, err := ioctl(loopFile.Fd(), ioctlSetStatus64, uintptr(unsafe.Pointer(&info))); err != nil {
115+
116+
if param.Direct {
146117
// Retry w/o direct IO flag in case kernel does not support it. The downside is that
147118
// it will suffer from double cache problem.
148-
info.flags &= ^(uint32(loFlagsDirectIO))
149-
if _, _, err := ioctl(loopFile.Fd(), ioctlSetStatus64, uintptr(unsafe.Pointer(&info))); err != nil {
150-
ioctl(loopFile.Fd(), ioctlClrFd, 0)
151-
return nil, errors.Errorf("cannot set loop info:%v", err)
119+
info.Flags &= ^(uint32(unix.LO_FLAGS_DIRECT_IO))
120+
_, _, err = ioctl(loop.Fd(), unix.LOOP_SET_STATUS64, uintptr(unsafe.Pointer(&info)))
121+
if err == nil {
122+
return nil
152123
}
153124
}
154125

155-
return loopFile, nil
126+
// Cleanup loop fd and return error
127+
_, _, _ = ioctl(loop.Fd(), unix.LOOP_CLR_FD, 0)
128+
return errors.Errorf("failed to set loop device info: %v", err)
156129
}
157130

158131
// setupLoop looks for (and possibly creates) a free loop device, and
@@ -169,18 +142,15 @@ func setupLoopDev(backingFile, loopDev string, param LoopParams) (devFile *os.Fi
169142
// the loop device when done with it.
170143
//
171144
// Upon success, the file handle to the loop device is returned.
172-
func setupLoop(backingFile string, param LoopParams) (*os.File, error) {
173-
var loopDev string
174-
145+
func setupLoop(backingFile string, param LoopParams) (string, error) {
175146
for retry := 1; retry < 200; retry++ {
176147
num, err := getFreeLoopDev()
177148
if err != nil {
178-
return nil, err
149+
return "", err
179150
}
180151

181-
loopDev = fmt.Sprintf(loopDevFormat, num)
182-
loopFile, err := setupLoopDev(backingFile, loopDev, param)
183-
if err != nil {
152+
loopDev := fmt.Sprintf(loopDevFormat, num)
153+
if err := setupLoopDev(backingFile, loopDev, param); err != nil {
184154
// Per util-linux/sys-utils/losetup.c:create_loop(),
185155
// free loop device can race and we end up failing
186156
// with EBUSY when trying to set it up.
@@ -189,40 +159,36 @@ func setupLoop(backingFile string, param LoopParams) (*os.File, error) {
189159
time.Sleep(time.Millisecond * time.Duration(rand.Intn(retry*10)))
190160
continue
191161
}
192-
return nil, err
162+
return "", err
193163
}
194-
return loopFile, nil
164+
165+
return loopDev, nil
195166
}
196167

197-
return nil, errors.New("Timeout creating new loopback device")
168+
return "", errors.New("timeout creating new loopback device")
198169
}
199170

200171
func removeLoop(loopdev string) error {
201-
dev, err := os.Open(loopdev)
172+
file, err := os.Open(loopdev)
202173
if err != nil {
203174
return err
204175
}
205-
_, _, err = ioctl(dev.Fd(), ioctlClrFd, 0)
206-
dev.Close()
176+
defer file.Close()
177+
178+
_, _, err = ioctl(file.Fd(), unix.LOOP_CLR_FD, 0)
207179
return err
208180
}
209181

210182
// Attach a specified backing file to a loop device
211183
func AttachLoopDevice(backingFile string) (string, error) {
212-
dev, err := setupLoop(backingFile, LoopParams{})
213-
if err != nil {
214-
return "", err
215-
}
216-
dev.Close()
217-
218-
return dev.Name(), nil
184+
return setupLoop(backingFile, LoopParams{})
219185
}
220186

221187
// Detach a loop device
222188
func DetachLoopDevice(devices ...string) error {
223189
for _, dev := range devices {
224190
if err := removeLoop(dev); err != nil {
225-
return err
191+
return errors.Wrapf(err, "failed to remove loop device: %s", dev)
226192
}
227193
}
228194

mount/losetup_linux_test.go

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -53,11 +53,11 @@ func TestSetupLoop(t *testing.T) {
5353
}()
5454

5555
/* RO loop */
56-
f, err = setupLoop(backingFile, LoopParams{Readonly: true, Autoclear: true})
56+
path, err := setupLoop(backingFile, LoopParams{Readonly: true, Autoclear: true})
5757
if err != nil {
5858
t.Fatal(err)
5959
}
60-
ff, err := os.OpenFile(f.Name(), os.O_RDWR, 0)
60+
ff, err := os.OpenFile(path, os.O_RDWR, 0)
6161
if err != nil {
6262
t.Fatal(err)
6363
}
@@ -67,16 +67,13 @@ func TestSetupLoop(t *testing.T) {
6767
if err = ff.Close(); err != nil {
6868
t.Fatal(err)
6969
}
70-
if err = f.Close(); err != nil {
71-
t.Fatal(err)
72-
}
7370

7471
/* RW loop */
75-
f, err = setupLoop(backingFile, LoopParams{Autoclear: true})
72+
path, err = setupLoop(backingFile, LoopParams{Autoclear: true})
7673
if err != nil {
7774
t.Fatal(err)
7875
}
79-
ff, err = os.OpenFile(f.Name(), os.O_RDWR, 0)
76+
ff, err = os.OpenFile(path, os.O_RDWR, 0)
8077
if err != nil {
8178
t.Fatal(err)
8279
}
@@ -86,9 +83,6 @@ func TestSetupLoop(t *testing.T) {
8683
if err = ff.Close(); err != nil {
8784
t.Fatal(err)
8885
}
89-
if err = f.Close(); err != nil {
90-
t.Fatal(err)
91-
}
9286
}
9387

9488
func TestAttachDetachLoopDevice(t *testing.T) {

mount/mount_linux.go

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -85,9 +85,8 @@ func (m *Mount) Mount(target string) (err error) {
8585
if err != nil {
8686
return err
8787
}
88-
defer devFile.Close()
8988
// Mount the loop device instead
90-
source = devFile.Name()
89+
source = devFile
9190
}
9291
if err := mountAt(chdir, source, target, m.Type, uintptr(oflags), data); err != nil {
9392
return err

0 commit comments

Comments
 (0)