From bb02302c9cf1c193a75c0bbf76ab8aedfb339405 Mon Sep 17 00:00:00 2001
From: yason <yan.xuean@zte.com.cn>
Date: Fri, 1 Dec 2017 23:24:48 +0800
Subject: [PATCH] improve rollback for overlay.prepare

improve err message
Signed-off-by: yason <yan.xuean@zte.com.cn>
---
 snapshots/overlay/overlay.go | 34 +++++++++++++++-------------------
 snapshots/storage/bolt.go    | 12 ++++++------
 2 files changed, 21 insertions(+), 25 deletions(-)

diff --git a/snapshots/overlay/overlay.go b/snapshots/overlay/overlay.go
index f3de4bcc2..e875d16a7 100644
--- 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
 	}
-
-	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")
-	}
-
-	if len(s.ParentIDs) > 0 {
-		st, err := os.Stat(filepath.Join(o.upperPath(s.ParentIDs[0])))
-		if err != nil {
+	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 {
+		return nil, errors.Wrap(err, "failed to create snapshot")
+	}
+
+	if len(s.ParentIDs) > 0 {
+		st, err := os.Stat(o.upperPath(s.ParentIDs[0]))
+		if err != nil {
 			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
index 08e9241c8..b120548dc 100644
--- 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)
 }