On 01/12/2012 12:28 PM, Laine Stump wrote:
This eliminates the warning message reported in:
https://bugzilla.redhat.com/show_bug.cgi?id=624447
It was caused by a failure to open an image file that is not
accessible by root (the uid libvirtd is running as) because it's on a
root-squash NFS share, owned by a different user, with permissions of
660 (or maybe 600).
The solution is to re-try the open using virFileOpenAs() when
appropriate. The codepath that generates the error is during
qemuSetupDiskCGroup(), but the actual open() is in a lower-level
generic function called from many places (virDomainDiskDefForeachPath),
so some other pieces of the code were touched just to add dummy (or
possibly useful) uid and gid arguments.
Eliminating this warning message has the nice side effect that the
requested opearation may even succeed (which in this case isn't
s/opearation/operation/
necessary, but shouldn't hurt anything either).
---
src/conf/domain_conf.c | 13 ++++++++++++-
src/conf/domain_conf.h | 1 +
src/qemu/qemu_cgroup.c | 2 ++
src/security/security_dac.c | 1 +
src/security/security_selinux.c | 7 +++++++
src/security/virt-aa-helper.c | 6 +++++-
6 files changed, 28 insertions(+), 2 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 180dd2b..2b7ec91 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -13370,6 +13370,7 @@ done:
int virDomainDiskDefForeachPath(virDomainDiskDefPtr disk,
bool allowProbing,
bool ignoreOpenFailure,
+ uid_t uid, gid_t gid,
virDomainDiskDefPathIterator iter,
void *opaque)
{
@@ -13425,8 +13426,18 @@ int virDomainDiskDefForeachPath(virDomainDiskDefPtr disk,
disk->src);
goto cleanup;
}
+ fd = open(path, O_RDONLY);
+ if ((fd < 0) && (errno == EACCES || errno == EPERM) &&
+ (uid > 0 || gid > 0)) {
Repeating my complaints from 1/2, it is possible for uid_t to be an
unsigned type (and in fact, this is the case on Linux), which means this
will not filter out the case of (uid_t)-1 on all platforms.
Is your goal to avoid a pointless virFileOpenAs() call? Remember,
virFileOpenAs called with both uid and gid of -1 will end up opening
with the current ids, but that's the same as what open() already does,
so it should fail in the same manner. The only time virFileOpenAs() is
useful is if there is no earlier open() attempt, or if the requested uid
or gid differs from the current process ids.
So I think you might be able to get away with:
if (... &&
(uid != (uid_t)-1 || gid != (gid_t)-1)) {
or just rely on a single virFileOpenAs() call without bothering with an
initial open() attempt.
--
Eric Blake eblake(a)redhat.com +1-919-301-3266
Libvirt virtualization library
http://libvirt.org