This new API function provides a way to start a persistently-defined
mediate device that was defined by virNodeDeviceDefineXML() (or one that
was defined externally via mdevctl)
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
This interface allows you to undefine a persistently defined (but
inactive) mediated devices. It is implemented via 'mdevctl'
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
With mediated devices, we can now define persistent node devices that
can be started and stopped. In order to take advantage of this, we need
an API to define new node devices.
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
Abstract out the function used to generate the commandline for 'mdevctl
start' since they take the same arguments. Add tests to ensure that
we're generating the command properly.
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
We need to query mdevctl for changes to device definitions since an
administrator can define new devices by executing mdevctl outside of
libvirt.
In the future, mdevctl may add a way to signal device add/remove via
events, but for now we resort to a bit of a workaround: monitoring the
mdevctl config directory for changes to files. When a change is
detected, we query mdevctl and update our device list. The mdevctl
querying is handled in a throwaway thread, and these threads are
synchronized with a mutex.
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
mdevctl does not currently provide any events when the list of defined
devices changes, so we will need to poll mdevctl for the list of defined
devices periodically. When a mediated device no longer exists from one
iteration to the next, we need to treat it as an "undefine" event.
When we get such an event, we remove the device from the list if it's
not active. Otherwise, we simply mark it as non-persistent.
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
At startup, query devices that are defined by 'mdevctl' and add them to
the node device list.
This adds a complication: we now have two potential sources of
information for a node device:
- udev for all devices and for activated mediated devices
- mdevctl for persistent mediated devices
Unfortunately, neither backend returns full information for a mediated
device. For example, if a persistent mediated device in the list (with
information provided from mdevctl) is 'started', that same device will
now be detected by udev. If we simply overwrite the existing device
definition with the new one provided by the udev backend, we will lose
extra information that was provided by mdevctl (e.g. attributes, etc).
To avoid this, make sure to copy the extra information into the new
device definition.
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
This adds an internal API to query for persistent mediated devices
that are defined by mdevctl. Upcoming commits will make use of this
information.
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
This function will parse the list of mediated devices that are returned
by mdevctl and convert it into our internal node device representation.
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
Expose a helper function that can be used by udev and mdevctl to
generate device names for node devices.
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
Add two flag values for virConnectListAllNodeDevices() so that we can
list only node devices that are active or inactive.
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
we will be able to define mediated devices that can be started or
stopped, so we need to be able to indicate whether the device is active
or not, similar to other resources (storage pools, domains, etc.)
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
When an mdevctl command fails, there is not much information available
to the user about why it failed. This is partly because we were not
making use of the error message that mdevctl itself prints upon failure.
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
Since the functions provided by libpciaccess are not thread-safe,
when the udev-event and nodedev-init threads of libvirt call the
pci_get_strings function provided by libpaciaccess at the same
time the following can happen:
nodedev-init thread:
nodeStateInitializeEnumerate ->
udevEnumerateDevices->
udevProcessDeviceListEntry ->
udevAddOneDevice ->
udevGetDeviceDetails->
udevProcessPCI ->
udevTranslatePCIIds ->
pci_get_strings -> (libpciaccess)
find_device_name ->
populate_vendor ->
d = realloc( vend->devices, (vend->num_devices + 1), * sizeof( struct pci_device_leaf ) );
vend->num_devices++;
udev-event thread:
udevEventHandleThread ->
udevHandleOneDevice ->
udevAddOneDevice->
udevGetDeviceDetails->
udevProcessPCI ->
udevTranslatePCIIds ->
pci_get_strings -> (libpciaccess)
find_device_name ->
populate_vendor ->
d = realloc( vend->devices, (vend->num_devices + 1), * sizeof( struct pci_device_leaf ) );
vend->num_devices++;
Signed-off-by: WangJian <wangjian161@huawei.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Generated by the following spatch:
@@
expression a, b;
@@
+ b = g_steal_pointer(&a);
- b = a;
... when != a
- a = NULL;
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
If libvirtd is terminated before the node driver finishes
initialization, it can crash with a backtrace similar to the following:
Stack trace of thread 1922933:
#0 0x00007f8515178774 g_hash_table_find (libglib-2.0.so.0)
#1 0x00007f851593ea98 virHashSearch (libvirt.so.0)
#2 0x00007f8515a1dd83 virNodeDeviceObjListSearch (libvirt.so.0)
#3 0x00007f84cceb40a1 udevAddOneDevice (libvirt_driver_nodedev.so)
#4 0x00007f84cceb5fae nodeStateInitializeEnumerate (libvirt_driver_nodedev.so)
#5 0x00007f85159840cb virThreadHelper (libvirt.so.0)
#6 0x00007f8511c7d14a start_thread (libpthread.so.0)
#7 0x00007f851442bdb3 __clone (libc.so.6)
Stack trace of thread 1922863:
#0 0x00007f851442651d syscall (libc.so.6)
#1 0x00007f85159842d4 virThreadSelfID (libvirt.so.0)
#2 0x00007f851594e240 virLogFormatString (libvirt.so.0)
#3 0x00007f851596635d vir_object_finalize (libvirt.so.0)
#4 0x00007f8514efe8e9 g_object_unref (libgobject-2.0.so.0)
#5 0x00007f85159667f8 virObjectUnref (libvirt.so.0)
#6 0x00007f851517755f g_hash_table_remove_all_nodes.part.0 (libglib-2.0.so.0)
#7 0x00007f8515177e62 g_hash_table_unref (libglib-2.0.so.0)
#8 0x00007f851596637e vir_object_finalize (libvirt.so.0)
#9 0x00007f8514efe8e9 g_object_unref (libgobject-2.0.so.0)
#10 0x00007f85159667f8 virObjectUnref (libvirt.so.0)
#11 0x00007f84cceb2b42 nodeStateCleanup (libvirt_driver_nodedev.so)
#12 0x00007f8515b37950 virStateCleanup (libvirt.so.0)
#13 0x00005648085348e8 main (libvirtd)
#14 0x00007f8514352493 __libc_start_main (libc.so.6)
#15 0x00005648085350fe _start (libvirtd)
This is because the initial population of the device list is done in a
separate initialization thread. If we attempt to exit libvirtd before
this init thread has completed, we'll try to free the device list while
accessing it from the other thread. In order to guarantee that this
init thread is not accessing the device list when we're cleaning up the
nodedev driver, make it joinable and wait for it to finish before
proceding with the cleanup. This is similar to how we handle the udev
event handler thread.
The separate initialization thread was added in commit
9f0ae0b1.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1836865
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
These functions are identical. Made using this spatch:
@@
expression path, mode;
@@
- virFileMakePathWithMode(path, mode)
+ g_mkdir_with_parents(path, mode)
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The parent array takes ownership of the inserted value once all checks
pass. Don't make the callers second-guess when that happens and modify
the function to take a double pointer so that it can be cleared once the
ownership is taken.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
The parent object takes ownership of the inserted value once all checks
pass. Don't make the callers second-guess when that happens and modify
the function to take a double pointer so that it can be cleared once the
ownership is taken.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
The 'conflict' key in a virt_daemon_unit dictionary is not used when
generating systemd service and socket files. The comment associated
with the key claims the default is 'true', and a few build files
needlessly set it to 'true' when defining their virt_daemon_unit.
Remove the 'conflict' key and its use in the affect build files.
Signed-off-by: Jim Fehlig <jfehlig@suse.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
`udevGetIntSysfsAttr` does not necessarily write to the third parameter,
even when it returns 0.
This was found by clang-tidy's
"clang-analyzer-core.UndefinedBinaryOperatorResult" check.
Signed-off-by: Tim Wiederhake <twiederh@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
The current virPCIDeviceNew() signature, receiving 4 uints in sequence
(domain, bus, slot, function), is not neat.
We already have a way to represent a PCI address in virPCIDeviceAddress
that is used in the code. Aside from the test files, most of
virPCIDeviceNew() callers have access to a virPCIDeviceAddress reference,
but then we need to retrieve the 4 required uints (addr.domain, addr.bus,
addr.slot, addr.function) to satisfy virPCIDeviceNew(). The result is
that we have extra verbosity/boilerplate to retrieve an information that
is already available in virPCIDeviceAddress.
A better way is presented by virNVMEDeviceNew(), where the caller just
supplies a virPCIDeviceAddress pointer and the function handles the
details internally.
This patch changes virPCIDeviceNew() to receive a virPCIDeviceAddress
pointer instead of 4 uints.
Reviewed-by: Laine Stump <laine@redhat.com>
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
With commit 09364608b4 node_device: refactor address retrieval of node device
"if-else if" was replaced by "switch".
The contained break statement now is no longer in context of the for loop
but instead of the switch causing the legitimate grumpiness of coverity.
Signed-off-by: Boris Fiuczynski <fiuczy@linux.ibm.com>
Suggested-by: John Ferlan <jferlan@redhat.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Allow mdev devices to be created on the matrix device.
Signed-off-by: Shalini Chellathurai Saroja <shalini@linux.ibm.com>
Reviewed-by: Bjoern Walk <bwalk@linux.ibm.com>
Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
Use switch statements instead of if-else condition in the method
nodeDeviceFindAddressByName to retrieve address of a node device.
Signed-off-by: Shalini Chellathurai Saroja <shalini@linux.ibm.com>
Reviewed-by: Bjoern Walk <bwalk@linux.ibm.com>
Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
Add support for AP matrix device in libvirt node device driver.
https://www.kernel.org/doc/html/latest/s390/vfio-ap.html#the-design
Signed-off-by: Shalini Chellathurai Saroja <shalini@linux.ibm.com>
Reviewed-by: Bjoern Walk <bwalk@linux.ibm.com>
Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
Each AP card device can support upto 256 AP queues. AP queues are
also detected by udev, so add support for libvirt nodedev driver.
https://www.kernel.org/doc/html/latest/s390/vfio-ap.html#ap-architectural-overview
Signed-off-by: Farhan Ali <alifm@linux.ibm.com>
Signed-off-by: Shalini Chellathurai Saroja <shalini@linux.ibm.com>
Reviewed-by: Bjoern Walk <bwalk@linux.ibm.com>
Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
Introduce support for the Adjunct Processor (AP) crypto card device.
Udev already detects the device, so add support for libvirt nodedev
driver.
https://www.kernel.org/doc/html/latest/s390/vfio-ap.html#ap-architectural-overview
Signed-off-by: Farhan Ali <alifm@linux.ibm.com>
Signed-off-by: Shalini Chellathurai Saroja <shalini@linux.ibm.com>
Reviewed-by: Bjoern Walk <bwalk@linux.ibm.com>
Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
The helper methods for getting integer properties ignore a missing
property setting its value to zero. This lack of error reporting
resulted in missing the regression handling hotplug of USB devices
with the vendor and model IDs getting set to zero silently.
The few callers which relied on this silent defaulting have been fixed,
so now we can report fatal errors immediately.
Reviewed-by: Laine Stump <laine@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The access of DKD_MEDIA_AVAILABLE for floppy disks, is mistakenly
protected by a check for ID_CDROM_MEDIA, introduced in:
commit 10427db779
Author: Ján Tomko <jtomko@redhat.com>
Date: Fri Jun 3 16:10:21 2016 +0200
Only return two values in udevGetUintProperty
Thus the check of DKD_MEDIA_AVAILABLE never run. In practice this didn't
matter since this property is set by the DeviceKit-Disks daemon which
was only around for 3 Fedora releases before being killed off around
F13. Thus we can just remove this legacy property.
Reviewed-by: Laine Stump <laine@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The udevProcessStorage method relies on udevGetIntProperty ignoring
errors about non-existant properties and instead setting the value to
zero. In theory when seeing ID_CDROM=1, you might expect that devices
which are not CDs will get ID_CDROM=0, but that's not what happens in
practice. Instead the property simply won't get set at all.
IOW, the code does not need to care about the value of the property,
merely whether it exists or not.
Reviewed-by: Laine Stump <laine@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
In v6.3.0-rc1~67 I've made a switch: instead of listening on udev
events the nodedev driver started listening for kernel events.
This was because when a device changes its name (e.g. NICs) we
will get "move" event with DEVPATH_OLD property set, which we can
then use to remove the old device and thus keep our internal list
up to date. The switch to "kernel" source was made because if the
old NICs naming (eth0, eth1, ...) is enabled (e.g. via
net.ifnames=0 on the kernel cmd line) then udev overwrites the
property with the new name making our internal list go out of
sync. Interestingly, when the od NICs naming is not enabled then
the DEVPATH_OLD contains the correct value.
But as it turns out, "kernel" source might be missing some other
important properties, e.g. USB vendor/product IDs. Therefore,
switch back to "udev" source and wish the best of luck to users
using the old NICs naming.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1897625
Fixes: 9a13704818
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Add detection of mdev_types capability to channel subsystem devices.
Signed-off-by: Boris Fiuczynski <fiuczy@linux.ibm.com>
Reviewed-by: Bjoern Walk <bwalk@linux.ibm.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
All of these conversions are trivial - VIR_DIR_CLOSE() (aka
virDirClose()) is called only once on the DIR*, and it happens just
before going out of scope.
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Commit 53aec799fa introduced the function udevGetVDPACharDev(),
which scans a directory using virDirOpenIfExists() and
virDirRead(). It unfortunately forgets to close the DIR* when it is
finished with it. This patch fixes that omission.
Fixes: 53aec799fa
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The current udev node device driver ignores all events related to vdpa
devices. Since libvirt now supports vDPA network devices, include these
devices in the device list.
Example output:
virsh # nodedev-list
[...ommitted long list of nodedevs...]
vdpa_vdpa0
virsh # nodedev-dumpxml vdpa_vdpa0
<device>
<name>vdpa_vdpa0</name>
<path>/sys/devices/vdpa0</path>
<parent>computer</parent>
<driver>
<name>vhost_vdpa</name>
</driver>
<capability type='vdpa'>
<chardev>/dev/vhost-vdpa-0</chardev>
</capability>
</device>
NOTE: normally the 'parent' would be a PCI device instead of 'computer',
but this example output is from the vdpa_sim kernel module, so it
doesn't have a normal parent device.
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
Assume nobody runs current libvirt on kernels such as 2.6.26.
Kernel commit 9f9c9cbb60576a1518d0bf93fb8e499cffccf377 (released
in 3.8) mentions the new path and I believe it was added by:
commit 948af1f0bbc8526448e8cbe3f8d3bf211bdf5181
firmware: Basic dmi-sysfs support
(released in 2.6.39), but I cannot figure out how all that
kernel automagic works.
This reverts commit 4c81b0fdc5
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
There was one attempt a year ago done by me to drop HAL [1] but it was
never resolved. There was another time when Dan suggested to drop HAL
driver [2] but it was decided to keep it around in case device
assignment will be implemented for FreeBSD and the fact that
virt-manager uses node device driver [3].
I checked git history and code and it doesn't look like bhyve supports
device assignment so from that POV it should not block removing HAL.
The argument about virt-manager is not strong as well because libvirt
installed from FreeBSD packages doesn't have HAL support so it will not
affect these users as well [4].
The only users affected by this change would be the ones compiling
libvirt from GIT on FreeBSD.
I looked into alternatives and there is libudev-devd package on FreeBSD
but unfortunately it doesn't work as it doesn't list any devices when
used with libvirt. It provides libudev APIs using devd.
I also looked into devd directly and it provides some APIs but there are
no APIs for device monitoring and events so that would have to be
somehow done by libvirt.
Main motivation for dropping HAL support is to replace libdbus with GLib
dbus implementation and it cannot be done with HAL driver present in
libvirt because HAL APIs heavily depends on symbols provided by libdbus.
[1] <https://www.redhat.com/archives/libvir-list/2019-May/msg00203.html>
[2] <https://www.redhat.com/archives/libvir-list/2016-April/msg00992.html>
[3] <https://www.redhat.com/archives/libvir-list/2016-April/msg00994.html>
[4] <https://svnweb.freebsd.org/ports/head/devel/libvirt/Makefile?view=markup>
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Allow vfio-ccw mdev devices to be created besides vfio-pci mdev devices
as well.
Reviewed-by: Erik Skultety <eskultet@redhat.com>
Reviewed-by: Bjoern Walk <bwalk@linux.ibm.com>
Signed-off-by: Boris Fiuczynski <fiuczy@linux.ibm.com>
Make Direct Access Storage Devices (DASDs) available in the node_device driver.
Reviewed-by: Bjoern Walk <bwalk@linux.ibm.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
Signed-off-by: Boris Fiuczynski <fiuczy@linux.ibm.com>
Make channel subsystem (CSS) devices available in the node_device driver.
The CCS devices reside in the computer system and provide CCW devices, e.g.:
+- css_0_0_003a
|
+- ccw_0_0_1a2b
|
+- scsi_host0
|
+- scsi_target0_0_0
|
+- scsi_0_0_0_0
Reviewed-by: Erik Skultety <eskultet@redhat.com>
Reviewed-by: Bjoern Walk <bwalk@linux.ibm.com>
Signed-off-by: Boris Fiuczynski <fiuczy@linux.ibm.com>
Refactor out CCW address parsing for later reuse.
Reviewed-by: Erik Skultety <eskultet@redhat.com>
Reviewed-by: Bjoern Walk <bwalk@linux.ibm.com>
Signed-off-by: Boris Fiuczynski <fiuczy@linux.ibm.com>