[libvirt PATCH 0/2] util: simplify virKMod code
Daniel P. Berrangé (2): src: remove redundant arg to virKModLoad util: remove unused virKModConfig method src/libvirt_private.syms | 1 - src/util/virfile.c | 2 +- src/util/virkmod.c | 27 ++------------- src/util/virkmod.h | 3 +- src/util/virpci.c | 2 +- tests/virkmodtest.c | 71 ++++++---------------------------------- 6 files changed, 16 insertions(+), 90 deletions(-) -- 2.24.1
All callers except for the test suite pass the same value for the second arg, so it can be removed, simplifying the code. Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- src/util/virfile.c | 2 +- src/util/virkmod.c | 8 +++----- src/util/virkmod.h | 2 +- src/util/virpci.c | 2 +- tests/virkmodtest.c | 44 +++++++++++--------------------------------- 5 files changed, 17 insertions(+), 41 deletions(-) diff --git a/src/util/virfile.c b/src/util/virfile.c index 20260a2e58..7b92c8767c 100644 --- a/src/util/virfile.c +++ b/src/util/virfile.c @@ -905,7 +905,7 @@ virFileNBDLoadDriver(void) } else { g_autofree char *errbuf = NULL; - if ((errbuf = virKModLoad(NBD_DRIVER, true))) { + if ((errbuf = virKModLoad(NBD_DRIVER))) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Failed to load nbd module")); return false; diff --git a/src/util/virkmod.c b/src/util/virkmod.c index 12cb5920e8..59cec69816 100644 --- a/src/util/virkmod.c +++ b/src/util/virkmod.c @@ -31,8 +31,7 @@ doModprobe(const char *opts, const char *module, char **outbuf, char **errbuf) g_autoptr(virCommand) cmd = NULL; cmd = virCommandNew(MODPROBE); - if (opts) - virCommandAddArg(cmd, opts); + virCommandAddArg(cmd, opts); if (module) virCommandAddArg(cmd, module); if (outbuf) @@ -83,7 +82,6 @@ virKModConfig(void) /** * virKModLoad: * @module: Name of the module to load - * @useBlacklist: True if honoring blacklist * * Attempts to load a kernel module * @@ -92,11 +90,11 @@ virKModConfig(void) * by the caller */ char * -virKModLoad(const char *module, bool useBlacklist) +virKModLoad(const char *module) { char *errbuf = NULL; - if (doModprobe(useBlacklist ? "-b" : NULL, module, NULL, &errbuf) < 0) + if (doModprobe("-b", module, NULL, &errbuf) < 0) return errbuf; VIR_FREE(errbuf); diff --git a/src/util/virkmod.h b/src/util/virkmod.h index f05cf83cf6..605da74416 100644 --- a/src/util/virkmod.h +++ b/src/util/virkmod.h @@ -24,7 +24,7 @@ #include "internal.h" char *virKModConfig(void); -char *virKModLoad(const char *, bool) +char *virKModLoad(const char *) ATTRIBUTE_NONNULL(1); char *virKModUnload(const char *) ATTRIBUTE_NONNULL(1); diff --git a/src/util/virpci.c b/src/util/virpci.c index 786b1393e6..24de69c75e 100644 --- a/src/util/virpci.c +++ b/src/util/virpci.c @@ -1013,7 +1013,7 @@ virPCIProbeStubDriver(virPCIStubDriver driver) if (!probed) { g_autofree char *errbuf = NULL; probed = true; - if ((errbuf = virKModLoad(drvname, true))) { + if ((errbuf = virKModLoad(drvname))) { VIR_WARN("failed to load driver %s: %s", drvname, errbuf); goto cleanup; } diff --git a/tests/virkmodtest.c b/tests/virkmodtest.c index e013f8f3b6..2906ad992e 100644 --- a/tests/virkmodtest.c +++ b/tests/virkmodtest.c @@ -27,11 +27,7 @@ # include "virkmod.h" # include "virstring.h" -struct testInfo { - const char *module; - const char *exp_cmd; - bool useBlacklist; -}; +# define MODNAME "vfio-pci" # define VIR_FROM_THIS VIR_FROM_NONE @@ -87,24 +83,21 @@ checkOutput(virBufferPtr buf, const char *exp_cmd) static int -testKModLoad(const void *args) +testKModLoad(const void *args G_GNUC_UNUSED) { int ret = -1; char *errbuf = NULL; - const struct testInfo *info = args; - const char *module = info->module; - bool useBlacklist = info->useBlacklist; virBuffer buf = VIR_BUFFER_INITIALIZER; virCommandSetDryRun(&buf, NULL, NULL); - errbuf = virKModLoad(module, useBlacklist); + errbuf = virKModLoad(MODNAME); if (errbuf) { fprintf(stderr, "Failed to load, error: %s\n", errbuf); goto cleanup; } - if (checkOutput(&buf, info->exp_cmd) < 0) + if (checkOutput(&buf, MODPROBE " -b " MODNAME "\n") < 0) goto cleanup; ret = 0; @@ -117,23 +110,21 @@ testKModLoad(const void *args) static int -testKModUnload(const void *args) +testKModUnload(const void *args G_GNUC_UNUSED) { int ret = -1; char *errbuf = NULL; - const struct testInfo *info = args; - const char *module = info->module; virBuffer buf = VIR_BUFFER_INITIALIZER; virCommandSetDryRun(&buf, NULL, NULL); - errbuf = virKModUnload(module); + errbuf = virKModUnload(MODNAME); if (errbuf) { fprintf(stderr, "Failed to unload, error: %s\n", errbuf); goto cleanup; } - if (checkOutput(&buf, info->exp_cmd) < 0) + if (checkOutput(&buf, RMMOD " " MODNAME "\n") < 0) goto cleanup; ret = 0; @@ -152,23 +143,10 @@ mymain(void) if (virTestRun("config", testKModConfig, NULL) < 0) ret = -1; - - /* Although we cannot run the command on the host, we can compare - * the output of the created command against what we'd expect to be - * created. So let's at least do that. - */ -# define DO_TEST(_name, _cb, _blkflag, _exp_cmd) \ - do { \ - struct testInfo data = {.module = "vfio-pci", \ - .exp_cmd = _exp_cmd, \ - .useBlacklist = _blkflag}; \ - if (virTestRun(_name, _cb, &data) < 0) \ - ret = -1; \ - } while (0) - - DO_TEST("load", testKModLoad, false, MODPROBE " vfio-pci\n"); - DO_TEST("unload", testKModUnload, false, RMMOD " vfio-pci\n"); - DO_TEST("blklist", testKModLoad, true, MODPROBE " -b vfio-pci\n"); + if (virTestRun("load", testKModLoad, NULL) < 0) + ret = -1; + if (virTestRun("unload", testKModUnload, NULL) < 0) + ret = -1; return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE; -- 2.24.1
On 6/18/20 5:42 PM, Daniel P. Berrangé wrote:
All callers except for the test suite pass the same value for the second arg, so it can be removed, simplifying the code.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- src/util/virfile.c | 2 +- src/util/virkmod.c | 8 +++----- src/util/virkmod.h | 2 +- src/util/virpci.c | 2 +- tests/virkmodtest.c | 44 +++++++++++--------------------------------- 5 files changed, 17 insertions(+), 41 deletions(-)
diff --git a/src/util/virfile.c b/src/util/virfile.c index 20260a2e58..7b92c8767c 100644 --- a/src/util/virfile.c +++ b/src/util/virfile.c @@ -905,7 +905,7 @@ virFileNBDLoadDriver(void) } else { g_autofree char *errbuf = NULL;
- if ((errbuf = virKModLoad(NBD_DRIVER, true))) { + if ((errbuf = virKModLoad(NBD_DRIVER))) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Failed to load nbd module")); return false; diff --git a/src/util/virkmod.c b/src/util/virkmod.c index 12cb5920e8..59cec69816 100644 --- a/src/util/virkmod.c +++ b/src/util/virkmod.c @@ -31,8 +31,7 @@ doModprobe(const char *opts, const char *module, char **outbuf, char **errbuf) g_autoptr(virCommand) cmd = NULL;
cmd = virCommandNew(MODPROBE); - if (opts) - virCommandAddArg(cmd, opts); + virCommandAddArg(cmd, opts);
Or: - cmd = virCommandNew(MODPROBE); - virCommandAddArg(cmd, opts); + cmd = virCommandNewArgList(MODPROBE, opts, NULL); Michal
Using virKModConfig would not simplify any existing code. Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- src/libvirt_private.syms | 1 - src/util/virkmod.c | 19 ------------------- src/util/virkmod.h | 1 - tests/virkmodtest.c | 29 ----------------------------- 4 files changed, 50 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 7509916dfd..691b5bc5bc 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2402,7 +2402,6 @@ virKeycodeValueTranslate; # util/virkmod.h -virKModConfig; virKModIsBlacklisted; virKModLoad; virKModUnload; diff --git a/src/util/virkmod.c b/src/util/virkmod.c index 59cec69816..4400245906 100644 --- a/src/util/virkmod.c +++ b/src/util/virkmod.c @@ -59,25 +59,6 @@ doRmmod(const char *module, char **errbuf) return 0; } -/** - * virKModConfig: - * - * 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 * -virKModConfig(void) -{ - char *outbuf = NULL; - - if (doModprobe("-c", NULL, &outbuf, NULL) < 0) - return NULL; - - return outbuf; -} - /** * virKModLoad: diff --git a/src/util/virkmod.h b/src/util/virkmod.h index 605da74416..3bb161df52 100644 --- a/src/util/virkmod.h +++ b/src/util/virkmod.h @@ -23,7 +23,6 @@ #include "internal.h" -char *virKModConfig(void); char *virKModLoad(const char *) ATTRIBUTE_NONNULL(1); char *virKModUnload(const char *) diff --git a/tests/virkmodtest.c b/tests/virkmodtest.c index 2906ad992e..3ba8c542d1 100644 --- a/tests/virkmodtest.c +++ b/tests/virkmodtest.c @@ -31,33 +31,6 @@ # define VIR_FROM_THIS VIR_FROM_NONE -static int -testKModConfig(const void *args G_GNUC_UNUSED) -{ - int ret = -1; - char *outbuf = NULL; - - /* This will return the contents of a 'modprobe -c' which can differ - * from machine to machine - be happy that we get something. - */ - outbuf = virKModConfig(); - if (!outbuf) { - if (virFileIsExecutable(MODPROBE)) { - fprintf(stderr, "Failed to get config\n"); - } else { - /* modprobe doesn't exist, do not claim error. */ - ret = 0; - } - goto cleanup; - } - ret = 0; - - cleanup: - VIR_FREE(outbuf); - return ret; -} - - static int checkOutput(virBufferPtr buf, const char *exp_cmd) { @@ -141,8 +114,6 @@ mymain(void) { int ret = 0; - if (virTestRun("config", testKModConfig, NULL) < 0) - ret = -1; if (virTestRun("load", testKModLoad, NULL) < 0) ret = -1; if (virTestRun("unload", testKModUnload, NULL) < 0) -- 2.24.1
On 6/18/20 5:42 PM, Daniel P. Berrangé wrote:
Daniel P. Berrangé (2): src: remove redundant arg to virKModLoad util: remove unused virKModConfig method
src/libvirt_private.syms | 1 - src/util/virfile.c | 2 +- src/util/virkmod.c | 27 ++------------- src/util/virkmod.h | 3 +- src/util/virpci.c | 2 +- tests/virkmodtest.c | 71 ++++++---------------------------------- 6 files changed, 16 insertions(+), 90 deletions(-)
Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Michal
participants (2)
-
Daniel P. Berrangé -
Michal Privoznik