Commit d77ffb68 added not only reporting of the PCI header type, but
also parsing of that information.  However, because there was no parsing
done for the other sub-PCI capabilities, if there was any other
capability then a valid header type name (like phys_function or
virt_functions) the parsing would fail.  This prevented passing node
device XMLs that we generated into our own functions when dealing with,
e.g. with SRIOV cards.

Instead of reworking the whole parsing, just fix this one occurence and
remove a test for it for the time being.  Future patches will deal with
the rest.
Signed-off-by: NMartin Kletzander <mkletzan@redhat.com>

When reading in an XML definition for a SCSI target device, the name
property of struct scsi_target refers to the @target element.

Let's fix this obvious typo and also extend the XML schema to provide
validation.
Signed-off-by: NBjoern Walk <bwalk@linux.vnet.ibm.com>

If we expose this information, which is one byte in every PCI config
file, we let all mgmt apps know whether the device itself is an endpoint
or not so it's easier for them to decide whether such device can be
passed through into a VM (endpoint) or not (*-bridge).

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1317531Signed-off-by: NMartin Kletzander <mkletzan@redhat.com>

Just a cleanup I stumbled upon in one of my older branches I did when
browsing through some code and forgot to send it.
Signed-off-by: NMartin Kletzander <mkletzan@redhat.com>

Report the maximum possible number of VFs for an SRIOV PF, like this:

   <capability type='virtual_functions' maxCount='7'>
      ...
   </capability>

I've just discovered that the virtual_functions and physical_functions
capabilities are not supported in the virNodeDeviceParse functions,
only in virNodeDeviceFormat (I suppose because they are only reported,
not set from XML). This should probably be remedied, but is less
immediately useful than the current patch.

For some reason a union (_virNodeDevCapData) that had only been
declared inside the toplevel struct virNodeDevCapsDef was being used
as an argument to functions all over the place. Since it was only a
union, the "type" attribute wasn't necessarily sent with it. While
this works, it just seems wrong.

This patch creates a toplevel typedef for virNodeDevCapData and
virNodeDevCapDataPtr, making it a struct that has the type attribute
as a member, along with an anonymous union of everything that used to
be in union _virNodeDevCapData. This way we only have to change the
following:

  s/union _virNodeDevCapData */virNodeDevCapDataPtr /

and

  s/caps->type/caps->data.type/

This will make me feel less guilty when adding functions that need a
pointer to one of these.

There is a lot of places, were it's pretty easy for user to enter some
characters that we need to escape to create a valid XML description.

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1197580Signed-off-by: NPavel Hrdina <phrdina@redhat.com>

A helper that never returns an error and treats bits out of bitmap range
as false.

Use it everywhere we use ignore_value on virBitmapGetBit, or loop over
the bitmap size.

Commit id 'c9027d8f' added parsing of the CapNet for offload SRIOV NIC
discovery, but forgot to free the nodes

Adding functionality to libvirt that will allow it
query the ethtool interface for the availability
of certain NIC HW offload features

Here is an example of the feature XML definition:

<device>
<name>net_eth4_90_e2_ba_5e_a5_45</name>
  <path>/sys/devices/pci0000:00/0000:00:03.0/0000:08:00.1/net/eth4</path>
  <parent>pci_0000_08_00_1</parent>
  <capability type='net'>
    <interface>eth4</interface>
    <address>90:e2:ba:5e:a5:45</address>
    <link speed='10000' state='up'/>
    <feature name='rx'/>
    <feature name='tx'/>
    <feature name='sg'/>
    <feature name='tso'/>
    <feature name='gso'/>
    <feature name='gro'/>
    <feature name='rxvlan'/>
    <feature name='txvlan'/>
    <feature name='rxhash'/>
    <capability type='80203'/>
  </capability>
</device>
Signed-off-by: NJán Tomko <jtomko@redhat.com>

Commit id '652a2ec6' introduced two new node device capability flags
and the ability to use those flags as a way to search for a specific
subset of a 'scsi_host' device - namely a 'fc_host' and/or 'vports'.
The code modified the virNodeDeviceCapMatch whichs allows for searching
using the 'virsh nodedev-list [cap]' via virConnectListAllNodeDevices.

