First version of reworked snapshot implementation

builder/virtualbox/common/driver.go

@@ -42,6 +42,7 @@ type Driver interface {
 	SuppressMessages() error
 
 	// VBoxManage executes the given VBoxManage command
+	// and returns the stdout channel as string
 	VBoxManage(...string) error
 
 	// Verify checks to make sure that this driver should function
@@ -52,26 +53,24 @@ type Driver interface {
 	// Version reads the version of VirtualBox that is installed.
 	Version() (string, error)
 
-	//
+	// LoadSnapshots Loads all defined snapshots for a vm.
+	// if no snapshots are defined nil will be returned
+	LoadSnapshots(string) (*VBoxSnapshot, error)
+
+	// CreateSnapshot Creates a snapshot for a vm with a given name
 	CreateSnapshot(string, string) error
 
-	//
+	// HasSnapshots tests if a vm has snapshots
 	HasSnapshots(string) (bool, error)
 
-	//
+	// GetCurrentSnapshot Returns the current snapshot for a vm
-	GetCurrentSnapshot(string) (string, error)
+	GetCurrentSnapshot(string) (*VBoxSnapshot, error)
 
-	//
+	// SetSnapshot sets the for a vm
-	SetSnapshot(string, string) error
+	SetSnapshot(string, *VBoxSnapshot) error
 
-	//
+	// DeleteSnapshot deletes the specified snapshot from a vm
-	DeleteSnapshot(string, string) error
+	DeleteSnapshot(string, *VBoxSnapshot) error
-
-	//
-	SnapshotExists(string, string) (bool, error)
-
-	//
-	GetParentSnapshot(string, string) (string, error)
 }
 
 func NewDriver() (Driver, error) {

builder/virtualbox/common/driver_4_2.go

@@ -1,6 +1,7 @@
 package common
 
 import (
+	"bufio"
 	"bytes"
 	"context"
 	"fmt"
@@ -11,8 +12,8 @@ import (
 	"strings"
 	"time"
 
+	"github.com/golang-collections/collections/stack"
 	versionUtil "github.com/hashicorp/go-version"
-
 	packer "github.com/hashicorp/packer/common"
 )
 
@@ -178,6 +179,11 @@ func (d *VBox42Driver) SuppressMessages() error {
 }
 
 func (d *VBox42Driver) VBoxManage(args ...string) error {
+	_, err := d.VBoxManageWithOutput(args...)
+	return err
+}
+
+func (d *VBox42Driver) VBoxManageWithOutput(args ...string) (string, error) {
 	var stdout, stderr bytes.Buffer
 
 	log.Printf("Executing VBoxManage: %#v", args)
@@ -205,7 +211,7 @@ func (d *VBox42Driver) VBoxManage(args ...string) error {
 	log.Printf("stdout: %s", stdoutString)
 	log.Printf("stderr: %s", stderrString)
 
-	return err
+	return stdoutString, err
 }
 
 func (d *VBox42Driver) Verify() error {
@@ -243,89 +249,133 @@ func (d *VBox42Driver) Version() (string, error) {
 
 // LoadSnapshots load the snapshots for a VM instance
 func (d *VBox42Driver) LoadSnapshots(vmName string) (*VBoxSnapshot, error) {
-	return nil, nil
+	if vmName == "" {
+		panic("Argument empty exception: vmName")
+	}
+	log.Printf("Executing LoadSnapshots: VM: %s", vmName)
+
+	stdoutString, err := d.VBoxManageWithOutput("snapshot", vmName, "list", "--machinereadable")
+	if nil != err {
+		return nil, err
+	}
+
+	var rootNode *VBoxSnapshot
+	if stdoutString != "This machine does not have any snapshots" {
+		scanner := bufio.NewScanner(strings.NewReader(stdoutString))
+		SnapshotNamePartsRe := regexp.MustCompile("Snapshot(?P<Type>Name|UUID)(?P<Path>(-[1-9]+)*)=\"(?P<Value>[^\"]*)\"")
+		var currentIndicator string
+		parentStack := stack.New()
+		var node *VBoxSnapshot
+		for scanner.Scan() {
+			txt := scanner.Text()
+			idx := strings.Index(txt, "=")
+			if idx > 0 {
+				if strings.HasPrefix(txt, "Current") {
+					node.IsCurrent = true
+				} else {
+					matches := SnapshotNamePartsRe.FindStringSubmatch(txt)
+					log.Printf("************ Snapshot %s name parts", txt)
+					log.Printf("Matches %#v\n", matches)
+					log.Printf("Node %s\n", matches[0])
+					log.Printf("Type %s\n", matches[1])
+					log.Printf("Path %s\n", matches[2])
+					log.Printf("Leaf %s\n", matches[3])
+					log.Printf("Value %s\n", matches[4])
+					if matches[1] == "Name" {
+						if nil == rootNode {
+							node = new(VBoxSnapshot)
+							rootNode = node
+							currentIndicator = matches[2]
+						} else {
+							pathLenCur := strings.Count(currentIndicator, "-")
+							pathLen := strings.Count(matches[2], "-")
+							if pathLen > pathLenCur {
+								currentIndicator = matches[2]
+								parentStack.Push(node)
+							} else if pathLen < pathLenCur {
+								for i := 0; i < pathLenCur-1; i++ {
+									parentStack.Pop()
+								}
+							}
+							node = new(VBoxSnapshot)
+							parent := parentStack.Peek().(*VBoxSnapshot)
+							if nil != parent {
+								parent.Children = append(parent.Children, node)
+							}
+						}
+						node.Name = matches[4]
+					} else if matches[1] == "UUID" {
+						node.UUID = matches[4]
+					}
+				}
+			} else {
+				log.Printf("Invalid key,value pair [%s]", txt)
+			}
+		}
+	}
+
+	return rootNode, nil
 }
 
 func (d *VBox42Driver) CreateSnapshot(vmname string, snapshotName string) error {
+	if vmname == "" {
+		panic("Argument empty exception: vmname")
+	}
 	log.Printf("Executing CreateSnapshot: VM: %s, SnapshotName %s", vmname, snapshotName)
 
 	return d.VBoxManage("snapshot", vmname, "take", snapshotName)
 }
 
 func (d *VBox42Driver) HasSnapshots(vmname string) (bool, error) {
+	if vmname == "" {
+		panic("Argument empty exception: vmname")
+	}
 	log.Printf("Executing HasSnapshots: VM: %s", vmname)
 
-	var stdout, stderr bytes.Buffer
+	sn, err := d.LoadSnapshots(vmname)
-	var hasSnapshots = false
+	if nil != err {
-
+		return false, err
-	cmd := exec.Command(d.VBoxManagePath, "snapshot", vmname, "list", "--machinereadable")
-	cmd.Stdout = &stdout
-	cmd.Stderr = &stderr
-	err := cmd.Run()
-
-	stdoutString := strings.TrimSpace(stdout.String())
-	stderrString := strings.TrimSpace(stderr.String())
-
-	if _, ok := err.(*exec.ExitError); ok {
-		if stdoutString != "This machine does not have any snapshots" {
-			err = fmt.Errorf("VBoxManage error: %s", stderrString)
-		}
-	} else {
-		hasSnapshots = true
 	}
-
+	return nil != sn, nil
-	return hasSnapshots, err
 }
 
-func (d *VBox42Driver) GetCurrentSnapshot(vmname string) (string, error) {
+func (d *VBox42Driver) GetCurrentSnapshot(vmname string) (*VBoxSnapshot, error) {
+	if vmname == "" {
+		panic("Argument empty exception: vmname")
+	}
 	log.Printf("Executing GetCurrentSnapshot: VM: %s", vmname)
 
-	var stdout, stderr bytes.Buffer
+	sn, err := d.LoadSnapshots(vmname)
-
+	if nil != err {
-	cmd := exec.Command(d.VBoxManagePath, "snapshot", vmname, "list", "--machinereadable")
+		return nil, err
-	cmd.Stdout = &stdout
-	cmd.Stderr = &stderr
-	err := cmd.Run()
-
-	stdoutString := strings.TrimSpace(stdout.String())
-	stderrString := strings.TrimSpace(stderr.String())
-
-	if _, ok := err.(*exec.ExitError); ok {
-		if stdoutString == "This machine does not have any snapshots" {
-			return "", nil
-		} else {
-			return "", (fmt.Errorf("VBoxManage error: %s", stderrString))
-		}
 	}
-
+	return sn.GetCurrentSnapshot(), nil
-	CurrentSnapshotNameRe := regexp.MustCompile("CurrentSnapshotName=\"(?P<snapshotName>[^\"]*)\"")
-
-	for _, line := range strings.Split(stdout.String(), "\n") {
-		result := CurrentSnapshotNameRe.FindStringSubmatch(line)
-		if len(result) > 1 {
-			return result[1], nil
-		}
-	}
-
-	return "", (fmt.Errorf("VBoxManage unable to find current snapshot name"))
 }
 
-func (d *VBox42Driver) SetSnapshot(vmname string, snapshotName string) error {
+func (d *VBox42Driver) SetSnapshot(vmname string, sn *VBoxSnapshot) error {
-	log.Printf("Executing SetSnapshot: VM: %s, SnapshotName %s", vmname, snapshotName)
+	if vmname == "" {
-
+		panic("Argument empty exception: vmname")
-	var err error
-	if snapshotName == "" {
-		err = d.VBoxManage("snapshot", vmname, "restorecurrent")
-	} else {
-		err = d.VBoxManage("snapshot", vmname, "restore", snapshotName)
 	}
-	return err
+	if nil == sn {
+		panic("Argument null exception: sn")
+	}
+	log.Printf("Executing SetSnapshot: VM: %s, SnapshotName %s", vmname, sn.UUID)
+
+	return d.VBoxManage("snapshot", vmname, "restore", sn.UUID)
 }
 
-func (d *VBox42Driver) DeleteSnapshot(vmname string, snapshotName string) error {
+func (d *VBox42Driver) DeleteSnapshot(vmname string, sn *VBoxSnapshot) error {
-	return d.VBoxManage("snapshot", vmname, "delete", snapshotName)
+	if vmname == "" {
+		panic("Argument empty exception: vmname")
+	}
+	if nil == sn {
+		panic("Argument null exception: sn")
+	}
+	log.Printf("Executing DeleteSnapshot: VM: %s, SnapshotName %s", vmname, sn.UUID)
+	return d.VBoxManage("snapshot", vmname, "delete", sn.UUID)
 }
 
+/*
 func (d *VBox42Driver) SnapshotExists(vmname string, snapshotName string) (bool, error) {
 	log.Printf("Executing SnapshotExists: VM %s, SnapshotName %s", vmname, snapshotName)
 
@@ -412,3 +462,4 @@ func (d *VBox42Driver) GetParentSnapshot(vmname string, snapshotName string) (st
 	}
 	return "", nil
 }
+*/

builder/virtualbox/common/snapshot.go

@@ -1,15 +1,127 @@
 package common
 
+import (
+	"strings"
+)
+
 // VBoxSnapshot stores the hierarchy of snapshots for a VM instance
 type VBoxSnapshot struct {
 	Name      string
 	UUID      string
 	IsCurrent bool
 	Parent    *VBoxSnapshot // nil if topmost (root) snapshot
-	Children  []VBoxSnapshot
+	Children  []*VBoxSnapshot
 }
 
 // IsChildOf verifies if the current snaphot is a child of the passed as argument
 func (sn *VBoxSnapshot) IsChildOf(candidate *VBoxSnapshot) bool {
-	return false
+	if nil == candidate {
+		panic("Missing parameter value: candidate")
+	}
+	node := sn
+	for nil != node {
+		if candidate.UUID == node.UUID {
+			break
+		}
+		node = node.Parent
+	}
+	return nil != node
+}
+
+// the walker uses a channel to return nodes from a snapshot tree in breadth approach
+func walk(sn *VBoxSnapshot, ch chan *VBoxSnapshot) {
+	if nil == sn {
+		return
+	}
+	if 0 < len(sn.Children) {
+		for _, child := range sn.Children {
+			walk(child, ch)
+		}
+	} else {
+		ch <- sn
+	}
+}
+
+func walker(sn *VBoxSnapshot) <-chan *VBoxSnapshot {
+	if nil == sn {
+		panic("Argument null exception: sn")
+	}
+
+	ch := make(chan *VBoxSnapshot)
+	go func() {
+		walk(sn, ch)
+		close(ch)
+	}()
+	return ch
+}
+
+// GetRoot returns the top-most (root) snapshot for a given snapshot
+func (sn *VBoxSnapshot) GetRoot() *VBoxSnapshot {
+	if nil == sn {
+		panic("Argument null exception: sn")
+	}
+
+	node := sn
+	for nil != node.Parent {
+		node = node.Parent
+	}
+	return node
+}
+
+// GetSnapshotsByName find all snapshots with a given name
+func (sn *VBoxSnapshot) GetSnapshotsByName(name string) []*VBoxSnapshot {
+	var result []*VBoxSnapshot
+	root := sn.GetRoot()
+	ch := walker(root)
+	for {
+		node, ok := <-ch
+		if !ok {
+			panic("Internal channel error while traversing the snapshot tree")
+		}
+		if strings.EqualFold(node.Name, name) {
+			result = append(result, node)
+		}
+	}
+	return result
+}
+
+// GetSnapshotByUUID returns a snapshot by it's UUID
+func (sn *VBoxSnapshot) GetSnapshotByUUID(uuid string) *VBoxSnapshot {
+	root := sn.GetRoot()
+	ch := walker(root)
+	for {
+		node, ok := <-ch
+		if !ok {
+			panic("Internal channel error while traversing the snapshot tree")
+		}
+		if strings.EqualFold(node.UUID, uuid) {
+			return node
+		}
+	}
+	return nil
+}
+
+// GetCurrentSnapshot returns the currently attached snapshot
+func (sn *VBoxSnapshot) GetCurrentSnapshot() *VBoxSnapshot {
+	root := sn.GetRoot()
+	ch := walker(root)
+	for {
+		node, ok := <-ch
+		if !ok {
+			panic("Internal channel error while traversing the snapshot tree")
+		}
+		if node.IsCurrent {
+			return node
+		}
+	}
+	return nil
+}
+
+func (sn *VBoxSnapshot) GetChildWithName(name string) *VBoxSnapshot {
+	for _, child := range sn.Children {
+		if child.Name == name {
+			return child
+		}
+	}
+	return nil
 }

builder/virtualbox/vm/builder.go

@@ -32,7 +32,7 @@ func (b *Builder) Prepare(raws ...interface{}) ([]string, error) {
 
 // Run executes a Packer build and returns a packer.Artifact representing
 // a VirtualBox appliance.
-func (b *Builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packer.Artifact, error) {
+func (b *Builder) Run(ui packer.Ui, hook packer.Hook) (packer.Artifact, error) {
 	// Create the driver that we'll use to communicate with VirtualBox
 	driver, err := vboxcommon.NewDriver()
 	if err != nil {
@@ -44,7 +44,6 @@ func (b *Builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packe
 	state.Put("config", b.config)
 	state.Put("debug", b.config.PackerDebug)
 	state.Put("driver", driver)
-	state.Put("cache", cache)
 	state.Put("hook", hook)
 	state.Put("ui", ui)
 

builder/virtualbox/vm/config.go

@@ -2,6 +2,7 @@ package vm
 
 import (
 	"fmt"
+	"log"
 	"strings"
 
 	vboxcommon "github.com/hashicorp/packer/builder/virtualbox/common"
@@ -128,51 +129,38 @@ func NewConfig(raws ...interface{}) (*Config, []string, error) {
 	if err != nil {
 		errs = packer.MultiErrorAppend(errs, fmt.Errorf("Failed creating VirtualBox driver: %s", err))
 	} else {
-		if c.AttachSnapshot != "" {
+		snapshotTree, err := driver.LoadSnapshots(c.VMName)
-			snapshotExists, err := driver.SnapshotExists(c.VMName, c.AttachSnapshot)
+		if err != nil {
-			if err != nil {
+			errs = packer.MultiErrorAppend(errs, fmt.Errorf("Failed creating VirtualBox driver: %s", err))
-				errs = packer.MultiErrorAppend(errs, fmt.Errorf("Failed to check for snapshot: %s with VM %s ; Error: %s", c.AttachSnapshot, c.VMName, err))
+		} else {
-			} else {
+			if c.AttachSnapshot != "" && c.TargetSnapshot != "" && c.AttachSnapshot == c.TargetSnapshot {
-				if !snapshotExists {
+				errs = packer.MultiErrorAppend(errs, fmt.Errorf("Attach snapshot %s and target snapshot %s cannot be the same", c.AttachSnapshot, c.TargetSnapshot))
+			}
+			attachSnapshot := snapshotTree.GetCurrentSnapshot()
+			if c.AttachSnapshot != "" {
+				snapshots := snapshotTree.GetSnapshotsByName(c.AttachSnapshot)
+				if 0 >= len(snapshots) {
 					errs = packer.MultiErrorAppend(errs, fmt.Errorf("Snapshot %s does not exist on with VM %s", c.AttachSnapshot, c.VMName))
+				} else if 1 < len(snapshots) {
+					errs = packer.MultiErrorAppend(errs, fmt.Errorf("Multiple Snapshots %s exist on with VM %s", c.AttachSnapshot, c.VMName))
+				} else {
+					attachSnapshot = snapshots[0]
 				}
 			}
-		}
+			if c.TargetSnapshot != "" {
-		if c.TargetSnapshot != "" {
+				snapshots := snapshotTree.GetSnapshotsByName(c.TargetSnapshot)
-			snapshotExists, err := driver.SnapshotExists(c.VMName, c.TargetSnapshot)
+				if 0 >= len(snapshots) {
-			if err != nil {
+					isChild := false
-				errs = packer.MultiErrorAppend(errs, fmt.Errorf("Failed to check for snapshot: %s", err))
+					for _, snapshot := range snapshots {
-			} else {
+						log.Printf("Checking if target snaphot %v is child of %s")
-				if snapshotExists {
+						isChild = nil != snapshot.Parent && snapshot.Parent.UUID == attachSnapshot.UUID
-					parent, err := driver.GetParentSnapshot(c.VMName, c.TargetSnapshot)
+					}
-					if err != nil {
+					if !isChild {
-						errs = packer.MultiErrorAppend(errs, fmt.Errorf("Failed to get parent for snapshot %s: %s", c.TargetSnapshot, err))
+						errs = packer.MultiErrorAppend(errs, fmt.Errorf("Target snapshot %s already exists and is not a direct child of %s", c.TargetSnapshot, attachSnapshot.Name))
-						return nil, warnings, errs
-					} else {
-						var selfSnapshotName string
-						if "" != c.AttachSnapshot {
-							selfSnapshotName = c.AttachSnapshot
-						} else {
-							currentSnapshot, err := driver.GetCurrentSnapshot(c.VMName)
-							if err != nil {
-								errs = packer.MultiErrorAppend(errs, fmt.Errorf("Failed to get current snapshot for VM %s: %s", c.VMName, err))
-								return nil, warnings, errs
-							}
-							selfSnapshotName = currentSnapshot
-						}
-						selfSnapshotParent, err := driver.GetParentSnapshot(c.VMName, selfSnapshotName)
-						if err != nil {
-							errs = packer.MultiErrorAppend(errs, fmt.Errorf("Failed to get parent for snapshot %s: %s", selfSnapshotName, err))
-						} else if parent != selfSnapshotName {
-							errs = packer.MultiErrorAppend(errs, fmt.Errorf("Target snapshot %s already exists and is not a direct child of %s", c.TargetSnapshot, selfSnapshotParent))
-						}
 					}
 				}
 			}
 		}
-		if c.AttachSnapshot != "" && c.TargetSnapshot != "" && c.AttachSnapshot == c.TargetSnapshot {
-			errs = packer.MultiErrorAppend(errs, fmt.Errorf("Attach snapshot %s and target snapshot %s cannot be the same", c.AttachSnapshot, c.TargetSnapshot))
-		}
 	}
 	// Check for any errors.
 	if errs != nil && len(errs.Errors) > 0 {

builder/virtualbox/vm/step_create_snapshot.go

@@ -22,15 +22,19 @@ func (s *StepCreateSnapshot) Run(_ context.Context, state multistep.StateBag) mu
 	if s.TargetSnapshot != "" {
 		time.Sleep(10 * time.Second) // Wait after the Vm has been shutdown, otherwise creating the snapshot might make the VM unstartable
 		ui.Say(fmt.Sprintf("Creating snapshot %s on virtual machine %s", s.TargetSnapshot, s.Name))
-		snapshotExists, err := driver.SnapshotExists(s.Name, s.TargetSnapshot)
+		snapshotTree, err := driver.LoadSnapshots(s.Name)
 		if err != nil {
-			err = fmt.Errorf("Failed to check for snapshot: %s", err)
+			err = fmt.Errorf("Failed to load snapshots for VM %s: %s", s.Name, err)
 			state.Put("error", err)
 			ui.Error(err.Error())
 			return multistep.ActionHalt
-		} else if snapshotExists {
+		}
+
+		currentSnapshot := snapshotTree.GetCurrentSnapshot()
+		targetSnapshot := currentSnapshot.GetChildWithName(s.TargetSnapshot)
+		if nil != targetSnapshot {
 			log.Printf("Deleting existing target snapshot %s", s.TargetSnapshot)
-			err = driver.DeleteSnapshot(s.Name, s.TargetSnapshot)
+			err = driver.DeleteSnapshot(s.Name, targetSnapshot)
 			if nil != err {
 				err = fmt.Errorf("Unable to delete snapshot %s from VM %s: %s", s.TargetSnapshot, s.Name, err)
 				state.Put("error", err)

builder/virtualbox/vm/step_set_snapshot.go

@@ -19,36 +19,43 @@ type StepSetSnapshot struct {
 func (s *StepSetSnapshot) Run(_ context.Context, state multistep.StateBag) multistep.StepAction {
 	driver := state.Get("driver").(vboxcommon.Driver)
 	ui := state.Get("ui").(packer.Ui)
-	hasSnapshots, err := driver.HasSnapshots(s.Name)
+	snapshotTree, err := driver.LoadSnapshots(s.Name)
 	if err != nil {
-		err := fmt.Errorf("Error checking for snapshots VM: %s", err)
+		err := fmt.Errorf("Error loading snapshots for VM: %s", err)
 		state.Put("error", err)
 		ui.Error(err.Error())
 		return multistep.ActionHalt
-	} else if hasSnapshots {
-		currentSnapshot, err := driver.GetCurrentSnapshot(s.Name)
-		if err != nil {
-			err := fmt.Errorf("Unable to get current snapshot for VM: %s", err)
-			state.Put("error", err)
-			ui.Error(err.Error())
-			return multistep.ActionHalt
-		}
-		s.revertToSnapshot = currentSnapshot
 	}
+
 	if s.AttachSnapshot != "" {
-		if !hasSnapshots {
+		if nil == snapshotTree {
 			err := fmt.Errorf("Unable to attach snapshot on VM %s when no snapshots exist", s.Name)
 			state.Put("error", err)
 			ui.Error(err.Error())
 			return multistep.ActionHalt
 		}
+		currentSnapshot := snapshotTree.GetCurrentSnapshot()
+		s.revertToSnapshot = currentSnapshot.UUID
 		ui.Say(fmt.Sprintf("Attaching snapshot %s on virtual machine %s", s.AttachSnapshot, s.Name))
-		err = driver.SetSnapshot(s.Name, s.AttachSnapshot)
+		candidateSnapshots := snapshotTree.GetSnapshotsByName(s.AttachSnapshot)
-		if err != nil {
+		if 0 <= len(candidateSnapshots) {
-			err := fmt.Errorf("Unable to set snapshot for VM: %s", err)
+			err := fmt.Errorf("Snapshot %s not found on VM %s", s.AttachSnapshot, s.Name)
 			state.Put("error", err)
 			ui.Error(err.Error())
 			return multistep.ActionHalt
+		} else if 1 > len(candidateSnapshots) {
+			err := fmt.Errorf("More than one Snapshot %s found on VM %s", s.AttachSnapshot, s.Name)
+			state.Put("error", err)
+			ui.Error(err.Error())
+			return multistep.ActionHalt
+		} else {
+			err = driver.SetSnapshot(s.Name, candidateSnapshots[0])
+			if err != nil {
+				err := fmt.Errorf("Unable to set snapshot for VM: %s", err)
+				state.Put("error", err)
+				ui.Error(err.Error())
+				return multistep.ActionHalt
+			}
 		}
 	}
 	return multistep.ActionContinue
@@ -63,7 +70,15 @@ func (s *StepSetSnapshot) Cleanup(state multistep.StateBag) {
 			return
 		} else {
 			ui.Say(fmt.Sprintf("Reverting to snapshot %s on virtual machine %s", s.revertToSnapshot, s.Name))
-			err := driver.SetSnapshot(s.Name, s.revertToSnapshot)
+			snapshotTree, err := driver.LoadSnapshots(s.Name)
+			revertTo := snapshotTree.GetSnapshotByUUID(s.revertToSnapshot)
+			if nil == revertTo {
+				err := fmt.Errorf("Snapshot with UUID %s not found for VM %s", s.revertToSnapshot, s.Name)
+				state.Put("error", err)
+				ui.Error(err.Error())
+				return
+			}
+			err = driver.SetSnapshot(s.Name, revertTo)
 			if err != nil {
 				err := fmt.Errorf("Unable to set snapshot for VM: %s", err)
 				state.Put("error", err)