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