From 7704fe72d06ca52037b2411c8e63cb844130531a Mon Sep 17 00:00:00 2001
From: Kazuyoshi Kato <katokazu@amazon.com>
Date: Fri, 19 Mar 2021 10:36:16 -0700
Subject: [PATCH] Specifically mention "mkfs.ext4" on the error from the
 command

Before the change, the error on the caller-side (e.g. ctr) was
something like

> unpack: failed to prepare extraction snapshot "...": exit status 5:
> unknown

which was too cryptic.

Signed-off-by: Kazuyoshi Kato <katokazu@amazon.com>
---
 snapshots/devmapper/snapshotter.go      | 15 ++++++++-------
 snapshots/devmapper/snapshotter_test.go |  6 ++++++
 2 files changed, 14 insertions(+), 7 deletions(-)

diff --git a/snapshots/devmapper/snapshotter.go b/snapshots/devmapper/snapshotter.go
index eebc0639e..d13c16c37 100644
--- a/snapshots/devmapper/snapshotter.go
+++ b/snapshots/devmapper/snapshotter.go
@@ -365,8 +365,9 @@ func (s *Snapshotter) createSnapshot(ctx context.Context, kind snapshots.Kind, k
 			return nil, err
 		}
 
-		if err := s.mkfs(ctx, deviceName); err != nil {
+		if err := mkfs(ctx, dmsetup.GetFullDevicePath(deviceName)); err != nil {
 			// Rollback thin device creation if mkfs failed
+			log.G(ctx).WithError(err).Errorf("failed to initialize thin device %q for snapshot %s", deviceName, snap.ID)
 			return nil, multierror.Append(err,
 				s.pool.RemoveDevice(ctx, deviceName))
 		}
@@ -393,22 +394,22 @@ func (s *Snapshotter) createSnapshot(ctx context.Context, kind snapshots.Kind, k
 }
 
 // mkfs creates ext4 filesystem on the given devmapper device
-func (s *Snapshotter) mkfs(ctx context.Context, deviceName string) error {
+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",
-		dmsetup.GetFullDevicePath(deviceName),
+		path,
 	}
 
 	log.G(ctx).Debugf("mkfs.ext4 %s", strings.Join(args, " "))
-	output, err := exec.Command("mkfs.ext4", args...).CombinedOutput()
+	b, err := exec.Command("mkfs.ext4", args...).CombinedOutput()
+	out := string(b)
 	if err != nil {
-		log.G(ctx).WithError(err).Errorf("failed to write fs:\n%s", string(output))
-		return err
+		return errors.Wrapf(err, "mkfs.ext4 couldn't initialize %q: %s", path, out)
 	}
 
-	log.G(ctx).Debugf("mkfs:\n%s", string(output))
+	log.G(ctx).Debugf("mkfs:\n%s", out)
 	return nil
 }
 
diff --git a/snapshots/devmapper/snapshotter_test.go b/snapshots/devmapper/snapshotter_test.go
index 0be158d14..7a3c64d74 100644
--- a/snapshots/devmapper/snapshotter_test.go
+++ b/snapshots/devmapper/snapshotter_test.go
@@ -139,3 +139,9 @@ func testUsage(t *testing.T, snapshotter snapshots.Snapshotter) {
 	assert.Check(t, layer2Usage.Size >= sizeBytes,
 		"%d > %d", layer2Usage.Size, sizeBytes)
 }
+
+func TestMkfs(t *testing.T) {
+	ctx := context.Background()
+	err := mkfs(ctx, "")
+	assert.ErrorContains(t, err, `mkfs.ext4 couldn't initialize ""`)
+}