From 91b64c58b128a8268fdbb3f03b2f238766be55cf Mon Sep 17 00:00:00 2001 From: Alakesh Haloi Date: Wed, 7 Jul 2021 15:06:18 -0700 Subject: [PATCH] add xfs support to devicemapper snapshotter ext4 file system was supported before. This adds support for xfs as well. Containerd config file can have fs_type as an additional option with possible values as "xfs" and "ext4" for now. In future other fstype support can be added. A snapshot created from a committed snapshot inherits the file system type of the parent. Any new snapshots that has no parent is created with the file system type indicated in config. If there is no config for file system type is found, then ext4 is assumed. This allows users to use xfs as an optional file system type. Signed-off-by: Alakesh Haloi --- snapshots/devmapper/config.go | 17 ++++ snapshots/devmapper/config_test.go | 10 ++- snapshots/devmapper/snapshotter.go | 106 +++++++++++++++++++----- snapshots/devmapper/snapshotter_test.go | 10 ++- 4 files changed, 117 insertions(+), 26 deletions(-) diff --git a/snapshots/devmapper/config.go b/snapshots/devmapper/config.go index 3d9558dd8..3c491df99 100644 --- a/snapshots/devmapper/config.go +++ b/snapshots/devmapper/config.go @@ -47,6 +47,9 @@ type Config struct { // Whether to discard blocks when removing a thin device. DiscardBlocks bool `toml:"discard_blocks"` + + // Defines file system to use for snapshout device mount. Defaults to "ext4" + FileSystemType fsType `toml:"fs_type"` } // LoadConfig reads devmapper configuration file from disk in TOML format @@ -86,6 +89,10 @@ func (c *Config) parse() error { return errors.Wrapf(err, "failed to parse base image size: '%s'", c.BaseImageSize) } + if c.FileSystemType == "" { + c.FileSystemType = fsTypeExt4 + } + c.BaseImageSizeBytes = uint64(baseImageSize) return nil } @@ -106,5 +113,15 @@ func (c *Config) Validate() error { result = multierror.Append(result, fmt.Errorf("base_image_size is required")) } + if c.FileSystemType != "" { + switch c.FileSystemType { + case fsTypeExt4, fsTypeXFS: + default: + result = multierror.Append(result, fmt.Errorf("unsupported Filesystem Type: %q", c.FileSystemType)) + } + } else { + result = multierror.Append(result, fmt.Errorf("filesystem type cannot be empty")) + } + return result.ErrorOrNil() } diff --git a/snapshots/devmapper/config_test.go b/snapshots/devmapper/config_test.go index a7195675f..80ba5859d 100644 --- a/snapshots/devmapper/config_test.go +++ b/snapshots/devmapper/config_test.go @@ -85,18 +85,20 @@ func TestFieldValidation(t *testing.T) { assert.Assert(t, err != nil) multErr := (err).(*multierror.Error) - assert.Assert(t, is.Len(multErr.Errors, 3)) + assert.Assert(t, is.Len(multErr.Errors, 4)) assert.Assert(t, multErr.Errors[0] != nil, "pool_name is empty") assert.Assert(t, multErr.Errors[1] != nil, "root_path is empty") assert.Assert(t, multErr.Errors[2] != nil, "base_image_size is empty") + assert.Assert(t, multErr.Errors[3] != nil, "filesystem type cannot be empty") } func TestExistingPoolFieldValidation(t *testing.T) { config := &Config{ - PoolName: "test", - RootPath: "test", - BaseImageSize: "10mb", + PoolName: "test", + RootPath: "test", + BaseImageSize: "10mb", + FileSystemType: "ext4", } err := config.Validate() diff --git a/snapshots/devmapper/snapshotter.go b/snapshots/devmapper/snapshotter.go index 35845cad5..d5d628f65 100644 --- a/snapshots/devmapper/snapshotter.go +++ b/snapshots/devmapper/snapshotter.go @@ -39,9 +39,13 @@ import ( exec "golang.org/x/sys/execabs" ) +type fsType string + const ( - metadataFileName = "metadata.db" - fsTypeExt4 = "ext4" + metadataFileName = "metadata.db" + fsTypeExt4 fsType = "ext4" + fsTypeXFS fsType = "xfs" + devmapperSnapshotFsType = "containerd.io/snapshot/devmapper/fstype" ) type closeFunc func() error @@ -183,7 +187,13 @@ func (s *Snapshotter) Mounts(ctx context.Context, key string) ([]mount.Mount, er return err }) - return s.buildMounts(snap), nil + snapInfo, err := s.Stat(ctx, key) + if err != nil { + log.G(ctx).WithError(err).Errorf("cannot retrieve snapshot info for key %s", key) + return nil, err + } + + return s.buildMounts(ctx, snap, fsType(snapInfo.Labels[devmapperSnapshotFsType])), nil } // Prepare creates thin device for an active snapshot identified by key @@ -227,7 +237,7 @@ func (s *Snapshotter) Commit(ctx context.Context, name, key string, opts ...snap log.G(ctx).WithFields(logrus.Fields{"name": name, "key": key}).Debug("commit") return s.withTransaction(ctx, true, func(ctx context.Context) error { - id, _, _, err := storage.GetInfo(ctx, key) + id, snapInfo, _, err := storage.GetInfo(ctx, key) if err != nil { return err } @@ -242,6 +252,15 @@ func (s *Snapshotter) Commit(ctx context.Context, name, key string, opts ...snap Size: size, } + // Add file system type label if present. In case more than one file system + // type is supported file system type from parent will be used for creating + // snapshot. + fsTypeActive := snapInfo.Labels[devmapperSnapshotFsType] + if fsTypeActive != "" { + fsLabel := make(map[string]string) + fsLabel[devmapperSnapshotFsType] = fsTypeActive + opts = append(opts, snapshots.WithLabels(fsLabel)) + } _, err = storage.CommitActive(ctx, key, name, usage, opts...) if err != nil { return err @@ -351,6 +370,33 @@ func (s *Snapshotter) Close() error { } func (s *Snapshotter) createSnapshot(ctx context.Context, kind snapshots.Kind, key, parent string, opts ...snapshots.Opt) ([]mount.Mount, error) { + var fileSystemType fsType + + // For snapshots with no parents, we use file system type as configured in config. + // For snapshots with parents, we inherit the file system type. We use the same + // file system type derived here for building mount points later. + fsLabel := make(map[string]string) + if len(parent) == 0 { + fileSystemType = s.config.FileSystemType + } else { + _, snapInfo, _, err := storage.GetInfo(ctx, parent) + if err != nil { + log.G(ctx).Errorf("failed to read snapshotInfo for %s", parent) + return nil, err + } + fileSystemType = fsType(snapInfo.Labels[devmapperSnapshotFsType]) + if fileSystemType == "" { + // For parent snapshots created without label support, we can assume that + // they are ext4 type. Children of parents with no label for fsType will + // now have correct label and committed snapshots from them will carry fs type + // label. TODO: find out if it is better to update the parent's label with + // fsType as ext4. + fileSystemType = fsTypeExt4 + } + } + fsLabel[devmapperSnapshotFsType] = string(fileSystemType) + opts = append(opts, snapshots.WithLabels(fsLabel)) + snap, err := storage.CreateSnapshot(ctx, kind, key, parent, opts...) if err != nil { return nil, err @@ -366,7 +412,7 @@ func (s *Snapshotter) createSnapshot(ctx context.Context, kind snapshots.Kind, k return nil, err } - if err := mkfs(ctx, dmsetup.GetFullDevicePath(deviceName)); err != nil { + if err := mkfs(ctx, s.config.FileSystemType, dmsetup.GetFullDevicePath(deviceName)); err != nil { status, sErr := dmsetup.Status(s.pool.poolName) if sErr != nil { multierror.Append(err, sErr) @@ -380,16 +426,17 @@ func (s *Snapshotter) createSnapshot(ctx context.Context, kind snapshots.Kind, k } else { parentDeviceName := s.getDeviceName(snap.ParentIDs[0]) snapDeviceName := s.getDeviceName(snap.ID) - log.G(ctx).Debugf("creating snapshot device '%s' from '%s'", snapDeviceName, parentDeviceName) - err := s.pool.CreateSnapshotDevice(ctx, parentDeviceName, snapDeviceName, s.config.BaseImageSizeBytes) + log.G(ctx).Debugf("creating snapshot device '%s' from '%s' with fsType: '%s'", snapDeviceName, parentDeviceName, fileSystemType) + + err = s.pool.CreateSnapshotDevice(ctx, parentDeviceName, snapDeviceName, s.config.BaseImageSizeBytes) if err != nil { log.G(ctx).WithError(err).Errorf("failed to create snapshot device from parent %s", parentDeviceName) return nil, err } } - mounts := s.buildMounts(snap) + mounts := s.buildMounts(ctx, snap, fileSystemType) // Remove default directories not expected by the container image _ = mount.WithTempMount(ctx, mounts, func(root string) error { @@ -399,20 +446,35 @@ func (s *Snapshotter) createSnapshot(ctx context.Context, kind snapshots.Kind, k return mounts, nil } -// mkfs creates ext4 filesystem on the given devmapper device -func mkfs(ctx context.Context, path string) error { - args := []string{ - "-E", - // We don't want any zeroing in advance when running mkfs on thin devices (see "man mkfs.ext4") - "nodiscard,lazy_itable_init=0,lazy_journal_init=0", - path, +// mkfs creates filesystem on the given devmapper device based on type +// specified in config. +func mkfs(ctx context.Context, fs fsType, path string) error { + mkfsCommand := "" + var args []string + + switch fs { + case fsTypeExt4: + mkfsCommand = "mkfs.ext4" + args = []string{ + "-E", + // We don't want any zeroing in advance when running mkfs on thin devices (see "man mkfs.ext4") + "nodiscard,lazy_itable_init=0,lazy_journal_init=0", + path, + } + case fsTypeXFS: + mkfsCommand = "mkfs.xfs" + args = []string{ + path, + } + default: + return errors.New("file system not supported") } - log.G(ctx).Debugf("mkfs.ext4 %s", strings.Join(args, " ")) - b, err := exec.Command("mkfs.ext4", args...).CombinedOutput() + log.G(ctx).Debugf("%s %s", mkfsCommand, strings.Join(args, " ")) + b, err := exec.Command(mkfsCommand, args...).CombinedOutput() out := string(b) if err != nil { - return errors.Wrapf(err, "mkfs.ext4 couldn't initialize %q: %s", path, out) + return errors.Wrapf(err, "%s couldn't initialize %q: %s", mkfsCommand, path, out) } log.G(ctx).Debugf("mkfs:\n%s", out) @@ -429,9 +491,13 @@ func (s *Snapshotter) getDevicePath(snap storage.Snapshot) string { return dmsetup.GetFullDevicePath(name) } -func (s *Snapshotter) buildMounts(snap storage.Snapshot) []mount.Mount { +func (s *Snapshotter) buildMounts(ctx context.Context, snap storage.Snapshot, fileSystemType fsType) []mount.Mount { var options []string + if fileSystemType == "" { + log.G(ctx).Error("File system type cannot be empty") + return nil + } if snap.Kind != snapshots.KindActive { options = append(options, "ro") } @@ -439,7 +505,7 @@ func (s *Snapshotter) buildMounts(snap storage.Snapshot) []mount.Mount { mounts := []mount.Mount{ { Source: s.getDevicePath(snap), - Type: fsTypeExt4, + Type: string(fileSystemType), Options: options, }, } diff --git a/snapshots/devmapper/snapshotter_test.go b/snapshots/devmapper/snapshotter_test.go index e89503009..d60430c61 100644 --- a/snapshots/devmapper/snapshotter_test.go +++ b/snapshots/devmapper/snapshotter_test.go @@ -141,8 +141,14 @@ func testUsage(t *testing.T, snapshotter snapshots.Snapshotter) { "%d > %d", layer2Usage.Size, sizeBytes) } -func TestMkfs(t *testing.T) { +func TestMkfsExt4(t *testing.T) { ctx := context.Background() - err := mkfs(ctx, "") + err := mkfs(ctx, "ext4", "") assert.ErrorContains(t, err, `mkfs.ext4 couldn't initialize ""`) } + +func TestMkfsXfs(t *testing.T) { + ctx := context.Background() + err := mkfs(ctx, "xfs", "") + assert.ErrorContains(t, err, `mkfs.xfs couldn't initialize ""`) +}