From 96f4032f04b2a625b12d188cae4315887bf9fc66 Mon Sep 17 00:00:00 2001
From: Thomas Hipp <thipp@suse.de>
Date: Wed, 29 Nov 2017 12:51:21 +0100
Subject: check uniqueness of storage volume name

The storage volume name needs to be unique. If this is not the case, the
storage volume will be overwritten which is not problematic itself.
However, `terraform destroy` will fail since it will try and delete the
same storage volume twice.

Signed-off-by: Thomas Hipp <thipp@suse.de>
---
 libvirt/resource_libvirt_volume.go      | 24 +++++++-----------------
 libvirt/resource_libvirt_volume_test.go | 27 +++++++++++++++++++++++++++
 2 files changed, 34 insertions(+), 17 deletions(-)

(limited to 'libvirt')

diff --git a/libvirt/resource_libvirt_volume.go b/libvirt/resource_libvirt_volume.go
index 7a7f6a40..4f9c3322 100644
--- a/libvirt/resource_libvirt_volume.go
+++ b/libvirt/resource_libvirt_volume.go
@@ -105,12 +105,15 @@ func resourceLibvirtVolumeCreate(d *schema.ResourceData, meta interface{}) error
 		return pool.Refresh(0)
 	})
 
-	volumeDef := newDefVolume()
-
-	if name, ok := d.GetOk("name"); ok {
-		volumeDef.Name = name.(string)
+	// Check whether the storage volume already exists. Its name needs to be
+	// unique.
+	if _, err := pool.LookupStorageVolByName(d.Get("name").(string)); err == nil {
+		return fmt.Errorf("storage volume '%s' already exists", d.Get("name").(string))
 	}
 
+	volumeDef := newDefVolume()
+	volumeDef.Name = d.Get("name").(string)
+
 	volumeFormat := "qcow2"
 	if _, ok := d.GetOk("format"); ok {
 		volumeFormat = d.Get("format").(string)
@@ -136,19 +139,6 @@ func resourceLibvirtVolumeCreate(d *schema.ResourceData, meta interface{}) error
 			return fmt.Errorf("'base_volume_name' can't be specified when also 'source' is given")
 		}
 
-		// Check if we already have this image in the pool
-		if len(volumeDef.Name) > 0 {
-			if v, err := pool.LookupStorageVolByName(volumeDef.Name); err != nil {
-				log.Printf("could not find image %s in pool %s", volumeDef.Name, poolName)
-			} else {
-				volume = v
-				volumeDef, err = newDefVolumeFromLibvirt(volume)
-				if err != nil {
-					return fmt.Errorf("could not get a volume definition from XML for %s: %s", volumeDef.Name, err)
-				}
-			}
-		}
-
 		if img, err = newImage(source.(string)); err != nil {
 			return err
 		}
diff --git a/libvirt/resource_libvirt_volume_test.go b/libvirt/resource_libvirt_volume_test.go
index d185b65e..65c658f1 100644
--- a/libvirt/resource_libvirt_volume_test.go
+++ b/libvirt/resource_libvirt_volume_test.go
@@ -2,6 +2,7 @@ package libvirt
 
 import (
 	"fmt"
+	"regexp"
 	"testing"
 
 	"github.com/hashicorp/terraform/helper/resource"
@@ -109,6 +110,32 @@ func TestAccLibvirtVolume_Basic(t *testing.T) {
 	})
 }
 
+func TestAccLibvirtVolume_UniqueName(t *testing.T) {
+	const config = `
+	resource "libvirt_volume" "terraform-acceptance-test-1" {
+		name = "terraform-test"
+		size =  1073741824
+	}
+
+	resource "libvirt_volume" "terraform-acceptance-test-2" {
+		name = "terraform-test"
+		size =  1073741824
+	}
+	`
+
+	resource.Test(t, resource.TestCase{
+		PreCheck:     func() { testAccPreCheck(t) },
+		Providers:    testAccProviders,
+		CheckDestroy: testAccCheckLibvirtVolumeDestroy,
+		Steps: []resource.TestStep{
+			{
+				Config:      config,
+				ExpectError: regexp.MustCompile(`storage volume 'terraform-test' already exists`),
+			},
+		},
+	})
+}
+
 func TestAccLibvirtVolume_DownloadFromSource(t *testing.T) {
 	var volume libvirt.StorageVol
 
-- 
cgit v1.2.3