2:35 p.m.
Hi Daniel,
I have addressed the issues you brought up in the commit titled 'remove
superfluous state & omitempty entries' that has been posted to the list.
--
cheers
ry
On Mon, Apr 24, 2017 at 3:47 AM, Daniel P. Berrange <berrange(a)redhat.com>
wrote:
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 :|