On Wed, Jan 29, 2014 at 07:52:45PM -0500, John Ferlan wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=1045124
When loading modules, libvirt does not honor the modprobe blacklist. By adding a "-b" to the modprobe command libvirt will fail to load a module if it's on the blacklist
Check if the failure to load a driver was due to it being on the blacklist using the output of a "modprobe -c" searching for "blacklist <driver_Name>" where driver_Name is possibly a modified string of the input driver changing all '-' into '_' since that's what modprobe does.
Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/util/virpci.c | 49 ++++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 40 insertions(+), 9 deletions(-)
diff --git a/src/util/virpci.c b/src/util/virpci.c index e2d222e..5d4168a 100644 --- a/src/util/virpci.c +++ b/src/util/virpci.c @@ -42,6 +42,7 @@ #include "vircommand.h" #include "virerror.h" #include "virfile.h" +#include "virkmod.h" #include "virstring.h" #include "virutil.h"
@@ -979,6 +980,9 @@ virPCIProbeStubDriver(const char *driver) { char *drvpath = NULL; bool probed = false; + size_t i; + char *drvblklst = NULL; + char *outbuf = NULL;
recheck: if (virPCIDriverDir(&drvpath, driver) == 0 && virFileExists(drvpath)) { @@ -990,18 +994,44 @@ recheck: VIR_FREE(drvpath);
if (!probed) { - const char *const probecmd[] = { MODPROBE, driver, NULL }; + char *errbuf = NULL; probed = true; - if (virRun(probecmd, NULL) < 0) { - char ebuf[1024]; - VIR_WARN("failed to load driver %s: %s", driver, - virStrerror(errno, ebuf, sizeof(ebuf))); - return -1; + if ((errbuf = virModprobeUseBlacklist(driver))) { + VIR_WARN("failed to load driver %s: %s", driver, errbuf); + VIR_FREE(errbuf); + goto cleanup; }
goto recheck; }
+ /* All error path code - purpose is to determine whether the failure + * occurs because device is on blacklist in order to add an error + * message to help detect why load failed + */ + if (virAsprintfQuiet(&drvblklst, "blacklist %s", driver) < 0) + goto cleanup; + + /* modprobe will convert all '-' into '_', so we need to as well */ + for (i = 0; i < drvblklst[i]; i++) + if (drvblklst[i] == '-') + drvblklst[i] = '_'; + + outbuf = virModprobeConfig(); + if (!outbuf) + goto cleanup; + + /* Find driver on blacklist? */ + if (strstr(outbuf, drvblklst)) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Failed to load PCI stub module %s: " + "administratively prohibited"), + driver); + }
So when I suggested creation of virkmod.{c,h} I was really meaning that all this code would go in there. eg here you'd just invoke if (virKModIsBlacklisted(driver)) { ....report error... } so all the kmod config parsing would be isolated from this pci code. Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|