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

[Daniel P. Berrange]

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 at 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 :|