Skip to content

Commit db01afa

Browse files
dmacvicardmacvicar
authored
Merge pull request dmacvicar#573 from dmacvicar/issue-561
When volume size is not given, set the backing store size as the size (fixes dmacvicar#561)
2 parents f5b98d5 + 77eeafd commit db01afa

File tree

2 files changed

+51
-44
lines changed

2 files changed

+51
-44
lines changed

‎libvirt/resource_libvirt_volume.go‎

Lines changed: 26 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -164,13 +164,16 @@ func resourceLibvirtVolumeCreate(d *schema.ResourceData, meta interface{}) error
164164
volumeDef.Capacity.Unit = "B"
165165
volumeDef.Capacity.Value = size
166166
} else {
167+
// the volume does not have a source image to upload
167168

168-
// the volume does not have a source image to upload, first handle
169-
// whether it has a backing image
170-
//
169+
// if size is given, set it to the specified value
170+
if _, ok := d.GetOk("size"); ok {
171+
volumeDef.Capacity.Value = uint64(d.Get("size").(int))
172+
}
173+
174+
//first handle whether it has a backing image
171175
// backing images can be specified by either (id), or by (name, pool)
172176
var baseVolume *libvirt.StorageVol
173-
174177
if baseVolumeID, ok := d.GetOk("base_volume_id"); ok {
175178
if _, ok := d.GetOk("base_volume_name"); ok {
176179
return fmt.Errorf("'base_volume_name' can't be specified when also 'base_volume_id' is given")
@@ -179,9 +182,7 @@ func resourceLibvirtVolumeCreate(d *schema.ResourceData, meta interface{}) error
179182
if err != nil {
180183
return fmt.Errorf("Can't retrieve volume ID '%s': %v", baseVolumeID.(string), err)
181184
}
182-
}
183-
184-
if baseVolumeName, ok := d.GetOk("base_volume_name"); ok {
185+
} else if baseVolumeName, ok := d.GetOk("base_volume_name"); ok {
185186
baseVolumePool := pool
186187
if _, ok := d.GetOk("base_volume_pool"); ok {
187188
baseVolumePoolName := d.Get("base_volume_pool").(string)
@@ -196,29 +197,32 @@ func resourceLibvirtVolumeCreate(d *schema.ResourceData, meta interface{}) error
196197
return fmt.Errorf("Can't retrieve base volume with name '%s': %v", baseVolumeName.(string), err)
197198
}
198199
}
200+
199201
if baseVolume != nil {
200-
backingStoreDef, err := newDefBackingStoreFromLibvirt(baseVolume)
202+
backingStoreFragmentDef, err := newDefBackingStoreFromLibvirt(baseVolume)
201203
if err != nil {
202204
return fmt.Errorf("Could not retrieve backing store definition: %s", err.Error())
203205
}
204-
if _, ok := d.GetOk("size"); ok {
205-
// does the backing store have some size information?, check at least that it is not smaller than the backing store
206-
volumeDef.Capacity.Value = uint64(d.Get("size").(int))
207-
backingStoreVolumeDef, err := newDefVolumeFromLibvirt(baseVolume)
208-
if err != nil {
209-
return err
210-
}
211206

212-
if backingStoreVolumeDef.Capacity != nil && volumeDef.Capacity.Value < backingStoreVolumeDef.Capacity.Value {
213-
return fmt.Errorf("When 'size' is specified, it shouldn't be smaller than the backing store specified with 'base_volume_id' or 'base_volume_name/base_volume_pool'")
214-
}
207+
backingStoreVolumeDef, err := newDefVolumeFromLibvirt(baseVolume)
208+
if err != nil {
209+
return err
210+
}
211+
212+
// if the volume does not specify size, set it to the size of the backing store
213+
if _, ok := d.GetOk("size"); !ok {
214+
volumeDef.Capacity.Value = backingStoreVolumeDef.Capacity.Value
215215
}
216-
volumeDef.BackingStore = &backingStoreDef
216+
217+
// Always check that the size, specified or taken from the backing store
218+
// is at least the size of the backing store itself
219+
if backingStoreVolumeDef.Capacity != nil && volumeDef.Capacity.Value < backingStoreVolumeDef.Capacity.Value {
220+
return fmt.Errorf("When 'size' is specified, it shouldn't be smaller than the backing store specified with 'base_volume_id' or 'base_volume_name/base_volume_pool'")
221+
}
222+
volumeDef.BackingStore = &backingStoreFragmentDef
217223
}
218224
}
219-
if _, ok := d.GetOk("size"); ok {
220-
volumeDef.Capacity.Value = uint64(d.Get("size").(int))
221-
}
225+
222226
data, err := xmlMarshallIndented(volumeDef)
223227
if err != nil {
224228
return fmt.Errorf("Error serializing libvirt volume: %s", err)

‎libvirt/resource_libvirt_volume_test.go‎

Lines changed: 25 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -122,27 +122,28 @@ func TestAccLibvirtVolume_Basic(t *testing.T) {
122122
func TestAccLibvirtVolume_BackingStoreTestByID(t *testing.T) {
123123
var volume libvirt.StorageVol
124124
var volume2 libvirt.StorageVol
125-
randomVolumeResource := acctest.RandString(10)
126-
randomVolumeName := acctest.RandString(10)
125+
random := acctest.RandString(10)
127126
resource.Test(t, resource.TestCase{
128127
PreCheck: func() { testAccPreCheck(t) },
129128
Providers: testAccProviders,
130129
CheckDestroy: testAccCheckLibvirtVolumeDestroy,
131130
Steps: []resource.TestStep{
132131
{
133132
Config: fmt.Sprintf(`
134-
resource "libvirt_volume" "%s" {
135-
name = "%s"
133+
resource "libvirt_volume" "backing-%s" {
134+
name = "backing-%s"
136135
size = 1073741824
137136
}
138-
resource "libvirt_volume" "backing-store" {
139-
name = "backing-store"
140-
base_volume_id = "${libvirt_volume.%s.id}"
137+
resource "libvirt_volume" "%s" {
138+
name = "%s"
139+
base_volume_id = "${libvirt_volume.backing-%s.id}"
141140
}
142-
`, randomVolumeResource, randomVolumeName, randomVolumeResource),
141+
`, random, random, random, random, random),
143142
Check: resource.ComposeTestCheckFunc(
144-
testAccCheckLibvirtVolumeExists("libvirt_volume."+randomVolumeResource, &volume),
145-
testAccCheckLibvirtVolumeIsBackingStore("libvirt_volume.backing-store", &volume2),
143+
testAccCheckLibvirtVolumeExists("libvirt_volume.backing-" + random, &volume),
144+
testAccCheckLibvirtVolumeIsBackingStore("libvirt_volume." + random, &volume2),
145+
resource.TestCheckResourceAttr(
146+
"libvirt_volume."+random, "size", "1073741824"),
146147
),
147148
},
148149
},
@@ -152,26 +153,28 @@ func TestAccLibvirtVolume_BackingStoreTestByID(t *testing.T) {
152153
func TestAccLibvirtVolume_BackingStoreTestByName(t *testing.T) {
153154
var volume libvirt.StorageVol
154155
var volume2 libvirt.StorageVol
155-
randomVolumeResource := acctest.RandString(10)
156-
randomVolumeName := acctest.RandString(10)
156+
random := acctest.RandString(10)
157157
resource.Test(t, resource.TestCase{
158158
PreCheck: func() { testAccPreCheck(t) },
159159
Providers: testAccProviders,
160160
CheckDestroy: testAccCheckLibvirtVolumeDestroy,
161161
Steps: []resource.TestStep{
162162
{
163163
Config: fmt.Sprintf(`
164-
resource "libvirt_volume" "%s" {
165-
name = "%s"
166-
size = 1073741824
167-
}
168-
resource "libvirt_volume" "backing-store" {
169-
name = "backing-store"
170-
base_volume_name = "${libvirt_volume.%s.name}"
171-
} `, randomVolumeResource, randomVolumeName, randomVolumeResource),
164+
resource "libvirt_volume" "backing-%s" {
165+
name = "backing-%s"
166+
size = 1073741824
167+
}
168+
resource "libvirt_volume" "%s" {
169+
name = "%s"
170+
base_volume_name = "${libvirt_volume.backing-%s.name}"
171+
}
172+
`, random, random, random, random, random),
172173
Check: resource.ComposeTestCheckFunc(
173-
testAccCheckLibvirtVolumeExists("libvirt_volume."+randomVolumeResource, &volume),
174-
testAccCheckLibvirtVolumeIsBackingStore("libvirt_volume.backing-store", &volume2),
174+
testAccCheckLibvirtVolumeExists("libvirt_volume.backing-"+random, &volume),
175+
testAccCheckLibvirtVolumeIsBackingStore("libvirt_volume." + random, &volume2),
176+
resource.TestCheckResourceAttr(
177+
"libvirt_volume."+random, "size", "1073741824"),
175178
),
176179
},
177180
},

0 commit comments

Comments
 (0)