A PCI device may have the capability to setup virtual functions (VFs)
but have them currently all disabled. Prior to this patch, if that was
the case the the node device XML for the device wouldn't report any
virtual_functions capability.

With this patch, if a file called "sriov_totalvfs" is found in the
device's sysfs directory, its contents will be interpreted as a
decimal number, and that value will be reported as "maxCount" in a
capability element of the device's XML, e.g.:

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

This will be reported regardless of whether or not any VFs are
currently enabled for the device.

NB: sriov_numvfs (the number of VFs currently active) is also
available in sysfs, but that value is implied by the number of items
in the list that is inside the capability element, so there is no
reason to explicitly provide it as an attribute.

sriov_totalvfs and sriov_numvfs are available in kernels at least as far
back as the 2.6.32 that is in RHEL6.7, but in the case that they
simply aren't there, libvirt will behave as it did prior to this patch
- no maxCount will be displayed, and the virtual_functions capability
will be absent from the device's XML when 0 VFs are enabled.

Convert virPCIDriverDir to return the buffer allocated (or not) and make the
appropriate check in the caller.
Signed-off-by: NJohn Ferlan <jferlan@redhat.com>

Convert virPCIDriverFile to return the buffer allocated (or not) and make the
appropriate check in the caller.
Signed-off-by: NJohn Ferlan <jferlan@redhat.com>

Convert virPCIFile to return the buffer allocated (or not) and make the
appropriate check in the caller.
Signed-off-by: NJohn Ferlan <jferlan@redhat.com>

For some devices, the $PCIDIR/vendor and $PCIDIR/device need to be
read. Iterate over them to get them as well in the the generated
apparmor profile.

Basically a getter function which is implemented for accessing the
address fields in virPCIDevice.
Signed-off-by: NShivaprasad G Bhat <sbhat@linux.vnet.ibm.com>
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>

I noticed this while working on qemuDomainGetBlockInfo.  Assigning
a bool value to an int variable compiles fine, but raises red flags
on the maintenance front as it becomes too easy to assign -1 or 2
or any other non-bool value to the same variable.

* cfg.mk (sc_prohibit_int_assign_bool): New rule.
* src/conf/snapshot_conf.c (virDomainSnapshotRedefinePrep): Fix
offenders.
* src/qemu/qemu_driver.c (qemuDomainGetBlockInfo)
(qemuDomainSnapshotCreateXML): Likewise.
* src/test/test_driver.c (testDomainSnapshotAlignDisks):
Likewise.
* src/util/vircgroup.c (virCgroupSupportsCpuBW): Likewise.
* src/util/virpci.c (virPCIDeviceBindToStub): Likewise.
* src/util/virutil.c (virIsCapableVport): Likewise.
* tools/virsh-domain-monitor.c (cmdDomMemStat): Likewise.
* tools/virsh-domain.c (cmdBlockResize, cmdScreenshot)
(cmdInjectNMI, cmdSendKey, cmdSendProcessSignal)
(cmdDetachInterface): Likewise.
Signed-off-by: NEric Blake <eblake@redhat.com>

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 all remaining problems, where there weren't
enough issues to warrant splitting it further.

* src/remote/remote_driver.c (doRemoteOpen): Correct use of {}.
* src/security/virt-aa-helper.c (vah_add_path, valid_path, main):
Likewise.
* src/rpc/virnetsocket.c (virNetSocketNewConnectLibSSH2):
Likewise.
* src/esx/esx_vi_types.c (esxVI_Type_FromString): Likewise.
* src/uml/uml_driver.c (umlDomainDetachDevice): Likewise.
* src/util/viralloc.c (virShrinkN): Likewise.
* src/util/virbuffer.c (virBufferURIEncodeString): Likewise.
* src/util/virdbus.c (virDBusCall): Likewise.
* src/util/virnetdev.c (virNetDevValidateConfig): Likewise.
* src/util/virnetdevvportprofile.c
(virNetDevVPortProfileGetNthParent): Likewise.
* src/util/virpci.c (virPCIDeviceIterDevices)
(virPCIDeviceWaitForCleanup)
(virPCIDeviceIsBehindSwitchLackingACS): Likewise.
* src/util/virsocketaddr.c (virSocketAddrGetNumNetmaskBits):
Likewise.
* src/util/viruri.c (virURIParseParams): Likewise.
* daemon/stream.c (daemonStreamHandleAbort): Likewise.
* tests/testutils.c (virtTestResult): Likewise.
* tests/cputest.c (cpuTestBaseline): Likewise.
* tools/virsh-domain.c (cmdDomPMSuspend): Likewise.
* tools/virsh-host.c (cmdNodeSuspend): Likewise.
* src/esx/esx_vi_generator.py (Type.generate_typefromstring):
Tweak generated code.
Signed-off-by: NEric Blake <eblake@redhat.com>

