Add a few explaining lines to the docs about ICMP types/codes

builtin/providers/cloudstack/resource_cloudstack_disk.go

@@ -51,7 +51,7 @@ func resourceCloudStackDisk() *schema.Resource {
 				Default:  false,
 			},
 
-			"virtual_machine": &schema.Schema{
+			"virtual_machine_id": &schema.Schema{
 				Type:     schema.TypeString,
 				Optional: true,
 			},
@@ -119,7 +119,7 @@ func resourceCloudStackDiskCreate(d *schema.ResourceData, meta interface{}) erro
 	d.SetPartial("device")
 	d.SetPartial("disk_offering")
 	d.SetPartial("size")
-	d.SetPartial("virtual_machine")
+	d.SetPartial("virtual_machine_id")
 	d.SetPartial("project")
 	d.SetPartial("zone")
 
@@ -185,7 +185,7 @@ func resourceCloudStackDiskRead(d *schema.ResourceData, meta interface{}) error
 		}
 
 		d.Set("device", retrieveDeviceName(v.Deviceid, c.Name))
-		setValueOrID(d, "virtual_machine", v.Vmname, v.Virtualmachineid)
+		d.Set("virtual_machine_id", v.Virtualmachineid)
 	}
 
 	return nil
@@ -254,7 +254,7 @@ func resourceCloudStackDiskUpdate(d *schema.ResourceData, meta interface{}) erro
 		// Set the additional partials
 		d.SetPartial("attach")
 		d.SetPartial("device")
