[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH go-xml] Add support for Node Device with basic testing.



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.

> +
> +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.

>
> +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 redhat com
> 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 :|


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]