Coverity determined that 'conflict' would be leaked.
Signed-off-by: NWang Rui <moon.wangrui@huawei.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>

These functions will handle PCIe devices and their link capabilities
to query some info about it.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>

In making the conversion to the new API, I fixed a couple bugs:
virSCSIDeviceGetSgName would leak memory if a directory
unexpectedly contained multiple entries;
virNetDevTapGetRealDeviceName could report a spurious error
from a stale errno inherited before starting the readdir search.

The decision on whether to store the result of virDirRead into
a variable is based on whether the end of the loop falls through
to cleanup code automatically.  In some cases, we have loops that
are documented to return NULL on failure, and which raise an
error on most failure paths but not in the case where the directory
was unexpectedly empty; it may be worth a followup patch to
explicitly report an error if readdir was successful but the
directory was empty, so that a NULL return always has an error set.

* src/util/vircgroup.c (virCgroupRemoveRecursively): Use new
interface.
(virCgroupKillRecursiveInternal, virCgroupSetOwner): Report
readdir failures.
* src/util/virfile.c (virFileLoopDeviceOpenSearch)
(virFileNBDDeviceFindUnused, virFileDeleteTree): Use new
interface.
* src/util/virnetdevtap.c (virNetDevTapGetRealDeviceName):
Properly check readdir errors.
* src/util/virpci.c (virPCIDeviceIterDevices)
(virPCIDeviceFileIterate, virPCIGetNetName): Report readdir
failures.
(virPCIDeviceAddressIOMMUGroupIterate): Use new interface.
* src/util/virscsi.c (virSCSIDeviceGetSgName): Report readdir
failures, and avoid memory leak.
(virSCSIDeviceGetDevName): Report readdir failures.
* src/util/virusb.c (virUSBDeviceSearch): Report readdir
failures.
* src/util/virutil.c (virGetFCHostNameByWWN)
(virFindFCHostCapableVport): Report readdir failures.
Signed-off-by: NEric Blake <eblake@redhat.com>

Any source file which calls the logging APIs now needs
to have a VIR_LOG_INIT("source.name") declaration at
the start of the file. This provides a static variable
of the virLogSource type.
Signed-off-by: NDaniel P. Berrange <berrange@redhat.com>

Specify which driver and which domain in used_by area to avoid conflict among
different drivers.
Signed-off-by: NChunyan Liu <cyliu@suse.com>

Coverity complains about "USE_AFTER_FREE" due to how virPCIDeviceSetStubDriver
"could" return either -1, 0, or 1 from the VIR_STRDUP() and then possibly makes
a call to virPCIDeviceDetach().

The only way this could happen is if NULL were passed as the "driver" name
and virStrdup() returned 0.  Since the calling functions check < 0 on the
initial function call, the 0 possibility causes Coverity to complain.

To fix this - enforce that the second parameter is not NULL using
ATTRIBUTE_NONNULL(2) for the function prototype, then in virPCIDeviceDetach
add an sa_assert(dev->stubDriver). This will result in Coverity not complaining
any more.

https://bugzilla.redhat.com/show_bug.cgi?id=1045124

