[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