Skip to content

Commit

Permalink
Merge pull request #380 from dmacvicar/fix_hardcoded_file_pools_2
Browse files Browse the repository at this point in the history 
Automatic disk driver selection based on volume format and automatic volume format detection
  • Loading branch information
@dmacvicar
dmacvicar committed Aug 30, 2018
2 parents 725307a + d81ee83 commit 676b5a3
Show file tree
Hide file tree
Showing 9 changed files with 374 additions and 30 deletions.
2 changes: 1 addition & 1 deletion libvirt/disk_def.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ func newDefDisk(i int) libvirtxml.DomainDisk {
},
Driver: &libvirtxml.DomainDiskDriver{
Name: "qemu",
Type: "qcow2",
Type: "raw",
},
}
}
Expand Down
73 changes: 68 additions & 5 deletions libvirt/resource_libvirt_domain.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -731,7 +731,10 @@ func resourceLibvirtDomainRead(d *schema.ResourceData, meta interface{}) error {
disk = map[string]interface{}{
"file": diskDef.Source.File,
}
} else {
} else if diskDef.Source.File != nil {
// LEGACY way of handling volumes using "file", which we replaced
// by the diskdef.Source.Volume once we realized it existed.
// This code will be removed in future versions of the provider.
virVol, err := virConn.LookupStorageVolByPath(diskDef.Source.File.File)
if err != nil {
return fmt.Errorf("Error retrieving volume for disk: %s", err)
Expand All @@ -743,10 +746,32 @@ func resourceLibvirtDomainRead(d *schema.ResourceData, meta interface{}) error {
return fmt.Errorf("Error retrieving volume for disk: %s", err)
}

disk = map[string]interface{}{
"volume_id": virVolKey,
}
} else {
pool, err := virConn.LookupStoragePoolByName(diskDef.Source.Volume.Pool)
if err != nil {
return fmt.Errorf("Error retrieving pool for disk: %s", err)
}
defer pool.Free()

virVol, err := pool.LookupStorageVolByName(diskDef.Source.Volume.Volume)
if err != nil {
return fmt.Errorf("Error retrieving volume for disk: %s", err)
}
defer virVol.Free()

virVolKey, err := virVol.GetKey()
if err != nil {
return fmt.Errorf("Error retrieving volume key for disk: %s", err)
}

disk = map[string]interface{}{
"volume_id": virVolKey,
}
}

disks = append(disks, disk)
}
d.Set("disks", disks)
Expand Down Expand Up @@ -1133,14 +1158,51 @@ func setDisks(d *schema.ResourceData, domainDef *libvirtxml.Domain, virConn *lib
if err != nil {
return fmt.Errorf("Can't retrieve volume %s: %v", volumeKey.(string), err)
}
diskVolumeFile, err := diskVolume.GetPath()

diskVolumeName, err := diskVolume.GetName()
if err != nil {
return fmt.Errorf("Error retrieving volume file: %s", err)
return fmt.Errorf("Can't retrieve name for volume %s", volumeKey.(string))
}

diskPool, err := diskVolume.LookupPoolByVolume()
if err != nil {
return fmt.Errorf("Can't retrieve pool for volume %s", volumeKey.(string))
}

diskPoolName, err := diskPool.GetName()
if err != nil {
return fmt.Errorf("Can't retrieve name for pool of volume %s", volumeKey.(string))
}

// find out the format of the volume in order to set the appropriate
// driver
volumeDef, err := newDefVolumeFromLibvirt(diskVolume)
if err != nil {
return err
}
if volumeDef.Target != nil && volumeDef.Target.Format != nil && volumeDef.Target.Format.Type != "" {
if volumeDef.Target.Format.Type == "qcow2" {
log.Print("[DEBUG] Setting disk driver to 'qcow2' to match disk volume format")
disk.Driver = &libvirtxml.DomainDiskDriver{
Name: "qemu",
Type: "qcow2",
}
}
if volumeDef.Target.Format.Type == "raw" {
log.Print("[DEBUG] Setting disk driver to 'raw' to match disk volume format")
disk.Driver = &libvirtxml.DomainDiskDriver{
Name: "qemu",
Type: "raw",
}
}
} else {
log.Printf("[WARN] Disk volume has no format specified: %s", volumeKey.(string))
}

disk.Source = &libvirtxml.DomainDiskSource{
File: &libvirtxml.DomainDiskSourceFile{
File: diskVolumeFile,
Volume: &libvirtxml.DomainDiskSourceVolume{
Pool: diskPoolName,
Volume: diskVolumeName,
},
}
} else if rawURL, ok := d.GetOk(prefix + ".url"); ok {
Expand All @@ -1163,6 +1225,7 @@ func setDisks(d *schema.ResourceData, domainDef *libvirtxml.Domain, virConn *lib
},
},
}

