[libvirt] [PATCH go-xml] add support for domain features

[Daniel P. Berrange]

On Sat, Apr 22, 2017 at 02:53:48PM -0700, Ryan Goodfellow wrote:
> This commit adds support for domain features. It does so by introducing
> a new family of types DomainFeature*. The aggregate type
> DomainFeatureList has been added to the Domain type to plumb in the new
> type family. Testing has also been added in domain_test.go
> ---
>  domain.go      | 91 +++++++++++++++++++++++++++++++++++++++++++++++-----------
>  domain_test.go | 55 +++++++++++++++++++++++++++++++++++
>  2 files changed, 130 insertions(+), 16 deletions(-)
> 
> diff --git a/domain.go b/domain.go
> index cccd9a6..c9ffaef 100644
> --- a/domain.go
> +++ b/domain.go
> @@ -371,23 +371,82 @@ type DomainCPU struct {
>  	Features []DomainCPUFeature `xml:"feature"`
>  }
>  
> +type DomainFeature struct {
> +	State string `xml:"state,attr,omitempty"`
> +}

There is no 'state' attribute on most top level features - this is just
something used under the hyperv features....

> +
> +type DomainFeatureAPIC struct {
> +	DomainFeature
> +     EOI string `xml:"eio,attr,omitempty"`
> +}

So this is wrong for example

> +
> +type DomainFeatureVendorId struct {
> +	DomainFeature
> +	Value string `xml:"value,attr,omitempty"`
> +}
> +
> +type DomainFeatureSpinlocks struct {
> +	DomainFeature
> +	Retries uint `xml:"retries,attr,omitempty"`
> +}

Since these are hyperv featurs, the struct name should have
HyperV in the name too.

> +
> +type DomainFeatureHyperV struct {
> +	DomainFeature
> +	Relaxed   *DomainFeature          `xml:"relaxed,omitempty"`
> +	VAPIC     *DomainFeature          `xml:"vapic,omitempty"`
> +	Spinlocks *DomainFeatureSpinlocks `xml:"spinlocks,omitempty"`
> +	VPIndex   *DomainFeature          `xml:"vpindex,omitempty"`
> +	Runtime   *DomainFeature          `xml:"runtime,omitempty"`
> +	Synic     *DomainFeature          `xml:"synic,omitempty"`
> +	STimer    *DomainFeature          `xml:"stimer,omitempty"`
> +	Reset     *DomainFeature          `xml:"reset,omitempty"`
> +	VendorId  *DomainFeatureVendorId  `xml:"vendor_id,omitempty"`

There's no need to use 'omitempty' when the type is a struct
pointer - its only needed for scalar types.

> +}
> +
> +type DomainFeatureKVM struct {
> +	DomainFeature
> +	Hidden *DomainFeature `xml:"hidden,omitempty"`
> +}
> +
> +type DomainFeatureGIC struct {
> +	DomainFeature
> +	Version string `xml:"version,attr,omitempty"`
> +}
> +
> +type DomainFeatureList struct {
> +	PAE        *DomainFeature       `xml:"pae,omitempty"`
> +	ACPI       *DomainFeature       `xml:"acpi,omitempty"`
> +	APIC       *DomainFeatureAPIC   `xml:"apic,omitempty"`
> +	HAP        *DomainFeature       `xml:"hap,omitempty"`
> +	Viridian   *DomainFeature       `xml:"viridian,omitempty"`
> +	PrivNet    *DomainFeature       `xml:"privnet,omitempty"`
> +	HyperV     *DomainFeatureHyperV `xml:"hyperv,omitempty"`
> +	KVM        *DomainFeatureKVM    `xml:"kvm,omitempty"`
> +	PVSpinlock *DomainFeature       `xml:"pvspinlock,omitempty"`
> +	PMU        *DomainFeature       `xml:"pmu,omitempty"`
> +	VMPort     *DomainFeature       `xml:"vmport,omitempty"`
> +	GIC        *DomainFeatureGIC    `xml:"gic,omitempty"`
> +	SMM        *DomainFeature       `xml:"smm,omitempty"`
> +}

None of these should use 'DomainFeature', since they do not
want a 'state' attribute. Also same note about omitempty not
being needed.

> +
>  type Domain struct {
> -	XMLName       xml.Name          `xml:"domain"`
> -	Type          string            `xml:"type,attr,omitempty"`
> -	Name          string            `xml:"name"`
> -	UUID          string            `xml:"uuid,omitempty"`
> -	Memory        *DomainMemory     `xml:"memory"`
> -	CurrentMemory *DomainMemory     `xml:"currentMemory"`
> -	MaximumMemory *DomainMaxMemory  `xml:"maxMemory"`
> -	VCPU          *DomainVCPU       `xml:"vcpu"`
> -	CPU           *DomainCPU        `xml:"cpu"`
> -	Resource      *DomainResource   `xml:"resource"`
> -	Devices       *DomainDeviceList `xml:"devices"`
> -	OS            *DomainOS         `xml:"os"`
> -	SysInfo       *DomainSysInfo    `xml:"sysinfo"`
> -	OnPoweroff    string            `xml:"on_poweroff,omitempty"`
> -	OnReboot      string            `xml:"on_reboot,omitempty"`
> -	OnCrash       string            `xml:"on_crash,omitempty"`
> +	XMLName       xml.Name           `xml:"domain"`
> +	Type          string             `xml:"type,attr,omitempty"`
> +	Name          string             `xml:"name"`
> +	UUID          string             `xml:"uuid,omitempty"`
> +	Memory        *DomainMemory      `xml:"memory"`
> +	CurrentMemory *DomainMemory      `xml:"currentMemory"`
> +	MaximumMemory *DomainMaxMemory   `xml:"maxMemory"`
> +	VCPU          *DomainVCPU        `xml:"vcpu"`
> +	CPU           *DomainCPU         `xml:"cpu"`
> +	Resource      *DomainResource    `xml:"resource"`
> +	Devices       *DomainDeviceList  `xml:"devices"`
> +	OS            *DomainOS          `xml:"os"`
> +	SysInfo       *DomainSysInfo     `xml:"sysinfo"`
> +	OnPoweroff    string             `xml:"on_poweroff,omitempty"`
> +	OnReboot      string             `xml:"on_reboot,omitempty"`
> +	OnCrash       string             `xml:"on_crash,omitempty"`
> +	Features      *DomainFeatureList `xml:"features,omitempty"`
>  }

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|