From e0c52cd23b414706ac630ce3d4c9651c3e74c146 Mon Sep 17 00:00:00 2001 From: Amit Barve Date: Fri, 1 Mar 2024 01:59:33 -0800 Subject: [PATCH] Remove unused CimFS related code CimFS is currently not supported with HyperV isolation. However, we still have code that handles processing of UtilityVM layer during image import. After the layer refactoring change we need to update this code as well. But since this code isn't being used anywhere updating it doesn't make much sense. There are no tests for this either. This code is removed for now and we can add it back later once the plan for running HyperV isolated containers with CimFS is more solid. Signed-off-by: Amit Barve --- internal/wclayer/cim/LayerWriter.go | 48 +---------- internal/wclayer/cim/bcd.go | 107 ------------------------ internal/wclayer/cim/common.go | 41 ---------- internal/wclayer/cim/process.go | 83 +------------------ internal/wclayer/cim/registry.go | 123 ---------------------------- pkg/ociwclayer/cim/import.go | 4 - 6 files changed, 3 insertions(+), 403 deletions(-) delete mode 100644 internal/wclayer/cim/bcd.go delete mode 100644 internal/wclayer/cim/common.go diff --git a/internal/wclayer/cim/LayerWriter.go b/internal/wclayer/cim/LayerWriter.go index 28c8df079a..9315971b64 100644 --- a/internal/wclayer/cim/LayerWriter.go +++ b/internal/wclayer/cim/LayerWriter.go @@ -6,16 +6,12 @@ import ( "context" "fmt" "io" - "os" "path/filepath" - "strconv" "strings" "github.com/Microsoft/go-winio" - "github.com/Microsoft/hcsshim/internal/log" "github.com/Microsoft/hcsshim/internal/oc" "github.com/Microsoft/hcsshim/internal/wclayer" - "github.com/Microsoft/hcsshim/osversion" "github.com/Microsoft/hcsshim/pkg/cimfs" "go.opencensus.io/trace" ) @@ -174,31 +170,8 @@ func (cw *CimLayerWriter) Close(ctx context.Context) (retErr error) { } }() - // Find out the osversion of this layer, both base & non-base layers can have UtilityVM layer. + // UVM based containers aren't supported with CimFS, don't process the UVM layer processUtilityVM := false - if cw.hasUtilityVM { - uvmSoftwareHivePath := filepath.Join(cw.layerPath, wclayer.UtilityVMPath, wclayer.RegFilesPath, "SOFTWARE") - osvStr, err := getOsBuildNumberFromRegistry(uvmSoftwareHivePath) - if err != nil { - return fmt.Errorf("read os version string from UtilityVM SOFTWARE hive: %w", err) - } - - osv, err := strconv.ParseUint(osvStr, 10, 16) - if err != nil { - return fmt.Errorf("parse os version string (%s): %w", osvStr, err) - } - - // write this version to a file for future reference by the shim process - if err = wclayer.WriteLayerUvmBuildFile(cw.layerPath, uint16(osv)); err != nil { - return fmt.Errorf("write uvm build version: %w", err) - } - - // CIMFS for hyperV isolated is only supported after 20348, processing UtilityVM layer on 2048 - // & lower will cause failures since those images won't have CIMFS specific UVM files (mostly - // BCD entries required for CIMFS) - processUtilityVM = (osv > osversion.LTSC2022) - log.G(ctx).Debugf("import image os version %d, processing UtilityVM layer: %t\n", osv, processUtilityVM) - } if len(cw.parentLayerPaths) == 0 { if err := cw.processBaseLayer(ctx, processUtilityVM); err != nil { @@ -264,22 +237,3 @@ func NewCimLayerWriter(ctx context.Context, layerPath, cimPath string, parentLay stdFileWriter: sfw, }, nil } - -// DestroyCimLayer destroys a cim layer i.e it removes all the cimfs files for the given layer as well as -// all of the other files that are stored in the layer directory (at path `layerPath`). -// If this is not a cimfs layer (i.e a cim file for the given layer does not exist) then nothing is done. -func DestroyCimLayer(ctx context.Context, layerPath string) error { - cimPath := GetCimPathFromLayer(layerPath) - - // verify that such a cim exists first, sometimes containerd tries to call - // this with the root snapshot directory as the layer path. We don't want to - // destroy everything inside the snapshots directory. - if _, err := os.Stat(cimPath); err != nil { - if os.IsNotExist(err) { - return nil - } - return err - } - - return cimfs.DestroyCim(ctx, cimPath) -} diff --git a/internal/wclayer/cim/bcd.go b/internal/wclayer/cim/bcd.go deleted file mode 100644 index 23a3ce6776..0000000000 --- a/internal/wclayer/cim/bcd.go +++ /dev/null @@ -1,107 +0,0 @@ -//go:build windows - -package cim - -import ( - "bytes" - "fmt" - "os/exec" - - "github.com/Microsoft/go-winio/pkg/guid" -) - -const ( - bcdFilePath = "UtilityVM\\Files\\EFI\\Microsoft\\Boot\\BCD" - cimfsDeviceOptionsID = "{763e9fea-502d-434f-aad9-5fabe9c91a7b}" - vmbusDeviceID = "{c63c9bdf-5fa5-4208-b03f-6b458b365592}" - compositeDeviceOptionsID = "{e1787220-d17f-49e7-977a-d8fe4c8537e2}" - bootContainerID = "{b890454c-80de-4e98-a7ab-56b74b4fbd0c}" -) - -func bcdExec(storePath string, args ...string) error { - var out bytes.Buffer - argsArr := []string{"/store", storePath, "/offline"} - argsArr = append(argsArr, args...) - cmd := exec.Command("bcdedit.exe", argsArr...) - cmd.Stdout = &out - if err := cmd.Run(); err != nil { - return fmt.Errorf("bcd command (%s) failed: %w", cmd, err) - } - return nil -} - -// A registry configuration required for the uvm. -func setBcdRestartOnFailure(storePath string) error { - return bcdExec(storePath, "/set", "{default}", "restartonfailure", "yes") -} - -func setBcdCimBootDevice(storePath, cimPathRelativeToVSMB string, diskID, partitionID guid.GUID) error { - // create options for cimfs boot device - if err := bcdExec(storePath, "/create", cimfsDeviceOptionsID, "/d", "CimFS Device Options", "/device"); err != nil { - return err - } - - // Set options. For now we need to set 2 options. First is the parent device i.e the device under - // which all cim files will be available. Second is the path of the cim (from which this UVM should - // boot) relative to the parent device. Note that even though the 2nd option is named - // `cimfsrootdirectory` it expects a path to the cim file and not a directory path. - if err := bcdExec(storePath, "/set", cimfsDeviceOptionsID, "cimfsparentdevice", fmt.Sprintf("vmbus=%s", vmbusDeviceID)); err != nil { - return err - } - - if err := bcdExec(storePath, "/set", cimfsDeviceOptionsID, "cimfsrootdirectory", fmt.Sprintf("\\%s", cimPathRelativeToVSMB)); err != nil { - return err - } - - // create options for the composite device - if err := bcdExec(storePath, "/create", compositeDeviceOptionsID, "/d", "Composite Device Options", "/device"); err != nil { - return err - } - - // We need to specify the diskID & the partition ID of the boot disk and we need to set the cimfs boot - // options ID - partitionStr := fmt.Sprintf("gpt_partition={%s};{%s}", diskID, partitionID) - if err := bcdExec(storePath, "/set", compositeDeviceOptionsID, "primarydevice", partitionStr); err != nil { - return err - } - - if err := bcdExec(storePath, "/set", compositeDeviceOptionsID, "secondarydevice", fmt.Sprintf("cimfs=%s,%s", bootContainerID, cimfsDeviceOptionsID)); err != nil { - return err - } - - if err := bcdExec(storePath, "/set", "{default}", "device", fmt.Sprintf("composite=0,%s", compositeDeviceOptionsID)); err != nil { - return err - } - - if err := bcdExec(storePath, "/set", "{default}", "osdevice", fmt.Sprintf("composite=0,%s", compositeDeviceOptionsID)); err != nil { - return err - } - - // Since our UVM file are stored under UtilityVM\Files directory inside the CIM we must prepend that - // directory in front of paths used by bootmgr - if err := bcdExec(storePath, "/set", "{default}", "path", "\\UtilityVM\\Files\\Windows\\System32\\winload.efi"); err != nil { - return err - } - - if err := bcdExec(storePath, "/set", "{default}", "systemroot", "\\UtilityVM\\Files\\Windows"); err != nil { - return err - } - - return nil -} - -// updateBcdStoreForBoot Updates the bcd store at path layerPath + UtilityVM\Files\EFI\Microsoft\Boot\BCD` to -// boot with the disk with given ID and given partitionID. cimPathRelativeToVSMB is the path of the cim which -// will be used for booting this UVM relative to the VSMB share. (Usually, the entire snapshots directory will -// be shared over VSMB, so if this is the cim-layers\1.cim under that directory, the value of -// `cimPathRelativeToVSMB` should be cim-layers\1.cim) -func updateBcdStoreForBoot(storePath string, cimPathRelativeToVSMB string, diskID, partitionID guid.GUID) error { - if err := setBcdRestartOnFailure(storePath); err != nil { - return err - } - - if err := setBcdCimBootDevice(storePath, cimPathRelativeToVSMB, diskID, partitionID); err != nil { - return err - } - return nil -} diff --git a/internal/wclayer/cim/common.go b/internal/wclayer/cim/common.go deleted file mode 100644 index bdeebd3c03..0000000000 --- a/internal/wclayer/cim/common.go +++ /dev/null @@ -1,41 +0,0 @@ -//go:build windows - -package cim - -import ( - "os" - "path/filepath" -) - -const ( - // name of the directory in which cims are stored - cimDir = "cim-layers" -) - -// Usually layers are stored at ./root/io.containerd.snapshotter.v1.windows/snapshots/. For cimfs we -// must store all layer cims in the same directory (for forked cims to work). So all cim layers are stored in -// /root/io.containerd.snapshotter.v1.windows/snapshots/cim-layers. And the cim file representing each -// individual layer is stored at /root/io.containerd.snapshotter.v1.windows/snapshots/cim-layers/.cim - -// CimName is the filename (.cim) of the file representing the cim -func GetCimNameFromLayer(layerPath string) string { - return filepath.Base(layerPath) + ".cim" -} - -// CimPath is the path to the CimDir/.cim file that represents a layer cim. -func GetCimPathFromLayer(layerPath string) string { - return filepath.Join(GetCimDirFromLayer(layerPath), GetCimNameFromLayer(layerPath)) -} - -// CimDir is the directory inside which all cims are stored. -func GetCimDirFromLayer(layerPath string) string { - dir := filepath.Dir(layerPath) - return filepath.Join(dir, cimDir) -} - -// IsCimLayer returns `true` if the layer at path `layerPath` is a cim layer. Returns `false` otherwise. -func IsCimLayer(layerPath string) bool { - cimPath := GetCimPathFromLayer(layerPath) - _, err := os.Stat(cimPath) - return (err == nil) -} diff --git a/internal/wclayer/cim/process.go b/internal/wclayer/cim/process.go index d926272645..8fdb3bad3f 100644 --- a/internal/wclayer/cim/process.go +++ b/internal/wclayer/cim/process.go @@ -7,95 +7,16 @@ import ( "fmt" "os" "path/filepath" - "syscall" "time" "github.com/Microsoft/go-winio" - "github.com/Microsoft/go-winio/vhd" - "github.com/Microsoft/hcsshim/computestorage" - "github.com/Microsoft/hcsshim/internal/memory" - "github.com/Microsoft/hcsshim/internal/security" - "github.com/Microsoft/hcsshim/internal/vhdx" "github.com/Microsoft/hcsshim/internal/wclayer" "golang.org/x/sys/windows" ) -const defaultVHDXBlockSizeInMB = 1 - -// processUtilityVMLayer is similar to createContainerBaseLayerVHDs but along with the scratch creation it -// also does some BCD modifications to allow the UVM to boot from the CIM. It expects that the UVM BCD file is -// present at layerPath/`wclayer.BcdFilePath` and a UVM SYSTEM hive is present at -// layerPath/UtilityVM/`wclayer.RegFilesPath`/SYSTEM. The scratch VHDs are created under the `layerPath` -// directory. +// processUtilityVMLayer will handle processing of UVM specific files when we start +// supporting UVM based containers with CimFS in the future. func processUtilityVMLayer(ctx context.Context, layerPath string) error { - // func createUtilityVMLayerVHDs(ctx context.Context, layerPath string) error { - baseVhdPath := filepath.Join(layerPath, wclayer.UtilityVMPath, wclayer.UtilityVMBaseVhd) - diffVhdPath := filepath.Join(layerPath, wclayer.UtilityVMPath, wclayer.UtilityVMScratchVhd) - defaultVhdSize := uint64(10) - - // Just create the vhdx for utilityVM layer, no need to format it. - createParams := &vhd.CreateVirtualDiskParameters{ - Version: 2, - Version2: vhd.CreateVersion2{ - MaximumSize: defaultVhdSize * memory.GiB, - BlockSizeInBytes: defaultVHDXBlockSizeInMB * memory.MiB, - }, - } - - handle, err := vhd.CreateVirtualDisk(baseVhdPath, vhd.VirtualDiskAccessNone, vhd.CreateVirtualDiskFlagNone, createParams) - if err != nil { - return fmt.Errorf("failed to create vhdx: %w", err) - } - - defer func() { - if err != nil { - os.RemoveAll(baseVhdPath) - os.RemoveAll(diffVhdPath) - } - }() - - err = computestorage.FormatWritableLayerVhd(ctx, windows.Handle(handle)) - closeErr := syscall.CloseHandle(handle) - if err != nil { - return err - } else if closeErr != nil { - return fmt.Errorf("failed to close vhdx handle: %w", closeErr) - } - - partitionInfo, err := vhdx.GetScratchVhdPartitionInfo(ctx, baseVhdPath) - if err != nil { - return fmt.Errorf("failed to get base vhd layout info: %w", err) - } - // relativeCimPath needs to be the cim path relative to the snapshots directory. The snapshots - // directory is shared inside the UVM over VSMB, so during the UVM boot this relative path will be - // used to find the cim file under that VSMB share. - relativeCimPath := filepath.Join(filepath.Base(GetCimDirFromLayer(layerPath)), GetCimNameFromLayer(layerPath)) - bcdPath := filepath.Join(layerPath, bcdFilePath) - if err = updateBcdStoreForBoot(bcdPath, relativeCimPath, partitionInfo.DiskID, partitionInfo.PartitionID); err != nil { - return fmt.Errorf("failed to update BCD: %w", err) - } - - if err := enableCimBoot(filepath.Join(layerPath, wclayer.UtilityVMPath, wclayer.RegFilesPath, "SYSTEM")); err != nil { - return fmt.Errorf("failed to setup cim image for uvm boot: %w", err) - } - - // Note: diff vhd creation and granting of vm group access must be done AFTER - // getting the partition info of the base VHD. Otherwise it causes the vhd parent - // chain to get corrupted. - // TODO(ambarve): figure out why this happens so that bcd update can be moved to a separate function - - // Create the differencing disk that will be what's copied for the final rw layer - // for a container. - if err = vhd.CreateDiffVhd(diffVhdPath, baseVhdPath, defaultVHDXBlockSizeInMB); err != nil { - return fmt.Errorf("failed to create differencing disk: %w", err) - } - - if err := security.GrantVmGroupAccess(baseVhdPath); err != nil { - return fmt.Errorf("failed to grant vm group access to %s: %w", baseVhdPath, err) - } - if err := security.GrantVmGroupAccess(diffVhdPath); err != nil { - return fmt.Errorf("failed to grant vm group access to %s: %w", diffVhdPath, err) - } return nil } diff --git a/internal/wclayer/cim/registry.go b/internal/wclayer/cim/registry.go index dd2af81cf3..c95b03ca37 100644 --- a/internal/wclayer/cim/registry.go +++ b/internal/wclayer/cim/registry.go @@ -3,88 +3,13 @@ package cim import ( - "encoding/binary" "fmt" - "os" - "unsafe" - "github.com/Microsoft/hcsshim/internal/log" "github.com/Microsoft/hcsshim/internal/winapi" "github.com/Microsoft/hcsshim/osversion" "github.com/pkg/errors" - "github.com/sirupsen/logrus" - "golang.org/x/sys/windows" ) -// enableCimBoot Opens the SYSTEM registry hive at path `hivePath` and updates it to include a CIMFS Start -// registry key. This prepares the uvm to boot from a cim file if requested. The registry changes required to -// actually make the uvm boot from a cim will be added in the uvm config (look at -// addBootFromCimRegistryChanges for details). This registry key needs to be available in the early boot -// phase and so including it in the uvm config doesn't work. -func enableCimBoot(hivePath string) (err error) { - dataZero := make([]byte, 4) - dataOne := make([]byte, 4) - binary.LittleEndian.PutUint32(dataOne, 1) - dataFour := make([]byte, 4) - binary.LittleEndian.PutUint32(dataFour, 4) - - bootGUID, err := windows.UTF16FromString(bootContainerID) - if err != nil { - return fmt.Errorf("failed to encode boot guid to utf16: %w", err) - } - - overrideBootPath, err := windows.UTF16FromString("\\Windows\\") - if err != nil { - return fmt.Errorf("failed to encode override boot path to utf16: %w", err) - } - - regChanges := []struct { - keyPath string - valueName string - valueType winapi.RegType - data *byte - dataLen uint32 - }{ - {"ControlSet001\\Control", "BootContainerGuid", winapi.REG_TYPE_SZ, (*byte)(unsafe.Pointer(&bootGUID[0])), 2 * uint32(len(bootGUID))}, - {"ControlSet001\\Services\\UnionFS", "Start", winapi.REG_TYPE_DWORD, &dataZero[0], uint32(len(dataZero))}, - {"ControlSet001\\Services\\wcifs", "Start", winapi.REG_TYPE_DWORD, &dataFour[0], uint32(len(dataZero))}, - // The bootmgr loads the uvm files from the cim and so uses the relative path `UtilityVM\\Files` inside the cim to access the uvm files. However, once the cim is mounted UnionFS will merge the correct directory (UtilityVM\\Files) of the cim with the scratch and then that point onwards we don't need to use the relative path. Below registry key tells the kernel that the boot path that was provided in BCD should now be overriden with this new path. - {"Setup", "BootPathOverride", winapi.REG_TYPE_SZ, (*byte)(unsafe.Pointer(&overrideBootPath[0])), 2 * uint32(len(overrideBootPath))}, - } - - var storeHandle winapi.ORHKey - if err = winapi.OROpenHive(hivePath, &storeHandle); err != nil { - return fmt.Errorf("failed to open registry store at %s: %w", hivePath, err) - } - - for _, change := range regChanges { - var changeKey winapi.ORHKey - if err = winapi.ORCreateKey(storeHandle, change.keyPath, 0, 0, 0, &changeKey, nil); err != nil { - return fmt.Errorf("failed to open reg key %s: %w", change.keyPath, err) - } - - if err = winapi.ORSetValue(changeKey, change.valueName, uint32(change.valueType), change.data, change.dataLen); err != nil { - return fmt.Errorf("failed to set value for regkey %s\\%s : %w", change.keyPath, change.valueName, err) - } - } - - // remove the existing file first - if err := os.Remove(hivePath); err != nil { - return fmt.Errorf("failed to remove existing registry %s: %w", hivePath, err) - } - - if err = winapi.ORSaveHive(winapi.ORHKey(storeHandle), hivePath, uint32(osversion.Get().MajorVersion), uint32(osversion.Get().MinorVersion)); err != nil { - return fmt.Errorf("error saving the registry store: %w", err) - } - - // close hive irrespective of the errors - if err := winapi.ORCloseHive(winapi.ORHKey(storeHandle)); err != nil { - return fmt.Errorf("error closing registry store; %w", err) - } - return nil - -} - // mergeHive merges the hive located at parentHivePath with the hive located at deltaHivePath and stores // the result into the file at mergedHivePath. If a file already exists at path `mergedHivePath` then it // throws an error. @@ -122,51 +47,3 @@ func mergeHive(parentHivePath, deltaHivePath, mergedHivePath string) (err error) } return } - -// getOsBuildNumberFromRegistry fetches the "CurrentBuild" value at path -// "Microsoft\Windows NT\CurrentVersion" from the SOFTWARE registry hive at path -// `regHivePath`. This is used to detect the build version of the uvm. -func getOsBuildNumberFromRegistry(regHivePath string) (_ string, err error) { - var storeHandle, keyHandle winapi.ORHKey - var dataType, dataLen uint32 - keyPath := "Microsoft\\Windows NT\\CurrentVersion" - valueName := "CurrentBuild" - dataLen = 16 // build version string can't be more than 5 wide chars? - dataBuf := make([]byte, dataLen) - - if err = winapi.OROpenHive(regHivePath, &storeHandle); err != nil { - return "", fmt.Errorf("failed to open registry store at %s: %w", regHivePath, err) - } - defer func() { - if closeErr := winapi.ORCloseHive(storeHandle); closeErr != nil { - log.L.WithFields(logrus.Fields{ - "error": closeErr, - "hive": regHivePath, - }).Warnf("failed to close hive") - } - }() - - if err = winapi.OROpenKey(storeHandle, keyPath, &keyHandle); err != nil { - return "", fmt.Errorf("failed to open key at %s: %w", keyPath, err) - } - defer func() { - if closeErr := winapi.ORCloseKey(keyHandle); closeErr != nil { - log.L.WithFields(logrus.Fields{ - "error": closeErr, - "hive": regHivePath, - "key": keyPath, - "value": valueName, - }).Warnf("failed to close hive key") - } - }() - - if err = winapi.ORGetValue(keyHandle, "", valueName, &dataType, &dataBuf[0], &dataLen); err != nil { - return "", fmt.Errorf("failed to get value of %s: %w", valueName, err) - } - - if dataType != uint32(winapi.REG_TYPE_SZ) { - return "", fmt.Errorf("unexpected build number data type (%d)", dataType) - } - - return winapi.ParseUtf16LE(dataBuf[:(dataLen - 2)]), nil -} diff --git a/pkg/ociwclayer/cim/import.go b/pkg/ociwclayer/cim/import.go index 0ee27f8d56..d8f4a1aa95 100644 --- a/pkg/ociwclayer/cim/import.go +++ b/pkg/ociwclayer/cim/import.go @@ -159,7 +159,3 @@ func writeCimLayerFromTar(ctx context.Context, r io.Reader, w *cim.CimLayerWrite } return size, nil } - -func DestroyCimLayer(layerPath string) error { - return cim.DestroyCimLayer(context.Background(), layerPath) -}