improve rollback for overlay.prepare

yanxuean · yanxuean · commit bb02302c9cf1 · 2017-12-02T00:09:44.000+08:00
improve err message
Signed-off-by: yason &lt;yan.xuean@zte.com.cn&gt;
diff --git a/snapshots/overlay/overlay.go b/snapshots/overlay/overlay.go
@@ -287,41 +287,37 @@ func (o *snapshotter) createSnapshot(ctx context.Context, kind snapshots.Kind, k
 	if err != nil {
 		return nil, err
 	}
+	isRollback := true
+	defer func() {
+		if isRollback {
+			if rerr := t.Rollback(); rerr != nil {
+				log.G(ctx).WithError(rerr).Warn("Failure rolling back transaction")
+			}
+		}
+	}()
 
 	s, err := storage.CreateSnapshot(ctx, kind, key, parent, opts...)
 	if err != nil {
-		if rerr := t.Rollback(); rerr != nil {
-			log.G(ctx).WithError(rerr).Warn("Failure rolling back transaction")
-		}
-		return nil, errors.Wrap(err, "failed to create active")
+		return nil, errors.Wrap(err, "failed to create snapshot")
 	}
 
 	if len(s.ParentIDs) > 0 {
-		st, err := os.Stat(filepath.Join(o.upperPath(s.ParentIDs[0])))
+		st, err := os.Stat(o.upperPath(s.ParentIDs[0]))
 		if err != nil {
-			if rerr := t.Rollback(); rerr != nil {
-				log.G(ctx).WithError(rerr).Warn("Failure rolling back transaction")
-			}
 			return nil, errors.Wrap(err, "failed to stat parent")
 		}
 
 		stat := st.Sys().(*syscall.Stat_t)
-
 		if err := os.Lchown(fs, int(stat.Uid), int(stat.Gid)); err != nil {
-			if rerr := t.Rollback(); rerr != nil {
-				log.G(ctx).WithError(rerr).Warn("Failure rolling back transaction")
-			}
 			return nil, errors.Wrap(err, "failed to chown")
 		}
 	}
 
 	path = filepath.Join(snapshotDir, s.ID)
 	if err = os.Rename(td, path); err != nil {
-		if rerr := t.Rollback(); rerr != nil {
-			log.G(ctx).WithError(rerr).Warn("Failure rolling back transaction")
-		}
 		return nil, errors.Wrap(err, "failed to rename")
 	}
+	isRollback = false
 	td = ""
 
 	if err = t.Commit(); err != nil {
diff --git a/snapshots/storage/bolt.go b/snapshots/storage/bolt.go
@@ -207,11 +207,11 @@ func CreateSnapshot(ctx context.Context, kind snapshots.Kind, key, parent string
 		if parent != "" {
 			spbkt = bkt.Bucket([]byte(parent))
 			if spbkt == nil {
-				return errors.Wrap(errdefs.ErrNotFound, "missing parent bucket")
+				return errors.Wrapf(errdefs.ErrNotFound, "missing parent %q bucket", parent)
 			}
 
 			if readKind(spbkt) != snapshots.KindCommitted {
-				return errors.Wrap(errdefs.ErrInvalidArgument, "parent is not committed snapshot")
+				return errors.Wrapf(errdefs.ErrInvalidArgument, "parent %q is not committed snapshot", parent)
 			}
 		}
 		sbkt, err := bkt.CreateBucket([]byte(key))
@@ -224,7 +224,7 @@ func CreateSnapshot(ctx context.Context, kind snapshots.Kind, key, parent string
 
 		id, err := bkt.NextSequence()
 		if err != nil {
-			return errors.Wrap(err, "unable to get identifier")
+			return errors.Wrapf(err, "unable to get identifier for snapshot %q", key)
 		}
 
 		t := time.Now().UTC()
@@ -245,12 +245,12 @@ func CreateSnapshot(ctx context.Context, kind snapshots.Kind, key, parent string
 			// Store a backlink from the key to the parent. Store the snapshot name
 			// as the value to allow following the backlink to the snapshot value.
 			if err := pbkt.Put(parentKey(pid, id), []byte(key)); err != nil {
-				return errors.Wrap(err, "failed to write parent link")
+				return errors.Wrapf(err, "failed to write parent link for snapshot %q", key)
 			}
 
 			s.ParentIDs, err = parents(bkt, spbkt, pid)
 			if err != nil {
-				return errors.Wrap(err, "failed to get parent chain")
+				return errors.Wrapf(err, "failed to get parent chain for snapshot %q", key)
 			}
 		}
 
@@ -438,7 +438,7 @@ func createBucketIfNotExists(ctx context.Context, fn func(context.Context, *bolt
 	}
 	pbkt, err := bkt.CreateBucketIfNotExists(bucketKeyParents)
 	if err != nil {
-		return errors.Wrap(err, "failed to create snapshots bucket")
+		return errors.Wrap(err, "failed to create parents bucket")
 	}
 	return fn(ctx, sbkt, pbkt)
 }

Original file line number	Diff line number	Diff line change
`@@ -207,11 +207,11 @@ func CreateSnapshot(ctx context.Context, kind snapshots.Kind, key, parent string`
`207`	`207`	`if parent != "" {`
`208`	`208`	`spbkt = bkt.Bucket([]byte(parent))`
`209`	`209`	`if spbkt == nil {`
`210`		`- return errors.Wrap(errdefs.ErrNotFound, "missing parent bucket")`
	`210`	`+ return errors.Wrapf(errdefs.ErrNotFound, "missing parent %q bucket", parent)`
`211`	`211`	`}`
`212`	`212`
`213`	`213`	`if readKind(spbkt) != snapshots.KindCommitted {`
`214`		`- return errors.Wrap(errdefs.ErrInvalidArgument, "parent is not committed snapshot")`
	`214`	`+ return errors.Wrapf(errdefs.ErrInvalidArgument, "parent %q is not committed snapshot", parent)`
`215`	`215`	`}`
`216`	`216`	`}`
`217`	`217`	`sbkt, err := bkt.CreateBucket([]byte(key))`
`@@ -224,7 +224,7 @@ func CreateSnapshot(ctx context.Context, kind snapshots.Kind, key, parent string`
`224`	`224`
`225`	`225`	`id, err := bkt.NextSequence()`
`226`	`226`	`if err != nil {`
`227`		`- return errors.Wrap(err, "unable to get identifier")`
	`227`	`+ return errors.Wrapf(err, "unable to get identifier for snapshot %q", key)`
`228`	`228`	`}`
`229`	`229`
`230`	`230`	`t := time.Now().UTC()`
`@@ -245,12 +245,12 @@ func CreateSnapshot(ctx context.Context, kind snapshots.Kind, key, parent string`
`245`	`245`	`// Store a backlink from the key to the parent. Store the snapshot name`
`246`	`246`	`// as the value to allow following the backlink to the snapshot value.`
`247`	`247`	`if err := pbkt.Put(parentKey(pid, id), []byte(key)); err != nil {`
`248`		`- return errors.Wrap(err, "failed to write parent link")`
	`248`	`+ return errors.Wrapf(err, "failed to write parent link for snapshot %q", key)`
`249`	`249`	`}`
`250`	`250`
`251`	`251`	`s.ParentIDs, err = parents(bkt, spbkt, pid)`
`252`	`252`	`if err != nil {`
`253`		`- return errors.Wrap(err, "failed to get parent chain")`
	`253`	`+ return errors.Wrapf(err, "failed to get parent chain for snapshot %q", key)`
`254`	`254`	`}`
`255`	`255`	`}`
`256`	`256`
`@@ -438,7 +438,7 @@ func createBucketIfNotExists(ctx context.Context, fn func(context.Context, *bolt`
`438`	`438`	`}`
`439`	`439`	`pbkt, err := bkt.CreateBucketIfNotExists(bucketKeyParents)`
`440`	`440`	`if err != nil {`
`441`		`- return errors.Wrap(err, "failed to create snapshots bucket")`
	`441`	`+ return errors.Wrap(err, "failed to create parents bucket")`
`442`	`442`	`}`
`443`	`443`	`return fn(ctx, sbkt, pbkt)`
`444`	`444`	`}`