On Wed, Jan 06, 2021 at 11:27:09AM +0100, Erik Skultety wrote:
On Thu, Dec 24, 2020 at 08:14:31AM -0600, Jonathon Jongsma wrote:
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> --- src/node_device/node_device_driver.c | 76 ++++++++++++++++++++++++++++ src/node_device/node_device_driver.h | 3 ++ src/node_device/node_device_udev.c | 48 ++++++++++++++++++ 3 files changed, 127 insertions(+)
diff --git a/src/node_device/node_device_driver.c b/src/node_device/node_device_driver.c index 5309b8abd5..0267005af1 100644 --- a/src/node_device/node_device_driver.c +++ b/src/node_device/node_device_driver.c @@ -1144,3 +1144,79 @@ nodeDeviceGenerateName(virNodeDeviceDefPtr def, *(def->name + i) = '_'; } } + + +static int +virMdevctlListDefined(virNodeDeviceDefPtr **devs) +{ + int status; + g_autofree char *output = NULL; + g_autoptr(virCommand) cmd = nodeDeviceGetMdevctlListCommand(true, &output); + + if (virCommandRun(cmd, &status) < 0) + return -1; + + if (!output) + return -1; + + return nodeDeviceParseMdevctlJSON(output, devs); +} + + +int +nodeDeviceUpdateMediatedDevices(void)
^This was called mdevctlEnumerateDevices in v2, so I'm wondering why did you change the name? I think it was okay the way it was (I double checked that I didn't suggest this change in v2 by accident).
In patch 9 I see the reason and it's indeed better to be named this way, so you can ignore ^this comment, sorry, I was too hasty. Erik