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@xxxxxxxxxx +1-919-301-3266 Libvirt virtualization library http://libvirt.org
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list