When loading modules, libvirt does not honor the modprobe blacklist.
Use the new virKModLoad() API in order to attempt load with blacklist check.
Use the new virKModIsBlacklisted() API to check if the failure to load
was due to the blacklist
Signed-off-by: NJohn Ferlan <jferlan@redhat.com>

Signed-off-by: NJiri Denemark <jdenemar@redhat.com>

https://bugzilla.redhat.com/show_bug.cgi?id=1046919

Since commit v0.9.0-47-g4e8969eb (released in 0.9.1) some failures during
device detach were reported to callers of virPCIDeviceBindToStub as
success. For example, even though a device seemed to be detached

    virsh # nodedev-detach pci_0000_07_05_0 --driver vfio
    Device pci_0000_07_05_0 detached

one could find similar message in libvirt logs:

    Failed to bind PCI device '0000:07:05.0' to vfio-pci: No such device

This patch fixes these paths and also avoids overwriting real errors
with errors encountered during a cleanup phase.

https://bugzilla.redhat.com/show_bug.cgi?id=1046919

When a PCI device is not bound to any driver, reattach should just
trigger driver probe rather than failing with

    Invalid device 0000:00:19.0 driver file
    /sys/bus/pci/devices/0000:00:19.0/driver is not a symlink

While virPCIDeviceGetDriverPathAndName was documented to return success
and NULL driver and path when a device is not attached to any driver but
didn't do so. Thus callers could not distinguish unbound devices from
failures.
Signed-off-by: NJiri Denemark <jdenemar@redhat.com>

Like commit 94a26c7e from Eric Blake, the old fuzzy code should
be replaced by the new array management macros now.

And the type of scsi->count should be changed into "size_t", and
thus virSCSIDeviceListCount should return size_t instead, similar
for vir{PCI,USB}DeviceListCount.

When determining if a device is behind a PCI bridge, the PCI device
class is checked by reading the config space. However, there are some
devices which have the wrong class on the config space, but the class is
initialized by Linux correctly as a PCI BRIDGE. This class can be read
by the sysfs file '/sys/bus/pci/devices/xxxx:xx:xx.x/class'.

One example of such bridge is IBM PCI Bridge 1014:03b9, which is
identified as a Host Bridge when reading the config space.
Signed-off-by: NThadeu Lima de Souza Cascardo <cascardo@linux.vnet.ibm.com>

Most of our code base uses space after comma but not before;
fix the remaining uses before adding a syntax check.

* src/util/vircommand.c: Consistently use commas.
* src/util/virlog.c: Likewise.
* src/util/virnetdevbandwidth.c: Likewise.
* src/util/virnetdevmacvlan.c: Likewise.
* src/util/virnetdevvportprofile.c: Likewise.
* src/util/virnetlink.c: Likewise.
* src/util/virpci.c: Likewise.
* src/util/virsysinfo.c: Likewise.
* src/util/virusb.c: Likewise.
Signed-off-by: NEric Blake <eblake@redhat.com>

These two chunks had to be part of df4283a5. But for some unclear
reason, the weren't. Anyway, these two variables are not used anywhere
within function. They're initialized to NULL and then VIR_FREE()-d. And
there's no reason do do two NOPs, right?
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>

This resolves:

  https://bugzilla.redhat.com/show_bug.cgi?id=1025397

When virPCIGetVirtualFunctions created the list of an SRIOV Physical
Function's (PF) Virtual Functions (VF), it had assumed that the order
of "virtfn*" links returned by readdir() from the PF's sysfs directory
was already in the correct order. Experience has shown that this is
not always the case - it can be in alphabetical order (which would
e.g. place virtfn11 before virtfn2) or even some seemingly random
order (see the example in the bugzilla report)

This results in 1) incorrect assumptions made by consumers of the
output of the virt_functions list of virsh nodedev-dumpxml, and 2)
setting MAC address and vlan tag on the wrong VF (since libvirt uses
netlink to set mac address and vlan tag, netlink requires the VF#, and
the function virPCIGetVirtualFunctionIndex() returns the wrong index
due to the improperly ordered VF list).

