[libvirt] [PATCH 0/3] quietly ignore missing pm-is-supported
On systems with no logind or pm-is-supported binary, stop spamming the logs about a missing executable. Ján Tomko (3): util: use VIR_AUTOPTR virNodeSuspendSupportsTargetPMUtils util: do not repeat the pm-is-supported string util: be quiet when pm-is-supported is unavailable src/util/virnodesuspend.c | 25 +++++++++++++------------ 1 file changed, 13 insertions(+), 12 deletions(-) -- 2.19.2
Get rid of the ret variable as well as the cleanup label. Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/util/virnodesuspend.c | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/src/util/virnodesuspend.c b/src/util/virnodesuspend.c index 3fc5b93ac7..af0ed615e1 100644 --- a/src/util/virnodesuspend.c +++ b/src/util/virnodesuspend.c @@ -238,9 +238,8 @@ int virNodeSuspend(unsigned int target, static int virNodeSuspendSupportsTargetPMUtils(unsigned int target, bool *supported) { - virCommandPtr cmd; + VIR_AUTOPTR(virCommand) cmd = NULL; int status; - int ret = -1; *supported = false; @@ -259,18 +258,14 @@ virNodeSuspendSupportsTargetPMUtils(unsigned int target, bool *supported) } if (virCommandRun(cmd, &status) < 0) - goto cleanup; + return -1; /* * Check return code of command == 0 for success * (i.e., the PM capability is supported) */ *supported = (status == 0); - ret = 0; - - cleanup: - virCommandFree(cmd); - return ret; + return 0; } #else /* ! WITH_PM_UTILS */ static int -- 2.19.2
On Tue, 2019-08-13 at 17:24 +0200, Ján Tomko wrote:
+++ b/src/util/virnodesuspend.c @@ -238,9 +238,8 @@ int virNodeSuspend(unsigned int target, /* * Check return code of command == 0 for success * (i.e., the PM capability is supported) */ *supported = (status == 0); - ret = 0; - - cleanup: - virCommandFree(cmd); - return ret; + return 0;
Please leave an empty line before 'return'. Also this doesn't build: util/virnodesuspend.c: In function 'virNodeSuspendSupportsTargetPMUtils': util/virnodesuspend.c:257:16: error: 'ret' undeclared (first use in this function) 257 | return ret; | ^~~ With the obvious fix (s/ret/-1/) applied and the empty line added, Reviewed-by: Andrea Bolognani <abologna@redhat.com> -- Andrea Bolognani / Red Hat / Virtualization
Use a 'binary' variable to hold it. Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/util/virnodesuspend.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/util/virnodesuspend.c b/src/util/virnodesuspend.c index af0ed615e1..62e62bb171 100644 --- a/src/util/virnodesuspend.c +++ b/src/util/virnodesuspend.c @@ -239,22 +239,23 @@ static int virNodeSuspendSupportsTargetPMUtils(unsigned int target, bool *supported) { VIR_AUTOPTR(virCommand) cmd = NULL; + const char *binary = "pm-is-supported"; int status; *supported = false; switch (target) { case VIR_NODE_SUSPEND_TARGET_MEM: - cmd = virCommandNewArgList("pm-is-supported", "--suspend", NULL); + cmd = virCommandNewArgList(binary, "--suspend", NULL); break; case VIR_NODE_SUSPEND_TARGET_DISK: - cmd = virCommandNewArgList("pm-is-supported", "--hibernate", NULL); + cmd = virCommandNewArgList(binary, "--hibernate", NULL); break; case VIR_NODE_SUSPEND_TARGET_HYBRID: - cmd = virCommandNewArgList("pm-is-supported", "--suspend-hybrid", NULL); + cmd = virCommandNewArgList(binary, "--suspend-hybrid", NULL); break; default: - return ret; + return -1; } if (virCommandRun(cmd, &status) < 0) -- 2.19.2
On Tue, 2019-08-13 at 17:24 +0200, Ján Tomko wrote:
+++ b/src/util/virnodesuspend.c case VIR_NODE_SUSPEND_TARGET_HYBRID: - cmd = virCommandNewArgList("pm-is-supported", "--suspend-hybrid", NULL); + cmd = virCommandNewArgList(binary, "--suspend-hybrid", NULL); break; default: - return ret; + return -1; }
Yeah, this last hunk should have been in the first patch O:-) Reviewed-by: Andrea Bolognani <abologna@redhat.com> -- Andrea Bolognani / Red Hat / Virtualization
On Wed, Aug 14, 2019 at 11:05:39AM +0200, Andrea Bolognani wrote:
On Tue, 2019-08-13 at 17:24 +0200, Ján Tomko wrote:
+++ b/src/util/virnodesuspend.c case VIR_NODE_SUSPEND_TARGET_HYBRID: - cmd = virCommandNewArgList("pm-is-supported", "--suspend-hybrid", NULL); + cmd = virCommandNewArgList(binary, "--suspend-hybrid", NULL); break; default: - return ret; + return -1; }
Yeah, this last hunk should have been in the first patch O:-)
Oops, --amend vs. rebase --continue bit me again. Jano
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
-- Andrea Bolognani / Red Hat / Virtualization
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Look up the binary name upfront to avoid the error: Cannot find 'pm-is-supported' in path: No such file or directory In that case, we just assume nodesuspend is not available. Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/util/virnodesuspend.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/util/virnodesuspend.c b/src/util/virnodesuspend.c index 62e62bb171..fea2ebcd76 100644 --- a/src/util/virnodesuspend.c +++ b/src/util/virnodesuspend.c @@ -29,6 +29,7 @@ #include "datatypes.h" #include "viralloc.h" +#include "virfile.h" #include "virlog.h" #include "virerror.h" @@ -239,11 +240,15 @@ static int virNodeSuspendSupportsTargetPMUtils(unsigned int target, bool *supported) { VIR_AUTOPTR(virCommand) cmd = NULL; - const char *binary = "pm-is-supported"; + VIR_AUTOFREE(char *) binary = NULL; int status; *supported = false; + binary = virFindFileInPath("pm-is-supported"); + if (!binary) + return -2; + switch (target) { case VIR_NODE_SUSPEND_TARGET_MEM: cmd = virCommandNewArgList(binary, "--suspend", NULL); -- 2.19.2
On Tue, 2019-08-13 at 17:24 +0200, Ján Tomko wrote:
Look up the binary name upfront to avoid the error: Cannot find 'pm-is-supported' in path: No such file or directory
In that case, we just assume nodesuspend is not available.
Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/util/virnodesuspend.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)
Reviewed-by: Andrea Bolognani <abologna@redhat.com> -- Andrea Bolognani / Red Hat / Virtualization
participants (2)
-
Andrea Bolognani -
Ján Tomko