Specifically mention "mkfs.ext4" on the error from the command

Kazuyoshi Kato · Kazuyoshi Kato · commit 7704fe72d06c · 2021-03-19T10:38:47.000-07:00
Before the change, the error on the caller-side (e.g. ctr) was
something like

&gt; unpack: failed to prepare extraction snapshot "...": exit status 5:
&gt; unknown

which was too cryptic.

Signed-off-by: Kazuyoshi Kato &lt;katokazu@amazon.com&gt;
diff --git 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
@@ -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 ""`)
+}

Original file line number	Diff line number	Diff line change
`@@ -365,8 +365,9 @@ func (s *Snapshotter) createSnapshot(ctx context.Context, kind snapshots.Kind, k`
`365`	`365`	`return nil, err`
`366`	`366`	`}`
`367`	`367`
`368`		`- if err := s.mkfs(ctx, deviceName); err != nil {`
	`368`	`+ if err := mkfs(ctx, dmsetup.GetFullDevicePath(deviceName)); err != nil {`
`369`	`369`	`// Rollback thin device creation if mkfs failed`
	`370`	`+ log.G(ctx).WithError(err).Errorf("failed to initialize thin device %q for snapshot %s", deviceName, snap.ID)`
`370`	`371`	`return nil, multierror.Append(err,`
`371`	`372`	`s.pool.RemoveDevice(ctx, deviceName))`
`372`	`373`	`}`
`@@ -393,22 +394,22 @@ func (s *Snapshotter) createSnapshot(ctx context.Context, kind snapshots.Kind, k`
`393`	`394`	`}`
`394`	`395`
`395`	`396`	`// mkfs creates ext4 filesystem on the given devmapper device`
`396`		`-func (s *Snapshotter) mkfs(ctx context.Context, deviceName string) error {`
	`397`	`+func mkfs(ctx context.Context, path string) error {`
`397`	`398`	`args := []string{`
`398`	`399`	`"-E",`
`399`	`400`	`// We don't want any zeroing in advance when running mkfs on thin devices (see "man mkfs.ext4")`
`400`	`401`	`"nodiscard,lazy_itable_init=0,lazy_journal_init=0",`
`401`		`- dmsetup.GetFullDevicePath(deviceName),`
	`402`	`+ path,`
`402`	`403`	`}`
`403`	`404`
`404`	`405`	`log.G(ctx).Debugf("mkfs.ext4 %s", strings.Join(args, " "))`
`405`		`- output, err := exec.Command("mkfs.ext4", args...).CombinedOutput()`
	`406`	`+ b, err := exec.Command("mkfs.ext4", args...).CombinedOutput()`
	`407`	`+ out := string(b)`
`406`	`408`	`if err != nil {`
`407`		`- log.G(ctx).WithError(err).Errorf("failed to write fs:\n%s", string(output))`
`408`		`- return err`
	`409`	`+ return errors.Wrapf(err, "mkfs.ext4 couldn't initialize %q: %s", path, out)`
`409`	`410`	`}`
`410`	`411`
`411`		`- log.G(ctx).Debugf("mkfs:\n%s", string(output))`
	`412`	`+ log.G(ctx).Debugf("mkfs:\n%s", out)`
`412`	`413`	`return nil`
`413`	`414`	`}`
`414`	`415`