On Thu, May 04, 2017 at 12:43:47PM -0700, Ryan Goodfellow wrote: > ping, ref < > https://www.redhat.com/archives/libvir-list/2017-April/msg01073.html> Oh, sorry, I didn't realize that was a follow up to this - usually we would recommand posting a complete standalone v2 patch, rather than just posting incremental changes. > > On Thu, Apr 27, 2017 at 12:35 PM, Ryan Goodfellow <rgoodfel@xxxxxxx> wrote: > > > 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@xxxxxxxxxx> > > 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/dber > >> range :| > > > > > > > -- > *ry**@isi* 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 :| -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
