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 <[email protected]> wrote: > On Thu, Jan 05, 2017 at 09:54:25AM +0100, Alexey Slaykovsky wrote: > > Signed-off-by: Alexey Slaykovsky <[email protected]> > > --- > > 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/ > :| >
-- libvir-list mailing list [email protected] https://www.redhat.com/mailman/listinfo/libvir-list
