Skip to content

Commit c0f8af3

Browse files
Hazanelmnecas
Hazanel
authored and
mnecas
committed
MTV-2019
Warn if CBT not enabled for individual volumes Ref: https://issues.redhat.com/browse/MTV-2019 Signed-off-by: ehazan <[email protected]>
1 parent 5d9dc30 commit c0f8af3

File tree

5 files changed

+401
-39
lines changed

5 files changed

+401
-39
lines changed

‎pkg/controller/provider/container/vsphere/cbt_disks_test.go‎

Lines changed: 112 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,112 @@
1+
package vsphere
2+
3+
import (
4+
modelVsphere "github.com/konveyor/forklift-controller/pkg/controller/provider/model/vsphere"
5+
. "github.com/onsi/ginkgo"
6+
. "github.com/onsi/gomega"
7+
)
8+
9+
var _ = Describe("isCBTEnabledForDisks", func() {
10+
var disks []modelVsphere.Disk
11+
var ctkMap map[string]bool
12+
13+
BeforeEach(func() {
14+
disks = []modelVsphere.Disk{}
15+
ctkMap = map[string]bool{}
16+
})
17+
18+
Context("All disks with CBT enabled across types", func() {
19+
BeforeEach(func() {
20+
disks = []modelVsphere.Disk{
21+
{ControllerKey: 16000, UnitNumber: 0, Bus: "scsi"}, // scsi0:0
22+
{ControllerKey: 17000, UnitNumber: 1, Bus: "sata"}, // sata0:1
23+
{ControllerKey: 18000, UnitNumber: 2, Bus: "nvme"}, // nvme0:2
24+
{ControllerKey: 19000, UnitNumber: 0, Bus: "ide"}, // ide0:0
25+
}
26+
ctkMap = map[string]bool{
27+
"scsi0:0": true,
28+
"sata0:1": true,
29+
"nvme0:2": true,
30+
"ide0:0": true,
31+
}
32+
})
33+
34+
It("should enable CBT for all disks", func() {
35+
isCBTEnabledForDisks(ctkMap, disks)
36+
for _, d := range disks {
37+
Expect(d.ChangeTrackingEnabled).To(BeTrue())
38+
}
39+
})
40+
})
41+
42+
Context("Mixed CBT state and missing entries", func() {
43+
BeforeEach(func() {
44+
disks = []modelVsphere.Disk{
45+
{ControllerKey: 16000, UnitNumber: 0, Bus: "scsi"}, // scsi0:0 → true
46+
{ControllerKey: 17000, UnitNumber: 1, Bus: "sata"}, // sata0:1 → false
47+
{ControllerKey: 18001, UnitNumber: 0, Bus: "nvme"}, // nvme1:0 → true
48+
{ControllerKey: 19000, UnitNumber: 1, Bus: "ide"}, // ide0:1 → not in map
49+
}
50+
ctkMap = map[string]bool{
51+
"scsi0:0": true,
52+
"sata0:1": false,
53+
"nvme1:0": true,
54+
// ide0:1 missing
55+
}
56+
})
57+
58+
It("should correctly reflect CBT state per device key", func() {
59+
isCBTEnabledForDisks(ctkMap, disks)
60+
Expect(disks[0].ChangeTrackingEnabled).To(BeTrue()) // scsi0:0
61+
Expect(disks[1].ChangeTrackingEnabled).To(BeFalse()) // sata0:1
62+
Expect(disks[2].ChangeTrackingEnabled).To(BeTrue()) // nvme1:0
63+
Expect(disks[3].ChangeTrackingEnabled).To(BeFalse()) // ide0:1 default false
64+
})
65+
})
66+
67+
Context("No entries in the CBT map", func() {
68+
BeforeEach(func() {
69+
disks = []modelVsphere.Disk{
70+
{ControllerKey: 16000, UnitNumber: 1, Bus: "scsi"},
71+
{ControllerKey: 17000, UnitNumber: 2, Bus: "sata"},
72+
}
73+
})
74+
75+
It("should default all CBT flags to false", func() {
76+
isCBTEnabledForDisks(ctkMap, disks)
77+
for _, d := range disks {
78+
Expect(d.ChangeTrackingEnabled).To(BeFalse())
79+
}
80+
})
81+
})
82+
83+
Context("CBT enabled for some and missing others", func() {
84+
BeforeEach(func() {
85+
disks = []modelVsphere.Disk{
86+
{ControllerKey: 16000, UnitNumber: 0, Bus: "scsi"}, // scsi0:0
87+
{ControllerKey: 17000, UnitNumber: 1, Bus: "sata"}, // sata0:1
88+
{ControllerKey: 18000, UnitNumber: 2, Bus: "nvme"}, // nvme0:2 (missing)
89+
}
90+
ctkMap = map[string]bool{
91+
"scsi0:0": true,
92+
"sata0:1": false,
93+
}
94+
})
95+
96+
It("should match enabled state and default missing to false", func() {
97+
isCBTEnabledForDisks(ctkMap, disks)
98+
Expect(disks[0].ChangeTrackingEnabled).To(BeTrue()) // scsi0:0
99+
Expect(disks[1].ChangeTrackingEnabled).To(BeFalse()) // sata0:1
100+
Expect(disks[2].ChangeTrackingEnabled).To(BeFalse()) // nvme0:2
101+
})
102+
})
103+
104+
Context("Empty disks slice", func() {
105+
It("should not panic or modify anything", func() {
106+
Expect(func() {
107+
isCBTEnabledForDisks(ctkMap, disks)
108+
}).ToNot(Panic())
109+
Expect(disks).To(BeEmpty())
110+
})
111+
})
112+
})