if strings.HasSuffix(url.Path, ".iso") {
disk.Device = "cdrom"
}
Expand Down
105 changes: 92 additions & 13 deletions libvirt/resource_libvirt_domain_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,13 @@ import (
"log"
"net/url"
"os"
"path/filepath"
"testing"

"github.com/hashicorp/terraform/helper/resource"
"github.com/hashicorp/terraform/terraform"
libvirt "github.com/libvirt/libvirt-go"
libvirtxml "github.com/libvirt/libvirt-go-xml"
)

func TestAccLibvirtDomain_Basic(t *testing.T) {
Expand Down Expand Up @@ -170,6 +172,60 @@ func TestAccLibvirtDomain_VolumeTwoDisks(t *testing.T) {
})
}

// tests that disk driver is set correctly for the volume format
func TestAccLibvirtDomain_VolumeDriver(t *testing.T) {
var domain libvirt.Domain
var volumeRaw libvirt.StorageVol
var volumeQCOW2 libvirt.StorageVol

var config = fmt.Sprintf(`
resource "libvirt_volume" "acceptance-test-volume-raw" {
name = "terraform-test-raw"
format = "raw"
}
resource "libvirt_volume" "acceptance-test-volume-qcow2" {
name = "terraform-test-qcow2"
format = "qcow2"
}
resource "libvirt_domain" "acceptance-test-domain" {
name = "terraform-test-domain"
disk {
volume_id = "${libvirt_volume.acceptance-test-volume-raw.id}"
}
disk {
volume_id = "${libvirt_volume.acceptance-test-volume-qcow2.id}"
}
}`)

resource.Test(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
Providers: testAccProviders,
CheckDestroy: testAccCheckLibvirtDomainDestroy,
Steps: []resource.TestStep{
{
Config: config,
Check: resource.ComposeTestCheckFunc(
testAccCheckLibvirtDomainExists("libvirt_domain.acceptance-test-domain", &domain),
testAccCheckLibvirtVolumeExists("libvirt_volume.acceptance-test-volume-raw", &volumeRaw),
testAccCheckLibvirtVolumeExists("libvirt_volume.acceptance-test-volume-qcow2", &volumeQCOW2),
// Check that each disk has the appropriate driver
testAccCheckLibvirtDomainDescription(&domain, func(domainDef libvirtxml.Domain) error {
if domainDef.Devices.Disks[0].Driver.Type != "raw" {
return fmt.Errorf("Expected disk to have RAW driver")
}
if domainDef.Devices.Disks[1].Driver.Type != "qcow2" {
return fmt.Errorf("Expected disk to have QCOW2 driver")
}
return nil
})),
},
},
})
}

