[v1] Add address support for memory device

[libvirt] [PATCH go-xml] Add address support for memory device

Posted by zhenwei.pi 6 years, 5 months ago

Add Base element for DomainAddress.
Add address element for DomainMemorydev.
Add test code for new DomainMemorydev.

Signed-off-by: zhenwei.pi <zhenwei.pi@youruncloud.com>
---
 domain.go      | 2 ++
 domain_test.go | 9 +++++++++
 2 files changed, 11 insertions(+)

diff --git a/domain.go b/domain.go
index bacab11..3d9404f 100644
--- a/domain.go
+++ b/domain.go
@@ -297,6 +297,7 @@ type DomainAddress struct {
 	Function   *HexUint `xml:"function,attr"`
 	Target     *uint    `xml:"target,attr"`
 	Unit       *uint    `xml:"unit,attr"`
+	Base       *HexUint `xml:"base,attr"`
 }
 
 type DomainConsole struct {
@@ -450,6 +451,7 @@ type DomainMemorydev struct {
 	Model   string                 `xml:"model,attr"`
 	Access  string                 `xml:"access,attr"`
 	Target  *DomainMemorydevTarget `xml:"target"`
+	Address *DomainAddress         `xml:"address"`
 }
 
 type DomainDeviceList struct {
diff --git a/domain_test.go b/domain_test.go
index dbebe42..cbc5d7f 100644
--- a/domain_test.go
+++ b/domain_test.go
@@ -66,6 +66,9 @@ var vcpuId0 uint = 0
 var vcpuOrder0 uint = 1
 var vcpuId1 uint = 1
 
+var memorydevAddressSlot HexUint = 0
+var memorydevAddressBase HexUint = 4294967296
+
 var domainTestData = []struct {
 	Object   Document
 	Expected []string
@@ -385,6 +388,11 @@ var domainTestData = []struct {
 								Value: 0,
 							},
 						},
+						Address: &DomainAddress{
+							Type: "dimm",
+							Slot: &memorydevAddressSlot,
+							Base: &memorydevAddressBase,
+						},
 					},
 				},
 			},
@@ -434,6 +442,7 @@ var domainTestData = []struct {
 			`        <size unit="GiB">1</size>`,
 			`        <node>0</node>`,
 			`      </target>`,
+			`      <address type="dimm" slot="0" base="4294967296"></address>`,
 			`    </memory>`,
 			`  </devices>`,
 			`</domain>`,
-- 
2.7.4

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH go-xml] Add address support for memory device

Posted by Peter Krempa 6 years, 5 months ago

On Mon, Oct 30, 2017 at 11:05:13 +0800, zhenwei.pi wrote:
> Add Base element for DomainAddress.
> Add address element for DomainMemorydev.
> Add test code for new DomainMemorydev.
> 
> Signed-off-by: zhenwei.pi <zhenwei.pi@youruncloud.com>
> ---
>  domain.go      | 2 ++
>  domain_test.go | 9 +++++++++
>  2 files changed, 11 insertions(+)
> 
> diff --git a/domain.go b/domain.go
> index bacab11..3d9404f 100644
> --- a/domain.go
> +++ b/domain.go
> @@ -297,6 +297,7 @@ type DomainAddress struct {
>  	Function   *HexUint `xml:"function,attr"`
>  	Target     *uint    `xml:"target,attr"`
>  	Unit       *uint    `xml:"unit,attr"`
> +	Base       *HexUint `xml:"base,attr"`
>  }
>  
>  type DomainConsole struct {
> @@ -450,6 +451,7 @@ type DomainMemorydev struct {
>  	Model   string                 `xml:"model,attr"`
>  	Access  string                 `xml:"access,attr"`
>  	Target  *DomainMemorydevTarget `xml:"target"`
> +	Address *DomainAddress         `xml:"address"`
>  }
>  
>  type DomainDeviceList struct {
> diff --git a/domain_test.go b/domain_test.go
> index dbebe42..cbc5d7f 100644
> --- a/domain_test.go
> +++ b/domain_test.go
> @@ -66,6 +66,9 @@ var vcpuId0 uint = 0
>  var vcpuOrder0 uint = 1
>  var vcpuId1 uint = 1
>  
> +var memorydevAddressSlot HexUint = 0
> +var memorydevAddressBase HexUint = 4294967296
> +
>  var domainTestData = []struct {
>  	Object   Document
>  	Expected []string
> @@ -385,6 +388,11 @@ var domainTestData = []struct {
>  								Value: 0,
>  							},
>  						},
> +						Address: &DomainAddress{
> +							Type: "dimm",
> +							Slot: &memorydevAddressSlot,
> +							Base: &memorydevAddressBase,
> +						},
>  					},
>  				},
>  			},
> @@ -434,6 +442,7 @@ var domainTestData = []struct {
>  			`        <size unit="GiB">1</size>`,
>  			`        <node>0</node>`,
>  			`      </target>`,
> +			`      <address type="dimm" slot="0" base="4294967296"></address>`,

This is not a hex number. you are missing the 0x prefix. Also the
element has no content so no closing tag is required.

>  			`    </memory>`,
>  			`  </devices>`,
>  			`</domain>`,
> -- 
> 2.7.4
> 
> --
> libvir-list mailing list
> libvir-list@redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH go-xml] Add address support for memory device