However, the original patches did not account for other searches for
the same capability key from virNodeListDevices using virNodeDeviceHasCap.
Since 'fc_host' and 'vports' are self defined bits of a 'scsi_host'
device mere string comparison against the basic/root type is not
sufficient.

This patch adds the check for the 'fc_host' and 'vports' bits within
a 'scsi_host' device and allows the following python code to find the
capabilities for the device:

import libvirt
conn = libvirt.openReadOnly('qemu:///system')
fc = conn.listDevices('fc_host', 0)
print(fc)
fc = conn.listDevices('vports', 0)
print(fc)
Signed-off-by: NShivaprasad G Bhat <sbhat@linux.vnet.ibm.com>

Since virNodeDeviceFree will call virObjectUnref anyway, let's just use that
directly so as to avoid the possibility that we inadvertently clear out
a pending error message when using the public API.

Signed-off-by: NMartin Kletzander <mkletzan@redhat.com>

I'm about to add a syntax check that enforces our documented
HACKING style of always using matching {} on if-else statements.

This patch focuses on code shared between multiple drivers.

* src/conf/domain_conf.c (virDomainFSDefParseXML)
(virSysinfoParseXML, virDomainNetDefParseXML)
(virDomainWatchdogDefParseXML)
(virDomainRedirFilterUSBDevDefParseXML): Correct use of {}.
* src/conf/interface_conf.c (virInterfaceDefParseDhcp)
(virInterfaceDefParseIp, virInterfaceVlanDefFormat)
(virInterfaceDefParseStartMode, virInterfaceDefParseBondMode)
(virInterfaceDefParseBondMiiCarrier)
(virInterfaceDefParseBondArpValid): Likewise.
* src/conf/node_device_conf.c (virNodeDevCapStorageParseXML):
Likewise.
* src/conf/nwfilter_conf.c (virNWFilterRuleDetailsParse)
(virNWFilterRuleParse, virNWFilterDefParseXML): Likewise.
* src/conf/secret_conf.c (secretXMLParseNode): Likewise.
* src/cpu/cpu_x86.c (x86Baseline, x86FeatureLoad, x86ModelLoad):
Likewise.
* src/network/bridge_driver.c (networkKillDaemon)
(networkDnsmasqConfContents): Likewise.
* src/node_device/node_device_hal.c (dev_refresh): Likewise.
* src/nwfilter/nwfilter_gentech_driver.c (virNWFilterInstantiate):
Likewise.
* src/nwfilter/nwfilter_ebiptables_driver.c
(_iptablesCreateRuleInstance): Likewise.
* src/storage/storage_backend_disk.c
(virStorageBackendDiskBuildPool): Likewise.
Signed-off-by: NEric Blake <eblake@redhat.com>

And add a syntax-check for '){$'.  It's not perfect, but better than
nothing.
Signed-off-by: NMartin Kletzander <mkletzan@redhat.com>

In a system with Fiber Channel Host Adapters, a query to list all Fibre Channel
HBAs OR Vports currently returns empty list:
  $ virsh nodedev-list --cap fc_host

  $

Libvirt correctly discovers properties for all HBAs. However, the reporting
fails because of incorrect flag comparison while filtering these types.

This is fixed by removing references to 'VIR_CONNECT_LIST_NODE_DEVICES_CAP_*'
for comparison and replacing those with 'VIR_NODE_DEV_CAP_*'

Introduced by original commit id '652a2ec6'
Signed-off-by: NPrerna Saxena <prerna@linux.vnet.ibm.com>

Fix a -Werror=maybe-uninitialized warning
Signed-off-by: NChen Hanxiao <chenhanxiao@cn.fujitsu.com>
Signed-off-by: NEric Blake <eblake@redhat.com>

Leak introduced in commit 16ebf10f (v1.2.6), detected by valgrind:

==9816== 216 (96 direct, 120 indirect) bytes in 6 blocks are definitely lost in loss record 665 of 821
==9816==    at 0x4A081D4: calloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==9816==    by 0x50836FB: virAlloc (viralloc.c:144)
==9816==    by 0x1DBDBE27: udevProcessPCI (node_device_udev.c:546)
==9816==    by 0x1DBDD79D: udevGetDeviceDetails (node_device_udev.c:1293)

