Move xfs formatting for encrypted devices out of crypt pkg into scsi

Signed-off-by: Kathryn Baldauf <kabaldau@microsoft.com>

internal/guest/storage/crypt/crypt.go

@@ -10,7 +10,6 @@ import (
  "os/exec"
  "path/filepath"
 
- "github.com/Microsoft/hcsshim/internal/guest/storage/xfs"
  "github.com/Microsoft/hcsshim/internal/log"
  "github.com/pkg/errors"
 )
@@ -22,7 +21,6 @@ var (
  _cryptsetupOpen = cryptsetupOpen
  _generateKeyFile = generateKeyFile
  _osMkdirTemp = os.MkdirTemp
- _xfsFormat = xfs.Format
  _osRemoveAll = os.RemoveAll
  _zeroFirstBlock = zeroFirstBlock
 )
@@ -111,9 +109,8 @@ func cryptsetupClose(deviceName string) error {
 // /dev/mapper/`cryptDeviceTemplate`. This can be mounted directly, but it
 // doesn't have any format yet.
 //
-// 4. Format the unencrypted block device as xfs.
+// 4. Prepare the unecrypted block device to be later encrypted as xfs
 // 4.1. Zero the first block. It appears that mkfs.xfs reads this before formatting.
-// 4.2. Format the device as xfs.
 
 func EncryptDevice(ctx context.Context, source string, dmCryptName string) (path string, err error) {
  // Create temporary directory to store the keyfile and xfs image
@@ -162,11 +159,6 @@ func EncryptDevice(ctx context.Context, source string, dmCryptName string) (path
  return "", fmt.Errorf("failed to zero first block: %w", err)
  }
 
- // 4.2. Format it as xfs
- if err = _xfsFormat(deviceNamePath); err != nil {
- return "", fmt.Errorf("mkfs.xfs failed to format %s: %w", deviceNamePath, err)
- }
-
  return deviceNamePath, nil
 }
 

internal/guest/storage/crypt/crypt_test.go

@@ -125,46 +125,6 @@ func Test_Encrypt_Cryptsetup_Open_Error(t *testing.T) {
  }
 }
 
