Thanks for the review.
On Tue, May 30, 2017 at 8:46 AM, Daniel P. Berrange <berrange@xxxxxxxxxx> wrote:
> On Mon, May 29, 2017 at 03:58:52AM -0400, Vladik Romanovsky wrote:
>> ---
>> node_device.go | 277 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>> node_device_test.go | 111 +++++++++++++++++++++
>> 2 files changed, 388 insertions(+)
>> create mode 100644 node_device.go
>> create mode 100644 node_device_test.go
>>
>> diff --git a/node_device.go b/node_device.go
>> new file mode 100644
>> index 0000000..5375d32
>> --- /dev/null
>> +++ b/node_device.go
>> @@ -0,0 +1,277 @@
>> +/*
>> + * This file is part of the libvirt-go-xml project
>> + *
>> + * Permission is hereby granted, free of charge, to any person obtaining a copy
>> + * of this software and associated documentation files (the "Software"), to deal
>> + * in the Software without restriction, including without limitation the rights
>> + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
>> + * copies of the Software, and to permit persons to whom the Software is
>> + * furnished to do so, subject to the following conditions:
>> + *
>> + * The above copyright notice and this permission notice shall be included in
>> + * all copies or substantial portions of the Software.
>> + *
>> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
>> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
>> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
>> + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
>> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
>> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
>> + * THE SOFTWARE.
>> + *
>> + * Copyright (C) 2017 Red Hat, Inc.
>> + *
>> + */
>> +
>> +package libvirtxml
>> +
>> +import (
>> + "encoding/xml"
>> + "fmt"
>> +)
>> +
>> +type NodeDevice struct {
>> + Name string `xml:"name"`
>> + Path string `xml:"path,omitempty"`
>> + Parent string `xml:"parent,omitempty"`
>> + Driver string `xml:"driver>name,omitempty"`
>> + Capability *CapabilityType `xml:"capability"`
>> +}
>
> A single device can have multiple capabilities.
Do you mean it should be:
Capability []CapabilityType `xml:"capability"` ?
>
>> +
>> +type CapabilityType struct {
>> + Type interface{} `xml:"type,attr"`
>> +}
>
> I'm not convinced about this approach - it differs from what we've
> done in other schemas, where we just have a single struct containing
> the union of data from all types. The user has no idea what they
> should be providing for 'Type' here since its a generic interface
> type.
>
Yes. I wasn't sure about this either. Originally, I wanted to avoid
the Type attribute and make
CapabilityType to be an interface type.
However, I figured that it's not possible to write an UnmarshalXML for
an interface type.
The reason I didn't use the same approach as in the rest of the
schemas is because the Capability
determins it's type by a XML attribute and not an element name.
I didn't find an easy way to query the xml attribue as something
like.. `xml:"type=something,attr"`
OK, I'll try a ddiffernt approach.
>>
>> +type IDName struct {
>> + ID string `xml:"id,attr"`
>> + Name string `xml:",chardata"`
>> +}
>> +
>> +type PciExpress struct {
>> + Links []PciExpressLink `xml:"link"`
>> +}
>> +
>> +type PciExpressLink struct {
>> + Validity string `xml:"validity,attr,omitempty"`
>> + Speed float64 `xml:"speed,attr,omitempty"`
>> + Port int `xml:"port,attr,omitempty"`
>> + Width int `xml:"width,attr,omitempty"`
>> +}
>> +
>> +type IOMMUGroupType struct {
>> + Number int `xml:"number,attr"`
>> +}
>> +
>> +type NUMA struct {
>> + Node int `xml:"node,attr"`
>> +}
>> +
>> +type PciCapabilityType struct {
>> + Domain int `xml:"domain,omitempty"`
>> + Bus int `xml:"bus,omitempty"`
>> + Slot int `xml:"slot,omitempty"`
>> + Function int `xml:"function,omitempty"`
>> + Product IDName `xml:"product,omitempty"`
>> + Vendor IDName `xml:"vendor,omitempty"`
>> + IommuGroup IOMMUGroupType `xml:"iommuGroup,omitempty"`
>> + Numa NUMA `xml:"numa,omitempty"`
>> + PciExpress PciExpress `xml:"pci-express,omitempty"`
>> + Capabilities []PciCapability `xml:"capability,omitempty"`
>> +}
>> +
>> +type PCIAddress struct {
>> + Domain string `xml:"domain,attr"`
>> + Bus string `xml:"bus,attr"`
>> + Slot string `xml:"slot,attr"`
>> + Function string `xml:"function,attr"`
>> +}
>> +
>> +type PciCapability struct {
>
> There's alot of inconsistency in capitalization of abbreviations
> in this file. PCI vs Pci, Numa vs NUMA, IOMMU vs Iiommu. They
> should generally always be capitalized.
>
>> + Type string`xml:"type,attr"`
>> + Address []PCIAddress `xml:"address,omitempty"`
>> + MaxCount int `xml:"maxCount,attr,omitempty"`
>> +}
>> +
>> +type SystemHardware struct {
>> + Vendor string `xml:"vendor"`
>> + Version string `xml:"version"`
>> + Serial string `xml:"serial"`
>> + UUID string `xml:"uuid"`
>> +}
>> +
>> +type SystemFirmware struct {
>> + Vendor string `xml:"vendor"`
>> + Version string `xml:"version"`
>> + ReleaseData string `xml:"release_date"`
>> +}
>> +
>> +type SystemCapabilityType struct {
>> + Product string `xml:"product"`
>> + Hardware SystemHardware `xml:"hardware"`
>> + Firmware SystemFirmware `xml:"firmware"`
>> +}
>> +
>> +type USBDeviceCapabilityType struct {
>> + Bus int `xml:"bus"`
>> + Device int `xml:"device"`
>> + Product IDName `xml:"product,omitempty"`
>> + Vendor IDName `xml:"vendor,omitempty"`
>> +}
>> +
>> +type USBCapabilityType struct {
>> + Number int `xml:"number"`
>> + Class int `xml:"class"`
>> + Subclass int `xml:"subclass"`
>> + Protocol int `xml:"protocol"`
>> + Description string `xml:"description,omitempty"`
>> +}
>> +
>> +type NetOffloadFeatures struct {
>> + Name string `xml:"number"`
>> +}
>> +
>> +type NetLink struct {
>> + State string `xml:"state,attr"`
>> + Speed string `xml:"speed,attr,omitempty"`
>> +}
>> +
>> +type NetCapability struct {
>> + Type string `xml:"type,attr"`
>> +}
>> +
>> +type NetCapabilityType struct {
>> + Interface string `xml:"interface"`
>> + Address string `xml:"address"`
>> + Link NetLink `xml:"link"`
>> + Features []NetOffloadFeatures `xml:"feature,omitempty"`
>> + Capability NetCapability `xml:"capability"`
>> +}
>> +
>> +type SCSIVportsOPS struct {
>> + Vports int `xml:"vports,omitempty"`
>> + MaxVports int `xml:"maxvports,,omitempty"`
>> +}
>> +
>> +type SCSIFCHost struct {
>> + WWNN string `xml:"wwnn,omitempty"`
>> + WWPN string `xml:"wwpn,omitempty"`
>> + FabricWWN string `xml:"fabric_wwn,omitempty"`
>> +}
>> +
>> +type SCSIHostCapability struct {
>> + VportsOPS SCSIVportsOPS `xml:"vports_ops"`
>> + FCHost SCSIFCHost `xml:"fc_host"`
>> +}
>> +
>> +type SCSIHostCapabilityType struct {
>> + Host int `xml:"host"`
>> + UniqueID int `xml:"unique_id"`
>> + Capability SCSIHostCapability `xml:"capability"`
>> +}
>> +
>> +type SCSICapabilityType struct {
>> + Host int `xml:"host"`
>> + Bus int `xml:"bus"`
>> + Target int `xml:"target"`
>> + Lun int `xml:"lun"`
>> + Type string `xml:"type"`
>> +}
>> +
>> +type StroageCap struct {
>
> s/Stroage/Storage/
>
>> + Type string `xml:"match,attr"`
>> + MediaAvailable int `xml:"media_available,omitempty"`
>> + MediaSize int `xml:"media_size,omitempty"`
>> + MediaLable int `xml:"media_label,omitempty"`
>> +}
>> +
>> +type StorageCapabilityType struct {
>> + Block string `xml:"block"`
>> + Bus string `xml:"bus"`
>> + DriverType string `xml:"drive_type"`
>> + Model string `xml:"model"`
>> + Vendor string `xml:"vendor"`
>> + Serial string `xml:"serial"`
>> + Size int `xml:"size"`
>> + Capatibility StroageCap `xml:"capability,omitempty"`
>> +}
>> +
>> +type DRMCapabilityType struct {
>> + Type string `xml:"type"`
>> +}
>
> Note that every struct is in the same 'libvirtxml' namespace, even if the
> declarations are spread across different .go files. We need the structs
> for each XML document to be isolated from each other, so you need to have
> a 'NodeDevice' prefix on all these struct names.
>
>
>> +
>> +func (c *CapabilityType) UnmarshalXML(d *xml.Decoder, start xml.StartElement) error {
>> + for _, attr := range start.Attr {
>> + fmt.Println(attr.Name.Local)
>> + if attr.Name.Local == "type" {
>> + switch attr.Value {
>> + case "pci":
>> + var pciCaps PciCapabilityType
>> + if err := d.DecodeElement(&pciCaps, &start); err != nil {
>> + return err
>> + }
>> + c.Type = pciCaps
>> + case "system":
>> + var systemCaps SystemCapabilityType
>> + if err := d.DecodeElement(&systemCaps, &start); err != nil {
>> + return err
>> + }
>> + c.Type = systemCaps
>> + case "usb_device":
>> + var usbdevCaps USBDeviceCapabilityType
>> + if err := d.DecodeElement(&usbdevCaps, &start); err != nil {
>> + return err
>> + }
>> + c.Type = usbdevCaps
>> + case "usb":
>> + var usbCaps USBCapabilityType
>> + if err := d.DecodeElement(&usbCaps, &start); err != nil {
>> + return err
>> + }
>> + c.Type = usbCaps
>> + case "net":
>> + var netCaps NetCapabilityType
>> + if err := d.DecodeElement(&netCaps, &start); err != nil {
>> + return err
>> + }
>> + c.Type = netCaps
>> + case "scsi_host":
>> + var scsiHostCaps SCSIHostCapabilityType
>> + if err := d.DecodeElement(&scsiHostCaps, &start); err != nil {
>> + return err
>> + }
>> + c.Type = scsiHostCaps
>> + case "scsi":
>> + var scsiCaps SCSICapabilityType
>> + if err := d.DecodeElement(&scsiCaps, &start); err != nil {
>> + return err
>> + }
>> + c.Type = scsiCaps
>> + case "storage":
>> + var storageCaps StorageCapabilityType
>> + if err := d.DecodeElement(&storageCaps, &start); err != nil {
>> + return err
>> + }
>> + c.Type = storageCaps
>> + case "drm":
>> + var drmCaps DRMCapabilityType
>> + if err := d.DecodeElement(&drmCaps, &start); err != nil {
>> + return err
>> + }
>> + c.Type = drmCaps
>> + }
>> + }
>> + }
>> + return nil
>> +}
>> +
>> +func (c *NodeDevice) Unmarshal(doc string) error {
>> + return xml.Unmarshal([]byte(doc), c)
>> +}
>> +
>> +func (c *NodeDevice) Marshal() (string, error) {
>> + doc, err := xml.MarshalIndent(c, "", " ")
>> + if err != nil {
>> + return "", err
>> + }
>> + return string(doc), nil
>> +}
>> diff --git a/node_device_test.go b/node_device_test.go
>> new file mode 100644
>> index 0000000..129956b
>> --- /dev/null
>> +++ b/node_device_test.go
>> @@ -0,0 +1,111 @@
>> +/*
>> + * This file is part of the libvirt-go-xml project
>> + *
>> + * Permission is hereby granted, free of charge, to any person obtaining a copy
>> + * of this software and associated documentation files (the "Software"), to deal
>> + * in the Software without restriction, including without limitation the rights
>> + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
>> + * copies of the Software, and to permit persons to whom the Software is
>> + * furnished to do so, subject to the following conditions:
>> + *
>> + * The above copyright notice and this permission notice shall be included in
>> + * all copies or substantial portions of the Software.
>> + *
>> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
>> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
>> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
>> + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
>> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
>> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
>> + * THE SOFTWARE.
>> + *
>> + * Copyright (C) 2017 Red Hat, Inc.
>> + *
>> + */
>> +
>> +package libvirtxml
>> +
>> +import (
>> + "reflect"
>> + "strings"
>> + "testing"
>> +)
>> +
>> +var NodeDeviceTestData = []struct {
>> + Object *NodeDevice
>> + XML []string
>> +}{
>> + {
>> + Object: &NodeDevice{
>> + Name: "pci_0000_81_00_0",
>> + Parent: "pci_0000_80_01_0",
>> + Driver: "ixgbe",
>> + Capability: &CapabilityType{
>> + Type: PciCapabilityType{
>> + Domain: 1,
>> + Bus: 21,
>> + Slot: 10,
>> + Function: 50,
>> + Product: IDName{
>> + ID: "0x1528",
>> + Name: "Ethernet Controller 10-Gigabit X540-AT2",
>> + },
>> + Vendor: IDName{
>> + ID: "0x8086",
>> + Name: "Intel Corporation",
>> + },
>> + IommuGroup: IOMMUGroupType{
>> + Number: 3,
>> + },
>> + Numa: NUMA{
>> + Node: 1,
>> + },
>> + Capabilities: []PciCapability{
>> + PciCapability{
>> + Type: "virt_functions",
>> + MaxCount: 63,
>> + },
>> + },
>> + },
>> + },
>> + },
>> + XML: []string{
>> + `<device>`,
>> + ` <name>pci_0000_81_00_0</name>`,
>> + ` <parent>pci_0000_80_01_0</parent>`,
>> + ` <driver>`,
>> + ` <name>ixgbe</name>`,
>> + ` </driver>`,
>> + ` <capability type='pci'>`,
>> + ` <domain>1</domain>`,
>> + ` <bus>21</bus>`,
>> + ` <slot>10</slot>`,
>> + ` <function>50</function>`,
>> + ` <product id='0x1528'>Ethernet Controller 10-Gigabit X540-AT2</product>`,
>> + ` <vendor id='0x8086'>Intel Corporation</vendor>`,
>> + ` <capability type='virt_functions' maxCount='63'/>`,
>> + ` <iommuGroup number='3'>`,
>> + ` <address domain='0x0000' bus='0x15' slot='0x00' function='0x4'/>`,
>> + ` </iommuGroup>`,
>> + ` <numa node='1'/>`,
>> + ` </capability>`,
>> + `</device>`,
>> + },
>> + },
>> +}
>> +
>> +func TestNodeDevice(t *testing.T) {
>> + for _, test := range NodeDeviceTestData {
>> + xmlDoc := strings.Join(test.XML, "\n")
>> + nodeDevice := NodeDevice{}
>> + err := nodeDevice.Unmarshal(xmlDoc)
>> + if err != nil {
>> + t.Fatal(err)
>> + }
>> +
>> + res := reflect.DeepEqual(&nodeDevice, test.Object)
>> + if !res {
>> + t.Fatal("Bad NodeDevice object creation.")
>> + }
>> + }
>> +}
>> --
>> 2.9.4
>>
>> --
>> libvir-list mailing list
>> libvir-list@xxxxxxxxxx
>> https://www.redhat.com/mailman/listinfo/libvir-list
>
> 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@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