‎pkg/controller/provider/container/vsphere/model.go‎

Lines changed: 86 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -551,6 +551,9 @@ func (v *VmAdapter) Model() model.Model {
551551

552552
// Apply the update to the model.
553553
func (v *VmAdapter) Apply(u types.ObjectUpdate) {
554+
// ctkPerDisk map - CBT enabled disks, we need this here to update the model.Disks
555+
// which on initial state is ready only after the ctkPerDisk update
556+
ctkPerDisk := map[string]bool{}
554557
v.Base.Apply(&v.model.Base, u)
555558
for _, p := range u.ChangeSet {
556559
switch p.Op {
@@ -667,29 +670,44 @@ func (v *VmAdapter) Apply(u types.ObjectUpdate) {
667670
if options, cast := p.Val.(types.ArrayOfOptionValue); cast {
668671
for _, val := range options.OptionValue {
669672
opt := val.GetOptionValue()
670-
switch opt.Key {
671-
case "numa.nodeAffinity":
673+
674+
if opt.Key == "numa.nodeAffinity" {
672675
if s, cast := opt.Value.(string); cast {
673676
v.model.NumaNodeAffinity = strings.Split(s, ",")
674677
}
675-
case "ctkEnabled":
678+
} else if opt.Key == "ctkEnabled" {
676679
if s, cast := opt.Value.(string); cast {
677680
boolVal, err := strconv.ParseBool(s)
678681
if err != nil {
679682
return
680683
}
681684
v.model.ChangeTrackingEnabled = boolVal
682685
}
683-
case "disk.EnableUUID":
686+
} else if opt.Key == "disk.EnableUUID" {
684687
if s, cast := opt.Value.(string); cast {
685688
boolVal, err := strconv.ParseBool(s)
686689
if err != nil {
687690
return
688691
}
689692
v.model.DiskEnableUuid = boolVal
690693
}
694+
} else if hasDiskPrefix(opt.Key) && strings.HasSuffix(opt.Key, ".ctkEnabled") {
695+
696+
if s, cast := opt.Value.(string); cast {
697+
boolVal, err := strconv.ParseBool(s)
698+
if err != nil {
699+
return
700+
}
701+
if boolVal {
702+
ctkPerDisk[strings.Split(opt.Key, ".")[0]] = true
703+
}
704+
}
691705
}
706+
}
692707

708+
//In case of ExtraConfig update, on initial state model.Disks is not ready yet
709+
if len(v.model.Disks) > 0 {
710+
isCBTEnabledForDisks(ctkPerDisk, v.model.Disks)
693711
}
694712
}
695713
case fNestedHVEnabled:
@@ -814,12 +832,42 @@ func (v *VmAdapter) Apply(u types.ObjectUpdate) {
814832
v.model.NICs = nicList
815833
v.updateControllers(&devArray)
816834
v.updateDisks(&devArray)
835+
836+
if len(ctkPerDisk) > 0 {
837+
isCBTEnabledForDisks(ctkPerDisk, v.model.Disks)
838+
}
817839
}
818840
}
819841
}
820842
}
821843
}
822844

845+
func hasDiskPrefix(key string) bool {
846+
return strings.HasPrefix(key, SCSI) ||
847+
strings.HasPrefix(key, SATA) ||
848+
strings.HasPrefix(key, IDE) ||
849+
strings.HasPrefix(key, NVME)
850+
}
851+
852+
func isCBTEnabledForDisks(ctkPerDisk map[string]bool, disks []model.Disk) {
853+
for i := range disks {
854+
disk := &disks[i]
855+
856+
// In vSphere, ControllerKey values are typically large integers that encode the controller bus number.
857+
// To extract the actual controller index (e.g., scsi0, scsi1), we round down to the nearest 100 to get the base,
858+
// then subtract it from the ControllerKey. For example, 16001 → controllerIndex 1 (16001 - 16000).
859+
baseKey := (disk.ControllerKey / 100) * 100
860+
controllerIndex := disk.ControllerKey - baseKey
861+
deviceKey := fmt.Sprintf("%s%d:%d", disk.Bus, controllerIndex, disk.UnitNumber)
862+
863+
if ctkPerDisk[deviceKey] {
864+
disk.ChangeTrackingEnabled = true
865+
} else {
866+
disk.ChangeTrackingEnabled = false
867+
}
868+
}
869+
}
870+
823871
// Update virtual disk devices.
824872
func (v *VmAdapter) updateControllers(devArray *types.ArrayOfVirtualDevice) {
825873
controllers := []model.Controller{}
@@ -900,11 +948,13 @@ func (v *VmAdapter) updateDisks(devArray *types.ArrayOfVirtualDevice) {
900948
switch backing := disk.Backing.(type) {
901949
case *types.VirtualDiskFlatVer1BackingInfo:
902950
md := model.Disk{
903-
Key: disk.Key,
904-
File: backing.FileName,
905-
Capacity: disk.CapacityInBytes,
906-
Mode: backing.DiskMode,
907-
Bus: controller.Bus,
951+
Key: disk.Key,
952+
UnitNumber: *disk.UnitNumber,
953+
ControllerKey: disk.ControllerKey,
954+
File: backing.FileName,
955+
Capacity: disk.CapacityInBytes,
956+
Mode: backing.DiskMode,
957+
Bus: controller.Bus,
908958
}
909959
if backing.Datastore != nil {
910960
datastoreId, _ := sanitize(backing.Datastore.Value)
@@ -916,13 +966,15 @@ func (v *VmAdapter) updateDisks(devArray *types.ArrayOfVirtualDevice) {
916966
disks = append(disks, md)
917967
case *types.VirtualDiskFlatVer2BackingInfo:
918968
md := model.Disk{
919-
Key: disk.Key,
920-
File: backing.FileName,
921-
Capacity: disk.CapacityInBytes,
922-
Shared: backing.Sharing != "sharingNone" && backing.Sharing != "",
923-
Mode: backing.DiskMode,
924-
Bus: controller.Bus,
925-
Serial: backing.Uuid,
969+
Key: disk.Key,
970+
UnitNumber: *disk.UnitNumber,
971+
ControllerKey: disk.ControllerKey,
972+
File: backing.FileName,
973+
Capacity: disk.CapacityInBytes,
974+
Shared: backing.Sharing != "sharingNone" && backing.Sharing != "",
975+
Mode: backing.DiskMode,
976+
Bus: controller.Bus,
977+
Serial: backing.Uuid,
926978
}
927979
if backing.Datastore != nil {
928980
datastoreId, _ := sanitize(backing.Datastore.Value)
@@ -934,14 +986,16 @@ func (v *VmAdapter) updateDisks(devArray *types.ArrayOfVirtualDevice) {
934986
disks = append(disks, md)
935987
case *types.VirtualDiskRawDiskMappingVer1BackingInfo:
936988
md := model.Disk{
937-
Key: disk.Key,
938-
File: backing.FileName,
939-
Capacity: disk.CapacityInBytes,
940-
Shared: backing.Sharing != "sharingNone" && backing.Sharing != "",
941-
Mode: backing.DiskMode,
942-
RDM: true,
943-
Bus: controller.Bus,
944-
Serial: backing.Uuid,
989+
Key: disk.Key,
990+
UnitNumber: *disk.UnitNumber,
991+
ControllerKey: disk.ControllerKey,
992+
File: backing.FileName,
993+
Capacity: disk.CapacityInBytes,
994+
Shared: backing.Sharing != "sharingNone" && backing.Sharing != "",
995+
Mode: backing.DiskMode,
996+
RDM: true,
997+
Bus: controller.Bus,
998+
Serial: backing.Uuid,
945999
}
9461000
if backing.Datastore != nil {
9471001
datastoreId, _ := sanitize(backing.Datastore.Value)
@@ -953,12 +1007,14 @@ func (v *VmAdapter) updateDisks(devArray *types.ArrayOfVirtualDevice) {
9531007
disks = append(disks, md)
9541008
case *types.VirtualDiskRawDiskVer2BackingInfo:
9551009
md := model.Disk{
956-
Key: disk.Key,
957-
File: backing.DescriptorFileName,
958-
Capacity: disk.CapacityInBytes,
959-
Shared: backing.Sharing != "sharingNone" && backing.Sharing != "",
960-
RDM: true,
961-
Bus: controller.Bus,
1010+
Key: disk.Key,
1011+
UnitNumber: *disk.UnitNumber,
1012+
ControllerKey: disk.ControllerKey,
1013+
File: backing.DescriptorFileName,
1014+
Capacity: disk.CapacityInBytes,
1015+
Shared: backing.Sharing != "sharingNone" && backing.Sharing != "",
1016+
RDM: true,
1017+
Bus: controller.Bus,
9621018
}
9631019
disks = append(disks, md)
9641020
}

‎pkg/controller/provider/model/vsphere/model.go‎

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -301,15 +301,18 @@ type Controller struct {
301301

302302
// Virtual Disk.
303303
type Disk struct {
304-
Key int32 `json:"key"`
305-
File string `json:"file"`
306-
Datastore Ref `json:"datastore"`
307-
Capacity int64 `json:"capacity"`
308-
Shared bool `json:"shared"`
309-
RDM bool `json:"rdm"`
310-
Bus string `json:"bus"`
311-
Mode string `json:"mode,omitempty"`
312-
Serial string `json:"serial,omitempty"`
304+
Key int32 `json:"key"`
305+
UnitNumber int32 `json:"unitNumber"`
306+
ControllerKey int32 `json:"controllerKey"`
307+
File string `json:"file"`
308+
Datastore Ref `json:"datastore"`
309+
Capacity int64 `json:"capacity"`
310+
Shared bool `json:"shared"`
311+
RDM bool `json:"rdm"`
312+
Bus string `json:"bus"`
313+
Mode string `json:"mode,omitempty"`
314+
Serial string `json:"serial,omitempty"`
315+
ChangeTrackingEnabled bool `json:"changeTrackingEnabled"`
313316
}
314317

315318
// Virtual Device.

‎validation/policies/io/konveyor/forklift/vmware/changed_block_tracking_per_disk.rego‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
package io.konveyor.forklift.vmware
2+
import future.keywords.in
3+
4+
change_tracking_disabled_per_disk(disk) {
5+
disk.changeTrackingEnabled == false
6+
}
7+
8+
concerns[flag] {
9+
some disk in input.disks
10+
change_tracking_disabled_per_disk(disk)
11+
12+
path_parts := split(disk.file, "/")
13+
filename := trim(path_parts[count(path_parts) - 1], "] ")
14+
15+
baseKey := (disk.controllerKey / 100) * 100
16+
controllerIndex := disk.controllerKey - baseKey
17+
18+
deviceKey := sprintf("%s%d:%d", [disk.bus, controllerIndex, disk.unitNumber])
19+
20+
flag := {
21+
"category": "Warning",
22+
"label": sprintf("Disk - %s does not have CBT enabled", [deviceKey]),
23+
"assessment": "Changed Block Tracking (CBT) has not been enabled for this device. This feature is a prerequisite for VM warm migration."
24+
}
25+
}

0 commit comments

Comments
 (0)