4:53 p.m.
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
5:47 a.m.
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 :|
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 :|
2:43 p.m.
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(a)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(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/dber
> range :|
--
*ry**@isi*
9:32 a.m.
On Thu, May 04, 2017 at 12:43:47PM -0700, Ryan Goodfellow wrote:
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(a)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(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/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 :|
2792
days inactive
2805
days old
4 comments
2 participants
participants (2)
-
Daniel P. Berrange -
Ryan Goodfellow