Two out of three callers free it right after converting it to a number.
Also change the comment at the beginning of the function, because
the comment inside the function told me to.
The wrapper adds an error message or a debug log.
Since we already log the properties we get from udev as strings,
there is no much use for the debug logs.
Open code the error message and delete the function.
Most of the code paths had to reset it to -1 and returning 0 was
only possible if we made it to the end of the function.
Initialize it to -1 and only set it to 0 if we reach the end, as we do
in most of libvirt code.
Commit c8b1a83605e4 changed the function, making it
impossible for callers to be able to tell whether a
non-negative return value means "physical function
address found and parsed correctly" or "couldn't find
corresponding physical function".
The important difference between the two being that,
in the latter case, the returned pointer is NULL and
should never, ever be dereferenced.
In order to cope with these changes, the callers
have to be updated.
Both instances use VIR_WARN() to print the error from a failed
virDBusGetSystemBus() call. Rather than use the virGetLastError
and need to check for valid return err pointer, just use the
virGetLastErrorMessage.
Signed-off-by: John Ferlan <jferlan@redhat.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=1317531
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Best viewed with '-w' as this is just an adjustment for future patch to
be readable without that.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Add a stub for nodeDeviceSysfsGetPCIRelatedDevCaps() for non-Linux
platforms. It allows nodedev driver to work on non-Linux platoforms
that, however, have HAL.
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.
As with several other attributes of devices (link status, sriov VF
list, IOMMU group list), the detdev feature bits aren't automatically
updated in the nodedev driver's cache when they change. In order to
get a properly up-to-date list when getting the XML of a device, we
must reget them in update-caps prior to each dumpxml.
This resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1232880
In a couple of cases, the node device driver (and the test node device
driver which likely copied it) was only logging "Node device not
found" when it couldn't find the requested device. This patch changes
those cases to log the name (and in the case when it's relevant, the
wwnn and wwpn) as well.
commit ffc40b63b50ecb changed uniond _virNodeDevCapData into a typedef
named virNodeDevCapData with a struct that contains the union as well
as a type enum. This change necessitated changing every reference to
"caps->type" into "caps->data.type", but the author of that patch
failed to test a build "WITH_HAL". This patch fixes the one place in
the hal backend that needed changing.
Both the hal and udev drivers call virPCI*() functions to the the
SRIOV VF/PF info about PCI devices, and the UDEV backend calls
virPCI*() to get IOMMU group info. Since there is now a single
function call in node_device_linux_sysfs.c to do all of this, replace
all that code in the two backends with calls to
nodeDeviceSysfsGetPCIRelatedDevCaps().
Note that this results in the HAL driver (probably) unnecessarily
calling virPCIDevieAddressGetIOMMUGroupNum(), but in the case that the
host doesn't support IOMMU groups, that function turns into a NOP (it
returns -2, which causes the caller to skip the call to
virPCIDeviceAddressGetIOMMUGroupAddresses()). So in the worst case it
is a few extra cycles spent, and in the best case a mythical platform
that supported IOMMU groups but used HAL rather than UDEV would gain
proper reporting of IOMMU group info.
Because reloading a PF driver with a different number of VFs doesn't
result in any sort of event sent from udev to the libvirt node_device
driver, libvirt's cache of that info can be out of date when a request
arrives for the info about a device. To fix this, we refresh that data
at the time of the dumpxml request, similar to what is already done
for netdev link info and SCSI host capabilities.
Since the same is true for iommu group information (for example, some
other device in the same iommu group could have been detached from the
host), we also create a function to update the iommu group info from
sysfs, and a common function that does both. (a later patch will call
this common function from the udev and hal backends).
This resolves:
https://bugzilla.redhat.com/show_bug.cgi?id=981546
The udev and hal drivers both already call the same functions as these
new functions added to node_device_linux_sysfs.c, but 1) we need to
call them from node_device_driver.c, and 2) it would be nice to
eliminate the duplicated code from the hal and udev backends.
This file contains only a single function, detect_scsi_host_caps(),
which is declared in node_device_driver.h and called from both the hal
and udev backends. Other things common to the hal and udev drivers
can be placed in that file though. As a prelude to adding further
functions, this patch renames the existing function to something
closer in line with other internal libvirt function names
(nodeDeviceSysfsGetSCSIHostCaps()), and puts the declarations into a
separate .h file.
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.
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: Já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 virNodeDeviceNumOfCaps and virNodeDeviceListCaps
using nodeDeviceNumOfCaps and nodeDeviceListCaps. 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')
devs = conn.listAllDevices()
for dev in devs:
if 'fc_host' in dev.listCaps() or 'vports' in dev.listCaps():
print dev.name(),dev.numOfCaps(),dev.listCaps()
For stateless, client side drivers, it is never correct to
probe for secondary drivers. It is only ever appropriate to
use the secondary driver that is associated with the
hypervisor in question. As a result the ESX & HyperV drivers
have both been forced to do hacks where they register no-op
drivers for the ones they don't implement.
For stateful, server side drivers, we always just want to
use the same built-in shared driver. The exception is
virtualbox which is really a stateless driver and so wants
to use its own server side secondary drivers. To deal with
this virtualbox has to be built as 3 separate loadable
modules to allow registration to work in the right order.
This can all be simplified by introducing a new struct
recording the precise set of secondary drivers each
hypervisor driver wants
struct _virConnectDriver {
virHypervisorDriverPtr hypervisorDriver;
virInterfaceDriverPtr interfaceDriver;
virNetworkDriverPtr networkDriver;
virNodeDeviceDriverPtr nodeDeviceDriver;
virNWFilterDriverPtr nwfilterDriver;
virSecretDriverPtr secretDriver;
virStorageDriverPtr storageDriver;
};
Instead of registering the hypervisor driver, we now
just register a virConnectDriver instead. This allows
us to remove all probing of secondary drivers. Once we
have chosen the primary driver, we immediately know the
correct secondary drivers to use.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
In systemd >= 218, the udev_set_log_fn method has been marked
deprecated and turned into a no-op. Nothing in the udev client
library will print to stderr by default anymore, so we can
just stop installing a logging hook for new enough udev.
If detect_scsi_host_caps reports errors but keeps libvirtd going on
startup, the user is misled by the error messages. Transforming them
into warning still shows the problems, but indicates this is not fatal.
The manufacurer and product from USB device itself are usually not particularly
useful -- they tend to be missing, or ugly (all-uppercase, padded with spaces,
etc.). Prefer what's in the usb id database and fall back to descriptors only
if the device is too new to be in database.
https://bugzilla.redhat.com/show_bug.cgi?id=1138887
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: Eric 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.
Commit 5df813177c3b609a8cf5db26ae94b26d4a40063d forgot to adjust a few
callers of virStrToLong_ui to ignore the returned value in some ancient
parts of the 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 stops the error message spam when running unprivileged
libvirtd:
2014-06-30 12:38:47.990+0000: 631: error : virPCIDeviceConfigOpen:300 :
Failed to open config space file
'/sys/bus/pci/devices/0000:00:00.0/config': Permission denied
Reported by Daniel Berrange:
https://www.redhat.com/archives/libvir-list/2014-June/msg01082.html
This new element is there to represent PCI-Express capabilities
of a PCI devices, like link speed, number of lanes, etc.
Signed-off-by: Michal 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🇩🇪f1:2b:1b:f3</address>
<link speed='1000' state='up'/>
<capability type='80203'/>
</capability>
</device>
Signed-off-by: Michal 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: Michal Privoznik <mprivozn@redhat.com>
In "src/conf/" there are many enumeration (enum) declarations. Similar
to the recent cleanup to "src/util" directory, it's better to use a
typedef for variable types, function types and other usages. Other
enumeration and folders will be changed to typedef's in the future.
Most of the files changed in this commit are reltaed to Node and
Network (node_device_conf.h and nwfilter_params.*) enums.
Signed-off-by: Julio Faracco <jcfaracco@gmail.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
