[libvirt] [PATCH go-xml] add support for domain features
Ryan Goodfellow
rgoodfel at isi.edu
Thu May 4 19:43:47 UTC 2017
ping, ref <
https://www.redhat.com/archives/libvir-list/2017-April/msg01073.html>
On Thu, Apr 27, 2017 at 12:35 PM, Ryan Goodfellow <rgoodfel at isi.edu> 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 at 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/dber
>> range :|
>
>
--
*ry**@isi*
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20170504/b4dc6e5e/attachment-0001.htm>
More information about the libvir-list
mailing list