[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 Wed, May 31, 2017 at 11:35:14AM -0400, Vladik Romanovsky wrote:
> ---
>  node_device.go      | 168 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>  node_device_test.go | 128 +++++++++++++++++++++++++++++++++++++++
>  2 files changed, 296 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..d7850e9
> --- /dev/null
> +++ b/node_device.go
> @@ -0,0 +1,168 @@
> +/*
> + * 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"
> +)
> +
> +type NodeDevice struct {
> +	Name       string                 `xml:"name"`
> +	Path       string                 `xml:"path,omitempty"`
> +	Parent     string                 `xml:"parent,omitempty"`
> +	Driver     string                 `xml:"driver>name,omitempty"`
> +	Capability []NodeDeviceCapability `xml:"capability"`
> +}
> +
> +type NodeDeviceCapability struct {
> +	Domain      int                            `xml:"domain,omitempty"`
> +	Bus         int                            `xml:"bus,omitempty"`
> +	Slot        int                            `xml:"slot,omitempty"`
> +	Function    int                            `xml:"function,omitempty"`
> +	IommuGroup  *NodeDeviceIOMMUGroup          `xml:"iommuGroup,omitempty"`
> +	Numa        *NodeDeviceNUMA                `xml:"numa,omitempty"`
> +	PciExpress  *NodeDevicePciExpress          `xml:"pci-express,omitempty"`

You don't need omitempty on any field that is a pointer.

> +	Hardware    *NodeDeviceSystemHardware      `xml:"hardware"`
> +	Firmware    *NodeDeviceSystemFirmware      `xml:"firmware"`
> +	Device      int                            `xml:"device"`
> +	Number      int                            `xml:"number"`
> +	Class       int                            `xml:"class"`
> +	Subclass    int                            `xml:"subclass"`
> +	Protocol    int                            `xml:"protocol"`
> +	Description string                         `xml:"description,omitempty"`
> +	Interface   string                         `xml:"interface"`
> +	Address     string                         `xml:"address"`
> +	Link        *NodeDeviceNetLink             `xml:"link"`
> +	Features    []NodeDeviceNetOffloadFeatures `xml:"feature,omitempty"`
> +	UniqueID    int                            `xml:"unique_id"`
> +	Target      int                            `xml:"target"`
> +	Lun         int                            `xml:"lun"`
> +	Block       string                         `xml:"block"`
> +	DriverType  string                         `xml:"drive_type"`
> +	Model       string                         `xml:"model"`
> +	Serial      string                         `xml:"serial"`
> +	Size        int                            `xml:"size"`
> +	Host        int                            `xml:"host"`
> +	Type        string                         `xml:"type"`

I'm surprised we don't ahve omitempty on almost all of these above

> +	Product     *NodeDeviceProduct             `xml:"product,omitempty"`
> +	Vendor      *NodeDeviceVendor              `xml:"vendor,omitempty"`
> +	Capability  []NodeDeviceNestedCapabilities `xml:"capability,omitempty"`
> +}

Wow, we have a lot of capabilities here. Makes be wonder if your previous
patch was in fact better... will have to think about that more.

> +
> +type NodeDeviceVendor struct {
> +	ID   string `xml:"id,attr,omitempty"`
> +	Name string `xml:",chardata"`
> +}
> +type NodeDeviceProduct struct {
> +	ID   string `xml:"id,attr,omitempty"`
> +	Name string `xml:",chardata"`
> +}
> +
> +type NodeDeviceNestedCapabilities struct {
> +	Type           string                   `xml:"type,attr"`
> +	Address        []NodeDevicePCIAddress   `xml:"address,omitempty"`
> +	MaxCount       int                      `xml:"maxCount,attr,omitempty"`
> +	VportsOPS      *NodeDeviceSCSIVportsOPS `xml:"vports_ops,omitempty"`
> +	FCHost         *NodeDeviceSCSIFCHost    `xml:"fc_host,omitempty"`
> +	MediaAvailable int                      `xml:"media_available,omitempty"`
> +	MediaSize      int                      `xml:"media_size,omitempty"`
> +	MediaLable     int                      `xml:"media_label,omitempty"`
> +}
> +
> +type NodeDevicePciExpress struct {
> +	Links []NodeDevicePciExpressLink `xml:"link"`
> +}
> +
> +type NodeDevicePciExpressLink 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 NodeDeviceIOMMUGroup struct {
> +	Number int `xml:"number,attr"`
> +}
> +
> +type NodeDeviceNUMA struct {
> +	Node int `xml:"node,attr"`
> +}
> +
> +type NodeDevicePCIAddress struct {
> +	Domain   string `xml:"domain,attr"`
> +	Bus      string `xml:"bus,attr"`
> +	Slot     string `xml:"slot,attr"`
> +	Function string `xml:"function,attr"`
> +}
> +
> +type NodeDeviceSystemHardware struct {
> +	Vendor  string `xml:"vendor"`
> +	Version string `xml:"version"`
> +	Serial  string `xml:"serial"`
> +	UUID    string `xml:"uuid"`
> +}
> +
> +type NodeDeviceSystemFirmware struct {
> +	Vendor      string `xml:"vendor"`
> +	Version     string `xml:"version"`
> +	ReleaseData string `xml:"release_date"`
> +}
> +
> +type NodeDeviceNetOffloadFeatures struct {
> +	Name string `xml:"number"`
> +}
> +
> +type NodeDeviceNetLink struct {
> +	State string `xml:"state,attr"`
> +	Speed string `xml:"speed,attr,omitempty"`
> +}
> +
> +type NetCapability struct {
> +	Type string `xml:"type,attr"`
> +}
> +
> +type NodeDeviceSCSIVportsOPS struct {
> +	Vports    int `xml:"vports,omitempty"`
> +	MaxVports int `xml:"maxvports,,omitempty"`
> +}
> +
> +type NodeDeviceSCSIFCHost struct {
> +	WWNN      string `xml:"wwnn,omitempty"`
> +	WWPN      string `xml:"wwpn,omitempty"`
> +	FabricWWN string `xml:"fabric_wwn,omitempty"`
> +}
> +
> +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..bae15f3
> --- /dev/null
> +++ b/node_device_test.go
> @@ -0,0 +1,128 @@
> +/*
> + * 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: []NodeDeviceCapability{
> +				NodeDeviceCapability{
> +					Domain:   1,
> +					Bus:      21,
> +					Slot:     10,
> +					Function: 50,
> +					Product: &NodeDeviceProduct{
> +						ID:   "0x1528",
> +						Name: "Ethernet Controller 10-Gigabit X540-AT2",
> +					},
> +					Vendor: &NodeDeviceVendor{
> +						ID:   "0x8086",
> +						Name: "Intel Corporation",
> +					},
> +					IommuGroup: &NodeDeviceIOMMUGroup{
> +						Number: 3,
> +					},
> +					Numa: &NodeDeviceNUMA{
> +						Node: 1,
> +					},
> +					Capability: []NodeDeviceNestedCapabilities{
> +						NodeDeviceNestedCapabilities{
> +							Type: "virt_functions",
> +							Address: []NodeDevicePCIAddress{
> +								NodeDevicePCIAddress{
> +									Domain:   "0x0000",
> +									Bus:      "0x81",
> +									Slot:     "0x10",
> +									Function: "0x1",
> +								},
> +								NodeDevicePCIAddress{
> +									Domain:   "0x0000",
> +									Bus:      "0x81",
> +									Slot:     "0x10",
> +									Function: "0x3",
> +								},
> +							},
> +							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'>`,
> +			`	  <address domain='0x0000' bus='0x81' slot='0x10' function='0x1'/>`,
> +			`	  <address domain='0x0000' bus='0x81' slot='0x10' function='0x3'/>`,
> +			`	</capability>`,
> +			`	<iommuGroup number='3'>`,
> +			`	  <address domain='0x0000' bus='0x15' slot='0x00' function='0x4'/>`,
> +			`	</iommuGroup>`,
> +			`   <numa node='1'/>`,
> +			`  </capability>`,
> +			`</device>`,

Indetation is a bit inconsistenmt here - sometimes 2 spaces, sometimes 6 spaces, or more

> +		},
> +	},
> +}


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]