Posted by Daniel P. Berrange 6 years, 5 months ago

On Mon, Oct 30, 2017 at 10:27:13AM +0100, Peter Krempa wrote:
> On Mon, Oct 30, 2017 at 11:05:13 +0800, zhenwei.pi wrote:
> > Add Base element for DomainAddress.
> > Add address element for DomainMemorydev.
> > Add test code for new DomainMemorydev.
> > 
> > Signed-off-by: zhenwei.pi <zhenwei.pi@youruncloud.com>
> > ---
> >  domain.go      | 2 ++
> >  domain_test.go | 9 +++++++++
> >  2 files changed, 11 insertions(+)
> > 
> > diff --git a/domain.go b/domain.go
> > index bacab11..3d9404f 100644
> > --- a/domain.go
> > +++ b/domain.go
> > @@ -297,6 +297,7 @@ type DomainAddress struct {
> >  	Function   *HexUint `xml:"function,attr"`
> >  	Target     *uint    `xml:"target,attr"`
> >  	Unit       *uint    `xml:"unit,attr"`
> > +	Base       *HexUint `xml:"base,attr"`
> >  }
> >  
> >  type DomainConsole struct {
> > @@ -450,6 +451,7 @@ type DomainMemorydev struct {
> >  	Model   string                 `xml:"model,attr"`
> >  	Access  string                 `xml:"access,attr"`
> >  	Target  *DomainMemorydevTarget `xml:"target"`
> > +	Address *DomainAddress         `xml:"address"`
> >  }
> >  
> >  type DomainDeviceList struct {
> > diff --git a/domain_test.go b/domain_test.go
> > index dbebe42..cbc5d7f 100644
> > --- a/domain_test.go
> > +++ b/domain_test.go
> > @@ -66,6 +66,9 @@ var vcpuId0 uint = 0
> >  var vcpuOrder0 uint = 1
> >  var vcpuId1 uint = 1
> >  
> > +var memorydevAddressSlot HexUint = 0
> > +var memorydevAddressBase HexUint = 4294967296
> > +
> >  var domainTestData = []struct {
> >  	Object   Document
> >  	Expected []string
> > @@ -385,6 +388,11 @@ var domainTestData = []struct {
> >  								Value: 0,
> >  							},
> >  						},
> > +						Address: &DomainAddress{
> > +							Type: "dimm",
> > +							Slot: &memorydevAddressSlot,
> > +							Base: &memorydevAddressBase,
> > +						},
> >  					},
> >  				},
> >  			},
> > @@ -434,6 +442,7 @@ var domainTestData = []struct {
> >  			`        <size unit="GiB">1</size>`,
> >  			`        <node>0</node>`,
> >  			`      </target>`,
> > +			`      <address type="dimm" slot="0" base="4294967296"></address>`,
> 
> This is not a hex number. you are missing the 0x prefix. Also the
> element has no content so no closing tag is required.

This is matching against go XML output, and that puts an closing tag even
on empty elemnts.

The hex number stuff is a huge mess. We need a major change to address
handling in the go xml parser to fix this as multiple existing addr
types are broken in this regardless (using base 10 instead of 16 or
vica-verca). I'm going to push this despite being broken, and then
cleanup everything afterwards.


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