On Fri, Jan 05, 2024 at 10:33:33 -0500, Laine Stump wrote:
On 1/5/24 9:22 AM, Peter Krempa wrote:
On Fri, Jan 05, 2024 at 03:20:04 -0500, Laine Stump wrote:
[...]
Change from V1: I tried to simplify the explanation in the commit log message
I don't think this addresses Jason's comment from V1, stating that we should only deal with driver names and let modprobe find the correct module.
modprobe has no way of "finding" the proper module for a given driver - all it does is replace "-" with "_" in the name and hope for the best. The problem is that the way to get to module name from a driver name is to follow the links in the driver's directory in sysfs, but the driver's directory in sysfs doesn't exist until the module has been loaded, and if the module is already loaded then you don't need to know the name of the module any more.
Same thing when you are looking for the best *driver* for the device. Yes you find a module name, but you can query the module itself for the driver and just use the driver.
That is what we end up doing - when we find the module name for a variant, we load that module and then follow the link to get to the driver.
As Jason stated, we should not deal with modules at all.
I disagree. If the module for a driver isn't loaded, then (unless the modulename happens to be (underscore($drivername)) there is no way to figure out what module to load.
(If the module had already been loaded, then we could follow the links in sysfs from driver to module, but of course if the module is already loaded then we don't need to know the module name!)
In the end, though, with the currently existing VFIO/VFIO-variant drivers, the only point of contention is that my current code allows specifying <driver name='vfio_pci'/> in the XML, and you (and Jason) are saying that we shouldn't allow that, but should require <driver name='vfio-pci'/>. Since, at some point *internally* we do need to support starting with "vfio_pci" and end up with "vfio-pci", this all comes down to whether the extra flexibility is at a higher level (as in this patch) or if it's only done at a lower level (i.e. doing it during the virPCIDeviceFindBestVFIOVariant() in patch 13). I'll see if I can make something that does it at that level and resubmit (but will then forward any bug reports about failing with <driver name='vfio_pci'/> to you :-P)
Fair enough, I think this explanation is sufficient for me. I've done a proper review in another subthread.