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

Ryan Goodfellow posted 1 patch 6 years, 11 months ago
Failed in applying to current master (apply log)
domain.go      | 91 +++++++++++++++++++++++++++++++++++++++++++++++-----------
domain_test.go | 55 +++++++++++++++++++++++++++++++++++
2 files changed, 130 insertions(+), 16 deletions(-)
[libvirt] [PATCH go-xml] add support for domain features
Posted by Ryan Goodfellow 6 years, 11 months ago
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"`
+}
+
+type DomainFeatureAPIC struct {
+	DomainFeature
+	EOI string `xml:"eio,attr,omitempty"`
+}
+
+type DomainFeatureVendorId struct {
+	DomainFeature
+	Value string `xml:"value,attr,omitempty"`
+}
+
+type DomainFeatureSpinlocks struct {
+	DomainFeature
+	Retries uint `xml:"retries,attr,omitempty"`
+}
+
+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"`
+}
+
+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"`
+}
+
 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"`
 }
 
 func (d *Domain) Unmarshal(doc string) error {
diff --git a/domain_test.go b/domain_test.go
index 06d585c..e25007e 100644
--- a/domain_test.go
+++ b/domain_test.go
@@ -745,6 +745,61 @@ var domainTestData = []struct {
 			`</domain>`,
 		},
 	},
+	{
+		Object: &Domain{
+			Type: "kvm",
+			Name: "test",
+			Features: &DomainFeatureList{
+				PAE:     &DomainFeature{},
+				ACPI:    &DomainFeature{},
+				APIC:    &DomainFeatureAPIC{},
+				HAP:     &DomainFeature{},
+				PrivNet: &DomainFeature{},
+				HyperV: &DomainFeatureHyperV{
+					Relaxed:   &DomainFeature{State: "on"},
+					VAPIC:     &DomainFeature{State: "on"},
+					Spinlocks: &DomainFeatureSpinlocks{DomainFeature{State: "on"}, 4096},
+					VPIndex:   &DomainFeature{State: "on"},
+					Runtime:   &DomainFeature{State: "on"},
+					Synic:     &DomainFeature{State: "on"},
+					Reset:     &DomainFeature{State: "on"},
+					VendorId:  &DomainFeatureVendorId{DomainFeature{State: "on"}, "KVM Hv"},
+				},
+				KVM: &DomainFeatureKVM{
+					Hidden: &DomainFeature{State: "on"},
+				},
+				PVSpinlock: &DomainFeature{State: "on"},
+				GIC:        &DomainFeatureGIC{Version: "2"},
+			},
+		},
+		Expected: []string{
+			`<domain type="kvm">`,
+			`  <name>test</name>`,
+			`  <features>`,
+			`    <pae></pae>`,
+			`    <acpi></acpi>`,
+			`    <apic></apic>`,
+			`    <hap></hap>`,
+			`    <privnet></privnet>`,
+			`    <hyperv>`,
+			`      <relaxed state="on"></relaxed>`,
+			`      <vapic state="on"></vapic>`,
+			`      <spinlocks state="on" retries="4096"></spinlocks>`,
+			`      <vpindex state="on"></vpindex>`,
+			`      <runtime state="on"></runtime>`,
+			`      <synic state="on"></synic>`,
+			`      <reset state="on"></reset>`,
+			`      <vendor_id state="on" value="KVM Hv"></vendor_id>`,
+			`    </hyperv>`,
+			`    <kvm>`,
+			`      <hidden state="on"></hidden>`,
+			`    </kvm>`,
+			`    <pvspinlock state="on"></pvspinlock>`,
+			`    <gic version="2"></gic>`,
+			`  </features>`,
+			`</domain>`,
+		},
+	},
 }
 
 func TestDomain(t *testing.T) {
-- 
2.11.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH go-xml] add support for domain features
Posted by Daniel P. Berrange 6 years, 11 months ago
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 :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH go-xml] add support for domain features
Posted by Ryan Goodfellow 6 years, 11 months ago
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@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 :|
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH go-xml] add support for domain features
Posted by Ryan Goodfellow 6 years, 10 months ago
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@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@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*
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH go-xml] add support for domain features
Posted by Daniel P. Berrange 6 years, 10 months ago
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@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@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*

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@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list