On 12/28/2012 08:59 AM, Michal Privoznik wrote:
Since sanlock doesn't run under root:root, we have chown()'ed
the
__LIBVIRT__DISKS__ lease file to the user:group defined in the
sanlock config. However, when writing the patch I've forgot about
lease files for each disk (this is the
/var/lib/libvirt/sanlock/<md5>) file.
---
src/locking/lock_driver_sanlock.c | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/src/locking/lock_driver_sanlock.c b/src/locking/lock_driver_sanlock.c
index 75ced84..c955003 100644
--- a/src/locking/lock_driver_sanlock.c
+++ b/src/locking/lock_driver_sanlock.c
@@ -679,6 +679,17 @@ static int virLockManagerSanlockCreateLease(struct sanlk_resource
*res)
}
VIR_DEBUG("Someone else just created lockspace %s",
res->disks[0].path);
} else {
+ /* chown() the path to make sure sanlock can access it */
+ if ((driver->user != -1 || driver->group != -1) &&
I'm late in my review, but this needs to be:
(driver->user != (uid_t) -1 || driver->group != (gid_t) -1)
Why? Because POSIX makes no requirements on whether [ug]id_t are signed
or unsigned, nor whether they are smaller, equal, or wider than int.
Therefore, without the cast, it is possible to hit a platform where the
comparison fails to do what you want, if you don't also have the cast in
place (that is, it is possible to have '(uid_t)-1 != -1' evaluate to
true instead of the intended false, due to integer promotion rules).
Thankfully, it's only theoretical at this point (sanlock has only been
ported to Linux, where uid_t is sane), but we might as well make the fix.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library
http://libvirt.org