* src/util/virpci.h (virPCIEDeviceInfoFree): New prototype.
* src/util/virpci.c (virPCIEDeviceInfoFree): New function.
* src/conf/node_device_conf.c (virNodeDevCapsDefFree): Clear
pci_express under pci case.
(virNodeDevCapPCIDevParseXML): Avoid leak.
* src/node_device/node_device_udev.c (udevProcessPCI): Likewise.
* src/libvirt_private.syms (virpci.h): Export it.
Signed-off-by: NEric Blake <eblake@redhat.com>

Finding virPCIE* code is more intuitive if located in virpci.h
instead of node_device_conf.h.

* src/conf/node_device_conf.h (virPCIELinkSpeed, virPCIELink)
(virPCIEDeviceInfo): Move...
* src/util/virpci.h: ...here.
* src/conf/node_device_conf.c (virPCIELinkSpeed): Likewise.
Signed-off-by: NEric Blake <eblake@redhat.com>

The compiler can alert us to places where we need to expand switch
statements because we add a new enum value, but only if we don't
have a default case.

* src/conf/node_device_conf.c (virNodeDeviceDefFormat)
(virNodeDevCapsDefParseXML, virNodeDevCapsDefFree): Drop default
case.
Signed-off-by: NEric Blake <eblake@redhat.com>

Add an optional unique_id parameter to nodedev.  Allows for easier lookup
and display of the unique_id value in order to document for use with
scsi_host code.

Replace:
if (virBufferError(&buf)) {
    virBufferFreeAndReset(&buf);
    virReportOOMError();
    ...
}

with:
if (virBufferCheckError(&buf) < 0)
    ...

This should not be a functional change (unless some callers
misused the virBuffer APIs - a different error would be reported
then)

This new element is there to represent PCI-Express capabilities
of a PCI devices, like link speed, number of lanes, etc.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>

With one of my recent patches (1c702778) libvirt's capable of
reporting NUMA node locality for PCI devices. The node ID is
stored in pci_dev.numa_node variable. However, since zero is
valid NUMA node ID, the default is -1 as it is in kernel too.
So, if the PCI device is not tied to any specific NUMA node, the
default is then NOT printed into XML. Therefore, when parsing
node device XML, the <node/> element is optional. But currently,
if it's not there, we must set sane default, otherwise after
parsing in the memory representation doesn't match the XML. We
are already doing this in other place: udevProcessPCI().
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>

While exposing the info under <interface/> in previous patch works, it
may work only in cases where interface is configured on the host.
However, orchestrating application may want to know the link state and
speed even in that case. That's why we ought to expose this in nodedev
XML too:

virsh # nodedev-dumpxml net_eth0_f0_de_f1_2b_1b_f3
<device>
  <name>net_eth0_f0_de_f1_2b_1b_f3</name>
  <path>/sys/devices/pci0000:00/0000:00:19.0/net/eth0</path>
  <parent>pci_0000_00_19_0</parent>
  <capability type='net'>
    <interface>eth0</interface>
    <address>f0:de:f1:2b:1b:f3</address>
    <link speed='1000' state='up'/>
    <capability type='80203'/>
  </capability>
</device>
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>

A PCI device can be associated with a specific NUMA node. Later, when
a guest is pinned to one NUMA node the PCI device can be assigned on
different NUMA node. This makes DMA transfers travel across nodes and
thus results in suboptimal performance. We should expose the NUMA node
locality for PCI devices so management applications can make better
decisions.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>

Since it is an abbreviation, PCI should always be fully
capitalized or full lower case, never Pci.
Signed-off-by: NDaniel P. Berrange <berrange@redhat.com>

Since it is an abbreviation, USB should always be fully
capitalized or full lower case, never Usb.
Signed-off-by: NDaniel P. Berrange <berrange@redhat.com>

Since it is an abbreviation, SCSI should always be fully
capitalized or full lower case, never Scsi.
Signed-off-by: NDaniel P. Berrange <berrange@redhat.com>

Completely mechanical changes, but there were a lot of lines so I made
it a separate patch.

