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

[Ryan Goodfellow]

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/
> dberrange :|
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20170427/597e5974/attachment-0001.htm>