Re: Cyrus IMAPd 2.3.10 Released
Hi, On Sat, 27 Oct 2007 22:36:45 +0200 Tomas Janousek [EMAIL PROTECTED] said: tjanouse Hi, tjanouse On Sun, Oct 28, 2007 at 02:35:05AM +0900, Hajimu UMEMOTO wrote: tjanouse Yes. It should read ret == -1 ngroups != newstate-ngroups. I'm really tjanouse confused why I put the ret != -1 in there. As far as I read the FreeBSD's getgrouplist() implementation, when it returns -1, the number of the groups actually filled is set to ngroups. It is match with the following description in the manpage: RETURN VALUES The getgrouplist() function returns -1 if the size of the group list is too small to hold all the user's groups. Here, the group array will be filled with as many groups as will fit. tjanouse The manpage says that the actual number of groups found is returned in tjanouse ngroups. My understanding may be bad and/or they may have implemented tjanouse something else than they say in the manpage/than what I understand, though. tjanouse But I think this part of behaviour is really the same. If you really think tjanouse this is not what happens, I will check the sources. It seems to me from the source of getgrouplist() that it sets the actual number of groups found to ngroups only when it returns 0. When it returns -1, the number of groups actually filled is set to ngroups. I think that it is what the RETURN VALUES section in FreeBSD's manpage says. So, I think that ret == -1 ngroups != newstate-ngroups would be always FALSE at least on FreeBSD. Sincerely, -- Hajimu UMEMOTO @ Internet Mutual Aid Society Yokohama, Japan [EMAIL PROTECTED] [EMAIL PROTECTED],jp.}FreeBSD.org http://www.imasy.org/~ume/ Cyrus Home Page: http://cyrusimap.web.cmu.edu/ Cyrus Wiki/FAQ: http://cyrusimap.web.cmu.edu/twiki List Archives/Info: http://asg.web.cmu.edu/cyrus/mailing-list.html
Re: Cyrus IMAPd 2.3.10 Released
Tomas Janousek wrote: Hi, On Sun, Oct 28, 2007 at 03:52:24PM +0900, Hajimu UMEMOTO wrote: It seems to me from the source of getgrouplist() that it sets the actual number of groups found to ngroups only when it returns 0. When it returns -1, the number of groups actually filled is set to ngroups. I think that it is what the RETURN VALUES section in FreeBSD's manpage says. Ok, I looked at the source and now I see it. And I think the source is really wrong (e.g. the write to groups without checking ngroups is non-zero -- this was fixed in glibc a looong time ago). So, I think that ret == -1 ngroups != newstate-ngroups would be always FALSE at least on FreeBSD. Yeah. To make it work on BSD, we should either preallocate big enough an array and don't care, or realloc to two-times the size whenever ngroups == newstate-ngroups (and to given size when ngroups newstate-ngroups, to make it faster on glibc). Thanks for catching this :) I don't have easy access to a BSD platform. Would somebody be willing to write and test such a patch? -- Kenneth Murchison Systems Programmer Project Cyrus Developer/Maintainer Carnegie Mellon University Cyrus Home Page: http://cyrusimap.web.cmu.edu/ Cyrus Wiki/FAQ: http://cyrusimap.web.cmu.edu/twiki List Archives/Info: http://asg.web.cmu.edu/cyrus/mailing-list.html
Re: Cyrus IMAPd 2.3.10 Released
Hi,
On Sun, 28 Oct 2007 09:57:45 -0400
Ken Murchison [EMAIL PROTECTED] said:
murch I don't have easy access to a BSD platform. Would somebody be willing
murch to write and test such a patch?
How about this patch?
Index: lib/auth_unix.c
diff -u -p lib/auth_unix.c.orig lib/auth_unix.c
--- lib/auth_unix.c.orig2007-09-28 05:02:45.0 +0900
+++ lib/auth_unix.c 2007-10-29 03:14:22.0 +0900
@@ -45,6 +45,9 @@
*/
#include config.h
+#ifdef HAVE_SYS_PARAM_H
+#include sys/param.h
+#endif
#include stdlib.h
#include pwd.h
#include grp.h
@@ -225,7 +228,12 @@ static struct auth_state *mynewstate(con
struct group *grp;
#ifdef HAVE_GETGROUPLIST
gid_t gid, *groupids = NULL;
-int ret, ngroups = 0;
+int ret = -1;
+#ifdef __GLIBC__
+int ngroups = 0;
+#else
+int ngroups = 5;
+#endif
#else
char **mem;
#endif
@@ -248,22 +256,33 @@ static struct auth_state *mynewstate(con
#ifdef HAVE_GETGROUPLIST
gid = pwd ? pwd-pw_gid : (gid_t) -1;
+#ifdef __GLIBC__
/* get number of groups user is member of into ngroups */
getgrouplist(identifier, gid, NULL, ngroups);
+#endif
/* get the actual group ids */
-do {
+while (ngroups = NGROUPS) {
groupids = (gid_t *)xrealloc((gid_t *)groupids,
ngroups * sizeof(gid_t));
newstate-ngroups = ngroups; /* copy of ngroups for comparision */
ret = getgrouplist(identifier, gid, groupids, ngroups);
+ if (ret != -1)
+ break;
+#ifdef __GLIBC__
/*
* This is tricky. We do this as long as getgrouplist tells us to
* realloc _and_ the number of groups changes. It tells us to realloc
* also in the case of failure...
*/
-} while (ret != -1 ngroups != newstate-ngroups);
+ if (ngroups == newstate-ngroups)
+ break;
+#else
+ if ((ngroups = newstate-ngroups * 2) NGROUPS)
+ ngroups = NGROUPS;
+#endif
+}
if (ret == -1) {
newstate-ngroups = 0;
Sincerely,
--
Hajimu UMEMOTO @ Internet Mutual Aid Society Yokohama, Japan
[EMAIL PROTECTED] [EMAIL PROTECTED],jp.}FreeBSD.org
http://www.imasy.org/~ume/
Cyrus Home Page: http://cyrusimap.web.cmu.edu/
Cyrus Wiki/FAQ: http://cyrusimap.web.cmu.edu/twiki
List Archives/Info: http://asg.web.cmu.edu/cyrus/mailing-list.html
Re: Cyrus IMAPd 2.3.10 Released
Hi,
On Sun, 28 Oct 2007 19:42:02 +0100
Tomas Janousek [EMAIL PROTECTED] said:
tjanouse Looks correct. (will not terminate if it reaches NGROUPS, don't know
if that
tjanouse can happen though)
Oops, it never happen.
It is intended to be safe-keeping for avoiding endless-loop.
tjanouse But as I said earlier, it's probably better to use the old code on
non-glibc
tjanouse systems.
I'm not sure which one is better.
Index: lib/auth_unix.c
diff -u -p lib/auth_unix.c.orig lib/auth_unix.c
--- lib/auth_unix.c.orig2007-09-28 05:02:45.0 +0900
+++ lib/auth_unix.c 2007-10-29 04:23:57.0 +0900
@@ -45,6 +45,9 @@
*/
#include config.h
+#ifdef HAVE_SYS_PARAM_H
+#include sys/param.h
+#endif
#include stdlib.h
#include pwd.h
#include grp.h
@@ -225,7 +228,12 @@ static struct auth_state *mynewstate(con
struct group *grp;
#ifdef HAVE_GETGROUPLIST
gid_t gid, *groupids = NULL;
-int ret, ngroups = 0;
+int ret = -1;
+#ifdef __GLIBC__
+int ngroups = 0;
+#else
+int ngroups = 5;
+#endif
#else
char **mem;
#endif
@@ -248,22 +256,35 @@ static struct auth_state *mynewstate(con
#ifdef HAVE_GETGROUPLIST
gid = pwd ? pwd-pw_gid : (gid_t) -1;
+#ifdef __GLIBC__
/* get number of groups user is member of into ngroups */
getgrouplist(identifier, gid, NULL, ngroups);
+#endif
/* get the actual group ids */
-do {
+for (;;) {
groupids = (gid_t *)xrealloc((gid_t *)groupids,
ngroups * sizeof(gid_t));
newstate-ngroups = ngroups; /* copy of ngroups for comparision */
ret = getgrouplist(identifier, gid, groupids, ngroups);
+ if (ret != -1)
+ break;
+#ifdef __GLIBC__
/*
* This is tricky. We do this as long as getgrouplist tells us to
* realloc _and_ the number of groups changes. It tells us to realloc
* also in the case of failure...
*/
-} while (ret != -1 ngroups != newstate-ngroups);
+ if (ngroups == newstate-ngroups)
+ break;
+#else
+ if (newstate-ngroups = NGROUPS)
+ break;
+ if ((ngroups = newstate-ngroups * 2) NGROUPS)
+ ngroups = NGROUPS;
+#endif
+}
if (ret == -1) {
newstate-ngroups = 0;
Sincerely,
--
Hajimu UMEMOTO @ Internet Mutual Aid Society Yokohama, Japan
[EMAIL PROTECTED] [EMAIL PROTECTED],jp.}FreeBSD.org
http://www.imasy.org/~ume/
Cyrus Home Page: http://cyrusimap.web.cmu.edu/
Cyrus Wiki/FAQ: http://cyrusimap.web.cmu.edu/twiki
List Archives/Info: http://asg.web.cmu.edu/cyrus/mailing-list.html
FastMail.FM GUID upgrade process
Ok - we're doing our GUID upgrades across the board now. Here's the process we're using: a) wrote a tool that tied a DB_File called cyrus.sha1s in each meta directory on the replicas, parsed the index files looking for records with nothing but zeros in the last 16 characters of the GUID and calculated the sha1 on each of them. This took about 5 days to finish running, but makes the next part a lot quicker! b) wrote a daemon which runs on each host and allows the following 4 commands: *) LOCK user.name.URI%20Escaped.MailboxName - lock cyrus.header, cyrus.index, cyrus.expunge in that order using fcntl (our cyrus is build with it). Also if cyrus.sha1s is missing, attempt to fetch it from the replica (but it's OK if this fails, just means all old GUIDs will cause a re-calculation) *) UPGRADE - parse each of cyrus.index and cyrus.expunge. If any old-style GUIDs are found, then looks first in cyrus.sha1s and finally just re-calculates from the underlying message files. - if any index records need new GUIDs or the old index has not yet been upgraded to version 10, stream the index file thorugh a Cyrus::IndexFile-stream_copy, altering the necessary GUIDs and forcing the output format to version 10 (this module can also be used to downgrade if we ever need to!) - leave the new file in cyrus.$item.NEW, but mark internally that the file has been upgraded. *) ROLLBACK - if any file has been upgraded, unlink() the .NEW file. - unlock expunge, index, header (in that order) *) COMMIT - if any file has been upgraded, rename() the .NEW to the base filename. - unlock expunge, index, header (in that order) c) wrote a controller script which reads the mailbox listing from the master and opens connections to both the master and replica slotd, sending the following commands: 1) master LOCK mailbox (or die) 2) replica LOCK mailbox (or master ROLLBACK; die) 3) master UPGRADE (or replica ROLLBACK; master ROLLBACK; die) 4) replica UPGRADE (or replica ROLLBACK; master ROLLBACK; die) 5) replica COMMIT (or replica ROLLBACK; master ROLLBACK; die) 6) master COMMIT (or master ROLLBACK; die NOISILY!!!) The only danger point is (6), where you could wind up with an upgraded replica without the associated upgraded master. You can go ahead and fix them by hand though, assuming you read the NOISILY bit. d) I think the slashdot crowd would put Profit!!! here. With only a short lock time on each index (most sha1s precalculated) and no need to multi-rewrite any index file, this will run much faster than the alternatives. I guess I should go clean up the cyrus.sha1s files once it's all finished. Bron. Cyrus Home Page: http://cyrusimap.web.cmu.edu/ Cyrus Wiki/FAQ: http://cyrusimap.web.cmu.edu/twiki List Archives/Info: http://asg.web.cmu.edu/cyrus/mailing-list.html
