Skip to content

Commit f235d47

Browse files
openshift-merge-bot[bot]openshift-merge-bot[bot]
authored
Merge pull request #26308 from kolyshkin/blkdev
podman-update: fix block device handling
2 parents 547de92 + 5838bfe commit f235d47

File tree

3 files changed

+110
-76
lines changed

3 files changed

+110
-76
lines changed

‎docs/source/markdown/podman-update.1.md.in‎

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -95,21 +95,21 @@ Changing this setting resets the timer, depending on the state of the container.
9595
@@option unsetenv.update
9696

9797

98-
## EXAMPLEs
98+
## EXAMPLES
9999

100-
Update a container with a new cpu quota and period.
100+
Update a container with a new cpu quota and period:
101101
```
102-
podman update --cpus=5 myCtr
102+
podman update --cpus=0.5 ctrID
103103
```
104104

105-
Update a container with all available options for cgroups v2.
105+
Update a container with multiple options at ones:
106106
```
107-
podman update --cpus 5 --cpuset-cpus 0 --cpu-shares 123 --cpuset-mems 0 --memory 1G --memory-swap 2G --memory-reservation 2G --blkio-weight-device /dev/zero:123 --blkio-weight 123 --device-read-bps /dev/zero:10mb --device-write-bps /dev/zero:10mb --device-read-iops /dev/zero:1000 --device-write-iops /dev/zero:1000 --pids-limit 123 ctrID
108-
```
109-
110-
Update a container with all available options for cgroups v1.
111-
```
112-
podman update --cpus 5 --cpuset-cpus 0 --cpu-shares 123 --cpuset-mems 0 --memory 1G --memory-swap 2G --memory-reservation 2G --memory-swappiness 50 --pids-limit 123 ctrID
107+
podman update --cpus 5 --cpuset-cpus 0 --cpu-shares 123 --cpuset-mems 0 \\
108+
--memory 1G --memory-swap 2G --memory-reservation 2G \\
109+
--blkio-weight-device /dev/sda:123 --blkio-weight 123 \\
110+
--device-read-bps /dev/sda:10mb --device-write-bps /dev/sda:10mb \\
111+
--device-read-iops /dev/sda:1000 --device-write-iops /dev/sda:1000 \\
112+
--pids-limit 123 ctrID
113113
```
114114

115115
## SEE ALSO

‎pkg/specgen/utils_linux.go‎

Lines changed: 77 additions & 66 deletions
Original file line numberDiff line numberDiff line change
@@ -9,94 +9,105 @@ import (
99
"golang.org/x/sys/unix"
1010
)
1111

