filtered params before appending them to the vm.set call

[princess-glimmr]

Hello there, I created this PR containing a proposal, how passing the params to the vm.set call could also be realised. I already reported the issue in #171.

TBH, I think my current implementation still needs a bit of work, as this is my first time writing any go-lang code. If there are any problems or required style changes to my code, simply let me know & I will try to make the adjustments.

I would be happy to receive a review & which parts require further work.

[ddelnano]

@princess-glimmr greatly appreciate your detailed bug report and this PR! I'll be giving this a look very soon.

[ddelnano]

Apologies for the long delay. I've had some issues with the test environment that I run the acceptance tests in and I've been working to address that.

I will be able to review this in more detail tomorrow.

[ddelnano]

@princess-glimmr apologies again for the delay. I know how slow reviews can frustrate contributing to a new project :(.

I think we should split this into two changes:

1. Fix resource set bug with minimal changes to XO client code
2. Provide a better abstraction for the XO client (recently split out into the xo-sdk-go repo)

I want to do this in two steps is because the client code was recently moved to another repo. This is to facilitate a new k8s integration that @ringods is working on. I haven't liked the current xo-sdk-go API. I wrote portions of it when I was less experienced with go and I think it deserves a more holistic revamp rather than change how the vm's update functionality works.

Since the VM update code already removes the resourceSet parameter when it is not set, I think we should update the provider code to do something like the following:

rs := ""
if d.HasChange("resource_set") {
  rs = d.Get("resource_set").(string)
}

[ ... ]
        vmReq := client.Vm{
                Id: id,
                CPUs: client.CPUs{
                        Number: cpus,
                },
                Memory: client.MemoryObject{
                        Static: []int{
                                0, memoryMax,
                        },
                },
                NameLabel:         nameLabel,
                NameDescription:   nameDescription,
                HA:                ha,
                ResourceSet:       rs,
                AutoPoweron:       autoPowerOn,
                AffinityHost:      affinityHost,
                BlockedOperations: blockOperations,
                ExpNestedHvm:      d.Get("exp_nested_hvm").(bool),
                StartDelay:        d.Get("start_delay").(int),
                Vga:               d.Get("vga").(string),
                // TODO: (#145) Uncomment this once issues with secure_boot have been figured out
                // SecureBoot:        d.Get("secure_boot").(bool),
                Boot: client.Boot{
                        Firmware: d.Get("hvm_boot_firmware").(string),
                },
                Videoram: client.Videoram{
                        Value: d.Get("videoram").(int),
                },
        }

I'm not sure if you've tried to get the terraform acceptance tests running (which we will need in order to merge a change), but I can help with creating that test if you would like. Please let me know if you'd like to attempt that yourself or if you would like my help.

I really do like how your proposed change simplifies the resourceVmUpdate logic, but I think we need to overhaul the xo-sdk-go client first before doing so.

[ddelnano]

@princess-glimmr sorry this took so long to fix. I'm going to merge the fix for #171 with #179. I appreciate your work to filter out only the necessary changes before making the API call, but I think we should improve the API of the Xen orchestra go client first (#189). I'll be looking into that in the coming weeks.