-		d.SetPartial("virtual_machine")
+		d.SetPartial("virtual_machine_id")
 	} else {
 		// Detach the volume
 		if err := resourceCloudStackDiskDetach(d, meta); err != nil {
@@ -295,43 +295,34 @@ func resourceCloudStackDiskDelete(d *schema.ResourceData, meta interface{}) erro
 func resourceCloudStackDiskAttach(d *schema.ResourceData, meta interface{}) error {
 	cs := meta.(*cloudstack.CloudStackClient)
 
-	// First check if the disk isn't already attached
+	if virtualmachineid, ok := d.GetOk("virtual_machine_id"); ok {
-	if attached, err := isAttached(d, meta); err != nil || attached {
+		// First check if the disk isn't already attached
-		return err
+		if attached, err := isAttached(d, meta); err != nil || attached {
-	}
+			return err
-
-	// Retrieve the virtual_machine ID
-	virtualmachineid, e := retrieveID(
-		cs,
-		"virtual_machine",
-		d.Get("virtual_machine").(string),
-		cloudstack.WithProject(d.Get("project").(string)),
-	)
-	if e != nil {
-		return e.Error()
-	}
-
-	// Create a new parameter struct
-	p := cs.Volume.NewAttachVolumeParams(d.Id(), virtualmachineid)
-
-	if device, ok := d.GetOk("device"); ok {
-		// Retrieve the device ID
-		deviceid := retrieveDeviceID(device.(string))
-		if deviceid == -1 {
-			return fmt.Errorf("Device %s is not a valid device", device.(string))
 		}
 
-		// Set the device ID
+		// Create a new parameter struct
-		p.SetDeviceid(deviceid)
+		p := cs.Volume.NewAttachVolumeParams(d.Id(), virtualmachineid.(string))
-	}
 
-	// Attach the new volume
+		if device, ok := d.GetOk("device"); ok {
-	r, err := Retry(4, retryableAttachVolumeFunc(cs, p))
+			// Retrieve the device ID
-	if err != nil {
+			deviceid := retrieveDeviceID(device.(string))
-		return err
+			if deviceid == -1 {
-	}
+				return fmt.Errorf("Device %s is not a valid device", device.(string))
+			}
 
-	d.SetId(r.(*cloudstack.AttachVolumeResponse).Id)
+			// Set the device ID
+			p.SetDeviceid(deviceid)
+		}
+
+		// Attach the new volume
+		r, err := Retry(4, retryableAttachVolumeFunc(cs, p))
+		if err != nil {
+			return err
+		}
+
+		d.SetId(r.(*cloudstack.AttachVolumeResponse).Id)
+	}
 
 	return nil
 }
@@ -351,41 +342,33 @@ func resourceCloudStackDiskDetach(d *schema.ResourceData, meta interface{}) erro
 	p.SetId(d.Id())
 
 	// Detach the currently attached volume
-	if _, err := cs.Volume.DetachVolume(p); err != nil {
+	_, err := cs.Volume.DetachVolume(p)
-		// Retrieve the virtual_machine ID
+	if err != nil {
-		virtualmachineid, e := retrieveID(
+		if virtualmachineid, ok := d.GetOk("virtual_machine_id"); ok {
-			cs,
+			// Create a new parameter struct
-			"virtual_machine",
+			pd := cs.VirtualMachine.NewStopVirtualMachineParams(virtualmachineid.(string))
-			d.Get("virtual_machine").(string),
-			cloudstack.WithProject(d.Get("project").(string)),
-		)
-		if e != nil {
-			return e.Error()
-		}
 
-		// Create a new parameter struct
+			// Stop the virtual machine in order to be able to detach the disk
-		pd := cs.VirtualMachine.NewStopVirtualMachineParams(virtualmachineid)
+			if _, err := cs.VirtualMachine.StopVirtualMachine(pd); err != nil {
+				return err
+			}
 
-		// Stop the virtual machine in order to be able to detach the disk
+			// Try again to detach the currently attached volume
-		if _, err := cs.VirtualMachine.StopVirtualMachine(pd); err != nil {
+			if _, err := cs.Volume.DetachVolume(p); err != nil {
-			return err
+				return err
-		}
+			}
 
-		// Try again to detach the currently attached volume
+			// Create a new parameter struct
-		if _, err := cs.Volume.DetachVolume(p); err != nil {
+			pu := cs.VirtualMachine.NewStartVirtualMachineParams(virtualmachineid.(string))
-			return err
-		}
 
-		// Create a new parameter struct
+			// Start the virtual machine again
-		pu := cs.VirtualMachine.NewStartVirtualMachineParams(virtualmachineid)
+			if _, err := cs.VirtualMachine.StartVirtualMachine(pu); err != nil {
-
+				return err
-		// Start the virtual machine again
+			}
-		if _, err := cs.VirtualMachine.StartVirtualMachine(pu); err != nil {
-			return err
 		}
 	}
 
-	return nil
+	return err
 }
 
 func isAttached(d *schema.ResourceData, meta interface{}) (bool, error) {

builtin/providers/cloudstack/resource_cloudstack_disk_test.go

@@ -186,7 +186,7 @@ resource "cloudstack_disk" "foo" {
   attach = true
   device = "/dev/xvde"
   disk_offering = "%s"
-  virtual_machine = "${cloudstack_instance.foobar.name}"
+  virtual_machine_id = "${cloudstack_instance.foobar.id}"
   zone = "${cloudstack_instance.foobar.zone}"
 }`,
 	CLOUDSTACK_SERVICE_OFFERING_1,
@@ -210,7 +210,7 @@ resource "cloudstack_disk" "foo" {
   name = "terraform-disk"
   attach = true
   disk_offering = "%s"
-  virtual_machine = "${cloudstack_instance.foobar.name}"
+  virtual_machine_id = "${cloudstack_instance.foobar.id}"
   zone = "${cloudstack_instance.foobar.zone}"
 }`,
 	CLOUDSTACK_SERVICE_OFFERING_1,
@@ -234,7 +234,7 @@ resource "cloudstack_disk" "foo" {
   name = "terraform-disk"
   attach = true
   disk_offering = "%s"
-  virtual_machine = "${cloudstack_instance.foobar.name}"
+	virtual_machine_id = "${cloudstack_instance.foobar.id}"
   zone = "${cloudstack_instance.foobar.zone}"
 }`,
 	CLOUDSTACK_SERVICE_OFFERING_1,

builtin/providers/cloudstack/resource_cloudstack_instance.go

@@ -207,46 +207,38 @@ func resourceCloudStackInstanceCreate(d *schema.ResourceData, meta interface{})
 	}
 
 	// If there are affinity group IDs supplied, add them to the parameter struct
-	if ags := d.Get("affinity_group_ids").(*schema.Set); ags.Len() > 0 {
+	if agIDs := d.Get("affinity_group_ids").(*schema.Set); agIDs.Len() > 0 {
 		var groups []string
-
+		for _, group := range agIDs.List() {
-		for _, group := range ags.List() {
 			groups = append(groups, group.(string))
 		}
-
 		p.SetAffinitygroupids(groups)
 	}
 
-	// If there is a affinity_group_names supplied, add it to the parameter struct
+	// If there are affinity group names supplied, add them to the parameter struct
-	if agns := d.Get("affinity_group_names").(*schema.Set); agns.Len() > 0 {
+	if agNames := d.Get("affinity_group_names").(*schema.Set); agNames.Len() > 0 {
 		var groups []string
-
+		for _, group := range agNames.List() {
-		for _, group := range agns.List() {
 			groups = append(groups, group.(string))
 		}
-
 		p.SetAffinitygroupnames(groups)
 	}
 
-	// If there is a security_group_ids supplied, add it to the parameter struct
+	// If there are security group IDs supplied, add them to the parameter struct
-	if sgids := d.Get("security_group_ids").(*schema.Set); sgids.Len() > 0 {
+	if sgIDs := d.Get("security_group_ids").(*schema.Set); sgIDs.Len() > 0 {
 		var groups []string
-
+		for _, group := range sgIDs.List() {
-		for _, group := range sgids.List() {
 			groups = append(groups, group.(string))
 		}
-
 		p.SetSecuritygroupids(groups)
 	}
 
-	// If there is a security_group_names supplied, add it to the parameter struct
+	// If there are security group names supplied, add them to the parameter struct
-	if sgns := d.Get("security_group_names").(*schema.Set); sgns.Len() > 0 {
+	if sgNames := d.Get("security_group_names").(*schema.Set); sgNames.Len() > 0 {
 		var groups []string
-
+		for _, group := range sgNames.List() {
-		for _, group := range sgns.List() {
 			groups = append(groups, group.(string))
 		}
-
 		p.SetSecuritygroupnames(groups)
 	}
 
@@ -324,47 +316,43 @@ func resourceCloudStackInstanceRead(d *schema.ResourceData, meta interface{}) er
 	d.Set("ip_address", vm.Nic[0].Ipaddress)
 	d.Set("group", vm.Group)
 
+	if _, ok := d.GetOk("affinity_group_ids"); ok {
+		groups := &schema.Set{F: schema.HashString}
+		for _, group := range vm.Affinitygroup {
+			groups.Add(group.Id)
+		}
+		d.Set("affinity_group_ids", groups)
+	}
+
+	if _, ok := d.GetOk("affinity_group_names"); ok {
+		groups := &schema.Set{F: schema.HashString}
+		for _, group := range vm.Affinitygroup {
+			groups.Add(group.Name)
+		}
+		d.Set("affinity_group_names", groups)
+	}
+
+	if _, ok := d.GetOk("security_group_ids"); ok {
+		groups := &schema.Set{F: schema.HashString}
+		for _, group := range vm.Securitygroup {
+			groups.Add(group.Id)
+		}
+		d.Set("security_group_ids", groups)
+	}
+
+	if _, ok := d.GetOk("security_group_names"); ok {
+		groups := &schema.Set{F: schema.HashString}
+		for _, group := range vm.Securitygroup {
+			groups.Add(group.Name)
+		}
+		d.Set("security_group_names", groups)
+	}
+
 	setValueOrID(d, "service_offering", vm.Serviceofferingname, vm.Serviceofferingid)
 	setValueOrID(d, "template", vm.Templatename, vm.Templateid)
 	setValueOrID(d, "project", vm.Project, vm.Projectid)
 	setValueOrID(d, "zone", vm.Zonename, vm.Zoneid)
 
-	groups := &schema.Set{F: schema.HashString}
-	for _, group := range vm.Affinitygroup {
-		groups.Add(group.Id)
-	}
-
-	if groups.Len() > 0 {
-		d.Set("affinity_group_ids", groups)
-	}
-
-	agns := &schema.Set{F: schema.HashString}
-	for _, group := range vm.Affinitygroup {
-		agns.Add(group.Name)
-	}
-
-	if agns.Len() > 0 {
-		d.Set("affinity_group_names", agns)
-	}
-
-	sgids := &schema.Set{F: schema.HashString}
-	for _, group := range vm.Securitygroup {
-		sgids.Add(group.Id)
-	}
-
-	if sgids.Len() > 0 {
-		d.Set("security_group_ids", sgids)
-	}
-
-	sgns := &schema.Set{F: schema.HashString}
-	for _, group := range vm.Securitygroup {
-		sgns.Add(group.Name)
-	}
-
-	if sgns.Len() > 0 {
-		d.Set("security_group_names", sgns)
-	}
-
 	return nil
 }
 
@@ -415,7 +403,7 @@ func resourceCloudStackInstanceUpdate(d *schema.ResourceData, meta interface{})
 	}
 
 	// Attributes that require reboot to update
-	if d.HasChange("name") || d.HasChange("service_offering") || d.HasChange("affinity_group_ids") || d.HasChange("keypair") {
+	if d.HasChange("name") || d.HasChange("service_offering") || d.HasChange("affinity_group_ids") || d.HasChange("affinity_group_names") || d.HasChange("keypair") {
 		// Before we can actually make these changes, the virtual machine must be stopped
 		_, err := cs.VirtualMachine.StopVirtualMachine(
 			cs.VirtualMachine.NewStopVirtualMachineParams(d.Id()))
@@ -470,8 +458,21 @@ func resourceCloudStackInstanceUpdate(d *schema.ResourceData, meta interface{})
 			p := cs.AffinityGroup.NewUpdateVMAffinityGroupParams(d.Id())
 			groups := []string{}
 
-			if ags := d.Get("affinity_group_ids").(*schema.Set); ags.Len() > 0 {
+			if agIDs := d.Get("affinity_group_ids").(*schema.Set); agIDs.Len() > 0 {
-				for _, group := range ags.List() {
+				for _, group := range agIDs.List() {
+					groups = append(groups, group.(string))
+				}
+			}
+
+			p.SetAffinitygroupids(groups)
+		}
+
+		if d.HasChange("affinity_group_names") {
+			p := cs.AffinityGroup.NewUpdateVMAffinityGroupParams(d.Id())
+			groups := []string{}
+
+			if agNames := d.Get("affinity_group_names").(*schema.Set); agNames.Len() > 0 {
+				for _, group := range agNames.List() {
 					groups = append(groups, group.(string))
 				}
 			}

builtin/providers/cloudstack/resources.go

@@ -49,8 +49,6 @@ func retrieveID(cs *cloudstack.CloudStackClient, name string, value string, opts
 	switch name {
 	case "disk_offering":
 		id, err = cs.DiskOffering.GetDiskOfferingID(value)
-	case "virtual_machine":
-		id, err = cs.VirtualMachine.GetVirtualMachineID(value, opts...)
 	case "service_offering":
 		id, err = cs.ServiceOffering.GetServiceOfferingID(value)
 	case "network_offering":

website/source/docs/providers/cloudstack/r/disk.html.markdown

@@ -44,8 +44,8 @@ The following arguments are supported:
 * `shrink_ok` - (Optional) Verifies if the disk volume is allowed to shrink when
     resizing (defaults false).
 
-* `virtual_machine` - (Optional) The name or ID of the virtual machine to which you
+* `virtual_machine_id` - (Optional) The ID of the virtual machine to which you want
-    want to attach the disk volume.
+    to attach the disk volume.
 
 * `project` - (Optional) The name or ID of the project to deploy this
     instance to. Changing this forces a new resource to be created.

website/source/docs/providers/cloudstack/r/network_acl_rule.html.markdown

@@ -53,11 +53,11 @@ The `rule` block supports:
 * `protocol` - (Required) The name of the protocol to allow. Valid options are:
     `tcp`, `udp`, `icmp`, `all` or a valid protocol number.
 
-* `icmp_type` - (Optional) The ICMP type to allow. This can only be specified if
+* `icmp_type` - (Optional) The ICMP type to allow, or `-1` to allow `any`. This
-    the protocol is ICMP.
+    can only be specified if the protocol is ICMP. (defaults 0)
 
-* `icmp_code` - (Optional) The ICMP code to allow. This can only be specified if
+* `icmp_code` - (Optional) The ICMP code to allow, or `-1` to allow `any`. This
-    the protocol is ICMP.
+    can only be specified if the protocol is ICMP. (defaults 0)
 
 * `ports` - (Optional) List of ports and/or port ranges to allow. This can only
     be specified if the protocol is TCP, UDP, ALL or a valid protocol number.