12+
// statBlkDev returns path's major and minor, or an error, if path is not a block device.
13+
func statBlkDev(path string) (int64, int64, error) {
14+
var stat unix.Stat_t
15+
16+
if err := unix.Stat(path, &stat); err != nil {
17+
return 0, 0, fmt.Errorf("could not parse device %s: %w", path, err)
18+
}
19+
if stat.Mode&unix.S_IFMT != unix.S_IFBLK {
20+
return 0, 0, fmt.Errorf("%s: not a block device", path)
21+
}
22+
rdev := uint64(stat.Rdev) //nolint:unconvert // Some architectures have different type.
23+
return int64(unix.Major(rdev)), int64(unix.Minor(rdev)), nil
24+
}
25+
26+
// fillThrottleDev fills in dev.Major and dev.Minor fields based on path to a block device.
27+
func fillThrottleDev(path string, dev *spec.LinuxThrottleDevice) error {
28+
major, minor, err := statBlkDev(path)
29+
if err != nil {
30+
return err
31+
}
32+
33+
dev.Major, dev.Minor = major, minor
34+
35+
return nil
36+
}
37+
1238
// FinishThrottleDevices takes the temporary representation of the throttle
13-
// devices in the specgen and looks up the major and major minors. it then
14-
// sets the throttle devices proper in the specgen
39+
// devices in the specgen, fills in major and minor numbers, and amends the
40+
// specgen with the specified devices. It returns an error if any device
41+
// specified doesn't exist or is not a block device.
1542
func FinishThrottleDevices(s *SpecGenerator) error {
43+
if len(s.ThrottleReadBpsDevice)+len(s.ThrottleWriteBpsDevice)+len(s.ThrottleReadIOPSDevice)+len(s.ThrottleWriteIOPSDevice) == 0 {
44+
return nil
45+
}
46+
1647
if s.ResourceLimits == nil {
1748
s.ResourceLimits = &spec.LinuxResources{}
1849
}
19-
if bps := s.ThrottleReadBpsDevice; len(bps) > 0 {
20-
if s.ResourceLimits.BlockIO == nil {
21-
s.ResourceLimits.BlockIO = &spec.LinuxBlockIO{}
22-
}
23-
for k, v := range bps {
24-
statT := unix.Stat_t{}
25-
if err := unix.Stat(k, &statT); err != nil {
26-
return fmt.Errorf("could not parse throttle device at %s: %w", k, err)
27-
}
28-
v.Major = (int64(unix.Major(uint64(statT.Rdev)))) //nolint: unconvert
29-
v.Minor = (int64(unix.Minor(uint64(statT.Rdev)))) //nolint: unconvert
30-
if s.ResourceLimits.BlockIO == nil {
31-
s.ResourceLimits.BlockIO = new(spec.LinuxBlockIO)
32-
}
33-
s.ResourceLimits.BlockIO.ThrottleReadBpsDevice = append(s.ResourceLimits.BlockIO.ThrottleReadBpsDevice, v)
34-
}
50+
if s.ResourceLimits.BlockIO == nil {
51+
s.ResourceLimits.BlockIO = &spec.LinuxBlockIO{}
3552
}
36-
if bps := s.ThrottleWriteBpsDevice; len(bps) > 0 {
37-
if s.ResourceLimits.BlockIO == nil {
38-
s.ResourceLimits.BlockIO = &spec.LinuxBlockIO{}
39-
}
40-
for k, v := range bps {
41-
statT := unix.Stat_t{}
42-
if err := unix.Stat(k, &statT); err != nil {
43-
return fmt.Errorf("could not parse throttle device at %s: %w", k, err)
44-
}
45-
v.Major = (int64(unix.Major(uint64(statT.Rdev)))) //nolint: unconvert
46-
v.Minor = (int64(unix.Minor(uint64(statT.Rdev)))) //nolint: unconvert
47-
s.ResourceLimits.BlockIO.ThrottleWriteBpsDevice = append(s.ResourceLimits.BlockIO.ThrottleWriteBpsDevice, v)
53+
54+
for k, v := range s.ThrottleReadBpsDevice {
55+
if err := fillThrottleDev(k, &v); err != nil {
56+
return err
4857
}
58+
s.ResourceLimits.BlockIO.ThrottleReadBpsDevice = append(s.ResourceLimits.BlockIO.ThrottleReadBpsDevice, v)
4959
}
50-
if iops := s.ThrottleReadIOPSDevice; len(iops) > 0 {
51-
if s.ResourceLimits.BlockIO == nil {
52-
s.ResourceLimits.BlockIO = &spec.LinuxBlockIO{}
53-
}
54-
for k, v := range iops {
55-
statT := unix.Stat_t{}
56-
if err := unix.Stat(k, &statT); err != nil {
57-
return fmt.Errorf("could not parse throttle device at %s: %w", k, err)
58-
}
59-
v.Major = (int64(unix.Major(uint64(statT.Rdev)))) //nolint: unconvert
60-
v.Minor = (int64(unix.Minor(uint64(statT.Rdev)))) //nolint: unconvert
61-
s.ResourceLimits.BlockIO.ThrottleReadIOPSDevice = append(s.ResourceLimits.BlockIO.ThrottleReadIOPSDevice, v)
60+
61+
for k, v := range s.ThrottleWriteBpsDevice {
62+
if err := fillThrottleDev(k, &v); err != nil {
63+
return err
6264
}
65+
s.ResourceLimits.BlockIO.ThrottleWriteBpsDevice = append(s.ResourceLimits.BlockIO.ThrottleWriteBpsDevice, v)
6366
}
64-
if iops := s.ThrottleWriteIOPSDevice; len(iops) > 0 {
65-
if s.ResourceLimits.BlockIO == nil {
66-
s.ResourceLimits.BlockIO = &spec.LinuxBlockIO{}
67+
68+
for k, v := range s.ThrottleReadIOPSDevice {
69+
if err := fillThrottleDev(k, &v); err != nil {
70+
return err
6771
}
68-
for k, v := range iops {
69-
statT := unix.Stat_t{}
70-
if err := unix.Stat(k, &statT); err != nil {
71-
return fmt.Errorf("could not parse throttle device at %s: %w", k, err)
72-
}
73-
v.Major = (int64(unix.Major(uint64(statT.Rdev)))) //nolint: unconvert
74-
v.Minor = (int64(unix.Minor(uint64(statT.Rdev)))) //nolint: unconvert
75-
s.ResourceLimits.BlockIO.ThrottleWriteIOPSDevice = append(s.ResourceLimits.BlockIO.ThrottleWriteIOPSDevice, v)
72+
s.ResourceLimits.BlockIO.ThrottleReadIOPSDevice = append(s.ResourceLimits.BlockIO.ThrottleReadIOPSDevice, v)
73+
}
74+
75+
for k, v := range s.ThrottleWriteIOPSDevice {
76+
if err := fillThrottleDev(k, &v); err != nil {
77+
return err
7678
}
79+
s.ResourceLimits.BlockIO.ThrottleWriteIOPSDevice = append(s.ResourceLimits.BlockIO.ThrottleWriteIOPSDevice, v)
7780
}
81+
7882
return nil
7983
}
8084

8185
func WeightDevices(specgen *SpecGenerator) error {
82-
devs := []spec.LinuxWeightDevice{}
86+
if len(specgen.WeightDevice) == 0 {
87+
return nil
88+
}
89+
8390
if specgen.ResourceLimits == nil {
8491
specgen.ResourceLimits = &spec.LinuxResources{}
8592
}
93+
if specgen.ResourceLimits.BlockIO == nil {
94+
specgen.ResourceLimits.BlockIO = &spec.LinuxBlockIO{}
95+
}
96+
8697
for k, v := range specgen.WeightDevice {
87-
statT := unix.Stat_t{}
88-
if err := unix.Stat(k, &statT); err != nil {
89-
return fmt.Errorf("failed to inspect '%s' in --blkio-weight-device: %w", k, err)
90-
}
91-
dev := new(spec.LinuxWeightDevice)
92-
dev.Major = (int64(unix.Major(uint64(statT.Rdev)))) //nolint: unconvert
93-
dev.Minor = (int64(unix.Minor(uint64(statT.Rdev)))) //nolint: unconvert
94-
dev.Weight = v.Weight
95-
devs = append(devs, *dev)
96-
if specgen.ResourceLimits.BlockIO == nil {
97-
specgen.ResourceLimits.BlockIO = &spec.LinuxBlockIO{}
98+
major, minor, err := statBlkDev(k)
99+
if err != nil {
100+
return fmt.Errorf("bad --blkio-weight-device: %w", err)
98101
}
99-
specgen.ResourceLimits.BlockIO.WeightDevice = devs
102+
specgen.ResourceLimits.BlockIO.WeightDevice = append(specgen.ResourceLimits.BlockIO.WeightDevice,
103+
spec.LinuxWeightDevice{
104+
LinuxBlockIODevice: spec.LinuxBlockIODevice{
105+
Major: major,
106+
Minor: minor,
107+
},
108+
Weight: v.Weight,
109+
})
100110
}
111+
101112
return nil
102113
}

‎test/system/280-update.bats‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -326,4 +326,27 @@ function nrand() {
326326

327327
run_podman rm -t 0 -f $ctrname
328328
}
329+
330+
# bats test_tags=ci:parallel
331+
@test "podman update - non-block device rejected by --*device* options" {
332+
local dev=/dev/zero # Not a block device.
333+
local block_opts=(
334+
"--blkio-weight-device=$dev:123"
335+
"--device-read-bps=$dev:10mb"
336+
"--device-write-bps=$dev:10mb"
337+
"--device-read-iops=$dev:1000"
338+
"--device-write-iops=$dev:1000"
339+
)
340+
run_podman run -d "$IMAGE" /home/podman/pause
341+
cid="$output"
342+
343+
defer-assertion-failures
344+
for opt in "${block_opts[@]}"; do
345+
run_podman 125 update "$cid" "$opt"
346+
assert "$output" =~ "$dev: not a block device"
347+
done
348+
immediate-assertion-failures
349+
350+
run_podman rm -t 0 -f "$cid"
351+
}
329352
# vim: filetype=sh

0 commit comments

Comments
 (0)