func TestAccLibvirtDomain_ScsiDisk(t *testing.T) {
var domain libvirt.Domain
var configScsi = fmt.Sprintf(`
Expand Down Expand Up @@ -205,7 +261,24 @@ func TestAccLibvirtDomain_ScsiDisk(t *testing.T) {

func TestAccLibvirtDomainURLDisk(t *testing.T) {
var domain libvirt.Domain
u, err := url.Parse("http://download.opensuse.org/tumbleweed/iso/openSUSE-Tumbleweed-DVD-x86_64-Current.iso")

fws := fileWebServer{}
if err := fws.Start(); err != nil {
t.Fatal(err)
}
defer fws.Stop()

isoPath, err := filepath.Abs("testdata/tcl.iso")
if err != nil {
t.Fatal(err)
}

u, err := fws.AddFile(isoPath)
if err != nil {
t.Error(err)
}

url, err := url.Parse(u)
if err != nil {
t.Error(err)
}
Expand All @@ -216,7 +289,7 @@ func TestAccLibvirtDomainURLDisk(t *testing.T) {
disk {
url = "%s"
}
}`, u.String())
}`, url.String())

resource.Test(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
Expand All @@ -227,7 +300,7 @@ func TestAccLibvirtDomainURLDisk(t *testing.T) {
Config: configURL,
Check: resource.ComposeTestCheckFunc(
testAccCheckLibvirtDomainExists("libvirt_domain.acceptance-test-domain", &domain),
testAccCheckLibvirtURLDisk(u, &domain),
testAccCheckLibvirtURLDisk(url, &domain),
),
},
},
Expand Down Expand Up @@ -731,6 +804,21 @@ func TestHash(t *testing.T) {
}

func testAccCheckLibvirtScsiDisk(n string, domain *libvirt.Domain) resource.TestCheckFunc {
return testAccCheckLibvirtDomainDescription(domain, func(domainDef libvirtxml.Domain) error {
disks := domainDef.Devices.Disks
for _, disk := range disks {
if diskBus := disk.Target.Bus; diskBus != "scsi" {
return fmt.Errorf("Disk bus is not scsi")
}
if wwn := disk.WWN; wwn != n {
return fmt.Errorf("Disk wwn %s is not equal to %s", wwn, n)
}
}
return nil
})
}

func testAccCheckLibvirtDomainDescription(domain *libvirt.Domain, checkFunc func(libvirtxml.Domain) error) resource.TestCheckFunc {
return func(s *terraform.State) error {
xmlDesc, err := domain.GetXMLDesc(0)
if err != nil {
Expand All @@ -743,16 +831,7 @@ func testAccCheckLibvirtScsiDisk(n string, domain *libvirt.Domain) resource.Test
return fmt.Errorf("Error reading libvirt domain XML description: %s", err)
}

disks := domainDef.Devices.Disks
for _, disk := range disks {
if diskBus := disk.Target.Bus; diskBus != "scsi" {
return fmt.Errorf("Disk bus is not scsi")
}
if wwn := disk.WWN; wwn != n {
return fmt.Errorf("Disk wwn %s is not equal to %s", wwn, n)
}
}
return nil
return checkFunc(domainDef)
}
}

Expand Down
37 changes: 31 additions & 6 deletions libvirt/resource_libvirt_volume.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ func resourceLibvirtVolume() *schema.Resource {
},
"format": {
Type: schema.TypeString,
Computed: true,
Optional: true,
ForceNew: true,
},
Expand Down Expand Up @@ -100,17 +101,16 @@ func resourceLibvirtVolumeCreate(d *schema.ResourceData, meta interface{}) error
volumeDef := newDefVolume()
volumeDef.Name = d.Get("name").(string)

volumeFormat := "qcow2"
if _, ok := d.GetOk("format"); ok {
volumeFormat = d.Get("format").(string)
}
volumeDef.Target.Format.Type = volumeFormat

var (
img image
volume *libvirt.StorageVol
)

givenFormat, isFormatGiven := d.GetOk("format")
if isFormatGiven {
volumeDef.Target.Format.Type = givenFormat.(string)
}

// an source image was given, this mean we can't choose size
if source, ok := d.GetOk("source"); ok {
// source and size conflict
Expand All @@ -129,6 +129,19 @@ func resourceLibvirtVolumeCreate(d *schema.ResourceData, meta interface{}) error
return err
}

// figure out the format of the image
isQCOW2, err := img.IsQCOW2()
if err != nil {
return fmt.Errorf("Error while determining image type for %s: %s", img.String(), err)
}
if isQCOW2 {
volumeDef.Target.Format.Type = "qcow2"
}

if isFormatGiven && isQCOW2 && givenFormat != "qcow2" {
return fmt.Errorf("Format other than QCOW2 explicitly specified for image detected as QCOW2 image: %s", img.String())
}

// update the image in the description, even if the file has not changed
size, err := img.Size()
if err != nil {
Expand Down Expand Up @@ -279,6 +292,18 @@ func resourceLibvirtVolumeRead(d *schema.ResourceData, meta interface{}) error {
}
d.Set("size", info.Capacity)

volumeDef, err := newDefVolumeFromLibvirt(volume)
if err != nil {
return err
}

if volumeDef.Target == nil || volumeDef.Target.Format == nil || volumeDef.Target.Format.Type == "" {
log.Printf("Volume has no format specified: %s", volName)
} else {
log.Printf("[DEBUG] Volume %s format: %s", volName, volumeDef.Target.Format.Type)
d.Set("format", volumeDef.Target.Format.Type)
}

return nil
}

Expand Down

0 comments on commit 676b5a3

Please sign in to comment.