-func Test_Encrypt_Mkfs_Error(t *testing.T) {
- clearCryptTestDependencies()
-
- // Test what happens when mkfs fails to format the unencrypted device.
- // Verify that the arguments passed to it are the right ones.
- _generateKeyFile = func(path string, size int64) error {
- return nil
- }
- _osRemoveAll = func(path string) error {
- return nil
- }
- _cryptsetupFormat = func(source string, keyFilePath string) error {
- return nil
- }
- _cryptsetupOpen = func(source string, deviceName string, keyFilePath string) error {
- return nil
- }
- _cryptsetupClose = func(deviceName string) error {
- return nil
- }
- _zeroFirstBlock = func(_ string, _ int) error {
- return nil
- }
-
- source := "/dev/sda"
- formatTarget := "/dev/mapper/dm-crypt-name"
-
- expectedErr := errors.New("expected error message")
- _xfsFormat = func(arg string) error {
- if arg != formatTarget {
- t.Fatalf("expected args: '%v' got: '%v'", formatTarget, arg)
- }
- return expectedErr
- }
-
- if _, err := EncryptDevice(context.Background(), source, "dm-crypt-name"); errors.Unwrap(err) != expectedErr {
- t.Fatalf("expected err: '%v' got: '%v'", expectedErr, err)
- }
-}
-
 func Test_Encrypt_Success(t *testing.T) {
  clearCryptTestDependencies()
 
@@ -184,9 +144,6 @@ func Test_Encrypt_Success(t *testing.T) {
  _zeroFirstBlock = func(_ string, _ int) error {
  return nil
  }
- _xfsFormat = func(arg string) error {
- return nil
- }
 
  source := "/dev/sda"
  dmCryptName := "dm-crypt-name"

internal/guest/storage/scsi/scsi.go

@@ -23,6 +23,7 @@ import (
  "github.com/Microsoft/hcsshim/internal/guest/storage/crypt"
  dm "github.com/Microsoft/hcsshim/internal/guest/storage/devicemapper"
  "github.com/Microsoft/hcsshim/internal/guest/storage/ext4"
+ "github.com/Microsoft/hcsshim/internal/guest/storage/xfs"
  "github.com/Microsoft/hcsshim/internal/log"
  "github.com/Microsoft/hcsshim/internal/oc"
  "github.com/Microsoft/hcsshim/internal/protocol/guestrequest"
@@ -58,6 +59,9 @@ var (
  // ext4Format is stubbed for unit testing the `EnsureFilesystem` flow
  // in `mount`
  ext4Format = ext4.Format
+ // ext4Format is stubbed for unit testing the `EnsureFilesystem` and
+ // `Encrypt` flow in `mount`
+ xfsFormat = xfs.Format
 )
 
 const (
@@ -223,25 +227,29 @@ func Mount(
  return fmt.Errorf("getting device's filesystem: %w", err)
  }
  }
+ log.G(ctx).WithField("filesystem", deviceFS).Debug("filesystem found on device")
+
  mountType := deviceFS
  if config.Filesystem != "" {
  mountType = config.Filesystem
  }
 
  // if EnsureFilesystem is set, then we need to check if the device has the
  // correct filesystem configured on it. If it does not, format the device
- // with the corect filesystem. Right now, we only support formatting ext4.
- //
- // Do not attempt to reformat if the device has been encrpyted since the
- // encryption process involves reformatting already.
- if config.EnsureFilesystem && !config.Encrypted {
+ // with the corect filesystem. Right now, we only support formatting ext4
+ // and xfs.
+ if config.EnsureFilesystem {
  // compare the actual fs found on the device to the filesystem requested
  if deviceFS != config.Filesystem {
  // re-format device to the correct fs
  switch config.Filesystem {
  case "ext4":
  if err := ext4Format(ctx, source); err != nil {
- return err
+ return fmt.Errorf("ext4 format: %w", err)
+ }
+ case "xfs":
+ if err = xfsFormat(source); err != nil {
+ return fmt.Errorf("xfs format: %w", err)
  }
  default:
  return fmt.Errorf("unsupported filesystem %s requested for device", config.Filesystem)

internal/guest/storage/scsi/scsi_test.go

@@ -31,6 +31,7 @@ func clearTestDependencies() {
  _getDeviceFsType = nil
  _tar2ext4IsDeviceExt4 = nil
  ext4Format = nil
+ xfsFormat = nil
 }
 
 // fakeFileInfo is a mock os.FileInfo that can be used to return
@@ -928,21 +929,30 @@ func Test_Mount_EncryptDevice_Called(t *testing.T) {
  return nil
  }
  encryptDeviceCalled := false
+ expectedCryptTarget := fmt.Sprintf(cryptDeviceFmt, 0, 0, 0)
+ expectedDevicePath := "/dev/mapper/" + expectedCryptTarget
+
  encryptDevice = func(_ context.Context, source string, devName string) (string, error) {
- expectedCryptTarget := fmt.Sprintf(cryptDeviceFmt, 0, 0, 0)
  if devName != expectedCryptTarget {
  t.Fatalf("expected crypt device %q got %q", expectedCryptTarget, devName)
  }
  encryptDeviceCalled = true
- return "", nil
+ return expectedDevicePath, nil
  }
  osStat = osStatNoop
  _getDeviceFsType = getDeviceFsTypeUnknown
 
+ xfsFormat = func(arg string) error {
+ if arg != expectedDevicePath {
+ t.Fatalf("expected args: '%v' got: '%v'", expectedDevicePath, arg)
+ }
+ return nil
+ }
+
  config := &Config{
  Encrypted: true,
  VerityInfo: nil,
- EnsureFilesystem: false,
+ EnsureFilesystem: true,
  Filesystem: "xfs",
  }
  if err := Mount(
@@ -962,6 +972,60 @@ func Test_Mount_EncryptDevice_Called(t *testing.T) {
  }
 }
 
+func Test_Mount_EncryptDevice_Mkfs_Error(t *testing.T) {
+ clearTestDependencies()
+
+ osMkdirAll = func(string, os.FileMode) error {
+ return nil
+ }
+ getDevicePath = func(context.Context, uint8, uint8, uint64) (string, error) {
+ return "", nil
+ }
+ unixMount = func(string, string, string, uintptr, string) error {
+ return nil
+ }
+ expectedCryptTarget := fmt.Sprintf(cryptDeviceFmt, 0, 0, 0)
+ expectedDevicePath := "/dev/mapper/" + expectedCryptTarget
+
+ encryptDevice = func(_ context.Context, source string, devName string) (string, error) {
+ if devName != expectedCryptTarget {
+ t.Fatalf("expected crypt device %q got %q", expectedCryptTarget, devName)
+ }
+ return expectedDevicePath, nil
+ }
+ osStat = osStatNoop
+ _getDeviceFsType = getDeviceFsTypeUnknown
+
+ xfsFormat = func(arg string) error {
+ if arg != expectedDevicePath {
+ t.Fatalf("expected args: '%v' got: '%v'", expectedDevicePath, arg)
+ }
+ return fmt.Errorf("fake error")
+ }
+ osRemoveAll = func(string) error {
+ return nil
+ }
+
+ config := &Config{
+ Encrypted: true,
+ VerityInfo: nil,
+ EnsureFilesystem: true,
+ Filesystem: "xfs",
+ }
+ if err := Mount(
+ context.Background(),
+ 0,
+ 0,
+ 0,
+ "/fake/path",
+ false,
+ nil,
+ config,
+ ); err == nil {
+ t.Fatalf("expected to fail")
+ }
+}
+
 func Test_Mount_RemoveAllCalled_When_EncryptDevice_Fails(t *testing.T) {
  clearTestDependencies()
 
@@ -986,10 +1050,14 @@ func Test_Mount_RemoveAllCalled_When_EncryptDevice_Fails(t *testing.T) {
  osStat = osStatNoop
  _getDeviceFsType = getDeviceFsTypeUnknown
 
+ xfsFormat = func(arg string) error {
+ return nil
+ }
+
  config := &Config{
  Encrypted: true,
  VerityInfo: nil,
- EnsureFilesystem: false,
+ EnsureFilesystem: true,
  Filesystem: "xfs",
  }
  err := Mount(

internal/layers/layers.go

@@ -126,8 +126,8 @@ func MountLCOWLayers(ctx context.Context, containerID string, layers *LCOWLayers
  mConfig := &scsi.MountConfig{
  Encrypted: vm.ScratchEncryptionEnabled(),
  Verity: scsi.ReadVerityInfo(ctx, hostPath),
- // For scratch disks, we want to make sure that the disk is formatted
- // with a filesystem.
+ // For scratch disks, we support formatting the disk if it is not already
+ // formatted.
  EnsureFileystem: true,
  Filesystem: "ext4",
  }

internal/uvm/scsi/mount.go

@@ -44,11 +44,12 @@ type MountConfig struct {
  Encrypted bool
  Verity *guestresource.DeviceVerityInfo
  Options []string
- // EnsureFilesystem indicates if we should check that a mount has the
- // target `Filesystem` FS formatted.
+ // EnsureFilesystem indicates to format the mount as `Filesystem`
+ // if it is not already formatted with that fs type.
  // This is only supported for LCOW.
  EnsureFileystem bool
- // Filesystem is the target filesystem a mount should be mounted as.
+ // Filesystem is the target filesystem that a device will be
+ // mounted as.
  // This is only supported for LCOW.
  Filesystem string
 }