The solution provided by this patch is for virPCIGetVirtualFunctions
to no longer scan the entire device directory in its natural order,
but instead to check for links individually by name "virtfn%d" where
%d starts at 0 and increases with each success. Since VFs are created
contiguously by the kernel, this will guarantee that all VFs are
found, and placed in the arry in the correct order.

One note of use to the uninitiated is that VIR_APPEND_ELEMENT always
either increments *num_virtual_functions or fails, so no this isn't an
endless loop.

(NB: the SRIOV_* defines at the top of virpci.c were removed
because they are unnecessary and/or not used.)

This is a prerequisite to the fix for the fix to:

  https://bugzilla.redhat.com/show_bug.cgi?id=1025397

num_virtual_functions needs to be size_t in order to use the
VIR_APPEND_ELEMENT macro.

https://bugzilla.redhat.com/show_bug.cgi?id=1018897

If a PCI deivce is not binded to any driver (e.g. there's yet no PCI
driver in the linux kernel) but still users want to passthru the device
we fail the whole operation as we fail to resolve the 'driver' link
under the PCI device sysfs tree. Obviously, this is not a fatal error
and it shouldn't be error at all.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>

Semantics of the libvirt helper are more clear. This change also allows
to clean up some pieces of code.

Signed-off-by: NOskari Saarenmaa <os@ohmu.fi>
Signed-off-by: NEric Blake <eblake@redhat.com>

When virAsprintf was changed from a function to a macro
reporting OOM error in dc6f2dad, it was documented as returning
0 on success. This is incorrect, it returns the number of bytes
written as asprintf does.

Some of the functions were converted to use virAsprintf's return
value directly, changing the return value on success from 0 to >= 0.

For most of these, this is not a problem, but the change in
virPCIDriverDir breaks PCI passthrough.

The return value check in virhashtest pre-dates virAsprintf OOM
conversion.

vmwareMakePath seems to be unused.

I recently patches the callers to virPCIDeviceReset() to not call it
if the current driver for a device was vfio-pci (since that driver
will always reset the device itself when appropriate. At the time, Dan
Berrange suggested that I could instead modify virPCIDeviceReset
to check the currently bound driver for the device, and decide
for itself whether or not to go ahead with the reset.

This patch removes the previously added checks, and replaces them with
a check down in virPCIDeviceReset(), as suggested.

The functional difference here is that previously we were deciding
based on either the hostdev configuration or the value of
stubDriverName in the virPCIDevice object, but now we are actually
comparing to the "driver" link in the device's sysfs entry
directly. In practice, both should be the same.

virPCIDeviceGetDriverPathAndName is a static function that will need
to be called by another function that occurs above it in the
file. This patch reorders the static functions so that a forward
declaration isn't needed.

I had made the change locally, so make check and make syntax-check
were successful, but forgot to add/commit. Unfortunately, git allows a
push when the local directory is dirty, so it didn't catch my mistake.

Eliminate memmove() by using VIR_*_ELEMENT API instead.

In both pci and usb cases, the count that held the size of the list
was unsigned int so it had to be changed to size_t.

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>

Similarly to VIR_STRDUP, we want the OOM error to be reported in
VIR_ALLOC and friends.

This fixes https://bugzilla.redhat.com/show_bug.cgi?id=971325

The problem was that if virPCIGetVirtualFunctions was given the name
of a non-existent interface, it would return to its caller without
initializing the pointer to the array of virtual functions to NULL,
and the caller (virNetDevGetVirtualFunctions) would try to VIR_FREE()
the invalid pointer.

The final error message before the crash would be:

 virPCIGetVirtualFunctions:2088 :
  Failed to open dir '/sys/class/net/eth2/device':
  No such file or directory

In this patch I move the initialization in virPCIGetVirtualFunctions()
to the begining of the function, and also do an explicit
initialization in virNetDevGetVirtualFunctions, just in case someone
in the future adds code into that function prior to the call to
virPCIGetVirtualFunctions.