Re: [libvirt] [PATCH v2 3/3] Honor blacklist for modprobe command

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(a)redhat.com&gt; > --- > 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.