This fixes a possible double free. In virNetworkAssignDef() if
virBitmapNew() fails, then virNetworkObjFree(network) is called.
However, with network->def pointing to actual @def. So if caller
frees @def again, ...

Moreover, this fixes one possible memory leak too. In
virInterfaceAssignDef() if appending to the list of interfaces
fails, we ought to call virInterfaceObjFree() instead of bare
VIR_FREE().

Although, in order to do that some array size variables needs
to be turned into size_t rather than int.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>

Currently, during XML parsing, when a call to a FromString() function to
get an enum value fails, the error which is reported is either
VIR_ERR_CONFIG_UNSUPPORTED, VIR_ERR_INTERNAL_ERROR or VIR_ERR_XML_ERROR.

This commit makes such conversion failures consistently return
VIR_ERR_CONFIG_UNSUPPORTED.

'const fooPtr' is the same as 'foo * const' (the pointer won't
change, but it's contents can).  But in general, if an interface
is trying to be const-correct, it should be using 'const foo *'
(the pointer is to data that can't be changed).

Fix up remaining offenders in src/conf, and their fallout.

* src/conf/snapshot_conf.h (virDomainSnapshotAssignDef)
(virDomainSnapshotFindByName): Drop attempt at const.
* src/conf/interface_conf.h (virInterfaceObjIsActive)
(virInterfaceDefFormat): Use intended type.
(virInterfaceFindByMACString, virInterfaceFindByName)
(virInterfaceAssignDef, virInterfaceRemove): Drop attempt at
const.
* src/conf/network_conf.h (virNetworkObjIsActive)
(virNetworkDefFormat, virNetworkDefForwardIf)
(virNetworkDefGetIpByIndex, virNetworkIpDefPrefix)
(virNetworkIpDefNetmask): Use intended type.
(virNetworkFindByUUID, virNetworkFindByName, virNetworkAssignDef)
(virNetworkObjAssignDef, virNetworkRemoveInactive)
(virNetworkBridgeInUse, virNetworkSetBridgeName)
(virNetworkAllocateBridge): Drop attempt at const.
* src/conf/netdev_vlan_conf.h (virNetDevVlanFormat): Make
const-correct.
* src/conf/node_device_conf.h (virNodeDeviceHasCap)
(virNodeDeviceDefFormat): Use intended type.
(virNodeDeviceFindByName, virNodeDeviceFindBySysfsPath)
(virNodeDeviceAssignDef, virNodeDeviceObjRemove)
(virNodeDeviceGetParentHost): Drop attempt at const.
* src/conf/secret_conf.h (virSecretDefFormat): Use intended type.
* src/conf/snapshot_conf.c (virDomainSnapshotAssignDef)
(virDomainSnapshotFindByName): Fix fallout.
* src/conf/interface_conf.c (virInterfaceBridgeDefFormat)
(virInterfaceBondDefFormat, virInterfaceVlanDefFormat)
(virInterfaceProtocolDefFormat, virInterfaceDefDevFormat)
(virInterfaceDefFormat, virInterfaceFindByMACString)
(virInterfaceFindByName, virInterfaceAssignDef)
(virInterfaceRemove): Likewise.
* src/conf/network_conf.c
(VIR_ENUM_IMPL, virNetworkFindByName, virNetworkObjAssignDef)
(virNetworkAssignDef, virNetworkRemoveInactive)
(virNetworkDefGetIpByIndex, virNetworkIpDefPrefix)
(virNetworkIpDefNetmask, virNetworkDHCPHostDefParseXML)
(virNetworkIpDefFormat, virNetworkRouteDefFormat)
(virPortGroupDefFormat, virNetworkForwardNatDefFormat)
(virNetworkDefFormatInternal, virNetworkBridgeInUse)
(virNetworkAllocateBridge, virNetworkSetBridgeName)
(virNetworkDNSDefFormat, virNetworkDefFormat): Likewise.
* src/conf/netdev_vlan_conf.c (virNetDevVlanFormat): Likewise.
* src/conf/node_device_conf.c (virNodeDeviceHasCap)
(virNodeDeviceFindBySysfsPath, virNodeDeviceFindByName)
(virNodeDeviceAssignDef, virNodeDeviceObjRemove)
(virNodeDeviceDefFormat, virNodeDeviceGetParentHost): Likewise.
* src/conf/secret_conf.c (virSecretDefFormatUsage)
(virSecretDefFormat): Likewise.
Signed-off-by: NEric Blake <eblake@redhat.com>

Convert the type of loop iterators named 'i', 'j', k',
'ii', 'jj', 'kk', to be 'size_t' instead of 'int' or
'unsigned int', also santizing 'ii', 'jj', 'kk' to use
the normal 'i', 'j', 'k' naming
Signed-off-by: NDaniel P. Berrange <berrange@redhat.com>

Ensure that all APIs which list node device objects filter
them against the access control system.
Signed-off-by: NDaniel P. Berrange <berrange@redhat.com>

There were two errors, one as a direct result of commit id '8807b285'
and the other from cut-n-paste

TEST: nodedevxml2xmltest
      ..............                           14  OK
==25735== 3 bytes in 1 blocks are definitely lost in loss record 1 of 24
==25735==    at 0x4A0887C: malloc (vg_replace_malloc.c:270)
==25735==    by 0x344D2AF275: xmlStrndup (in /usr/lib64/libxml2.so.2.9.1)
==25735==    by 0x4D0C767: virNodeDeviceDefParseNode (node_device_conf.c:997)
==25735==    by 0x4D0D3D2: virNodeDeviceDefParse (node_device_conf.c:1337)
==25735==    by 0x401CA4: testCompareXMLToXMLHelper (nodedevxml2xmltest.c:28)
==25735==    by 0x402B2F: virtTestRun (testutils.c:158)
==25735==    by 0x401B27: mymain (nodedevxml2xmltest.c:81)
==25735==    by 0x40316A: virtTestMain (testutils.c:722)
==25735==    by 0x37C1021A04: (below main) (libc-start.c:225)
==25735==
==25735== 16 bytes in 1 blocks are definitely lost in loss record 10 of 24
==25735==    at 0x4A08A6E: realloc (vg_replace_malloc.c:662)
==25735==    by 0x4C7385E: virReallocN (viralloc.c:184)
==25735==    by 0x4C73906: virExpandN (viralloc.c:214)
==25735==    by 0x4C73B4A: virInsertElementsN (viralloc.c:324)
==25735==    by 0x4D0C84C: virNodeDeviceDefParseNode (node_device_conf.c:1026)
==25735==    by 0x4D0D3D2: virNodeDeviceDefParse (node_device_conf.c:1337)
==25735==    by 0x401CA4: testCompareXMLToXMLHelper (nodedevxml2xmltest.c:28)
==25735==    by 0x402B2F: virtTestRun (testutils.c:158)
==25735==    by 0x401B27: mymain (nodedevxml2xmltest.c:81)
==25735==    by 0x40316A: virtTestMain (testutils.c:722)
==25735==    by 0x37C1021A04: (below main) (libc-start.c:225)
==25735==
PASS: nodedevxml2xmltest

The first error was resolved by adding a missing VIR_FREE(numberStr); in
the new function virNodeDevCapPciDevIommuGroupParseXML().

The second error was a bit more opaque as the error was a result of copying
the free methodolgy of the existing code in virNodeDevCapsDefFree(). The code
would free each of the entries in the array, but not the memory for the
array itself.  Added the necessary VIR_FREE(data->pci_dev.iommuGroupDevices)
and while at it added the missing VIR_FREE(data->pci_dev.virtual_functions)
although there wasn't a test that tripped across it (thus it's been lurking
since commit id 'a010165d').

This includes adding it to the nodedev parser and formatter, docs, and
test.

An example of the new iommuGroup element that is a part of the output
from "virsh nodedev-dumpxml" (virNodeDeviceGetXMLDesc()):

  <device>
    <name>pci_0000_02_00_1</name>
    <capability type='pci'>
    ...
      <iommuGroup number='12'>
        <address domain='0x0000' bus='0x02' slot='0x00' function='0x0'/>
        <address domain='0x0000' bus='0x02' slot='0x00' function='0x1'/>
      </iommuGroup>
    </capability>
  </device>