Fix domain running status fails when shutdown expected (#622)

[luc-phan]

This PR fixes the issue of the guest not being shutdown when updating running to false (#622).

$ vim main.tf

terraform {
  required_providers {
    libvirt = {
      source = "dmacvicar/libvirt"
      version = ">= 0.7.1"
    }
  }
}

provider "libvirt" {
  uri = "qemu:///system"
}

resource "libvirt_domain" "myvm" {
  name   = "myvm"
  vcpu   = 1
  running = true
}

$ terraform init
$ terraform apply
$ virsh list --all
 Id    Name   State
-----------------------
 140   myvm   running

$ vim main.tf

terraform {
  required_providers {
    libvirt = {
      source = "dmacvicar/libvirt"
      version = ">= 0.7.1"
    }
  }
}

provider "libvirt" {
  uri = "qemu:///system"
}

resource "libvirt_domain" "myvm" {
  name   = "myvm"
  vcpu   = 1
  running = false  # I changed this!
}

$ terraform apply
$ virsh list --all
 Id    Name   State
-----------------------
 140   myvm   running

The expected state is shut off.

[tiaden]

@luc-phan, Thank you for the PR. I was about to create a PR for this issue and I noticed that you already did. I hope we can collaborate on this together. I looked at your code. It's well written. However if the domain is created with the running at false, then switched to true during update, it will start the domain and immediately issue a shutdown. What I did in my case was to leverage the existing code and implement the same functionality in just 4 lines of code and avoid the issue explained above.

diff --git a/libvirt/resource_libvirt_domain.go b/libvirt/resource_libvirt_domain.go
index 5565d8542..c0d86aeeb 100644
--- a/libvirt/resource_libvirt_domain.go
+++ b/libvirt/resource_libvirt_domain.go
@@ -675,7 +675,7 @@ func resourceLibvirtDomainUpdate(ctx context.Context, d *schema.ResourceData, me
 		return diag.FromErr(err)
 	}
 
-	if !domainRunningNow {
+	if !domainRunningNow && d.Get("running").(bool) {
 		err = virConn.DomainCreate(domain)
 		if err != nil {
 			return diag.Errorf("error creating libvirt domain: %s", err)
@@ -756,6 +756,9 @@ func resourceLibvirtDomainUpdate(ctx context.Context, d *schema.ResourceData, me
 		}
 	}
 
+	if err := destroyDomainByUserRequest(virConn, d, domain); err != nil {
+		return diag.FromErr(err)
+	}
 	return nil
 }
 

Let me know what you think. This is a link to the commit I made tiaden@06341a4

[luc-phan]

However if the domain is created with the running at false, then switched to true during update, it will start the domain and immediately issue a shutdown.

No. It will not issue a shutdown, but there is some redundant code... The VM is started...

• there :

  terraform-provider-libvirt/libvirt/resource_libvirt_domain.go

  Line 679 in 558fe47

  err = virConn.DomainCreate(domain)

• and there :

  terraform-provider-libvirt/libvirt/domain.go

  Line 883 in 558fe47

  err := virConn.DomainCreate(domain)

I think it's what you mean. Thank you, I didn't noticed that. I'm going to fix it.

Let me know what you think.

Indeed, your code is better, it leverages the existing code, and the VM is not started twice.

I'm going to fix my PR anyway, even though it's rejected, because I'm still learning, and it's a good exercise for me :)

[luc-phan]

I added 2 arguments :

• shutdown_timeout
• shutdown_force = DomainDestroy()