Skip to content

Commit 59e022d

Browse files
germaggermag
committed
Add a storage object
This is the first part of a bigger refactoring. I tried to make it as non-intrusive as possible, so it's not ideal. But it will simplify the review, since this should have the same behavior/mistakes than the previous version. The current architecture of having all the actions as methods of a VM object, and storing the config of the disk image, VM, and the running Vm in s single json, is very inflexible. In addition, the cache logic is all over the place, so let's start with a simple cache implementation, that will allow us to move to the final goal. Signed-off-by: German Maglione <[email protected]>
1 parent aec89a6 commit 59e022d

File tree

15 files changed

+906
-258
lines changed

15 files changed

+906
-258
lines changed

‎cmd/list.go‎

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

33
import (
4+
"fmt"
45
"os"
56

67
"github.com/containers/podman-bootc/pkg/config"
8+
"github.com/containers/podman-bootc/pkg/define"
79
"github.com/containers/podman-bootc/pkg/user"
8-
"github.com/containers/podman-bootc/pkg/utils"
910
"github.com/containers/podman-bootc/pkg/vm"
1011

1112
"github.com/containers/common/pkg/report"
@@ -60,46 +61,49 @@ func doList(_ *cobra.Command, _ []string) error {
6061
}
6162

6263
func CollectVmList(user user.User, libvirtUri string) (vmList []vm.BootcVMConfig, err error) {
63-
files, err := os.ReadDir(user.CacheDir())
64+
ids, err := user.Storage().List()
6465
if err != nil {
6566
return nil, err
6667
}
6768

68-
for _, f := range files {
69-
if f.IsDir() {
70-
cfg, err := getVMInfo(user, libvirtUri, f.Name())
71-
if err != nil {
72-
logrus.Warningf("skipping vm %s reason: %v", f.Name(), err)
73-
continue
74-
}
75-
76-
vmList = append(vmList, *cfg)
69+
for _, id := range ids {
70+
cfg, err := getVMInfo(user, libvirtUri, id)
71+
if err != nil {
72+
logrus.Warningf("skipping vm %s reason: %v", id, err)
73+
continue
7774
}
75+
76+
vmList = append(vmList, *cfg)
7877
}
7978
return vmList, nil
8079
}
8180

82-
func getVMInfo(user user.User, libvirtUri string, imageId string) (*vm.BootcVMConfig, error) {
81+
func getVMInfo(user user.User, libvirtUri string, imageId define.FullImageId) (*vm.BootcVMConfig, error) {
82+
guard, unlock, err := user.Storage().Get(imageId)
83+
if err != nil {
84+
return nil, fmt.Errorf("unable to lock the VM cache: %w", err)
85+
}
86+
defer func() {
87+
if err := unlock(); err != nil {
88+
logrus.Warningf("unable to unlock VM %s: %v", imageId, err)
89+
}
90+
}()
91+
8392
bootcVM, err := vm.NewVM(vm.NewVMParameters{
84-
ImageID: imageId,
93+
ImageID: string(imageId),
8594
User: user,
8695
LibvirtUri: libvirtUri,
87-
Locking: utils.Shared,
8896
})
8997

9098
if err != nil {
9199
return nil, err
92100
}
93101

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

102-
cfg, err := bootcVM.GetConfig()
106+
cfg, err := bootcVM.GetConfig(guard)
103107
if err != nil {
104108
return nil, err
105109
}

‎cmd/rm.go‎

Lines changed: 33 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,10 @@ package cmd
22

33
import (
44
"fmt"
5-
"os"
65

76
"github.com/containers/podman-bootc/pkg/config"
7+
"github.com/containers/podman-bootc/pkg/define"
88
"github.com/containers/podman-bootc/pkg/user"
9-
"github.com/containers/podman-bootc/pkg/utils"
109
"github.com/containers/podman-bootc/pkg/vm"
1110

1211
"github.com/sirupsen/logrus"
@@ -44,35 +43,49 @@ func oneOrAll() cobra.PositionalArgs {
4443
}
4544

4645
func doRemove(_ *cobra.Command, args []string) error {
46+
usr, err := user.NewUser()
47+
if err != nil {
48+
return err
49+
}
50+
4751
if removeAll {
48-
return pruneAll()
52+
return pruneAll(usr)
4953
}
5054

51-
return prune(args[0])
55+
id := args[0]
56+
fullImageId, err := usr.Storage().SearchByPrefix(id)
57+
if err != nil {
58+
return fmt.Errorf("searching for ID %s: %w", id, err)
59+
}
60+
if fullImageId == nil {
61+
return fmt.Errorf("local installation '%s' does not exists", id)
62+
}
63+
64+
return prune(usr, *fullImageId)
5265
}
5366

54-
func prune(id string) error {
55-
user, err := user.NewUser()
67+
func prune(usr user.User, id define.FullImageId) error {
68+
_, unlock, err := usr.Storage().GetExclusiveOrAdd(id)
5669
if err != nil {
57-
return err
70+
return fmt.Errorf("unable to lock the VM cache: %w", err)
5871
}
72+
defer func() {
73+
if err := unlock(); err != nil {
74+
logrus.Errorf("unable to unlock VM %s: %v", id, err)
75+
}
76+
}()
5977

6078
bootcVM, err := vm.NewVM(vm.NewVMParameters{
61-
ImageID: id,
79+
ImageID: string(id),
6280
LibvirtUri: config.LibvirtUri,
63-
User: user,
64-
Locking: utils.Exclusive,
81+
User: usr,
6582
})
6683
if err != nil {
6784
return fmt.Errorf("unable to get VM %s: %v", id, err)
6885
}
6986

70-
// Let's be explicit instead of relying on the defer exec order
7187
defer func() {
7288
bootcVM.CloseConnection()
73-
if err := bootcVM.Unlock(); err != nil {
74-
logrus.Warningf("unable to unlock VM %s: %v", id, err)
75-
}
7689
}()
7790

7891
if force {
@@ -90,24 +103,16 @@ func prune(id string) error {
90103
return nil
91104
}
92105

93-
func pruneAll() error {
94-
user, err := user.NewUser()
106+
func pruneAll(usr user.User) error {
107+
ids, err := usr.Storage().List()
95108
if err != nil {
96109
return err
97110
}
98111

99-
files, err := os.ReadDir(user.CacheDir())
100-
if err != nil {
101-
return err
102-
}
103-
104-
for _, f := range files {
105-
if f.IsDir() {
106-
vmID := f.Name()
107-
err := prune(vmID)
108-
if err != nil {
109-
logrus.Errorf("unable to remove %s: %v", vmID, err)
110-
}
112+
for _, id := range ids {
113+
err := prune(usr, id)
114+
if err != nil {
115+
logrus.Errorf("unable to remove %s: %v", id, err)
111116
}
112117
}
113118

‎cmd/run.go‎

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import (
1010

1111
"github.com/containers/podman-bootc/pkg/bootc"
1212
"github.com/containers/podman-bootc/pkg/config"
13+
"github.com/containers/podman-bootc/pkg/define"
1314
"github.com/containers/podman-bootc/pkg/user"
1415
"github.com/containers/podman-bootc/pkg/utils"
1516
"github.com/containers/podman-bootc/pkg/vm"
@@ -116,11 +117,20 @@ func doRun(flags *cobra.Command, args []string) error {
116117
return fmt.Errorf("unable to get free port for SSH: %w", err)
117118
}
118119

120+
guard, unlock, err := user.Storage().Get(define.FullImageId(bootcDisk.GetImageId()))
121+
if err != nil {
122+
return fmt.Errorf("unable to lock the VM cache: %w", err)
123+
}
124+
defer func() {
125+
if err := unlock(); err != nil {
126+
logrus.Warningf("unable to unlock VM %s: %v", bootcDisk.GetImageId(), err)
127+
}
128+
}()
129+
119130
bootcVM, err := vm.NewVM(vm.NewVMParameters{
120131
ImageID: bootcDisk.GetImageId(),
121132
User: user,
122133
LibvirtUri: config.LibvirtUri,
123-
Locking: utils.Shared,
124134
})
125135

126136
if err != nil {
@@ -130,9 +140,6 @@ func doRun(flags *cobra.Command, args []string) error {
130140
// Let's be explicit instead of relying on the defer exec order
131141
defer func() {
132142
bootcVM.CloseConnection()
133-
if err := bootcVM.Unlock(); err != nil {
134-
logrus.Warningf("unable to unlock VM %s: %v", bootcDisk.GetImageId(), err)
135-
}
136143
}()
137144

138145
cmd := args[1:]
@@ -153,6 +160,7 @@ func doRun(flags *cobra.Command, args []string) error {
153160
}
154161

155162
// write down the config file
163+
// FIXME: we are writing the config using a shared guard
156164
if err = bootcVM.WriteConfig(*bootcDisk); err != nil {
157165
return err
158166
}
@@ -191,7 +199,7 @@ func doRun(flags *cobra.Command, args []string) error {
191199
}
192200

193201
// ssh into the VM
194-
ExitCode, err = utils.WithExitCode(bootcVM.RunSSH(cmd))
202+
ExitCode, err = utils.WithExitCode(bootcVM.RunSSH(guard, cmd))
195203
if err != nil {
196204
return fmt.Errorf("ssh: %w", err)
197205
}

‎cmd/ssh.go‎

Lines changed: 23 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
package cmd
22

33
import (
4+
"fmt"
5+
46
"github.com/containers/podman-bootc/pkg/config"
57
"github.com/containers/podman-bootc/pkg/user"
68
"github.com/containers/podman-bootc/pkg/utils"
@@ -25,30 +27,41 @@ func init() {
2527
}
2628

2729
func doSsh(_ *cobra.Command, args []string) error {
28-
user, err := user.NewUser()
30+
usr, err := user.NewUser()
2931
if err != nil {
3032
return err
3133
}
3234

3335
id := args[0]
36+
fullImageId, err := usr.Storage().SearchByPrefix(id)
37+
if err != nil {
38+
return fmt.Errorf("searching for ID %s: %w", id, err)
39+
}
40+
if fullImageId == nil {
41+
return fmt.Errorf("local installation '%s' does not exists", id)
42+
}
43+
44+
guard, unlock, err := usr.Storage().Get(*fullImageId)
45+
if err != nil {
46+
return fmt.Errorf("unable to lock the VM cache: %w", err)
47+
}
48+
defer func() {
49+
if err := unlock(); err != nil {
50+
logrus.Warningf("unable to unlock VM %s: %v", id, err)
51+
}
52+
}()
3453

3554
vm, err := vm.NewVM(vm.NewVMParameters{
36-
ImageID: id,
37-
User: user,
55+
ImageID: string(*fullImageId),
56+
User: usr,
3857
LibvirtUri: config.LibvirtUri,
39-
Locking: utils.Shared,
4058
})
4159

4260
if err != nil {
4361
return err
4462
}
45-
46-
// Let's be explicit instead of relying on the defer exec order
4763
defer func() {
4864
vm.CloseConnection()
49-
if err := vm.Unlock(); err != nil {
50-
logrus.Warningf("unable to unlock VM %s: %v", id, err)
51-
}
5265
}()
5366

5467
err = vm.SetUser(sshUser)
@@ -61,6 +74,6 @@ func doSsh(_ *cobra.Command, args []string) error {
6174
cmd = args[1:]
6275
}
6376

64-
ExitCode, err = utils.WithExitCode(vm.RunSSH(cmd))
77+
ExitCode, err = utils.WithExitCode(vm.RunSSH(guard, cmd))
6578
return err
6679
}

‎cmd/stop.go‎

Lines changed: 23 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,10 @@
11
package cmd
22

33
import (
4+
"fmt"
5+
46
"github.com/containers/podman-bootc/pkg/config"
57
"github.com/containers/podman-bootc/pkg/user"
6-
"github.com/containers/podman-bootc/pkg/utils"
78
"github.com/containers/podman-bootc/pkg/vm"
89

910
"github.com/sirupsen/logrus"
@@ -23,28 +24,41 @@ func init() {
2324
}
2425

2526
func doStop(_ *cobra.Command, args []string) (err error) {
26-
user, err := user.NewUser()
27+
usr, err := user.NewUser()
2728
if err != nil {
2829
return err
2930
}
3031

3132
id := args[0]
33+
fullImageId, err := usr.Storage().SearchByPrefix(id)
34+
if err != nil {
35+
return fmt.Errorf("searching for ID %s: %w", id, err)
36+
}
37+
if fullImageId == nil {
38+
return fmt.Errorf("local installation '%s' does not exists", id)
39+
}
40+
41+
_, unlock, err := usr.Storage().GetExclusiveOrAdd(*fullImageId)
42+
if err != nil {
43+
return fmt.Errorf("unable to lock the VM cache: %w", err)
44+
}
45+
defer func() {
46+
if err := unlock(); err != nil {
47+
logrus.Errorf("unable to unlock VM %s: %v", id, err)
48+
}
49+
}()
50+
3251
bootcVM, err := vm.NewVM(vm.NewVMParameters{
33-
ImageID: id,
52+
ImageID: string(*fullImageId),
3453
LibvirtUri: config.LibvirtUri,
35-
User: user,
36-
Locking: utils.Exclusive,
54+
User: usr,
3755
})
3856
if err != nil {
3957
return err
4058
}
4159

42-
// Let's be explicit instead of relying on the defer exec order
4360
defer func() {
4461
bootcVM.CloseConnection()
45-
if err := bootcVM.Unlock(); err != nil {
46-
logrus.Warningf("unable to unlock VM %s: %v", id, err)
47-
}
4862
}()
4963

5064
return bootcVM.Delete()

0 commit comments

Comments
 (0)