krb5 commit: Use a single global dump for iprop full syncs
Greg Hudson
ghudson at MIT.EDU
Fri Oct 5 14:09:05 EDT 2012
https://github.com/krb5/krb5/commit/4fd4144b3222b060f3e9928a9cb4587df9979539
commit 4fd4144b3222b060f3e9928a9cb4587df9979539
Author: Nicolas Williams <nico at cryptonector.com>
Date: Fri Aug 17 18:19:31 2012 -0500
Use a single global dump for iprop full syncs
Use a global dump (the default dump file) for full syncs for iprop.
When a slave asks for a fullsync we kprop the existing global dump to it
if that is good enough, else we dump the DB and send the new global
dump.
Before this change kadmind would run kdb5_util dump -i... each time a
slave asked for a full dump. This was done in a sub-process,
thankfully, but it was still a waste of time and storage (e.g., if one
has a huge KDB).
Also, long dump times might cause a slave to give up (the timeout for
this is now configurable). But since iprop dumps bear a serial number
and timestamp and since slaves will resync from that point forward, it
doesn't matter if the dump we send a slave is fresh as long as it is
fresh enough (i.e., that its sno and timestamp are in the ulog).
Also:
- Rename dumps into place instead of unlink, create, write (but we
still keep the dump ok files as lock files and as a method of
signaling to kprop that the dump is complete).
ticket: 7371
src/include/kdb_log.h | 2 +
src/kadmin/dbutil/dump.c | 231 ++++++++++++++++++++++++++++++---------
src/kadmin/server/ipropd_svc.c | 68 ++++++------
src/lib/kdb/kdb_log.c | 2 +-
src/lib/kdb/libkdb5.exports | 1 +
5 files changed, 215 insertions(+), 89 deletions(-)
diff --git a/src/include/kdb_log.h b/src/include/kdb_log.h
index 51648c7..beecdc1 100644
--- a/src/include/kdb_log.h
+++ b/src/include/kdb_log.h
@@ -108,6 +108,8 @@ typedef struct kdb_hlog {
uint16_t kdb_block; /* Block size of each element */
} kdb_hlog_t;
+extern void ulog_sync_header(kdb_hlog_t *);
+
typedef struct kdb_ent_header {
uint32_t kdb_umagic; /* Update entry magic # */
kdb_sno_t kdb_entry_sno; /* Serial # of entry */
diff --git a/src/kadmin/dbutil/dump.c b/src/kadmin/dbutil/dump.c
index 9350dc1..e366104 100644
--- a/src/kadmin/dbutil/dump.c
+++ b/src/kadmin/dbutil/dump.c
@@ -123,6 +123,8 @@ typedef struct _dump_version {
char *header;
int updateonly;
int create_kadm5;
+ int iprop;
+ int ipropx;
dump_func dump_princ;
osa_adb_iter_policy_func dump_policy;
load_func load_record;
@@ -133,6 +135,8 @@ dump_version old_version = {
"kdb5_edit load_dump version 2.0\n",
0,
1,
+ 0,
+ 0,
dump_k5beta_iterator,
NULL,
process_k5beta_record,
@@ -142,6 +146,8 @@ dump_version beta6_version = {
"kdb5_edit load_dump version 3.0\n",
0,
1,
+ 0,
+ 0,
dump_k5beta6_iterator,
NULL,
process_k5beta6_record,
@@ -151,6 +157,8 @@ dump_version beta7_version = {
"kdb5_util load_dump version 4\n",
0,
0,
+ 0,
+ 0,
dump_k5beta7_princ,
dump_k5beta7_policy,
process_k5beta7_record,
@@ -160,6 +168,8 @@ dump_version iprop_version = {
"iprop",
0,
0,
+ 1,
+ 0,
dump_k5beta7_princ_withpolicy,
dump_k5beta7_policy,
process_k5beta7_record,
@@ -169,6 +179,8 @@ dump_version ov_version = {
"OpenV*Secure V1.0\t",
1,
1,
+ 0,
+ 0,
dump_ov_princ,
dump_k5beta7_policy,
process_ov_record
@@ -179,6 +191,8 @@ dump_version r1_3_version = {
"kdb5_util load_dump version 5\n",
0,
0,
+ 0,
+ 0,
dump_k5beta7_princ_withpolicy,
dump_k5beta7_policy,
process_k5beta7_record,
@@ -188,6 +202,8 @@ dump_version r1_8_version = {
"kdb5_util load_dump version 6\n",
0,
0,
+ 0,
+ 0,
dump_k5beta7_princ_withpolicy,
dump_r1_8_policy,
process_r1_8_record,
@@ -197,6 +213,8 @@ dump_version r1_11_version = {
"kdb5_util load_dump version 7\n",
0,
0,
+ 0,
+ 0,
dump_k5beta7_princ_withpolicy,
dump_r1_11_policy,
process_r1_11_record,
@@ -206,6 +224,8 @@ dump_version ipropx_1_version = {
"ipropx",
0,
0,
+ 1,
+ 1,
dump_k5beta7_princ_withpolicy,
dump_r1_11_policy,
process_r1_11_record,
@@ -281,6 +301,7 @@ static const char oldoption[] = "-old";
static const char b6option[] = "-b6";
static const char b7option[] = "-b7";
static const char ipropoption[] = "-i";
+static const char conditionaloption[] = "-c";
static const char verboseoption[] = "-verbose";
static const char updateoption[] = "-update";
static const char hashoption[] = "-hash";
@@ -349,6 +370,126 @@ krb5_error_code master_key_convert(context, db_entry)
return 0;
}
+/* Create temp file for new dump to be named ofile. */
+static FILE *
+create_ofile(char *ofile, char **tmpname)
+{
+ int fd = -1;
+ FILE *f;
+
+ *tmpname = NULL;
+ if (asprintf(tmpname, "%s-XXXXXX", ofile) < 0)
+ goto error;
+
+ fd = mkstemp(*tmpname);
+ if (fd == -1)
+ goto error;
+
+ f = fdopen(fd, "w+");
+ if (f != NULL)
+ return f;
+
+error:
+ com_err(progname, errno,
+ _("while allocating temporary filename dump"));
+ if (fd >= 0)
+ unlink(*tmpname);
+ exit(1);
+}
+
+/* Rename new dump file into place */
+static void
+finish_ofile(char *ofile, char **tmpname)
+{
+ if (rename(*tmpname, ofile) == -1) {
+ com_err(progname, errno, _("while renaming dump file into place"));
+ exit(1);
+ }
+ free(*tmpname);
+ *tmpname = NULL;
+}
+
+/*
+ * Read the dump header. Returns 1 on success, 0 if the file is not a
+ * recognized iprop dump format.
+ */
+static int
+parse_iprop_header(char *buf, dump_version **dv, uint32_t *last_sno,
+ uint32_t *last_seconds, uint32_t *last_useconds)
+{
+ char head[128];
+ int nread;
+ uint32_t u[4];
+ uint32_t *up = &u[0];
+
+ nread = sscanf(buf, "%127s %u %u %u %u", head, &u[0], &u[1], &u[2], &u[3]);
+ if (nread < 1)
+ return 0;
+
+ if (!strcmp(head, ipropx_1_version.header)) {
+ if (nread != 5)
+ return 0;
+ if (u[0] == IPROPX_VERSION_0)
+ *dv = &iprop_version;
+ else if (u[0] == IPROPX_VERSION_1)
+ *dv = &ipropx_1_version;
+ else {
+ fprintf(stderr, _("%s: Unknown iprop dump version %d\n"),
+ progname, u[0]);
+ return 0;
+ }
+ up = &u[1];
+ } else if (!strcmp(head, iprop_version.header)) {
+ if (nread != 4)
+ return 0;
+ *dv = &iprop_version;
+ } else {
+ fprintf(stderr, "Invalid iprop header\n");
+ return 0;
+ }
+
+ *last_sno = *(up++);
+ *last_seconds = *(up++);
+ *last_useconds = *(up++);
+ return 1;
+}
+
+/*
+ * Return 1 if the {sno, timestamp} in an existing dump file is in the
+ * ulog, else return 0.
+ */
+static int
+current_dump_sno_in_ulog(char *ifile, kdb_hlog_t *ulog)
+{
+ dump_version *junk;
+ uint32_t last_sno, last_seconds, last_useconds;
+ char buf[BUFSIZ];
+ FILE *f;
+
+ if (ulog->kdb_last_sno == 0)
+ return 0; /* nothing in ulog */
+
+ f = fopen(ifile, "r");
+ if (f == NULL)
+ return 0; /* aliasing other errors to ENOENT here is OK */
+
+ if (fgets(buf, sizeof(buf), f) == NULL)
+ return errno ? -1 : 0;
+ fclose(f);
+
+ if (!parse_iprop_header(buf, &junk, &last_sno, &last_seconds,
+ &last_useconds))
+ return 0;
+
+ if (ulog->kdb_first_sno > last_sno ||
+ ulog->kdb_first_time.seconds > last_seconds ||
+ (ulog->kdb_first_time.seconds == last_seconds &&
+ ulog->kdb_first_time.useconds > last_useconds))
+ return 0;
+
+ return 1;
+}
+
/*
* Update the "ok" file.
*/
@@ -1128,10 +1269,11 @@ dump_db(argc, argv)
FILE *f;
struct dump_args arglist;
char *ofile;
+ char *tmpofile = NULL;
krb5_error_code kret, retval;
dump_version *dump;
int aindex;
- krb5_boolean locked;
+ int conditional = 0;
char *new_mkey_file = 0;
bool_t dump_sno = FALSE;
kdb_log_context *log_ctx;
@@ -1187,7 +1329,9 @@ dump_db(argc, argv)
exit_status++;
return;
}
- } else if (!strcmp(argv[aindex], verboseoption))
+ } else if (!strcmp(argv[aindex], conditionaloption))
+ conditional = 1;
+ else if (!strcmp(argv[aindex], verboseoption))
arglist.flags |= FLAG_VERBOSE;
else if (!strcmp(argv[aindex], "-mkey_convert"))
mkey_convert = 1;
@@ -1214,6 +1358,22 @@ dump_db(argc, argv)
}
/*
+ * If a conditional ipropx dump we check if the existing dump is
+ * good enough.
+ */
+ if (ofile != NULL && conditional) {
+ if (!dump->iprop) {
+ com_err(progname, 0,
+ _("Conditional dump is an undocumented option for "
+ "use only for iprop dumps"));
+ exit_status++;
+ return;
+ }
+ if (current_dump_sno_in_ulog(ofile, log_ctx->ulog))
+ return;
+ }
+
+ /*
* Make sure the database is open. The policy database only has
* to be opened if we try a dump that uses it.
*/
@@ -1290,40 +1450,20 @@ dump_db(argc, argv)
}
kret = 0;
- locked = 0;
+
if (ofile && strcmp(ofile, "-")) {
/*
* Discourage accidental dumping to filenames beginning with '-'.
*/
if (ofile[0] == '-')
usage();
- /*
- * Make sure that we don't open and truncate on the fopen,
- * since that may hose an on-going kprop process.
- *
- * We could also control this by opening for read and
- * write, doing an flock with LOCK_EX, and then
- * truncating the file once we have gotten the lock,
- * but that would involve more OS dependencies than I
- * want to get into.
- */
- unlink(ofile);
- if (!(f = fopen(ofile, "w"))) {
+ f = create_ofile(ofile, &tmpofile);
+ if (f == NULL) {
fprintf(stderr, ofopen_error,
progname, ofile, error_message(errno));
exit_status++;
return;
}
- if ((kret = krb5_lock_file(util_context,
- fileno(f),
- KRB5_LOCKMODE_EXCLUSIVE))) {
- fprintf(stderr, oflock_error,
- progname, ofile, error_message(kret));
- fclose(f);
- exit_status++;
- }
- else
- locked = 1;
} else {
f = stdout;
}
@@ -1341,8 +1481,11 @@ dump_db(argc, argv)
if (krb5_db_lock(util_context, KRB5_LOCKMODE_SHARED)) {
fprintf(stderr,
_("%s: Couldn't grab lock\n"), progname);
+ if (tmpofile != NULL)
+ unlink(tmpofile);
+ free(tmpofile);
exit_status++;
- goto unlock_and_return;
+ return;
}
if (ipropx_version)
@@ -1373,17 +1516,15 @@ dump_db(argc, argv)
exit_status++;
}
if (ofile && f != stdout && !exit_status) {
- if (locked) {
- (void) krb5_lock_file(util_context, fileno(f), KRB5_LOCKMODE_UNLOCK);
- locked = 0;
- }
fclose(f);
+ finish_ofile(ofile, &tmpofile);
update_ok_file(ofile);
}
+ if (tmpofile != NULL)
+ unlink(tmpofile);
+ free(tmpofile);
+ return;
}
-unlock_and_return:
- if (locked)
- (void) krb5_lock_file(util_context, fileno(f), KRB5_LOCKMODE_UNLOCK);
}
/*
@@ -2512,7 +2653,6 @@ load_db(argc, argv)
krb5_int32 crflags;
int aindex;
int db_locked = 0;
- char iheader[1024];
kdb_log_context *log_ctx;
krb5_boolean add_update = TRUE;
uint32_t caller, last_sno, last_seconds, last_useconds;
@@ -2738,26 +2878,9 @@ load_db(argc, argv)
log_ctx->iproprole = IPROP_NULL;
if (!add_update) {
- unsigned int ipropx_version = IPROPX_VERSION_0;
-
- if (!strncmp(buf, "ipropx ", sizeof("ipropx ") - 1))
- sscanf(buf, "%1023s %u %u %u %u", iheader,
- &ipropx_version, &last_sno,
- &last_seconds, &last_useconds);
- else
- sscanf(buf, "%1023s %u %u %u", iheader, &last_sno,
- &last_seconds, &last_useconds);
-
- switch (ipropx_version) {
- case IPROPX_VERSION_0:
- load = &iprop_version;
- break;
- case IPROPX_VERSION_1:
- load = &ipropx_1_version;
- break;
- default:
- fprintf(stderr, _("%s: Unknown iprop dump version %d\n"),
- progname, ipropx_version);
+ if (!parse_iprop_header(buf, &load, &last_sno,
+ &last_seconds,
+ &last_useconds)) {
exit_status++;
goto error;
}
diff --git a/src/kadmin/server/ipropd_svc.c b/src/kadmin/server/ipropd_svc.c
index 0e45f6a..19c1bac 100644
--- a/src/kadmin/server/ipropd_svc.c
+++ b/src/kadmin/server/ipropd_svc.c
@@ -246,10 +246,10 @@ static kdb_fullresync_result_t *
ipropx_resync(uint32_t vers, struct svc_req *rqstp)
{
static kdb_fullresync_result_t ret;
- char *tmpf = 0;
char *ubuf = 0;
char clhost[MAXHOSTNAMELEN] = {0};
int pret, fret;
+ FILE *p;
kadm5_server_handle_t handle = global_server_handle;
OM_uint32 min_stat;
gss_name_t name = NULL;
@@ -320,23 +320,21 @@ ipropx_resync(uint32_t vers, struct svc_req *rqstp)
}
/*
- * construct db dump file name; kprop style name + clnt fqdn
- */
- if (asprintf(&tmpf, "%s_%s", KPROP_DEFAULT_FILE, clhost) < 0) {
- krb5_klog_syslog(LOG_ERR,
- _("%s: unable to construct db dump file name; out of memory"),
- whoami);
- goto out;
- }
-
- /*
- * note the -i; modified version of kdb5_util dump format
+ * Note the -i; modified version of kdb5_util dump format
* to include sno (serial number). This argument is now
* versioned (-i0 for legacy dump format, -i1 for ipropx
- * version 1 format, etc)
+ * version 1 format, etc).
+ *
+ * The -c option ("conditional") causes the dump to dump only if no
+ * dump already exists or that dump is not in ipropx format, or the
+ * sno and timestamp in the header of that dump are outside the
+ * ulog. This allows us to share a single global dump with all
+ * slaves, since it's OK to share an older dump, as long as its sno
+ * and timestamp are in the ulog (then the slaves can get the
+ * subsequent updates very iprop).
*/
- if (asprintf(&ubuf, "%s dump -i%d %s </dev/null 2>&1",
- KPROPD_DEFAULT_KDB5_UTIL, vers, tmpf) < 0) {
+ if (asprintf(&ubuf, "%s dump -i%d -c %s",
+ KPROPD_DEFAULT_KDB5_UTIL, vers, KPROP_DEFAULT_FILE) < 0) {
krb5_klog_syslog(LOG_ERR,
_("%s: cannot construct kdb5 util dump string too long; out of memory"),
whoami);
@@ -366,8 +364,14 @@ ipropx_resync(uint32_t vers, struct svc_req *rqstp)
DPRINT(("%s: run `%s' ...\n", whoami, ubuf));
(void) signal(SIGCHLD, SIG_DFL);
/* run kdb5_util(1M) dump for IProp */
- /* XXX popen can return NULL; is pclose(NULL) okay? */
- pret = pclose(popen(ubuf, "w"));
+ p = popen(ubuf, "w");
+ if (p == NULL) {
+ krb5_klog_syslog(LOG_ERR,
+ _("%s: popen failed: %s"),
+ whoami, error_message(errno));
+ _exit(1);
+ }
+ pret = pclose(p);
DPRINT(("%s: pclose=%d\n", whoami, pret));
if (pret != 0) {
/* XXX popen/pclose may not set errno
@@ -384,25 +388,22 @@ ipropx_resync(uint32_t vers, struct svc_req *rqstp)
}
DPRINT(("%s: exec `kprop -f %s %s' ...\n",
- whoami, tmpf, clhost));
+ whoami, KPROP_DEFAULT_FILE, clhost));
/* XXX Yuck! */
- if (getenv("KPROP_PORT"))
- pret = execl(KPROPD_DEFAULT_KPROP, "kprop", "-f", tmpf,
- "-P", getenv("KPROP_PORT"),
- clhost, NULL);
- else
- pret = execl(KPROPD_DEFAULT_KPROP, "kprop", "-f", tmpf,
+ if (getenv("KPROP_PORT")) {
+ pret = execl(KPROP_DEFAULT_FILE, "kprop", "-f",
+ KPROP_DEFAULT_FILE, "-P", getenv("KPROP_PORT"),
clhost, NULL);
- if (pret == -1) {
- if (nofork) {
- perror(whoami);
- }
- krb5_klog_syslog(LOG_ERR,
- _("%s: exec failed: %s"),
- whoami,
- error_message(errno));
- _exit(1);
+ } else {
+ pret = execl(KPROP_DEFAULT_FILE, "kprop", "-f",
+ KPROP_DEFAULT_FILE, clhost, NULL);
}
+ perror(whoami);
+ krb5_klog_syslog(LOG_ERR,
+ _("%s: exec failed: %s"),
+ whoami,
+ error_message(errno));
+ _exit(1);
default: /* parent */
ret.ret = UPDATE_OK;
@@ -427,7 +428,6 @@ out:
free(service_name);
if (name)
gss_release_name(&min_stat, &name);
- free(tmpf);
free(ubuf);
return (&ret);
}
diff --git a/src/lib/kdb/kdb_log.c b/src/lib/kdb/kdb_log.c
index 0153375..dc994dd 100644
--- a/src/lib/kdb/kdb_log.c
+++ b/src/lib/kdb/kdb_log.c
@@ -89,7 +89,7 @@ ulog_sync_update(kdb_hlog_t *ulog, kdb_ent_header_t *upd)
/*
* Sync memory to disk for the update log header.
*/
-static void
+void
ulog_sync_header(kdb_hlog_t *ulog)
{
diff --git a/src/lib/kdb/libkdb5.exports b/src/lib/kdb/libkdb5.exports
index 9aa8d1a..43a361d 100644
--- a/src/lib/kdb/libkdb5.exports
+++ b/src/lib/kdb/libkdb5.exports
@@ -88,6 +88,7 @@ krb5_db_promote
ulog_map
ulog_set_role
ulog_free_entries
+ulog_sync_header
xdr_kdb_last_t
xdr_kdb_incr_result_t
xdr_kdb_fullresync_result_t
More information about the cvs-krb5
mailing list