On 4/13/21 4:58 PM, Erik Skultety wrote:
On Tue, Apr 13, 2021 at 12:01:57PM +0200, Michal Privoznik wrote:
After all devices were enumerated, the enumeration thread call
nodeDeviceUpdateMediatedDevices() to refresh the state of
mediated devices. This means that 'mdevctl' will be executed. But
it may be missing on some systems (e.g. mine) in which case we
should just skip the update of mdevs instead of failing whole
device enumeration.
Signed-off-by: Michal Privoznik <mprivozn@xxxxxxxxxx>
---
src/node_device/node_device_driver.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/src/node_device/node_device_driver.c b/src/node_device/node_device_driver.c
index 2bb83c19f2..fab830c596 100644
--- a/src/node_device/node_device_driver.c
+++ b/src/node_device/node_device_driver.c
@@ -42,9 +42,12 @@
#include "virnetdev.h"
#include "virutil.h"
#include "vircommand.h"
+#include "virlog.h"
#define VIR_FROM_THIS VIR_FROM_NODEDEV
+VIR_LOG_INIT("node_device.node_device_driver");
+
virNodeDeviceDriverStatePtr driver;
virDrvOpenStatus
@@ -1605,6 +1608,13 @@ nodeDeviceUpdateMediatedDevices(void)
virMdevctlForEachData data = { 0, };
size_t i;
+ /* The code below assumes MDEVCTL to exist. Well, if it doesn't then exit
+ * early. */
This commentary can be dropped as the check is self explanatory.
+ if (!virFileExists(MDEVCTL)) {
I don't think ^this is correct, when you don't have mdevctl installed,
MDEVCTL == "mdevctl" which won't exist even after you install the program, so
update won't work properly until you reconfigure meson and rebuild. You need to
do virFindFileInPath so that everything starts transparently working.
Okay.
I'll give you an example where this can get annoying. I just tested this on a
machine where I had mdevctl installed previously, so I removed it, but I had
some mdevs defined. Because mdevctl is now missing, udev reports some mdev
devices, but that's it. After I re-installed mdevctl back I expected libvirtd
to at least be able to define a device, but I got an error from mdevctl that
that device already is defined, but libvirtd doesn't know it's persistent
because /etc/mdevctl.d was never scanned, so I suddenly see devices as
transient and have to manually remove the mdevctl definitions.
Fair enough, but is this even supposed to work? I mean, if you
install/uninstall a tool without libvirtd restart then you shouldn't
expect things to work. We use binaries, because some tools don't provide
libraries. If mdevctl was a library then no such thing as
install/uninstall without libvirtd would be possible. Either it would be
still mapped into libvirtd (even after you've removed it from disk), or
it won't be loaded after you've installed it.
I'll switch over to virFindFileInPath(), though.
Michal
