[libvirt] [PATCH go-xml] Added domain OS struct and tests
Aleksei Slaikovskii
aslaikov at redhat.com
Thu Jan 5 13:59:25 UTC 2017
Hi Daniel!
Thank you for review, you're absolutely right about omitempty on
structures, sorry for that. Will rename DomainEntry as well.
On Thu, Jan 5, 2017 at 1:58 PM, Daniel P. Berrange <berrange at redhat.com>
wrote:
> On Thu, Jan 05, 2017 at 09:54:25AM +0100, Alexey Slaykovsky wrote:
> > Signed-off-by: Alexey Slaykovsky <aslaikov at redhat.com>
> > ---
> > domain.go | 68 ++++++++++++++++++++++++
> > domain_test.go | 164 ++++++++++++++++++++++++++++++
> +++++++++++++++++++++++++++
> > 2 files changed, 232 insertions(+)
> >
> > diff --git a/domain.go b/domain.go
> > index c98908e..25155ee 100644
> > --- a/domain.go
> > +++ b/domain.go
> > @@ -126,6 +126,69 @@ type DomainMemory struct {
> > Unit string `xml:"unit,attr"`
> > }
> >
> > +type DomainOSType struct {
> > + Arch string `xml:"arch,attr"`
> > + Machine string `xml:"machine,attr"`
> > + Type string `xml:",chardata"`
> > +}
> > +
> > +type DomainSMBios struct {
> > + Mode string `xml:"mode,attr"`
> > +}
> > +
> > +type DomainNVRam struct {
> > + NVRam string `xml:",chardata"`
> > + Template string `xml:"template,attr,omitempty"`
> > +}
> > +
> > +type DomainBootDevice struct {
> > + Dev string `xml:"dev,attr"`
> > +}
> > +
> > +type DomainBootMenu struct {
> > + Enabled string `xml:"enabled,attr"`
> > + Timeout string `xml:"timeout,attr,omitempty"`
> > +}
> > +
> > +type DomainSysInfo struct {
> > + Type string `xml:"type,attr"`
> > + System []DomainEntry `xml:"system>entry"`
> > + BIOS []DomainEntry `xml:"bios>entry"`
> > + BaseBoard []DomainEntry `xml:"baseBoard>entry"`
> > +}
> > +
> > +type DomainEntry struct {
> > + Name string `xml:"name,attr"`
> > + Value string `xml:",chardata"`
> > +}
>
> Can you call this "DomainSysInfoEntry" so its more tightly scoped.
>
> > +
> > +type DomainBIOS struct {
> > + UseSerial string `xml:"useserial,attr"`
> > + RebootTimeout string `xml:"rebootTimeout,attr"`
> > +}
> > +
> > +type DomainLoader struct {
> > + Path string `xml:",chardata"`
> > + Readonly string `xml:"readonly,attr"`
> > + Secure string `xml:"secure,attr"`
> > + Type string `xml:"type,attr"`
> > +}
> > +
> > +type DomainOS struct {
> > + Type *DomainOSType `xml:"type"`
> > + Loader *DomainLoader `xml:"loader,omitempty"`
> > + NVRam *DomainNVRam `xml:"nvram"`
>
> I'm curious why we need 'omitempty' on "Loader" and a few
> others below, but don't need it on "NVRam". Based on your
> test case it seems the formatter omits NVRam automatically
> when it is nil, which would suggest we don't need omitempty
> on the other struct fields either - just need it on the
> string fields.
>
> > + Kernel string `xml:"kernel,omitempty"`
> > + Initrd string `xml:"initrd,omitempty"`
> > + KernelArgs string `xml:"cmdline,omitempty"`
> > + BootDevices []DomainBootDevice `xml:"boot"`
> > + BootMenu *DomainBootMenu `xml:"bootmenu,omitempty"`
> > + SMBios *DomainSMBios `xml:"smbios,omitempty"`
> > + BIOS *DomainBIOS `xml:"bios,omitempty"`
> > + Init string `xml:"init,omitempty"`
> > + InitArgs []string `xml:"initarg,omitempty"`
> > +}
>
> Looks good apart from this two nitpicks.
>
> Regards,
> Daniel
> --
> |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/
> :|
> |: http://libvirt.org -o- http://virt-manager.org
> :|
> |: http://entangle-photo.org -o- http://search.cpan.org/~danberr/
> :|
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20170105/f7aa3740/attachment-0001.htm>
More information about the libvir-list
mailing list