From 40ca035ec51dee218a46801ec6958515c5ac1b02 Mon Sep 17 00:00:00 2001 From: John Carden Date: Mon, 14 Oct 2024 10:23:19 -0700 Subject: [PATCH 1/4] intermediate commit to add RemoveISODisk to wmi --- pkg/virtualization/core/service/dvddrive.go | 21 +++++++++++++++++++ .../core/virtualsystem/virtualmachine.go | 5 +++++ 2 files changed, 26 insertions(+) diff --git a/pkg/virtualization/core/service/dvddrive.go b/pkg/virtualization/core/service/dvddrive.go index 80dc0fe34d..817d68d44e 100644 --- a/pkg/virtualization/core/service/dvddrive.go +++ b/pkg/virtualization/core/service/dvddrive.go @@ -84,6 +84,27 @@ func (vmms *VirtualSystemManagementService) AddISODisk(vm *virtualsystem.Virtual } +func (vmms *VirtualSystemManagementService) GetISODisk(vm *virtualsystem.VirtualMachine, isoPath string) (ld *disk.LogicalDisk, err error) { + // col, err := vm.GetDvdDrives() + // if err != nil { + // return + // } + // defer col.Close() + + // TODO: probably put this find functionality somewhere else + // for _, inst := range col { + // dvddrive, err := drive.NewDvdDrive(inst) + // if err != nil { + // return nil, err + // } + // Get the logical disk + // ld, err := disk.GetLogicalDiskByHostResource(vm, isoPath) + // if err != nil { + // return nil + // } + return nil, errors.NotImplemented +} + func (vmms *VirtualSystemManagementService) RemoveISODisk(ld *disk.LogicalDisk) (err error) { dvddrive, err := ld.GetDrive() if err != nil { diff --git a/pkg/virtualization/core/virtualsystem/virtualmachine.go b/pkg/virtualization/core/virtualsystem/virtualmachine.go index 37892613e0..cc990b890f 100644 --- a/pkg/virtualization/core/virtualsystem/virtualmachine.go +++ b/pkg/virtualization/core/virtualsystem/virtualmachine.go @@ -762,6 +762,11 @@ func (vm *VirtualMachine) GetVirtualHardDrives() (col resourceallocation.Resourc return } +func (vm *VirtualMachine) GetDvdDrives() (col resourceallocation.ResourceAllocationSettingDataCollection, err error) { + col, err = vm.GetResourceAllocationSettingData(v2.ResourcePool_ResourceType_DVD_drive) + return +} + func (vm *VirtualMachine) GetVirtualHardDiskByLocation(controllerNumber, controllerLocation int) (vhd *disk.VirtualHardDisk, err error) { col, err := vm.GetVirtualHardDisks() if err != nil { From 2ddac03199e1f5a0cf58d62d77fe4436b2c4907f Mon Sep 17 00:00:00 2001 From: Neeraj Dixit Date: Wed, 16 Oct 2024 23:26:38 +0000 Subject: [PATCH 2/4] Add code to retrieve and clean up the DVD disk attached to VM --- pkg/virtualization/core/service/dvddrive.go | 47 ++--------- .../core/storage/disk/logicaldisk.go | 12 +++ .../core/storage/drive/dvddrive.go | 25 ++++++ .../core/virtualsystem/virtualmachine.go | 84 ++++++++++++++++++- .../virtualsystem/virtualsystemsettingdata.go | 16 ++++ 5 files changed, 140 insertions(+), 44 deletions(-) diff --git a/pkg/virtualization/core/service/dvddrive.go b/pkg/virtualization/core/service/dvddrive.go index 817d68d44e..ea1116cf8f 100644 --- a/pkg/virtualization/core/service/dvddrive.go +++ b/pkg/virtualization/core/service/dvddrive.go @@ -84,48 +84,6 @@ func (vmms *VirtualSystemManagementService) AddISODisk(vm *virtualsystem.Virtual } -func (vmms *VirtualSystemManagementService) GetISODisk(vm *virtualsystem.VirtualMachine, isoPath string) (ld *disk.LogicalDisk, err error) { - // col, err := vm.GetDvdDrives() - // if err != nil { - // return - // } - // defer col.Close() - - // TODO: probably put this find functionality somewhere else - // for _, inst := range col { - // dvddrive, err := drive.NewDvdDrive(inst) - // if err != nil { - // return nil, err - // } - // Get the logical disk - // ld, err := disk.GetLogicalDiskByHostResource(vm, isoPath) - // if err != nil { - // return nil - // } - return nil, errors.NotImplemented -} - -func (vmms *VirtualSystemManagementService) RemoveISODisk(ld *disk.LogicalDisk) (err error) { - dvddrive, err := ld.GetDrive() - if err != nil { - return - } - defer dvddrive.Close() - - // Remove Disk - err1 := vmms.RemoveVirtualSystemResource(ld.CIM_ResourceAllocationSettingData, -1) - // Remove Drive - err = vmms.RemoveVirtualSystemResource(dvddrive.CIM_ResourceAllocationSettingData, -1) - if err != nil { - return - } - if err1 != nil { - err = err1 - return - } - return -} - func (vmms *VirtualSystemManagementService) AddDvdDrive(vm *virtualsystem.VirtualMachine) (dvd *drive.DvdDrive, err error) { // Add the dvd drive tmp, err := vm.NewDvdDrive() @@ -165,3 +123,8 @@ func (vmms *VirtualSystemManagementService) RemoveDvdDrive(dvd *drive.DvdDrive) err = vmms.RemoveVirtualSystemResource(dvd.CIM_ResourceAllocationSettingData, -1) return } + +func (vmms *VirtualSystemManagementService) RemoveDvdDisk(dvddisk *disk.LogicalDisk) (err error) { + err = vmms.RemoveVirtualSystemResource(dvddisk.CIM_ResourceAllocationSettingData, -1) + return +} diff --git a/pkg/virtualization/core/storage/disk/logicaldisk.go b/pkg/virtualization/core/storage/disk/logicaldisk.go index 51fc1cc616..45ceb50b3b 100644 --- a/pkg/virtualization/core/storage/disk/logicaldisk.go +++ b/pkg/virtualization/core/storage/disk/logicaldisk.go @@ -6,6 +6,7 @@ package disk import ( "github.com/microsoft/wmi/pkg/base/instance" "github.com/microsoft/wmi/pkg/constant" + "github.com/microsoft/wmi/pkg/errors" "github.com/microsoft/wmi/pkg/virtualization/core/resource/resourceallocation" wmi "github.com/microsoft/wmi/pkg/wmiinstance" v2 "github.com/microsoft/wmi/server2019/root/virtualization/v2" @@ -35,3 +36,14 @@ func (ld *LogicalDisk) GetDrive() (*resourceallocation.ResourceAllocationSetting } return resourceallocation.NewResourceAllocationSettingData(inst) } + +func (ld *LogicalDisk) GetPath() (string, error) { + value, err := ld.GetProperty("HostResource") + if err != nil { + return "", err + } + for _, hr := range value.([]interface{}) { + return hr.(string), nil + } + return "", errors.Wrapf(errors.NotFound, "") +} diff --git a/pkg/virtualization/core/storage/drive/dvddrive.go b/pkg/virtualization/core/storage/drive/dvddrive.go index 765f98e35e..96000c4b52 100644 --- a/pkg/virtualization/core/storage/drive/dvddrive.go +++ b/pkg/virtualization/core/storage/drive/dvddrive.go @@ -18,3 +18,28 @@ func NewDvdDrive(instance *wmi.WmiInstance) (*DvdDrive, error) { } return &DvdDrive{wmivm}, nil } + +type DvdDriveCollection []*DvdDrive + +func NewDvdDriveCollection(instances []*wmi.WmiInstance) (col DvdDriveCollection, err error) { + for _, inst := range instances { + na, err1 := NewDvdDrive(inst) + if err1 != nil { + err = err1 + return + } + col = append(col, na) + } + return +} + +func Close(vms *DvdDriveCollection) (err error) { + for _, value := range *vms { + value.Close() + } + return nil +} + +func GetRelatedStorageAllocationSettingData(instance *wmi.WmiInstance) (value *wmi.WmiInstance, err error) { + return instance.GetRelated("Msvm_StorageAllocationSettingData") +} diff --git a/pkg/virtualization/core/virtualsystem/virtualmachine.go b/pkg/virtualization/core/virtualsystem/virtualmachine.go index cc990b890f..239716c440 100644 --- a/pkg/virtualization/core/virtualsystem/virtualmachine.go +++ b/pkg/virtualization/core/virtualsystem/virtualmachine.go @@ -762,8 +762,88 @@ func (vm *VirtualMachine) GetVirtualHardDrives() (col resourceallocation.Resourc return } -func (vm *VirtualMachine) GetDvdDrives() (col resourceallocation.ResourceAllocationSettingDataCollection, err error) { - col, err = vm.GetResourceAllocationSettingData(v2.ResourcePool_ResourceType_DVD_drive) +func (vm *VirtualMachine) GetDvdDrives() (col drive.DvdDriveCollection, err error) { + settings, err := vm.GetVirtualSystemSettingData() + if err != nil { + return + } + defer settings.Close() + return settings.GetVirtualDvdDrives() +} + +func (vm *VirtualMachine) GetDvdConfigByIsoPath(isoPath string) (dvd *drive.DvdDrive, diskdvd *disk.LogicalDisk, err error) { + col, err := vm.GetDvdDrives() + if err != nil { + return + } + defer drive.Close(&col) + + for _, inst := range col { + dvddisk, err1 := drive.GetRelatedStorageAllocationSettingData(inst.WmiInstance) + if err1 != nil { + // Missing related storage allocation data is benign + // It just means that DVD disk is not available + continue + } + + dvdPathInterface, err1 := dvddisk.GetProperty("HostResource") + if err1 != nil { + err = fmt.Errorf("unable to read HostResource field from disk WMI %s", err1) + return + } + if dvdPathInterface == nil { + // Attached ISO path is not available, continue + continue + } + + dvdPath := "" + for _, interfaceValue := range dvdPathInterface.([]interface{}) { + valuetmp, ok := interfaceValue.(string) + if !ok { + err = errors.Wrapf(errors.InvalidType, " string is Invalid. Expected %s", reflect.TypeOf(interfaceValue)) + return + } + + dvdPath = string(valuetmp) + } + + if dvdPath != isoPath { + continue + } + + // Clone the DVD drive instance for return + dvdclone, err1 := inst.Clone() + if err1 != nil { + err = err1 + return + } + + retdvd, err1 := drive.NewDvdDrive(dvdclone) + if err1 != nil { + err = err1 + return + } + + // Also clone the logical disk instance for return + dvddiskclone, err1 := dvddisk.Clone() + if err1 != nil { + err = err1 + return + } + + retdvddisk, err1 := disk.NewLogicalDisk(dvddiskclone) + if err1 != nil { + err = err1 + return + } + + dvd = retdvd + diskdvd = retdvddisk + return + } + err = errors.Wrapf(errors.NotFound, + "Dvd drive with path [%s] not found in Vm [%s]", isoPath, vm.Name()) + dvd = nil return } diff --git a/pkg/virtualization/core/virtualsystem/virtualsystemsettingdata.go b/pkg/virtualization/core/virtualsystem/virtualsystemsettingdata.go index d698606955..3300a4e9c7 100644 --- a/pkg/virtualization/core/virtualsystem/virtualsystemsettingdata.go +++ b/pkg/virtualization/core/virtualsystem/virtualsystemsettingdata.go @@ -17,6 +17,7 @@ import ( "github.com/microsoft/wmi/pkg/virtualization/core/pcie" "github.com/microsoft/wmi/pkg/virtualization/core/processor" "github.com/microsoft/wmi/pkg/virtualization/core/storage/disk" + "github.com/microsoft/wmi/pkg/virtualization/core/storage/drive" na "github.com/microsoft/wmi/pkg/virtualization/network/virtualnetworkadapter" wmi "github.com/microsoft/wmi/pkg/wmiinstance" v2 "github.com/microsoft/wmi/server2019/root/virtualization/v2" @@ -251,6 +252,21 @@ func (vm *VirtualSystemSettingData) GetVirtualHardDisks() (col disk.VirtualHardD return } +func (vm *VirtualSystemSettingData) GetVirtualDvdDrives() (col drive.DvdDriveCollection, err error) { + resourceType := fmt.Sprintf("%d", int32(v2.ResourceAllocationSettingData_ResourceType_DVD_drive)) + query := query.NewWmiQuery("Cim_ResourceAllocationSettingData", "ResourceType", resourceType) + rasdcollection, err := instance.GetWmiInstancesFromHost(vm.GetWmiHost(), string(constant.Virtualization), query) + if err != nil { + return + } + + col, err = drive.NewDvdDriveCollection(rasdcollection) + if err != nil { + rasdcollection.Close() + } + return +} + func (vm *VirtualSystemSettingData) getResourceAllocationSettingData(rtype v2.ResourceAllocationSettingData_ResourceType) (col wmi.WmiInstanceCollection, err error) { resourceType := fmt.Sprintf("%d", int32(rtype)) query := query.NewWmiQuery("Cim_ResourceAllocationSettingData", "ResourceType", resourceType) From 2337ef6aff351f3667488025ce0af390685adbfc Mon Sep 17 00:00:00 2001 From: Neeraj Dixit Date: Thu, 17 Oct 2024 22:26:52 +0000 Subject: [PATCH 3/4] use filepath package for path comparison --- pkg/virtualization/core/virtualsystem/virtualmachine.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pkg/virtualization/core/virtualsystem/virtualmachine.go b/pkg/virtualization/core/virtualsystem/virtualmachine.go index 239716c440..3a4ec5a288 100644 --- a/pkg/virtualization/core/virtualsystem/virtualmachine.go +++ b/pkg/virtualization/core/virtualsystem/virtualmachine.go @@ -5,6 +5,7 @@ package virtualsystem import ( "fmt" + "path/filepath" //"log" "time" @@ -807,7 +808,7 @@ func (vm *VirtualMachine) GetDvdConfigByIsoPath(isoPath string) (dvd *drive.DvdD dvdPath = string(valuetmp) } - if dvdPath != isoPath { + if filepath.Clean(dvdPath) != filepath.Clean(isoPath) { continue } From 39d4a6a092aaa336c99e246987a43266649aa28c Mon Sep 17 00:00:00 2001 From: Neeraj Dixit Date: Fri, 18 Oct 2024 22:23:33 +0000 Subject: [PATCH 4/4] Addressed review comments and added unit tests --- pkg/virtualization/core/service/dvddrive.go | 21 +++++++ .../virtualmachinemanagementservice_test.go | 61 +++++++++++++++++++ .../core/storage/disk/logicaldisk.go | 12 ---- .../core/storage/drive/dvddrive.go | 21 ------- .../core/storage/drive/dvddrivecollection.go | 29 +++++++++ .../core/virtualsystem/virtualmachine.go | 8 +-- .../virtualmachinesetting_test.go | 21 +++++++ 7 files changed, 136 insertions(+), 37 deletions(-) create mode 100644 pkg/virtualization/core/storage/drive/dvddrivecollection.go diff --git a/pkg/virtualization/core/service/dvddrive.go b/pkg/virtualization/core/service/dvddrive.go index ea1116cf8f..4afdaab558 100644 --- a/pkg/virtualization/core/service/dvddrive.go +++ b/pkg/virtualization/core/service/dvddrive.go @@ -84,6 +84,27 @@ func (vmms *VirtualSystemManagementService) AddISODisk(vm *virtualsystem.Virtual } +func (vmms *VirtualSystemManagementService) RemoveISODisk(ld *disk.LogicalDisk) (err error) { + dvddrive, err := ld.GetDrive() + if err != nil { + return + } + defer dvddrive.Close() + + // Remove Disk + err1 := vmms.RemoveVirtualSystemResource(ld.CIM_ResourceAllocationSettingData, -1) + // Remove Drive + err = vmms.RemoveVirtualSystemResource(dvddrive.CIM_ResourceAllocationSettingData, -1) + if err != nil { + return + } + if err1 != nil { + err = err1 + return + } + return +} + func (vmms *VirtualSystemManagementService) AddDvdDrive(vm *virtualsystem.VirtualMachine) (dvd *drive.DvdDrive, err error) { // Add the dvd drive tmp, err := vm.NewDvdDrive() diff --git a/pkg/virtualization/core/service/virtualmachinemanagementservice_test.go b/pkg/virtualization/core/service/virtualmachinemanagementservice_test.go index bd30e29bc3..7238e32139 100644 --- a/pkg/virtualization/core/service/virtualmachinemanagementservice_test.go +++ b/pkg/virtualization/core/service/virtualmachinemanagementservice_test.go @@ -239,6 +239,67 @@ func TestAddRemoveIsoDiskGen1(t *testing.T) { } } +func TestAddRemoveIsoDiskByPath(t *testing.T) { + vmms, err := GetVirtualSystemManagementService(whost) + if err != nil { + t.Fatalf("Failed [%+v]", err) + } + vm, err := vmms.GetVirtualMachineByName("test") + if err != nil { + t.Fatalf("Failed [%+v]", err) + } + defer vm.Close() + t.Logf("Found [%s] VMs", vm.Name()) + + // make sure there is a controller + err = vmms.AddSCSIController(vm) + if err != nil { + t.Fatalf("Failed [%+v]", err) + } + + path := "c:\\test\\tmp.iso" + + // create an iso file + err = generateIso(path) + if err != nil { + t.Fatalf("Failed [%+v]", err) + } + + isodisk, dvddrive, err := vmms.AddISODisk(vm, path) + if err != nil { + t.Fatalf("Failed [%+v]", err) + } + defer isodisk.Close() + defer dvddrive.Close() + + // remove the iso disk + vmdvddrive, vmdvddisk, err := vm.GetDvdDriveAndLogicalDiskByIsoPath(path) + if err != nil { + t.Fatalf("Failed [%+v]", err) + } + t.Logf("Obtained ISO disk [%s] from [%s]", path, "test") + defer vmdvddisk.Close() + defer vmdvddrive.Close() + + // Remove logical disk + err = vmms.RemoveDvdDisk(vmdvddisk) + if err != nil { + t.Fatalf("Failed to remove DVD disk [%+v]", err) + } + + // Remove DVD drive + err = vmms.RemoveDvdDrive(vmdvddrive) + if err != nil { + t.Fatalf("Failed to remove DVD drive [%+v]", err) + } + + // Remove the ISO file + err = os.Remove(path) + if err != nil { + t.Fatalf("Failed [%+v]", err) + } +} + func generateIso(path string) error { writer, err := iso9660.NewWriter() if err != nil { diff --git a/pkg/virtualization/core/storage/disk/logicaldisk.go b/pkg/virtualization/core/storage/disk/logicaldisk.go index 45ceb50b3b..51fc1cc616 100644 --- a/pkg/virtualization/core/storage/disk/logicaldisk.go +++ b/pkg/virtualization/core/storage/disk/logicaldisk.go @@ -6,7 +6,6 @@ package disk import ( "github.com/microsoft/wmi/pkg/base/instance" "github.com/microsoft/wmi/pkg/constant" - "github.com/microsoft/wmi/pkg/errors" "github.com/microsoft/wmi/pkg/virtualization/core/resource/resourceallocation" wmi "github.com/microsoft/wmi/pkg/wmiinstance" v2 "github.com/microsoft/wmi/server2019/root/virtualization/v2" @@ -36,14 +35,3 @@ func (ld *LogicalDisk) GetDrive() (*resourceallocation.ResourceAllocationSetting } return resourceallocation.NewResourceAllocationSettingData(inst) } - -func (ld *LogicalDisk) GetPath() (string, error) { - value, err := ld.GetProperty("HostResource") - if err != nil { - return "", err - } - for _, hr := range value.([]interface{}) { - return hr.(string), nil - } - return "", errors.Wrapf(errors.NotFound, "") -} diff --git a/pkg/virtualization/core/storage/drive/dvddrive.go b/pkg/virtualization/core/storage/drive/dvddrive.go index 96000c4b52..dea0f969d6 100644 --- a/pkg/virtualization/core/storage/drive/dvddrive.go +++ b/pkg/virtualization/core/storage/drive/dvddrive.go @@ -19,27 +19,6 @@ func NewDvdDrive(instance *wmi.WmiInstance) (*DvdDrive, error) { return &DvdDrive{wmivm}, nil } -type DvdDriveCollection []*DvdDrive - -func NewDvdDriveCollection(instances []*wmi.WmiInstance) (col DvdDriveCollection, err error) { - for _, inst := range instances { - na, err1 := NewDvdDrive(inst) - if err1 != nil { - err = err1 - return - } - col = append(col, na) - } - return -} - -func Close(vms *DvdDriveCollection) (err error) { - for _, value := range *vms { - value.Close() - } - return nil -} - func GetRelatedStorageAllocationSettingData(instance *wmi.WmiInstance) (value *wmi.WmiInstance, err error) { return instance.GetRelated("Msvm_StorageAllocationSettingData") } diff --git a/pkg/virtualization/core/storage/drive/dvddrivecollection.go b/pkg/virtualization/core/storage/drive/dvddrivecollection.go new file mode 100644 index 0000000000..019c7accbd --- /dev/null +++ b/pkg/virtualization/core/storage/drive/dvddrivecollection.go @@ -0,0 +1,29 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. + +package drive + +import ( + wmi "github.com/microsoft/wmi/pkg/wmiinstance" +) + +type DvdDriveCollection []*DvdDrive + +func NewDvdDriveCollection(instances []*wmi.WmiInstance) (col DvdDriveCollection, err error) { + for _, inst := range instances { + na, err1 := NewDvdDrive(inst) + if err1 != nil { + err = err1 + return + } + col = append(col, na) + } + return +} + +func (dvdcol *DvdDriveCollection) Close() (err error) { + for _, value := range *dvdcol { + value.Close() + } + return nil +} diff --git a/pkg/virtualization/core/virtualsystem/virtualmachine.go b/pkg/virtualization/core/virtualsystem/virtualmachine.go index 3a4ec5a288..8991aa06a4 100644 --- a/pkg/virtualization/core/virtualsystem/virtualmachine.go +++ b/pkg/virtualization/core/virtualsystem/virtualmachine.go @@ -772,14 +772,14 @@ func (vm *VirtualMachine) GetDvdDrives() (col drive.DvdDriveCollection, err erro return settings.GetVirtualDvdDrives() } -func (vm *VirtualMachine) GetDvdConfigByIsoPath(isoPath string) (dvd *drive.DvdDrive, diskdvd *disk.LogicalDisk, err error) { - col, err := vm.GetDvdDrives() +func (vm *VirtualMachine) GetDvdDriveAndLogicalDiskByIsoPath(isoPath string) (dvd *drive.DvdDrive, diskdvd *disk.LogicalDisk, err error) { + dvdCol, err := vm.GetDvdDrives() if err != nil { return } - defer drive.Close(&col) + defer dvdCol.Close() - for _, inst := range col { + for _, inst := range dvdCol { dvddisk, err1 := drive.GetRelatedStorageAllocationSettingData(inst.WmiInstance) if err1 != nil { // Missing related storage allocation data is benign diff --git a/pkg/virtualization/core/virtualsystem/virtualmachinesetting_test.go b/pkg/virtualization/core/virtualsystem/virtualmachinesetting_test.go index cd11468823..0cbe6027c6 100644 --- a/pkg/virtualization/core/virtualsystem/virtualmachinesetting_test.go +++ b/pkg/virtualization/core/virtualsystem/virtualmachinesetting_test.go @@ -50,3 +50,24 @@ func TestGetVirtualHardDisks(t *testing.T) { defer vhds.Close() t.Logf("Virtual Hard Disks Found [%d]", len(vhds)) } + +func TestGetVirtualDvdDrives(t *testing.T) { + vm, err := GetVirtualMachineByVMName(whost, "test") + if err != nil { + t.Fatal("Failed " + err.Error()) + } + defer vm.Close() + + setting, err := vm.GetVirtualSystemSettingData() + if err != nil { + t.Fatal("Failed " + err.Error()) + } + defer setting.Close() + + dvds, err := setting.GetVirtualDvdDrives() + if err != nil { + t.Fatal("Failed " + err.Error()) + } + defer dvds.Close() + t.Logf("DVD drives Found [%d]", len(dvds)) +}