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 ffc40b63b5 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.
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>
Some CDROM devices are reported by udev to have an ID_TYPE="generic"
thus it is necessary to check if ID_CDROM is present.
As a side effect, treating ID_TYPE="generic" as a missing ID_TYPE will
enable checks for ID_DRIVE_FLASH_SD and ID_DRIVE_FLOPPY and the
udevKludgeStorageType heuristic.
Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
Since it is an abbreviation, SCSI should always be fully
capitalized or full lower case, never Scsi.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
virNodeDeviceListCaps will always return empty for a pci nodedevice,
actually it should return 'pci'.
It is because the loop variable ncaps isn't increased.
Introduced by commit be2636f.
https://bugzilla.redhat.com/show_bug.cgi?id=1081932
Signed-off-by: Jincheng Miao <jmiao@redhat.com>
Signed-off-by: Ján Tomko <jtomko@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: Daniel P. Berrange <berrange@redhat.com>
As part of the goal to get away from doing string matching on
filenames when deciding whether to emit a log message, turn
the virLogSource enum into a struct which contains a log
"name". There will eventually be one virLogSource instance
statically declared per source file. To minimise churn in this
commit though, a single global instance is used.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Most of our code base uses space after comma but not before;
fix the remaining uses before adding a syntax check.
* src/network/bridge_driver.c: Consistently use commas.
* src/node_device/node_device_hal.c: Likewise.
* src/node_device/node_device_udev.c: Likewise.
* src/storage/storage_backend_rbd.c: Likewise.
Signed-off-by: Eric Blake <eblake@redhat.com>
Thie patch fixes the segfault:
error : nodeStateInitialize:658 : DBus not available,
disabling HAL driver: internal error: Unable to get DBus
system bus connection: Failed to connect to socket
/var/run/dbus/system_bus_socket: No such file or directory
error : nodeStateInitialize:719 : ?:
Caught Segmentation violation dumping internal log buffer:
This segfault occurs at the below VIR_ERROR:
failure:
if (dbus_error_is_set(&err)) {
VIR_ERROR(_("%s: %s"), err.name, err.message);
When virDBusGetSystemBus fails, the code jumps to the above failure
path. However, the err variable is not correctly initialized
before calling virDBusGetSystemBus. As a result, dbus_error_is_set
may pass over the uninitialized err variable whose name or
message may point to somewhere unknown memory region, which
causes a segfault on VIR_ERROR.
The new code initializes the err variable before calling
virDBusGetSystemBus.
Signed-off-by: Ryota Ozaki <ozaki.ryota@gmail.com>
The VIR_FREE() macro will cast away any const-ness. This masked a
number of places where we passed a 'const char *' string to
VIR_FREE. Fortunately in all of these cases, the variable was not
in fact const data, but a heap allocated string. Fix all the
variable declarations to reflect this.
Signed-off-by: Daniel P. Berrange <berrange@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: Daniel P. Berrange <berrange@redhat.com>
Ensure that all APIs which list node device objects filter
them against the access control system.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
This fixes:
https://bugzilla.redhat.com/show_bug.cgi?id=979290https://bugzilla.redhat.com/show_bug.cgi?id=979330
The node device driver was written with the assumption that udev would
use a "change" event to notify libvirt of any change to device status
(including the name of the driver it was bound to). It turns out this
is not the case (see Comment 4 of BZ 979290). That means that a
dumpxml for a device would always show whatever driver happened to be
bound at the time libvirt was started (when the node device cache was
built).
There was already code in the driver (for the benefit of the HAL
backend) that updated the driver name from sysfs each time a device's
info was retrieved from the cache. This patch just enables that manual
update for the udev backend as well.
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>
The xml outputed by HAL backend for scsi generic device:
<device>
<name>pci_8086_2922_scsi_host_scsi_device_lun0_scsi_generic</name>
<path>/sys/devices/pci0000:00/0000:00:1f.2/host0/target0:0:0/0:0:0:0/scsi_generic/sg0</path>
<parent>pci_8086_2922_scsi_host_scsi_device_lun0</parent>
<capability type='scsi_generic'>
<char>/dev/sg0</char>
</capability>
</device>
Since scsi generic device doesn't have DEVTYPE property set, the
only way to know if it's a scsi generic device or not is to read
the "SUBSYSTEM" property.
The XML of the scsi generic device will be like:
<device>
<name>scsi_generic_sg0</name>
<path>/sys/devices/pci0000:00/0000:00:1f.2/ata1/host0/target0:0:0/0:0:0:0/scsi_generic/sg0</path>
<parent>scsi_0_0_0_0</parent>
<capability type='scsi_generic'>
<char>/dev/sg0</char>
</capability>
</device>
