Skip to content

Commit 0521272

Browse files
ckyrouacckyrouac
committed
locks: Refactor locking mechanism into cmd functions
Previously, due to the coupling of the container image pull to the bootc disk code, the cache directory couldn't be locked until the image id was obtained. Now that the image ID is retrieved first in the run function, the locks can be bound to each command. Signed-off-by: Chris Kyrouac <[email protected]>
1 parent 4ccf515 commit 0521272

File tree

13 files changed

+197
-163
lines changed

13 files changed

+197
-163
lines changed

‎cmd/list.go

Lines changed: 18 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,9 @@ package cmd
33
import (
44
"os"
55

6+
"gitlab.com/bootc-org/podman-bootc/pkg/cache"
67
"gitlab.com/bootc-org/podman-bootc/pkg/config"
78
"gitlab.com/bootc-org/podman-bootc/pkg/user"
8-
"gitlab.com/bootc-org/podman-bootc/pkg/utils"
99
"gitlab.com/bootc-org/podman-bootc/pkg/vm"
1010

1111
"github.com/containers/common/pkg/report"
@@ -79,24 +79,34 @@ func CollectVmList(user user.User, libvirtUri string) (vmList []vm.BootcVMConfig
7979
return vmList, nil
8080
}
8181

82-
func getVMInfo(user user.User, libvirtUri string, imageId string) (*vm.BootcVMConfig, error) {
82+
func getVMInfo(user user.User, libvirtUri string, fullImageId string) (*vm.BootcVMConfig, error) {
83+
cacheDir, err := cache.NewCache(fullImageId, user)
84+
if err != nil {
85+
return nil, err
86+
}
87+
err = cacheDir.Lock(cache.Shared)
88+
if err != nil {
89+
return nil, err
90+
}
91+
92+
defer func() {
93+
if err := cacheDir.Unlock(); err != nil {
94+
logrus.Warningf("unable to unlock VM %s: %v", fullImageId, err)
95+
}
96+
}()
97+
8398
bootcVM, err := vm.NewVM(vm.NewVMParameters{
84-
ImageID: imageId,
99+
ImageID: fullImageId,
85100
User: user,
86101
LibvirtUri: libvirtUri,
87-
Locking: utils.Shared,
88102
})
89103

90104
if err != nil {
91105
return nil, err
92106
}
93107

94-
// Let's be explicit instead of relying on the defer exec order
95108
defer func() {
96109
bootcVM.CloseConnection()
97-
if err := bootcVM.Unlock(); err != nil {
98-
logrus.Warningf("unable to unlock VM %s: %v", imageId, err)
99-
}
100110
}()
101111

102112
cfg, err := bootcVM.GetConfig()

‎cmd/rm.go

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import (
44
"fmt"
55
"os"
66

7+
"gitlab.com/bootc-org/podman-bootc/pkg/cache"
78
"gitlab.com/bootc-org/podman-bootc/pkg/config"
89
"gitlab.com/bootc-org/podman-bootc/pkg/user"
910
"gitlab.com/bootc-org/podman-bootc/pkg/utils"
@@ -44,6 +45,7 @@ func oneOrAll() cobra.PositionalArgs {
4445
}
4546

4647
func doRemove(_ *cobra.Command, args []string) error {
48+
4749
if removeAll {
4850
return pruneAll()
4951
}
@@ -57,20 +59,33 @@ func prune(id string) error {
5759
return err
5860
}
5961

62+
// take an exclusive lock on the cache directory
63+
fullImageId, err := utils.FullImageIdFromPartial(id, user)
64+
if err != nil {
65+
return err
66+
}
67+
cacheDir, err := cache.NewCache(fullImageId, user)
68+
if err != nil {
69+
return err
70+
}
71+
err = cacheDir.Lock(cache.Exclusive)
72+
if err != nil {
73+
return err
74+
}
75+
6076
bootcVM, err := vm.NewVM(vm.NewVMParameters{
6177
ImageID: id,
6278
LibvirtUri: config.LibvirtUri,
6379
User: user,
64-
Locking: utils.Exclusive,
6580
})
6681
if err != nil {
6782
return fmt.Errorf("unable to get VM %s: %v", id, err)
6883
}
6984

70-
// Let's be explicit instead of relying on the defer exec order
7185
defer func() {
86+
// Let's be explicit instead of relying on the defer exec order
7287
bootcVM.CloseConnection()
73-
if err := bootcVM.Unlock(); err != nil {
88+
if err := cacheDir.Unlock(); err != nil {
7489
logrus.Warningf("unable to unlock VM %s: %v", id, err)
7590
}
7691
}()

‎cmd/run.go

Lines changed: 47 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -109,9 +109,16 @@ func doRun(flags *cobra.Command, args []string) error {
109109
return fmt.Errorf("unable to pull container image: %w", err)
110110
}
111111

112-
// create the cache directory
113-
cache := cache.NewCache(containerImage.GetId(), user)
114-
err = cache.Create()
112+
// create the cacheDir directory
113+
cacheDir, err := cache.NewCache(containerImage.GetId(), user)
114+
if err != nil {
115+
return fmt.Errorf("unable to create cache: %w", err)
116+
}
117+
err = cacheDir.Lock(cache.Exclusive)
118+
if err != nil {
119+
return err
120+
}
121+
err = cacheDir.Create()
115122
if err != nil {
116123
return fmt.Errorf("unable to create cache: %w", err)
117124
}
@@ -121,13 +128,20 @@ func doRun(flags *cobra.Command, args []string) error {
121128
ImageID: containerImage.GetId(),
122129
User: user,
123130
LibvirtUri: config.LibvirtUri,
124-
Locking: utils.Shared,
125131
})
126132

127133
if err != nil {
128134
return fmt.Errorf("unable to initialize VM: %w", err)
129135
}
130136

137+
defer func() {
138+
// Let's be explicit instead of relying on the defer exec order
139+
bootcVM.CloseConnection()
140+
if err := cacheDir.Unlock(); err != nil {
141+
logrus.Warningf("unable to unlock cache %s: %v", cacheDir.ImageId, err)
142+
}
143+
}()
144+
131145
isRunning, err := bootcVM.IsRunning()
132146
if err != nil {
133147
return fmt.Errorf("unable to check if VM is running: %w", err)
@@ -145,7 +159,7 @@ func doRun(flags *cobra.Command, args []string) error {
145159
}
146160

147161
// create the disk image
148-
bootcDisk := bootc.NewBootcDisk(containerImage, ctx, user, cache, bustCache)
162+
bootcDisk := bootc.NewBootcDisk(containerImage, ctx, user, cacheDir, bustCache)
149163
err = bootcDisk.Install(vmConfig.Quiet, diskImageConfigInstance)
150164

151165
if err != nil {
@@ -159,14 +173,6 @@ func doRun(flags *cobra.Command, args []string) error {
159173
return fmt.Errorf("unable to get free port for SSH: %w", err)
160174
}
161175

162-
// Let's be explicit instead of relying on the defer exec order
163-
defer func() {
164-
bootcVM.CloseConnection()
165-
if err := bootcVM.Unlock(); err != nil {
166-
logrus.Warningf("unable to unlock VM %s: %v", containerImage.GetId(), err)
167-
}
168-
}()
169-
170176
cmd := args[1:]
171177
err = bootcVM.Run(vm.RunVMParameters{
172178
Cmd: cmd,
@@ -189,6 +195,20 @@ func doRun(flags *cobra.Command, args []string) error {
189195
return err
190196
}
191197

198+
// done modifying the cache, so remove the Exclusive lock
199+
err = cacheDir.Unlock()
200+
if err != nil {
201+
return fmt.Errorf("unable to unlock cache: %w", err)
202+
}
203+
204+
// take a RO lock for the SSH connection
205+
// it will be unlocked at the end of this function
206+
// by the previous defer()
207+
err = cacheDir.Lock(cache.Shared)
208+
if err != nil {
209+
return err
210+
}
211+
192212
if !vmConfig.Background {
193213
if !vmConfig.Quiet {
194214
var vmConsoleWg sync.WaitGroup
@@ -230,6 +250,20 @@ func doRun(flags *cobra.Command, args []string) error {
230250

231251
// Always remove when executing a command
232252
if vmConfig.RemoveVm || len(cmd) > 0 {
253+
// remove the RO lock
254+
err = cacheDir.Unlock()
255+
if err != nil {
256+
return err
257+
}
258+
259+
// take an exclusive lock to remove the VM
260+
// it will be unlocked at the end of this function
261+
// by the previous defer()
262+
err = cacheDir.Lock(cache.Exclusive)
263+
if err != nil {
264+
return err
265+
}
266+
233267
err = bootcVM.Delete() //delete the VM, but keep the disk image
234268
if err != nil {
235269
return fmt.Errorf("unable to remove VM from cache: %w", err)

‎cmd/ssh.go

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package cmd
22

33
import (
4+
"gitlab.com/bootc-org/podman-bootc/pkg/cache"
45
"gitlab.com/bootc-org/podman-bootc/pkg/config"
56
"gitlab.com/bootc-org/podman-bootc/pkg/user"
67
"gitlab.com/bootc-org/podman-bootc/pkg/utils"
@@ -32,21 +33,34 @@ func doSsh(_ *cobra.Command, args []string) error {
3233

3334
id := args[0]
3435

36+
//take a read only lock on the cache directory
37+
fullImageId, err := utils.FullImageIdFromPartial(id, user)
38+
if err != nil {
39+
return err
40+
}
41+
cacheDir, err := cache.NewCache(fullImageId, user)
42+
if err != nil {
43+
return err
44+
}
45+
err = cacheDir.Lock(cache.Shared)
46+
if err != nil {
47+
return err
48+
}
49+
3550
vm, err := vm.NewVM(vm.NewVMParameters{
3651
ImageID: id,
3752
User: user,
3853
LibvirtUri: config.LibvirtUri,
39-
Locking: utils.Shared,
4054
})
4155

4256
if err != nil {
4357
return err
4458
}
4559

46-
// Let's be explicit instead of relying on the defer exec order
4760
defer func() {
61+
// Let's be explicit instead of relying on the defer exec order
4862
vm.CloseConnection()
49-
if err := vm.Unlock(); err != nil {
63+
if err := cacheDir.Unlock(); err != nil {
5064
logrus.Warningf("unable to unlock VM %s: %v", id, err)
5165
}
5266
}()

‎cmd/stop.go

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package cmd
22

33
import (
4+
"gitlab.com/bootc-org/podman-bootc/pkg/cache"
45
"gitlab.com/bootc-org/podman-bootc/pkg/config"
56
"gitlab.com/bootc-org/podman-bootc/pkg/user"
67
"gitlab.com/bootc-org/podman-bootc/pkg/utils"
@@ -23,26 +24,40 @@ func init() {
2324
}
2425

2526
func doStop(_ *cobra.Command, args []string) (err error) {
27+
id := args[0]
28+
2629
user, err := user.NewUser()
2730
if err != nil {
2831
return err
2932
}
3033

31-
id := args[0]
34+
//take an exclusive lock on the cache directory
35+
fullImageId, err := utils.FullImageIdFromPartial(id, user)
36+
if err != nil {
37+
return err
38+
}
39+
cacheDir, err := cache.NewCache(fullImageId, user)
40+
if err != nil {
41+
return err
42+
}
43+
err = cacheDir.Lock(cache.Exclusive)
44+
if err != nil {
45+
return err
46+
}
47+
3248
bootcVM, err := vm.NewVM(vm.NewVMParameters{
3349
ImageID: id,
3450
LibvirtUri: config.LibvirtUri,
3551
User: user,
36-
Locking: utils.Exclusive,
3752
})
3853
if err != nil {
3954
return err
4055
}
4156

42-
// Let's be explicit instead of relying on the defer exec order
4357
defer func() {
58+
// Let's be explicit instead of relying on the defer exec order
4459
bootcVM.CloseConnection()
45-
if err := bootcVM.Unlock(); err != nil {
60+
if err := cacheDir.Unlock(); err != nil {
4661
logrus.Warningf("unable to unlock VM %s: %v", id, err)
4762
}
4863
}()

0 commit comments

Comments
 (0)