Earlier, I wrote:
> What I'm wondering is, is this an appropriate place to submit that
> patch? Are the Qpopper developpers known to use this mailing list?
> Are other people interested in my patch?
Joe Maimon replied:
> Yes
Ok, so here goes ...
Appended below my signature is what actually amounts to two separate
patches against Qpopper-4.0.6b3: The first to reduce the amount of mail
spool copying that takes place in "normal" (non-server) mode, and the
second to have the POP server update the system "lastlog" file when the
user is authenticated.
For the first patch, pop_dropcopy.c and pop_updt.c are changed as
follows:
In pop_dropcopy.c, link the mail spool to the temp spool, acquire a
lock on the mail spool, unlink and recreate the mail spool, and release
the lock. If the link fails, (either because the temp spool is on a
different file system than the mail spool, or because the temp spool
already exists), fall back to the original code that copies the mail
spool into the temp spool.
In pop_updt.c, if no updates are pending (no messages were read,
requiring Status: or X-UIDL: header updates, and no no messages were
marked to be deleted), after the lock has been acquired on the mail
spool, and any new messages have been appended to the temp spool,
rename the temp spool to mail spool (overwriting the existing mail
spool whose contents have already been copied), rather than copying
the contents back.
This significantly reduces I/O on the system, particular in the case
where users who leave their mail on the server check frequently for
new mail. In our case, it has made the difference between a system
that had been otherwise rendered unusable (user's do use this system
for other purposes than POP, including an IMAP based webmail interface
and extensive filtering with Procmail in at least some cases), and a
system whose response has returned to what we consider normal for the
capabilities of this hardware.
The second patch permits us to use the system lastlog file to monitor
which accounts are used regularly, or more to the point, which ones
haven't been used in any way (interactively, POP, IMAP, FTP, etc.; I've
made the same mod to IMAP and FTP servers) in some extended period of
time. I added a function to pop_pass.c to update the lastlog file, then
added a call to that after the user authentication succeeds.
Note that some of the line numbers affected in pop_pass.c will be
different than a stock source tree, as I've removed from the diff some
site-specific changes we also make to that file.
We've been using the lastlog patch with various versions of the Qpopper
server for years without any trouble. The reduced I/O patch has been in
place for nearly two weeks, (though it did undergo some development
during the first week or so), on a system with approximately 20,000
users, (though only 1656 of them have used the POP service in the past
two weeks). Of those users that do use the POP service, enough of them
have their client software configured to check mail frequently enough
that it was rendering a 1 GHz dual Alpha system (with 10GB RAM, but
memory was not a problem at all) completely unusable. I upgraded from
Qpopper-3.0.2 to Qpopper-4.0.6b3, then added this reduced I/O
functionality to the non-server mode in Qpopper-4.0.6b3. The difference
in performance has been quite remarkable.
I hope others find this useful, and certainly if anyone finds errors or
other reasons to be concerned they'll bring it up ...
--
----------------------------------------------------------------------
Sylvain Robitaille [EMAIL PROTECTED]
Systems analyst / Postmaster Concordia University
Instructional & Information Technology Montreal, Quebec, Canada
----------------------------------------------------------------------
--- popper/pop_dropcopy.c.original 2004-12-14 21:36:18.000000000 -0500
+++ popper/pop_dropcopy.c 2005-03-18 15:39:59.000000000 -0500
@@ -1485,7 +1485,95 @@
(long unsigned) geteuid(),
(long unsigned) getegid() );
- dfd = open ( p->temp_drop, O_RDWR | O_CREAT, 0660 );
+ /*
+ * 2005/03/10 Sylvain Robitaille: We want the temporary drop to be
+ * opened with the same permission as the real mail spool. Start
+ * by gathering that information.
+ */
+ if ( stat(p->drop_name, &mybuf) == -1 ) {
+ pop_log ( p, POP_PRIORITY, HERE,
+ "[SYS/TEMP] Unable to stat maildrop "
+ "'%s': %s (%d)",
+ p->drop_name, STRERROR(errno), errno );
+ return pop_msg ( p, POP_FAILURE, HERE,
+ "[SYS/TEMP] Failed to stat %s with uid %lu, "
+ "gid %lu. Change permissions.",
+ p->drop_name,
+ (long unsigned) pwp->pw_uid,
+ (long unsigned) pwp->pw_gid );
+ }
+ /*
+ * 2005/03/11 Sylvain Robitaille: Ideally, we can simply link the
+ * temp drop to the mail spool, unlink the mail spool,
+ * then recreate the mail spool, with suitable ownership
+ * and permission. Note that the link() system call
+ * will fail if the target already exists.
+ */
+ int CU_Drop_Linked = 0;
+ if ( link(p->drop_name, p->temp_drop) == -1 ) {
+ pop_log ( p, POP_PRIORITY, HERE,
+ "[SYS/TEMP] Link to temporary maildrop "
+ "'%s' failed: %s (%d)",
+ p->temp_drop, STRERROR(errno), errno );
+ } else {
+ CU_Drop_Linked++;
+ DEBUG_LOG1 ( p, "linked temp drop %s", p->temp_drop );
+ /*
+ * Make sure we lock the mail spool:
+ */
+ DEBUG_LOG0 ( p, "Getting mail lock" );
+ rslt = Qmaillock ( p->drop_name,
+ 2,
+ p->bNo_atomic_open,
+ p->pCfg_spool_dir,
+ p->trace,
+ HERE,
+ p->debug );
+ if ( rslt != 0 ) {
+ /*
+ * Can't get a lock, unlink the temp spool and return
+ */
+ unlink ( p->temp_drop );
+ return pop_msg ( p, POP_FAILURE, HERE,
+ "[SYS/TEMP] maillock error '%s' (%d) on '%s': %s
(%d)",
+ Qlockerr(rslt), rslt, p->drop_name,
+ strerror(errno), errno );
+ } else {
+ DEBUG_LOG1 ( p, "Unlinking spool %s", p->drop_name );
+ if ( unlink(p->drop_name) == -1 ) {
+ pop_log ( p, POP_PRIORITY, HERE,
+ "[SYS/TEMP] Unable to unlink old maildrop "
+ "'%s': %s (%d)",
+ p->drop_name, STRERROR(errno), errno );
+ return pop_msg ( p, POP_FAILURE, HERE,
+ "[SYS/TEMP] Failed to unlink %s "
+ "with uid %lu, gid %lu.",
+ p->temp_drop,
+ (long unsigned) pwp->pw_uid,
+ (long unsigned) pwp->pw_gid );
+ } else {
+ int nfd = open ( p->drop_name, O_RDWR|O_CREAT, mybuf.st_mode );
+ if ( nfd == -1 ) {
+ pop_log ( p, POP_PRIORITY, HERE,
+ "[SYS/TEMP] Unable to create new maildrop "
+ "'%s': %s (%d)",
+ p->drop_name, STRERROR(errno), errno );
+ return pop_msg ( p, POP_FAILURE, HERE,
+ "[SYS/TEMP] Failed to create %s "
+ "with uid %lu, gid %lu.",
+ p->temp_drop,
+ (long unsigned) pwp->pw_uid,
+ (long unsigned) pwp->pw_gid );
+ } /* new mail drop creation failed */
+ } /* old mail drop unlinked */
+ } /* we have a lock on the mail spool */
+ } /* temp spool linked to mail spool */
+
+ /*
+ * 2005/03/11 Sylvain Robitaille: this open is Ok, but match
+ * permission.
+ */
+ dfd = open ( p->temp_drop, O_RDWR | O_CREAT, mybuf.st_mode );
if ( dfd == -1 ) {
pop_log ( p, POP_PRIORITY, HERE,
"[SYS/TEMP] Unable to open temporary maildrop "
@@ -1620,6 +1708,14 @@
} /* mybuf.st_size != 0 */
/*
+ * 2005/03/17 Sylvain Robitaille: If the temp spool was linked to
+ * the mail spool, we already have a lock, but it's
+ * reasonable to touch it here.
+ */
+ if ( CU_Drop_Linked ) {
+ Qtouchlock ( HERE );
+ } else {
+ /*
* Always use Qmaillock().
*/
@@ -1639,6 +1735,7 @@
Qlockerr(rslt), rslt, p->drop_name,
strerror(errno), errno );
}
+ }
time_locked = time(0);
/* Open the user's maildrop; if this fails, no harm in assuming empty */
--- popper/pop_updt.c.original 2004-12-14 21:36:18.000000000 -0500
+++ popper/pop_updt.c 2005-03-18 15:24:34.000000000 -0500
@@ -483,8 +483,41 @@
mfd );
} /* server mode */
- if ( p->server_mode == FALSE
- || ( p->msgs_deleted != p->msg_count ) ) {
+ /*
+ * 2005/03/10 Sylvain Robitaille: an attempt to add the "fast-update"
+ * feature in non-server mode: if no updates to the mail spool are
+ * pending, (a common case on systems where users normally "leave mail
+ * on server") rather than recopy the spool one message at a time,
+ * simply rename() it if we can.
+ */
+ int CU_Drop_Linked = 0;
+ if ( p->server_mode == FALSE &&
+ p->dirty == FALSE ) {
+
+ DEBUG_LOG2 ( p, "Renaming %s to %s",
+ p->temp_drop, p->drop_name );
+ nchar = rename ( p->temp_drop, p->drop_name );
+ if ( nchar == 0 ) {
+ DEBUG_LOG2 ( p, "Moved %s to %s",
+ p->temp_drop, p->drop_name );
+ CU_Drop_Linked++;
+ }
+ else {
+ pop_log ( p, POP_NOTICE, HERE,
+ "Unable to move %s to %s: %s (%d)",
+ p->temp_drop, p->drop_name,
+ STRERROR(errno), errno );
+ /* ensure that we'll fall back to copying the spool */
+ p->dirty = TRUE;
+ }
+ } /* !p->server_mode && !p->dirty */
+
+ /*
+ * 2005/03/10 Sylvain Robitaille: if there are pending updates, or
+ * rename() failed, fall back to the original copying
+ * method.
+ */
+ if ( p->server_mode == FALSE && p->dirty ) {
DEBUG_LOG7 ( p, "Server mode=%i; %i out of %i msgs deleted; "
"copying msgs from %s (%d) to %s (%d)",
p->server_mode, p->msgs_deleted, p->msg_count,
@@ -628,7 +661,7 @@
p->drop_size += length;
} /* msg loop */
- /* flush and check for errors now! The new mail will be writen
+ /* flush and check for errors now! The new mail will be written
* without stdio, since we need not separate messages
*/
@@ -827,6 +860,7 @@
Qmailunlock ( HERE );
} /* server mode */
else {
+ if ( !CU_Drop_Linked ) {
DEBUG_LOG2 ( p, "non-server mode; copying any new msgs back from"
"temp drop (%d) to spool (%d)",
fileno(p->drop), mfd );
@@ -867,13 +901,13 @@
/*
* Close the maildrop and empty temporary maildrop
*/
- fclose ( md );
ftruncate ( fileno(p->drop), (OFF_T)0 );
DEBUG_LOG1 ( p, "truncated temp drop (%d)", fileno(p->drop) );
unlink_temp_drop ( p, p->temp_drop, HERE );
fclose ( p->drop );
+ } /* !CU_Drop_Linked */
+ fclose ( md );
Qmailunlock ( HERE );
-
} /* non-server mode */
if ( p->bDo_timing )
--- popper/popper.h.original 2004-09-02 01:24:08.000000000 -0400
+++ popper/popper.h 2005-03-08 15:35:01.000000000 -0500
@@ -101,6 +101,13 @@
#define ALLOC_MSGS 20
#define OUT_BUF_SIZE 512 /* Amount of output to buffer before forcing a
write */
+/* ----------------------------------------------------------------- */
+/* 1997/11/21 Sylvain Robitaille: Added ability to record POP access */
+/* in lastlog file, so let's define where the lastlog file is. */
+/* */
+# define LASTLOG "/var/adm/lastlog"
+/* ----------------------------------------------------------------- */
+
#ifndef MAXHOSTNAMELEN
# define MAXHOSTNAMELEN 64
#endif /* not MAXHOSTNAMELEN */
--- popper/pop_pass.c.original 2004-12-14 21:22:23.000000000 -0500
+++ popper/pop_pass.c 2005-03-08 16:40:29.000000000 -0500
@@ -1283,6 +1283,66 @@
#endif /* SPEC_POP_AUTH */
+/***************************************************************
+ * *
+ * Function: update_lastlog(POP *p, uid_t uid) *
+ * *
+ * Description: records the user's login in the system lastlog *
+ * file. *
+ * *
+ * Author: Sylvain Robitaille -- using code borrowed from the *
+ * ssh source. *
+ * *
+ * NOTE: This code is written specifically for Digital Unix on *
+ * an Alpha. No attempts for portability were made. *
+ * *
+ * Date: 1997/11/21 *
+ * 2000/06/01 copied into new version added HERE to *
+ * pop_log call args *
+ * *
+ ***************************************************************/
+void update_lastlog(POP *p, uid_t uid) {
+# include <lastlog.h>
+# include <fcntl.h>
+
+ int fd; /* file descriptor */
+ struct lastlog ll; /* lastlog information held here */
+
+ /* LASTLOG is defined in popper.h to be the path to the lastlog file. */
+ const char *lastlog = LASTLOG;
+
+ /*
+ * extract elements of p
+ */
+ const char *user = p->user;
+ const char *host = p->client;
+
+ /*
+ * Now update the lastlog file.
+ */
+
+ /* Check that things are sane. */
+ if(strcmp(user, "") != 0) {
+ /* Initialize the lastlog structure */
+ memset(&ll, 0, sizeof(ll));
+
+ /* Fill in the data */
+ ll.ll_time = time(NULL);
+ strncpy(ll.ll_line, "pop", sizeof(ll.ll_line));
+ strncpy(ll.ll_host, host, sizeof(ll.ll_host));
+
+ fd = open(lastlog, O_RDWR);
+ if(fd >= 0) {
+ lseek(fd, (off_t)((long)uid * sizeof(ll)), 0);
+ if(write(fd, &ll, sizeof(ll)) != sizeof(ll))
+ pop_log(p, POP_PRIORITY, HERE, "Could not write %.100s: %.100s",
+ lastlog, strerror(errno));
+ close(fd);
+ }
+ }
+}
+
+
/*
* pass: Obtain the user password from a POP client
*/
@@ -1439,6 +1523,14 @@
#endif /* SECURENISPLUS */
/*
+ * 1997/11/19 Sylvain Robitaille
+ * ----------
+ * Right here would be a fine spot for updating the lastlog file.
+ * We need to be sure to do so before dropping priviledges.
+ */
+ update_lastlog(p, pwp->pw_uid);
+
+ /*
* Check if server mode should be set or reset based on group membership.
*/
