Repository: firecracker-microvm/firecracker-containerd
Pull request #73 (2019-01-23 22:47:53)

Support container-storage-setup tool 
*Description of changes:*
This makes `devmapper_snapshotter` compatible with `container-storage-setup` tool and Docker's `dm.*` flags. See `README` for more details.

relates to #15 

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.



Patch:
diff --git a/snapshotter/cmd/devmapper/README.md b/snapshotter/cmd/devmapper/README.md
index 8f2be17..e4e49d6 100644
--- a/snapshotter/cmd/devmapper/README.md
+++ b/snapshotter/cmd/devmapper/README.md
@@ -62,3 +62,73 @@ Now you can use snapshotter with containerd:
 ```
 CONTAINERD_SNAPSHOTTER=firecracker-dm-snapshotter ctr images pull docker.io/library/alpine:latest
 ```
+
+## Using container storage setup tool
+
+The device mapper snapshotter is compatible with `container_storage_setup` [utility](https://github.com/projectatomic/container-storage-setup)
+(formerly known as `docker-storage-setup`). `container-storage-setup` is a script to
+configure COW file systems like devicemapper and overlayfs. It is usually run via a systemd service.
+
+The snapshotter supports a subset of `dm.*` command line flags generated by `container_storage_setup`.
+
+Here is a quick tutorial how it can be used togather.
+
+- Run an EC2 instance with additional EBS volume `/dev/sdb` (this example uses CentOS 7)
+- Install container storage setup tool
+
+  ```bash
+  $ yum update -y
+  $ yum install -y container-storage-setup
+  ```
+
+- Create thin-pool device using `container-storage-setup` tool
+
+  ```bash
+  $ touch /etc/sysconfig/docker-storage-setup
+  $ echo DEVS=/dev/xvdb >> /etc/sysconfig/docker-storage-setup
+  $ echo VG=test >> /etc/sysconfig/docker-storage-setup
+  $ container-storage-setup
+  INFO: Volume group backing root filesystem could not be determined
+  INFO: Writing zeros to first 4MB of device /dev/xvdb
+  4+0 records in
+  4+0 records out
+  4194304 bytes (4.2 MB) copied, 0.0162157 s, 259 MB/s
+  INFO: Device node /dev/xvdb1 exists.
+    Physical volume "/dev/xvdb1" successfully created.
+    Volume group "test" successfully created
+    Rounding up size to full physical extent 20.00 MiB
+    Thin pool volume with chunk size 512.00 KiB can address at most 126.50 TiB of data.
+    Logical volume "docker-pool" created.
+    Logical volume test/docker-pool changed.
+  ```
+
+  Run `man container-storage-setup` to get more information how to setup storage for container runtimes.
+
+- `container-storage-setup` creates a runtime storage file which looks as follows:
+
+  ```bash
+  $ cat /etc/sysconfig/docker-storage
+  DOCKER_STORAGE_OPTIONS="--storage-driver devicemapper --storage-opt dm.fs=xfs --storage-opt dm.thinpooldev=/dev/mapper/test-docker--pool "
+  ```
+
+- Now you can create a service script to run the device mapper snapshotter
+
+  ```
+  EnvironmentFile=/etc/sysconfig/docker-storage
+  ...
+  ExecStart=./devmapper_snapshotter -address /var/run/firecracker-dm-snapshotter.sock -debug $DOCKER_STORAGE_OPTIONS
+  ...
+  ```
+
+  The output should look like:
+  ```
+  INFO[0000] loaded configuration file "/etc/containerd/devmapper-snapshotter.json"
+  INFO[0000] applying storage opt: dm.fs=xfs
+  WARN[0000] "xfs" not supported, defaulting to ext4
+  INFO[0000] applying storage opt: dm.thinpooldev=/dev/mapper/test-docker--pool
+  INFO[0000] initializing pool device "test-docker--pool"
+  INFO[0000] using dmsetup:
+  Library version:   1.02.110 (2015-10-30)
+  Driver version:    4.37.0
+  INFO[0000] running gRPC server                           unix_addr=/var/run/firecracker-dm-snapshotter.sock
+  ```
\ No newline at end of file
diff --git a/snapshotter/cmd/devmapper/flag.go b/snapshotter/cmd/devmapper/flag.go
new file mode 100644
index 0000000..70906c6
--- /dev/null
+++ b/snapshotter/cmd/devmapper/flag.go
@@ -0,0 +1,66 @@
+// Copyright 2018 Amazon.com, Inc. or its affiliates. All Rights Reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License"). You may
+// not use this file except in compliance with the License. A copy of the
+// License is located at
+//
+//	http://aws.amazon.com/apache2.0/
+//
+// or in the "license" file accompanying this file. This file is distributed
+// on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either
+// express or implied. See the License for the specific language governing
+// permissions and limitations under the License.
+
+package main
+
+import (
+	"fmt"
+	"os"
+	"strings"
+)
+
+// parseKeyValueOpt splits a string to key=value
+func parseKeyValueOpt(opt string) (string, string, error) {
+	pair := strings.SplitN(opt, "=", 2)
+	if len(pair) != 2 {
+		return "", "", fmt.Errorf("failed to split option: %q", opt)
+	}
+
+	key := strings.TrimSpace(pair[0])
+	if key == "" {
+		return "", "", fmt.Errorf("option %q has no key", opt)
+	}
+
+	value := strings.TrimSpace(pair[1])
+
+	return key, value, nil
+}
+
+// visitKeyValueOpts finds all --optName key=value in command line string and invokes callback for each pair
+func visitKeyValueOpts(optName string, optFn func(key, value string) error) error {
+	// Nothing to visit
+	if len(os.Args) < 3 {
+		return nil
+	}
+
+	// Skip exec path
+	args := os.Args[1:]
+
+	for i := 0; i < len(args)-1; i++ {
+		if optName != args[i] {
+			continue
+		}
+
+		// Next element must be key=value pair
+		key, value, err := parseKeyValueOpt(args[i+1])
+		if err != nil {
+			return err
+		}
+
+		if err := optFn(key, value); err != nil {
+			return err
+		}
+	}
+
+	return nil
+}
diff --git a/snapshotter/cmd/devmapper/main.go b/snapshotter/cmd/devmapper/main.go
index 67fe538..9886607 100644
--- a/snapshotter/cmd/devmapper/main.go
+++ b/snapshotter/cmd/devmapper/main.go
@@ -17,11 +17,15 @@ import (
 	"context"
 	"flag"
 	"os"
+	"strings"
 
+	"github.com/containerd/containerd/log"
 	"github.com/containerd/containerd/snapshots"
+	"github.com/docker/go-units"
 
 	"github.com/firecracker-microvm/firecracker-containerd/snapshotter"
 	"github.com/firecracker-microvm/firecracker-containerd/snapshotter/devmapper"
+	"github.com/firecracker-microvm/firecracker-containerd/snapshotter/pkg/dmsetup"
 )
 
 const (
@@ -30,10 +34,52 @@ const (
 )
 
 func main() {
-	var configPath string
+	var (
+		ctx        = context.Background()
+		config     = &devmapper.Config{}
+		configPath = ""
+		rootPath   = ""
+	)
+
 	flag.StringVar(&configPath, "config", "", "Path to devmapper configuration file")
+	flag.StringVar(&rootPath, "path", "", "Path to snapshotter data")
+
+	// These flags are needed for compatibility with container-storage-setup tool as there is no way to
+	// ignore unknown flags in `flag` package. Storage options are parsed using visitKeyValueOpts func.
+	flag.String("storage-driver", "devicemapper", "Storage driver to use. Always devicemapper.")
+	flag.Bool("storage-opt", false, "Storage configuration options (compatible with Docker dm.* flags)")
+
 	flag.Parse()
 
+	// Try load file from disk
+	if cfg, err := loadConfig(ctx, configPath); err == nil {
+		config = cfg
+	} else if err != os.ErrNotExist {
+		log.G(ctx).WithError(err).Fatal("failed to load config file")
+	}
+
+	if rootPath != "" {
+		config.RootPath = rootPath
+	}
+
+	// Append and/or overwrite file configuration with --storage-opt dm.XXX=YYY command line flags
+	if err := visitKeyValueOpts("--storage-opt", func(key, value string) error {
+		return applyStorageOpt(ctx, key, value, config)
+	}); err != nil {
+		log.G(ctx).WithError(err).Fatal("failed to apply storage options")
+	}
+
+	if err := config.Validate(); err != nil {
+		log.G(ctx).WithError(err).Fatal("invalid configuration")
+	}
+
+	snapshotter.Run(func(ctx context.Context) (snapshots.Snapshotter, error) {
+		return devmapper.NewSnapshotter(ctx, config)
+	})
+}
+
+// loadConfig loads configuration file from disk
+func loadConfig(ctx context.Context, configPath string) (*devmapper.Config, error) {
 	if configPath == "" {
 		configPath = os.Getenv(configPathEnvName)
 	}
@@ -42,7 +88,50 @@ func main() {
 		configPath = defaultConfigPath
 	}
 
-	snapshotter.Run(func(ctx context.Context) (snapshots.Snapshotter, error) {
-		return devmapper.NewSnapshotter(ctx, configPath)
-	})
+	config, err := devmapper.LoadConfig(configPath)
+	if err != nil {
+		return nil, err
+	}
+
+	log.G(ctx).Infof("loaded configuration file %q", configPath)
+	return config, nil
+}
+
+// applyStorageOpt overwrites configuration with --storage-opt command line flags
+func applyStorageOpt(ctx context.Context, key, value string, config *devmapper.Config) error {
+	log.G(ctx).Infof("applying storage opt: %s=%s", key, value)
+
+	switch key {
+	case "dm.basesize":
+		size, err := units.RAMInBytes(value)
+		if err != nil {
+			return err
+		}
+
+		config.BaseImageSize = value
+		config.BaseImageSizeBytes = uint64(size)
+	case "dm.metadatadev":
+		config.MetadataDevice = value
+	case "dm.datadev":
+		config.DataDevice = value
+	case "dm.thinpooldev":
+		config.PoolName = strings.TrimPrefix(value, dmsetup.DevMapperDir)
+	case "dm.blocksize":
+		size, err := units.RAMInBytes(value)
+		if err != nil {
+			return err
+		}
+
+		config.DataBlockSize = value
+		config.DataBlockSizeSectors = uint32(size / dmsetup.SectorSize)
+	case "dm.fs":
+		// TODO: Support alternative file systems (https://github.com/firecracker-microvm/firecracker-containerd/issues/44)
+		if value != "ext4" {
+			log.G(ctx).Warnf("%q not supported, defaulting to ext4", value)
+		}
+	default:
+		log.G(ctx).Warnf("ignoring unsupported flag %q", key)
+	}
+
+	return nil
 }
diff --git a/snapshotter/devmapper/config.go b/snapshotter/devmapper/config.go
index 4006d05..a343759 100644
--- a/snapshotter/devmapper/config.go
+++ b/snapshotter/devmapper/config.go
@@ -15,7 +15,9 @@ package devmapper
 
 import (
 	"encoding/json"
+	"fmt"
 	"io/ioutil"
+	"os"
 
 	"github.com/docker/go-units"
 	"github.com/hashicorp/go-multierror"
@@ -64,6 +66,14 @@ type Config struct {
 
 // LoadConfig reads devmapper configuration file JSON format from disk
 func LoadConfig(path string) (*Config, error) {
+	if _, err := os.Stat(path); err != nil {
+		if os.IsNotExist(err) {
+			return nil, os.ErrNotExist
+		}
+
+		return nil, err
+	}
+
 	data, err := ioutil.ReadFile(path)
 	if err != nil {
 		return nil, errors.Wrap(err, "failed to read file")
@@ -78,7 +88,7 @@ func LoadConfig(path string) (*Config, error) {
 		return nil, err
 	}
 
-	if err := config.validate(); err != nil {
+	if err := config.Validate(); err != nil {
 		return nil, err
 	}
 
@@ -88,10 +98,12 @@ func LoadConfig(path string) (*Config, error) {
 func (c *Config) parse() error {
 	var result *multierror.Error
 
-	if blockSize, err := units.RAMInBytes(c.DataBlockSize); err != nil {
-		result = multierror.Append(result, errors.Wrapf(err, "failed to parse data block size: %q", c.DataBlockSize))
-	} else {
-		c.DataBlockSizeSectors = uint32(blockSize / dmsetup.SectorSize)
+	if c.DataBlockSize != "" {
+		if blockSize, err := units.RAMInBytes(c.DataBlockSize); err != nil {
+			result = multierror.Append(result, errors.Wrapf(err, "failed to parse data block size: %q", c.DataBlockSize))
+		} else {
+			c.DataBlockSizeSectors = uint32(blockSize / dmsetup.SectorSize)
+		}
 	}
 
 	if baseImageSize, err := units.RAMInBytes(c.BaseImageSize); err != nil {
@@ -103,31 +115,47 @@ func (c *Config) parse() error {
 	return result.ErrorOrNil()
 }
 
-func (c *Config) validate() error {
+// Validate makes sure configuration fields are valid
+func (c *Config) Validate() error {
 	var result *multierror.Error
 
-	strChecks := []struct {
-		field string
-		name  string
-	}{
-		{c.PoolName, "pool_name"},
-		{c.RootPath, "root_path"},
-		{c.DataDevice, "data_device"},
-		{c.MetadataDevice, "meta_device"},
+	if c.PoolName == "" {
+		result = multierror.Append(result, fmt.Errorf("pool_name is required"))
 	}
 
-	for _, check := range strChecks {
-		if check.field == "" {
-			result = multierror.Append(result, errors.Errorf("%s is empty", check.name))
-		}
+	if c.RootPath == "" {
+		result = multierror.Append(result, fmt.Errorf("root_path is required"))
 	}
 
-	if c.DataBlockSizeSectors < dataBlockMinSize || c.DataBlockSizeSectors > dataBlockMaxSize {
-		result = multierror.Append(result, errInvalidBlockSize)
+	if c.BaseImageSize == "" {
+		result = multierror.Append(result, fmt.Errorf("base_image_size is required"))
 	}
 
-	if c.DataBlockSizeSectors%dataBlockMinSize != 0 {
-		result = multierror.Append(result, errInvalidBlockAlignment)
+	// The following fields are required only if we want to create or reload pool.
+	// Otherwise existing pool with 'PoolName' (prepared in advance) can be used by snapshotter.
+	if c.DataDevice != "" || c.MetadataDevice != "" || c.DataBlockSize != "" || c.DataBlockSizeSectors != 0 {
+		strChecks := []struct {
+			field string
+			name  string
+		}{
+			{c.DataDevice, "data_device"},
+			{c.MetadataDevice, "meta_device"},
+			{c.DataBlockSize, "data_block_size"},
+		}
+
+		for _, check := range strChecks {
+			if check.field == "" {
+				result = multierror.Append(result, errors.Errorf("%s is empty", check.name))
+			}
+		}
+
+		if c.DataBlockSizeSectors < dataBlockMinSize || c.DataBlockSizeSectors > dataBlockMaxSize {
+			result = multierror.Append(result, errInvalidBlockSize)
+		}
+
+		if c.DataBlockSizeSectors%dataBlockMinSize != 0 {
+			result = multierror.Append(result, errInvalidBlockAlignment)
+		}
 	}
 
 	return result.ErrorOrNil()
diff --git a/snapshotter/devmapper/pool_device.go b/snapshotter/devmapper/pool_device.go
index 8e77885..974f190 100644
--- a/snapshotter/devmapper/pool_device.go
+++ b/snapshotter/devmapper/pool_device.go
@@ -42,7 +42,7 @@ func NewPoolDevice(ctx context.Context, config *Config) (*PoolDevice, error) {
 		return nil, err
 	}
 
-	log.G(ctx).Infof("using dmsetup: %s", version)
+	log.G(ctx).Infof("using dmsetup:\n%s", version)
 
 	dbpath := filepath.Join(config.RootPath, config.PoolName+".db")
 	poolMetaStore, err := NewPoolMetadata(dbpath)
@@ -50,21 +50,8 @@ func NewPoolDevice(ctx context.Context, config *Config) (*PoolDevice, error) {
 		return nil, err
 	}
 
-	poolPath := dmsetup.GetFullDevicePath(config.PoolName)
-	if _, err := os.Stat(poolPath); err == nil {
-		log.G(ctx).Debugf("reloading existing pool %q", poolPath)
-		if err := dmsetup.ReloadPool(config.PoolName, config.DataDevice, config.MetadataDevice, config.DataBlockSizeSectors); err != nil {
-			return nil, errors.Wrapf(err, "failed to reload pool %q", config.PoolName)
-		}
-	} else {
-		if !os.IsNotExist(err) {
-			return nil, errors.Wrapf(err, "failed to stat for %q", poolPath)
-		}
-
-		log.G(ctx).Debug("creating new pool device")
-		if err := dmsetup.CreatePool(config.PoolName, config.DataDevice, config.MetadataDevice, config.DataBlockSizeSectors); err != nil {
-			return nil, errors.Wrapf(err, "failed to create thin-pool with name %q", config.PoolName)
-		}
+	if err := openPool(ctx, config); err != nil {
+		return nil, err
 	}
 
 	return &PoolDevice{
@@ -73,6 +60,50 @@ func NewPoolDevice(ctx context.Context, config *Config) (*PoolDevice, error) {
 	}, nil
 }
 
+func openPool(ctx context.Context, config *Config) error {
+	if err := config.Validate(); err != nil {
+		return err
+	}
+
+	var (
+		poolPath   = dmsetup.GetFullDevicePath(config.PoolName)
+		poolExists = false
+	)
+
+	if _, err := os.Stat(poolPath); err != nil && !os.IsNotExist(err) {
+		return errors.Wrapf(err, "failed to stat for %q", poolPath)
+	} else if err == nil {
+		poolExists = true
+	}
+
+	// Create new pool if not exists
+	if !poolExists {
+		log.G(ctx).Debug("creating new pool device")
+		if err := dmsetup.CreatePool(config.PoolName, config.DataDevice, config.MetadataDevice, config.DataBlockSizeSectors); err != nil {
+			return errors.Wrapf(err, "failed to create thin-pool with name %q", config.PoolName)
+		}
+
+		return nil
+	}
+
+	// Pool exists, check if it needs to be reloaded
+	if config.DataDevice != "" && config.MetadataDevice != "" {
+		log.G(ctx).Debugf("reloading existing pool %q", poolPath)
+		if err := dmsetup.ReloadPool(config.PoolName, config.DataDevice, config.MetadataDevice, config.DataBlockSizeSectors); err != nil {
+			return errors.Wrapf(err, "failed to reload pool %q", config.PoolName)
+		}
+
+		return nil
+	}
+
+	// If data and meta devices are not provided, use existing pool. Query info to make sure it's OK.
+	if _, err := dmsetup.Info(poolPath); err != nil {
+		return errors.Wrapf(err, "failed to query info for existing pool %q", poolPath)
+	}
+
+	return nil
+}
+
 // transition invokes 'updateStateFn' callback to perform devmapper operation and reflects device state changes/errors in meta store.
 // 'tryingState' will be set before invoking callback. If callback succeeded 'successState' will be set, otherwise
 // error details will be recorded in meta store.
diff --git a/snapshotter/devmapper/snapshotter.go b/snapshotter/devmapper/snapshotter.go
index d08f474..85a5183 100644
--- a/snapshotter/devmapper/snapshotter.go
+++ b/snapshotter/devmapper/snapshotter.go
@@ -53,16 +53,9 @@ type Snapshotter struct {
 // NewSnapshotter creates new device mapper snapshotter.
 // Internally it creates thin-pool device (or reloads if it's already exists) and
 // initializes a database file for metadata.
-func NewSnapshotter(ctx context.Context, configPath string) (*Snapshotter, error) {
-	log.G(ctx).WithField("config-path", configPath).Info("creating devmapper snapshotter")
-
+func NewSnapshotter(ctx context.Context, config *Config) (*Snapshotter, error) {
 	var cleanupFn []closeFunc
 
-	config, err := LoadConfig(configPath)
-	if err != nil {
-		return nil, err
-	}
-
 	if err := os.MkdirAll(config.RootPath, 0750); err != nil && !os.IsExist(err) {
 		return nil, errors.Wrapf(err, "failed to create root directory: %s", config.RootPath)
 	}
diff --git a/snapshotter/naive/naive.go b/snapshotter/naive/naive.go
index 3d50c95..41f106c 100644
--- a/snapshotter/naive/naive.go
+++ b/snapshotter/naive/naive.go
@@ -36,6 +36,7 @@ const (
 	imageDirName      = "images"
 	imageFSType       = "ext4"
 	sparseImageSizeMB = 1024
+	mib               = 1048576
 )
 
 // Snapshotter implements naive snapshotter for containerd
@@ -302,7 +303,7 @@ func (s *Snapshotter) createImage(ctx context.Context, imagePath string, fileSiz
 		return err
 	}
 
-	if err := file.Truncate(int64(fileSizeMB) * 1048576); err != nil {
+	if err := file.Truncate(int64(fileSizeMB) * mib); err != nil {
 		return err
 	}
 
diff --git a/snapshotter/pkg/dmsetup/dmsetup.go b/snapshotter/pkg/dmsetup/dmsetup.go
index d177a17..55129a2 100644
--- a/snapshotter/pkg/dmsetup/dmsetup.go
+++ b/snapshotter/pkg/dmsetup/dmsetup.go
@@ -24,7 +24,8 @@ import (
 )
 
 const (
-	devMapperDir = "/dev/mapper/"
+	// DevMapperDir represents devmapper devices location
+	DevMapperDir = "/dev/mapper/"
 	// SectorSize represents the number of bytes in one sector on devmapper devices
 	SectorSize = 512
 )
@@ -259,11 +260,11 @@ func Version() (string, error) {
 
 // GetFullDevicePath returns full path for the given device name (like "/dev/mapper/name")
 func GetFullDevicePath(deviceName string) string {
-	if strings.HasPrefix(deviceName, devMapperDir) {
+	if strings.HasPrefix(deviceName, DevMapperDir) {
 		return deviceName
 	}
 
-	return devMapperDir + deviceName
+	return DevMapperDir + deviceName
 }
 
 // BlockDeviceSize returns size of block device in bytes


Test patch:
diff --git a/snapshotter/cmd/devmapper/flag_test.go b/snapshotter/cmd/devmapper/flag_test.go
new file mode 100644
index 0000000..fb4591c
--- /dev/null
+++ b/snapshotter/cmd/devmapper/flag_test.go
@@ -0,0 +1,130 @@
+// Copyright 2018 Amazon.com, Inc. or its affiliates. All Rights Reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License"). You may
+// not use this file except in compliance with the License. A copy of the
+// License is located at
+//
+//	http://aws.amazon.com/apache2.0/
+//
+// or in the "license" file accompanying this file. This file is distributed
+// on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either
+// express or implied. See the License for the specific language governing
+// permissions and limitations under the License.
+
+package main
+
+import (
+	"errors"
+	"os"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestParseKeyValueOpt(t *testing.T) {
+	key, value, err := parseKeyValueOpt("dm.baseSize=1")
+	assert.NoError(t, err)
+	assert.Equal(t, "dm.baseSize", key)
+	assert.Equal(t, "1", value)
+
+	key, value, err = parseKeyValueOpt(" dm.device = /dev/mapper/test ")
+	assert.NoError(t, err)
+	assert.Equal(t, "dm.device", key)
+	assert.Equal(t, "/dev/mapper/test", value)
+
+	key, value, err = parseKeyValueOpt(" dm.device =   ")
+	assert.NoError(t, err)
+	assert.Equal(t, "dm.device", key)
+	assert.Equal(t, "", value)
+}
+
+func TestParseInvalidKeyValueOpt(t *testing.T) {
+	_, _, err := parseKeyValueOpt("dm.baseSize")
+	assert.Error(t, err)
+
+	_, _, err = parseKeyValueOpt("=value")
+	assert.Error(t, err)
+}
+
+func TestVisitKeyValueOpts(t *testing.T) {
+	var (
+		args   = os.Args
+		called = false
+	)
+
+	os.Args = []string{"exec_path", "--opt", "a=b"}
+	err := visitKeyValueOpts("--opt", func(key, value string) error {
+		called = true
+		assert.Equal(t, "a", key)
+		assert.Equal(t, "b", value)
+		return nil
+	})
+
+	assert.NoError(t, err)
+	assert.True(t, called)
+
+	defer func() { os.Args = args }()
+}
+
+func TestVisitKeyValueOptCallbackErr(t *testing.T) {
+	var (
+		args  = os.Args
+		count = 0
+		cbErr = errors.New("test")
+	)
+
+	os.Args = []string{"exec_path", "--opt", "a=b", "--opt", "c=d"}
+	err := visitKeyValueOpts("--opt", func(key, value string) error {
+		count++
+		return cbErr
+	})
+
+	assert.Equal(t, cbErr, err)
+	assert.Equalf(t, 1, count, "callback should be called exactly once")
+
+	defer func() { os.Args = args }()
+}
+
+func TestVisitInvalidOpts(t *testing.T) {
+	args := os.Args
+
+	tests := []struct {
+		name string
+		args []string
+		err  error
+	}{
+		{"Empty args", []string{}, nil},
+		{"Nil", nil, nil},
+		{"Empty string", []string{""}, nil},
+		{"Empty strings", []string{"", "", "", "", "", ""}, nil},
+		{"Just reboot flag", []string{"-reboot"}, nil},
+		{"Single opt flag", []string{"--opt"}, nil},
+		{"Opt flag at end", []string{"--type", "devmapper", "--opt"}, nil},
+		{"Opt flag at start", []string{"--opt", "--type", "devmapper"}, errors.New("failed to split option: \"--type\"")},
+		{"Opt with invalid kv pair", []string{"--opt", "1+1"}, errors.New("failed to split option: \"1+1\"")},
+		{"Opt with empty kv", []string{"--opt", ""}, errors.New("failed to split option: \"\"")},
+	}
+
+	for _, test := range tests {
+		var (
+			expectedErr = test.err
+			args        = test.args
+		)
+
+		t.Run(test.name, func(t *testing.T) {
+			os.Args = append([]string{"exec_path_to_be_skipped"}, args...)
+			err := visitKeyValueOpts("--opt", func(_, _ string) error {
+				assert.Fail(t, "callback should not be called")
+				return nil
+			})
+
+			if expectedErr == nil {
+				assert.NoError(t, err)
+			} else {
+				assert.EqualError(t, err, expectedErr.Error())
+			}
+		})
+	}
+
+	defer func() { os.Args = args }()
+}
diff --git a/snapshotter/cmd/devmapper/main_test.go b/snapshotter/cmd/devmapper/main_test.go
new file mode 100644
index 0000000..824036d
--- /dev/null
+++ b/snapshotter/cmd/devmapper/main_test.go
@@ -0,0 +1,51 @@
+// Copyright 2018 Amazon.com, Inc. or its affiliates. All Rights Reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License"). You may
+// not use this file except in compliance with the License. A copy of the
+// License is located at
+//
+//	http://aws.amazon.com/apache2.0/
+//
+// or in the "license" file accompanying this file. This file is distributed
+// on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either
+// express or implied. See the License for the specific language governing
+// permissions and limitations under the License.
+
+package main
+
+import (
+	"context"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+
+	"github.com/firecracker-microvm/firecracker-containerd/snapshotter/devmapper"
+)
+
+func TestApplyStorageOpt(t *testing.T) {
+	var (
+		config = &devmapper.Config{}
+		ctx    = context.Background()
+	)
+
+	opts := map[string]string{
+		"dm.basesize":    "10gb",
+		"dm.metadatadev": "/meta_dev",
+		"dm.datadev":     "/data_dev",
+		"dm.thinpooldev": "/pool_dev",
+		"dm.blocksize":   "100mb",
+	}
+
+	for key, value := range opts {
+		err := applyStorageOpt(ctx, key, value, config)
+		assert.NoErrorf(t, err, "failed to apply opt %q", key)
+	}
+
+	assert.Equal(t, "10gb", config.BaseImageSize)
+	assert.EqualValues(t, 10*1024*1024*1024, config.BaseImageSizeBytes)
+	assert.Equal(t, "/meta_dev", config.MetadataDevice)
+	assert.Equal(t, "/data_dev", config.DataDevice)
+	assert.Equal(t, "/pool_dev", config.PoolName)
+	assert.Equal(t, "100mb", config.DataBlockSize)
+	assert.EqualValues(t, 100*1024*1024/512, config.DataBlockSizeSectors)
+}
diff --git a/snapshotter/devmapper/config_test.go b/snapshotter/devmapper/config_test.go
index 4be4608..998add3 100644
--- a/snapshotter/devmapper/config_test.go
+++ b/snapshotter/devmapper/config_test.go
@@ -68,7 +68,7 @@ func TestLoadConfig(t *testing.T) {
 
 func TestLoadConfigInvalidPath(t *testing.T) {
 	_, err := LoadConfig("")
-	require.Error(t, err)
+	require.Equal(t, os.ErrNotExist, err)
 
 	_, err = LoadConfig("/dev/null")
 	require.Error(t, err)
@@ -91,18 +91,31 @@ func TestParseInvalidData(t *testing.T) {
 }
 
 func TestFieldValidation(t *testing.T) {
-	config := Config{DataBlockSizeSectors: 1}
-	err := config.validate()
+	config := &Config{DataBlockSizeSectors: 1}
+	err := config.Validate()
 	require.Error(t, err)
 
 	multErr := (err).(*multierror.Error)
-	require.Len(t, multErr.Errors, 6)
+	require.Len(t, multErr.Errors, 8)
 
 	assert.Error(t, multErr.Errors[0], "pool_name is empty")
 	assert.Error(t, multErr.Errors[1], "root_path is empty")
-	assert.Error(t, multErr.Errors[2], "data_device is empty")
-	assert.Error(t, multErr.Errors[3], "meta_device is empty")
+	assert.Error(t, multErr.Errors[2], "base_image_size is empty")
+	assert.Error(t, multErr.Errors[3], "data_device is empty")
+	assert.Error(t, multErr.Errors[4], "meta_device is empty")
+	assert.Error(t, multErr.Errors[5], "data_block_size is empty")
+
+	assert.Equal(t, multErr.Errors[6], errInvalidBlockSize)
+	assert.Equal(t, multErr.Errors[7], errInvalidBlockAlignment)
+}
+
+func TestExistingPoolFieldValidation(t *testing.T) {
+	config := &Config{
+		PoolName:      "test",
+		RootPath:      "test",
+		BaseImageSize: "10mb",
+	}
 
-	assert.Equal(t, multErr.Errors[4], errInvalidBlockSize)
-	assert.Equal(t, multErr.Errors[5], errInvalidBlockAlignment)
+	err := config.Validate()
+	assert.NoError(t, err)
 }
diff --git a/snapshotter/devmapper/pool_device_test.go b/snapshotter/devmapper/pool_device_test.go
index 93b5c1d..840be05 100644
--- a/snapshotter/devmapper/pool_device_test.go
+++ b/snapshotter/devmapper/pool_device_test.go
@@ -75,7 +75,10 @@ func TestPoolDevice(t *testing.T) {
 		RootPath:             tempDir,
 		DataDevice:           loopDataDevice,
 		MetadataDevice:       loopMetaDevice,
+		DataBlockSize:        "65536",
 		DataBlockSizeSectors: 128,
+		BaseImageSize:        "16mb",
+		BaseImageSizeBytes:   16 * 1024 * 1024,
 	}
 
 	pool, err := NewPoolDevice(ctx, config)
diff --git a/snapshotter/devmapper/snapshotter_test.go b/snapshotter/devmapper/snapshotter_test.go
index e40f3b5..df2b41a 100644
--- a/snapshotter/devmapper/snapshotter_test.go
+++ b/snapshotter/devmapper/snapshotter_test.go
@@ -15,10 +15,7 @@ package devmapper
 
 import (
 	"context"
-	"encoding/json"
 	"fmt"
-	"io/ioutil"
-	"path/filepath"
 	"testing"
 	"time"
 
@@ -29,6 +26,7 @@ import (
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 
+	"github.com/firecracker-microvm/firecracker-containerd/snapshotter/pkg/dmsetup"
 	"github.com/firecracker-microvm/firecracker-containerd/snapshotter/pkg/losetup"
 )
 
@@ -41,19 +39,18 @@ func TestSnapshotterSuite(t *testing.T) {
 		_, loopDataDevice := createLoopbackDevice(t, root)
 		_, loopMetaDevice := createLoopbackDevice(t, root)
 
+		poolName := fmt.Sprintf("containerd-snapshotter-suite-pool-%d", time.Now().Nanosecond())
+		err := dmsetup.CreatePool(poolName, loopDataDevice, loopMetaDevice, 64*1024/dmsetup.SectorSize)
+		require.NoErrorf(t, err, "failed to create pool %q", poolName)
+
 		config := &Config{
-			RootPath:       root,
-			PoolName:       fmt.Sprintf("containerd-snapshotter-suite-pool-%d", time.Now().Nanosecond()),
-			DataDevice:     loopDataDevice,
-			MetadataDevice: loopMetaDevice,
-			DataBlockSize:  "64Kb",
-			BaseImageSize:  "16Mb",
+			RootPath:           root,
+			PoolName:           poolName,
+			BaseImageSize:      "16Mb",
+			BaseImageSizeBytes: 16 * 1024 * 1024,
 		}
 
-		configPath := filepath.Join(root, "config.json")
-		saveConfig(t, configPath, config)
-
-		snap, err := NewSnapshotter(context.Background(), configPath)
+		snap, err := NewSnapshotter(context.Background(), config)
 		if err != nil {
 			return nil, nil, err
 		}
@@ -77,11 +74,3 @@ func TestSnapshotterSuite(t *testing.T) {
 		}, nil
 	})
 }
-
-func saveConfig(t *testing.T, path string, config *Config) {
-	data, err := json.Marshal(config)
-	require.NoError(t, err)
-
-	err = ioutil.WriteFile(path, data, 0700)
-	require.NoError(t, err)
-}
diff --git a/snapshotter/naive/naive_test.go b/snapshotter/naive/naive_test.go
index 7d43a5c..0d973ac 100644
--- a/snapshotter/naive/naive_test.go
+++ b/snapshotter/naive/naive_test.go
@@ -40,11 +40,11 @@ func TestCreateImage(t *testing.T) {
 	imgPath := filepath.Join(tempDir, "x.img")
 
 	const (
-		sizeMB    = 100
-		sizeBytes = sizeMB * 100000
+		sizeMiB   = 100
+		sizeBytes = sizeMiB * mib
 	)
 
-	err = snap.createImage(context.Background(), imgPath, sizeMB)
+	err = snap.createImage(context.Background(), imgPath, sizeMiB)
 	if err != nil {
 		t.Fatal(err)
 	}