[libvirt] [PATCH 0/2] Remove some unnecessary warnings
cd . ./blurb Martin Kletzander (2): Add functions for checking if user or group exists qemu: Report less errors on driver startup src/libvirt_private.syms | 2 ++ src/qemu/qemu_conf.c | 6 ++++-- src/util/virutil.c | 40 ++++++++++++++++++++++++++++++++-------- src/util/virutil.h | 4 ++++ 4 files changed, 42 insertions(+), 10 deletions(-) -- 2.18.0
Instead of duplicating the code from virGet{User,Group}IDByName(), which are static anyway, extend those functions to accept NULL pointers for the result and a boolean for controlling the error reporting. Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- Feel free to suggest any other way of doing this, I just felt like this is the most readable way of doing things src/libvirt_private.syms | 2 ++ src/util/virutil.c | 40 ++++++++++++++++++++++++++++++++-------- src/util/virutil.h | 4 ++++ 3 files changed, 38 insertions(+), 8 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index a0d229a79f0c..e7a4d61f25fd 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -3090,6 +3090,8 @@ virUSBDeviceSetUsedBy; # util/virutil.h +virDoesGroupExist; +virDoesUserExist; virDoubleToStr; virEnumFromString; virEnumToString; diff --git a/src/util/virutil.c b/src/util/virutil.c index a908422feb7f..2290020e494e 100644 --- a/src/util/virutil.c +++ b/src/util/virutil.c @@ -971,9 +971,11 @@ char *virGetGroupName(gid_t gid) /* Search in the password database for a user id that matches the user name * `name`. Returns 0 on success, -1 on failure or 1 if name cannot be found. + * + * Warns if @missing_ok is false */ static int -virGetUserIDByName(const char *name, uid_t *uid) +virGetUserIDByName(const char *name, uid_t *uid, bool missing_ok) { char *strbuf = NULL; struct passwd pwbuf; @@ -996,7 +998,7 @@ virGetUserIDByName(const char *name, uid_t *uid) } if (!pw) { - if (rc != 0) { + if (rc != 0 && !missing_ok) { char buf[1024]; /* log the possible error from getpwnam_r. Unfortunately error * reporting from this function is bad and we can't really @@ -1009,7 +1011,8 @@ virGetUserIDByName(const char *name, uid_t *uid) goto cleanup; } - *uid = pw->pw_uid; + if (uid) + *uid = pw->pw_uid; ret = 0; cleanup: @@ -1032,7 +1035,7 @@ virGetUserID(const char *user, uid_t *uid) if (*user == '+') { user++; } else { - int rc = virGetUserIDByName(user, uid); + int rc = virGetUserIDByName(user, uid, false); if (rc <= 0) return rc; } @@ -1051,9 +1054,11 @@ virGetUserID(const char *user, uid_t *uid) /* Search in the group database for a group id that matches the group name * `name`. Returns 0 on success, -1 on failure or 1 if name cannot be found. + * + * Warns if @missing_ok is false */ static int -virGetGroupIDByName(const char *name, gid_t *gid) +virGetGroupIDByName(const char *name, gid_t *gid, bool missing_ok) { char *strbuf = NULL; struct group grbuf; @@ -1076,7 +1081,7 @@ virGetGroupIDByName(const char *name, gid_t *gid) } if (!gr) { - if (rc != 0) { + if (rc != 0 && !missing_ok) { char buf[1024]; /* log the possible error from getgrnam_r. Unfortunately error * reporting from this function is bad and we can't really @@ -1089,7 +1094,8 @@ virGetGroupIDByName(const char *name, gid_t *gid) goto cleanup; } - *gid = gr->gr_gid; + if (gid) + *gid = gr->gr_gid; ret = 0; cleanup: @@ -1112,7 +1118,7 @@ virGetGroupID(const char *group, gid_t *gid) if (*group == '+') { group++; } else { - int rc = virGetGroupIDByName(group, gid); + int rc = virGetGroupIDByName(group, gid, false); if (rc <= 0) return rc; } @@ -1129,6 +1135,24 @@ virGetGroupID(const char *group, gid_t *gid) return 0; } +/* Silently checks if User @name exists. + * Returns if the user exists and fallbacks to false on error. + */ +int +virDoesUserExist(const char *name, bool *exists) +{ + return virGetUserIDByName(name, NULL, true) == 0; +} + +/* Silently checks if Group @name exists. + * Returns if the group exists and fallbacks to false on error. + */ +int +virDoesGroupExist(const char *name) +{ + return virGetGroupIDByName(name, NULL, true) == 0; +} + /* Compute the list of primary and supplementary groups associated * with @uid, and including @gid in the list (unless it is -1), diff --git a/src/util/virutil.h b/src/util/virutil.h index 1ba9635bd991..8a27cca523c8 100644 --- a/src/util/virutil.h +++ b/src/util/virutil.h @@ -152,6 +152,10 @@ int virGetUserID(const char *name, int virGetGroupID(const char *name, gid_t *gid) ATTRIBUTE_RETURN_CHECK; +int virDoesUserExist(const char *name, bool *exists); +int virDoesGroupExist(const char *name, bool *exists); + + bool virIsDevMapperDevice(const char *dev_name) ATTRIBUTE_NONNULL(1); bool virValidateWWN(const char *wwn); -- 2.18.0
On Wed, Sep 12, 2018 at 05:09:12PM +0200, Martin Kletzander wrote:
Instead of duplicating the code from virGet{User,Group}IDByName(), which are static anyway, extend those functions to accept NULL pointers for the result and a boolean for controlling the error reporting.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- Feel free to suggest any other way of doing this, I just felt like this is the most readable way of doing things
src/libvirt_private.syms | 2 ++ src/util/virutil.c | 40 ++++++++++++++++++++++++++++++++-------- src/util/virutil.h | 4 ++++ 3 files changed, 38 insertions(+), 8 deletions(-)
+/* Silently checks if Group @name exists. + * Returns if the group exists and fallbacks to false on error. + */ +int +virDoesGroupExist(const char *name)
util/virutil.c:1142:42: error: unused parameter 'exists' [-Werror,-Wunused-parameter] virDoesUserExist(const char *name, bool *exists) ^ util/virutil.c:1151:1: error: conflicting types for 'virDoesGroupExist' virDoesGroupExist(const char *name)
+{ + return virGetGroupIDByName(name, NULL, true) == 0; +} +
/* Compute the list of primary and supplementary groups associated * with @uid, and including @gid in the list (unless it is -1),
It is not a problem at all if the `tss` user/group does not exist, the code fallbacks to the `root` user/group. However we report a warning for no reason on every start-up. Fix this by checking if the user/group actually exists. Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- src/qemu/qemu_conf.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index a4f545ef9243..4d69b599fed8 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -197,9 +197,11 @@ virQEMUDriverConfigPtr virQEMUDriverConfigNew(bool privileged) if (virAsprintf(&cfg->swtpmStorageDir, "%s/lib/libvirt/swtpm", LOCALSTATEDIR) < 0) goto error; - if (virGetUserID("tss", &cfg->swtpm_user) < 0) + if (virDoesUserExist("tss") != 0 || + virGetUserID("tss", &cfg->swtpm_user) < 0) cfg->swtpm_user = 0; /* fall back to root */ - if (virGetGroupID("tss", &cfg->swtpm_group) < 0) + if (virDoesGroupExist("tss") != 0 || + virGetGroupID("tss", &cfg->swtpm_group) < 0) cfg->swtpm_group = 0; /* fall back to root */ } else { char *rundir; -- 2.18.0
On 09/12/2018 05:09 PM, Martin Kletzander wrote:
cd . ./blurb
Martin Kletzander (2): Add functions for checking if user or group exists qemu: Report less errors on driver startup
src/libvirt_private.syms | 2 ++ src/qemu/qemu_conf.c | 6 ++++-- src/util/virutil.c | 40 ++++++++++++++++++++++++++++++++-------- src/util/virutil.h | 4 ++++ 4 files changed, 42 insertions(+), 10 deletions(-)
ACK series. --help I mean Michal
On Wed, Sep 12, 2018 at 05:09:11PM +0200, Martin Kletzander wrote:
cd . ./blurb
Martin Kletzander (2): Add functions for checking if user or group exists qemu: Report less errors on driver startup
src/libvirt_private.syms | 2 ++ src/qemu/qemu_conf.c | 6 ++++-- src/util/virutil.c | 40 ++++++++++++++++++++++++++++++++-------- src/util/virutil.h | 4 ++++ 4 files changed, 42 insertions(+), 10 deletions(-)
Thanks-by: Ján Tomko <jtomko@redhat.com> Jano
participants (3)
-
Ján Tomko -
Martin Kletzander -
Michal Privoznik