On Wed, Jan 29, 2014 at 07:52:43PM -0500, John Ferlan wrote: > diff --git a/src/util/virkmod.c b/src/util/virkmod.c > new file mode 100644 > index 0000000..b8de8ea > --- /dev/null > +++ b/src/util/virkmod.c > @@ -0,0 +1,138 @@ > +/* > + * virkmod.c: helper APIs for managing kernel modules > + * > + * Copyright (C) 2014 Red Hat, Inc. > + * > + * This library is free software; you can redistribute it and/or > + * modify it under the terms of the GNU Lesser General Public > + * License as published by the Free Software Foundation; either > + * version 2.1 of the License, or (at your option) any later version. > + * > + * This library is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > + * Lesser General Public License for more details. > + * > + * You should have received a copy of the GNU Lesser General Public > + * License along with this library. If not, see > + * <http://www.gnu.org/licenses/>. > + * > + */ > + > +#include <config.h> > +#include "virkmod.h" > +#include "vircommand.h" > + > +static int > +doModprobe(const char *opts, const char *module, char **outbuf, char **errbuf) > +{ > + int ret = -1; > + virCommandPtr cmd = NULL; > + > + cmd = virCommandNew(MODPROBE); > + if (opts) > + virCommandAddArg(cmd, opts); > + if (module) > + virCommandAddArg(cmd, module); > + if (outbuf) > + virCommandSetOutputBuffer(cmd, outbuf); > + if (errbuf) > + virCommandSetErrorBuffer(cmd, errbuf); > + > + if (virCommandRun(cmd, NULL) < 0) > + goto cleanup; > + > + ret = 0; > + > +cleanup: > + virCommandFree(cmd); > + return ret; > +} > + > +/** > + * virModprobeConfig: > + * > + * Get the current kernel module configuration > + * > + * Returns NULL on failure or a pointer to the output which > + * must be VIR_FREE()'d by the caller > + */ > +char * > +virModprobeConfig(void) > +{ > + char *outbuf = NULL; > + > + if (doModprobe("-c", NULL, &outbuf, NULL) < 0) > + return NULL; > + > + return outbuf; > +} > + > + > +/** > + * virModprobeLoad: > + * @module: Name of the module to load regardless of being on blacklist > + * > + * Attempts to load a kernel module > + * > + * returns NULL in case of success and the error buffer output from the > + * virCommandRun() on failure. The returned buffer must be VIR_FREE() > + * by the caller > + */ > +char * > +virModprobeLoad(const char *module) > +{ > + char *errbuf = NULL; > + > + if (doModprobe(NULL, module, NULL, &errbuf) < 0) > + return errbuf; > + > + VIR_FREE(errbuf); > + return NULL; > +} > + > + > +/** > + * virModprobeUseBlacklist: > + * @module: Name of the module to load applying blacklist lookup > + * > + * Like ModprobeLoad, except adhere to the blacklist if present > + * > + * returns NULL in case of success and the error buffer output from the > + * virCommandRun() on failure. The returned buffer must be VIR_FREE() > + * by the caller > + */ > +char * > +virModprobeUseBlacklist(const char *module) > +{ > + char *errbuf = NULL; > + > + if (doModprobe("-b", module, NULL, &errbuf) < 0) > + return errbuf; > + > + VIR_FREE(errbuf); > + return NULL; > +} I think we could probably just have a 'bool useBlacklist' parameter for the virModeprobeLoad method - we'll likely want all callers to use the blacklist anyway, so not much need to have separate methods for each usage. > + > + > +/** > + * virModprobeUnload: > + * @module: Name of the module to unload > + * > + * Remove or unload a module > + * > + * returns NULL in case of success and the error buffer output from the > + * virCommandRun() on failure. The returned buffer must be VIR_FREE() > + * by the caller > + */ > +char * > +virModprobeUnload(const char *module) > +{ > + char *errbuf = NULL; > + > + if (doModprobe("-r", module, NULL, &errbuf) < 0) > + return errbuf; > + > + VIR_FREE(errbuf); > + return NULL; > +} Oh actually this reminds me of a recent firewalld bug. Basically 'modprobe -r' has utterly broken semantics - it will recursively unload any modules that were dependancies of the one you're removing even if things still require them ! eg it'll see the 'bridge' module has refcount of 0 and remove it, even if you have bridges created on the host :-( 'rmmod modname' is what you actually want to use. > +#ifndef __VIR_KMOD_H__ > +# define __VIR_KMOD_H__ > + > +# include "internal.h" > +# include "viralloc.h" Put the viralloc.h in the .c file, since the header doesn't do need any memory allocation APIs. > + > +char *virModprobeConfig(void); > +char *virModprobeLoad(const char *) > + ATTRIBUTE_NONNULL(1); > +char *virModprobeUseBlacklist(const char *); > + ATTRIBUTE_NONNULL(1) > +char *virModprobeUnload(const char *) > + ATTRIBUTE_NONNULL(1); > +#endif /* __VIR_KMOD_H__ */ Nitpick on naming - general goal is to have function names match the filename. eg so we should use virKModLoad / virKModUnload 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 :| -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list