Skip to content

Commit e5fa238

Browse files
larsewilarsewi
authored
Merge pull request cfengine#5617 from larsewi/lastseen
ENT-11838: Fixed perpetual warnings that a deleted host has checked in
2 parents 3534477 + 526f354 commit e5fa238

File tree

10 files changed

+167
-115
lines changed

10 files changed

+167
-115
lines changed

‎cf-check/backup.c

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -70,8 +70,9 @@ const char *create_backup_dir()
7070
return NULL;
7171
}
7272

73-
const int n =
74-
snprintf(backup_dir, PATH_MAX, "%s%jd/", backup_root, (intmax_t)ts);
73+
int n =
74+
snprintf(backup_dir, PATH_MAX - 1, // trailing slash for later
75+
"%s%jd-XXXXXX", backup_root, (intmax_t)ts);
7576
if (n >= PATH_MAX)
7677
{
7778
Log(LOG_LEVEL_ERR,
@@ -81,7 +82,7 @@ const char *create_backup_dir()
8182
return NULL;
8283
}
8384

84-
if (mkdir(backup_dir, 0700) != 0)
85+
if (mkdtemp(backup_dir) == NULL)
8586
{
8687
Log(LOG_LEVEL_ERR,
8788
"Could not create directory '%s' (%s)",
@@ -90,6 +91,10 @@ const char *create_backup_dir()
9091
return NULL;
9192
}
9293

94+
// Add trailing forward slash
95+
backup_dir[n++] = FILE_SEPARATOR;
96+
backup_dir[n] = '\0';
97+
9398
return backup_dir;
9499
}
95100

@@ -102,6 +107,11 @@ int backup_files_copy(Seq *filenames)
102107
assert_or_return(length > 0, 1);
103108

104109
const char *backup_dir = create_backup_dir();
110+
if (backup_dir == NULL)
111+
{
112+
// Error already logged
113+
return -1;
114+
}
105115

106116
Log(LOG_LEVEL_INFO, "Backing up to '%s'", backup_dir);
107117

‎cf-check/db_structs.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
// Struct used for quality entries in /var/cfengine/state/cf_lastseen.lmdb:
1818
typedef struct
1919
{
20+
bool acknowledged;
2021
time_t lastseen;
2122
QPoint Q;
2223
} KeyHostSeen; // Keep in sync with lastseen.h

‎cf-check/dump.c

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,7 @@ static void print_struct_lastseen_quality(
107107
memcpy(&quality, value.mv_data, sizeof(quality));
108108
const time_t lastseen = quality.lastseen;
109109
const QPoint Q = quality.Q;
110+
bool acknowledged = quality.acknowledged;
110111

111112
JsonElement *q_json = JsonObjectCreate(4);
112113
JsonObjectAppendReal(q_json, "q", Q.q);
@@ -115,6 +116,7 @@ static void print_struct_lastseen_quality(
115116
JsonObjectAppendReal(q_json, "dq", Q.dq);
116117

117118
JsonElement *top_json = JsonObjectCreate(2);
119+
JsonObjectAppendBool(top_json, "acknowledged", acknowledged);
118120
JsonObjectAppendInteger(top_json, "lastseen", lastseen);
119121
JsonObjectAppendObject(top_json, "Q", q_json);
120122

‎libpromises/dbm_api.c

Lines changed: 15 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -426,17 +426,23 @@ bool OpenDBInstance(DBHandle **dbp, dbid id, DBHandle *handle)
426426
}
427427
}
428428

429-
DBPathUnLock(&lock);
430-
}
431-
432-
if (handle->priv)
433-
{
434-
if (!DBMigrate(handle, id))
429+
if (handle->priv != NULL)
435430
{
436-
DBPrivCloseDB(handle->priv);
437-
handle->priv = NULL;
438-
handle->open_tstamp = -1;
431+
if (!DBMigrate(handle, id))
432+
{
433+
/* Migration failed. The best we can do is to move the
434+
* broken DB to the side and start fresh. */
435+
DBPrivCloseDB(handle->priv);
436+
DBPathMoveBroken(handle->filename);
437+
handle->priv = DBPrivOpenDB(handle->filename, id);
438+
if (handle->priv == DB_PRIV_DATABASE_BROKEN)
439+
{
440+
handle->priv = NULL;
441+
}
442+
}
439443
}
444+
445+
DBPathUnLock(&lock);
440446
}
441447
}
442448

‎libpromises/dbm_lmdb.c

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,7 @@ struct DBCursorPriv_
6969
MDB_cursor *mc;
7070
MDB_val delkey;
7171
void *curkv;
72+
size_t curks;
7273
bool pending_delete;
7374
};
7475

@@ -1290,6 +1291,7 @@ bool DBPrivAdvanceCursor(
12901291
}
12911292
cursor->curkv = xmalloc(keybuf_size + data.mv_size);
12921293
memcpy(cursor->curkv, mkey.mv_data, mkey.mv_size);
1294+
cursor->curks = mkey.mv_size;
12931295
*key = cursor->curkv;
12941296
*key_size = mkey.mv_size;
12951297
*value_size = data.mv_size;
@@ -1355,7 +1357,7 @@ bool DBPrivWriteCursorEntry(
13551357
{
13561358
MDB_val curkey;
13571359
curkey.mv_data = cursor->curkv;
1358-
curkey.mv_size = sizeof(cursor->curkv);
1360+
curkey.mv_size = cursor->curks;
13591361

13601362
rc = mdb_cursor_put(cursor->mc, &curkey, &data, MDB_CURRENT);
13611363
CheckLMDBUsable(rc, cursor->db->env);

‎libpromises/dbm_migration.c

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,11 +26,12 @@
2626

2727
#include <lastseen.h>
2828
#include <string_lib.h>
29+
#include <backup.h>
2930

3031
extern const DBMigrationFunction dbm_migration_plan_lastseen[];
3132

3233

33-
#ifdef LMDB
34+
#ifndef LMDB
3435
bool DBMigrate(ARG_UNUSED DBHandle *db, ARG_UNUSED dbid id)
3536
{
3637
return true;
@@ -64,6 +65,11 @@ bool DBMigrate(DBHandle *db, dbid id)
6465
{
6566
if (step_version == DBVersion(db))
6667
{
68+
Seq *seq = SeqNew(1, free);
69+
SeqAppend(seq, DBIdToPath(id));
70+
backup_files_copy(seq);
71+
SeqDestroy(seq);
72+
6773
if (!(*step)(db))
6874
{
6975
return false;

‎libpromises/dbm_migration_lastseen.c

Lines changed: 60 additions & 100 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626

2727
#include <lastseen.h>
2828
#include <logging.h>
29+
#include <string_lib.h>
2930

3031
typedef struct
3132
{
@@ -34,6 +35,12 @@ typedef struct
3435
double var;
3536
} QPoint0;
3637

38+
typedef struct
39+
{
40+
time_t lastseen;
41+
QPoint Q; // Average time between connections (rolling weighted average)
42+
} KeyHostSeen1;
43+
3744
#define QPOINT0_OFFSET 128
3845

3946
/*
@@ -48,142 +55,95 @@ typedef struct
4855

4956
static bool LastseenMigrationVersion0(DBHandle *db)
5057
{
51-
bool errors = false;
52-
DBCursor *cursor;
58+
/* For some reason DB migration for LMDB was disabled in 2014 (in commit
59+
* 8611970bfa33be7b3cf0724eb684833e08582850). Unfortunately there is no
60+
* mention as to why this was done. Maybe it was not working?
61+
*
62+
* However, we're re-enabling it now (10 years later). Since this
63+
* migration function has not been active for the last 10 years, the
64+
* safest thing is to remove the migration logic, and only update the
65+
* version number.
66+
*
67+
* If you have not upgraded CFEngine in the last 10 years, this will be
68+
* the least of your problems.
69+
*/
70+
return WriteDB(db, "version", "1", sizeof("1"));
71+
}
5372

73+
static bool LastseenMigrationVersion1(DBHandle *db)
74+
{
75+
DBCursor *cursor;
5476
if (!NewDBCursor(db, &cursor))
5577
{
78+
Log(LOG_LEVEL_ERR,
79+
"Unable to create database cursor during lastseen migration");
5680
return false;
5781
}
5882

5983
char *key;
6084
void *value;
61-
int ksize, vsize;
85+
int key_size, value_size;
6286

63-
while (NextDB(cursor, &key, &ksize, &value, &vsize))
87+
// Iterate through all key-value pairs
88+
while (NextDB(cursor, &key, &key_size, &value, &value_size))
6489
{
65-
if (ksize == 0)
90+
if (key_size == 0)
6691
{
67-
Log(LOG_LEVEL_INFO, "LastseenMigrationVersion0: Database structure error -- zero-length key.");
92+
Log(LOG_LEVEL_WARNING,
93+
"Found zero-length key during lastseen migration");
6894
continue;
6995
}
7096

71-
/* Only look for old [+-]kH -> IP<QPoint> entries */
72-
if ((key[0] != '+') && (key[0] != '-'))
97+
// Only look for old KeyHostSeen entries
98+
if (key[0] != 'q')
7399
{
74-
/* Warn about completely unexpected keys */
75-
76-
if ((key[0] != 'q') && (key[0] != 'k') && (key[0] != 'a'))
100+
// Warn about completely unexpected keys
101+
if ((key[0] != 'k') && (key[0] != 'a') && !StringEqualN(key, "version", key_size))
77102
{
78-
Log(LOG_LEVEL_INFO, "LastseenMigrationVersion0: Malformed key found '%s'", key);
103+
Log(LOG_LEVEL_WARNING,
104+
"Found unexpected key '%s' during lastseen migration. "
105+
"Only expecting 'version' or 'k', 'a' and 'q[i|o]' prefixed keys.",
106+
key);
79107
}
80-
81-
continue;
82-
}
83-
84-
bool incoming = key[0] == '-';
85-
const char *hostkey = key + 1;
86-
87-
/* Only migrate sane data */
88-
if (vsize != QPOINT0_OFFSET + sizeof(QPoint0))
89-
{
90-
Log(LOG_LEVEL_INFO,
91-
"LastseenMigrationVersion0: invalid value size for key '%s', entry is deleted",
92-
key);
93-
DBCursorDeleteEntry(cursor);
94-
continue;
95-
}
96-
97-
/* Properly align the data */
98-
const char *old_data_address = (const char *) value;
99-
QPoint0 old_data_q;
100-
memcpy(&old_data_q, (const char *) value + QPOINT0_OFFSET,
101-
sizeof(QPoint0));
102-
103-
char hostkey_key[CF_BUFSIZE];
104-
snprintf(hostkey_key, CF_BUFSIZE, "k%s", hostkey);
105-
106-
if (!WriteDB(db, hostkey_key, old_data_address, strlen(old_data_address) + 1))
107-
{
108-
Log(LOG_LEVEL_INFO, "Unable to write version 1 lastseen entry for '%s'", key);
109-
errors = true;
110-
continue;
111-
}
112-
113-
char address_key[CF_BUFSIZE];
114-
snprintf(address_key, CF_BUFSIZE, "a%s", old_data_address);
115-
116-
if (!WriteDB(db, address_key, hostkey, strlen(hostkey) + 1))
117-
{
118-
Log(LOG_LEVEL_INFO, "Unable to write version 1 reverse lastseen entry for '%s'", key);
119-
errors = true;
120-
continue;
121-
}
122-
123-
char quality_key[CF_BUFSIZE];
124-
snprintf(quality_key, CF_BUFSIZE, "q%c%s", incoming ? 'i' : 'o', hostkey);
125-
126-
/*
127-
Ignore malformed connection quality data
128-
*/
129-
130-
if ((!isfinite(old_data_q.q))
131-
|| (old_data_q.q < 0)
132-
|| (!isfinite(old_data_q.expect))
133-
|| (!isfinite(old_data_q.var)))
134-
{
135-
Log(LOG_LEVEL_INFO, "Ignoring malformed connection quality data for '%s'", key);
136-
DBCursorDeleteEntry(cursor);
137108
continue;
138109
}
139110

140-
KeyHostSeen data = {
141-
.lastseen = (time_t)old_data_q.q,
142-
.Q = {
143-
/*
144-
Previously .q wasn't stored in database, but was calculated
145-
every time as a difference between previous timestamp and a
146-
new timestamp. Given we don't have this information during
147-
the database upgrade, just assume that last reading is an
148-
average one.
149-
*/
150-
.q = old_data_q.expect,
151-
.dq = 0,
152-
.expect = old_data_q.expect,
153-
.var = old_data_q.var,
154-
}
111+
KeyHostSeen1 *old_value = value;
112+
KeyHostSeen new_value = {
113+
.acknowledged = true, // We don't know, assume yes
114+
.lastseen = old_value->lastseen,
115+
.Q = old_value->Q,
155116
};
156117

157-
if (!WriteDB(db, quality_key, &data, sizeof(data)))
158-
{
159-
Log(LOG_LEVEL_INFO, "Unable to write version 1 connection quality key for '%s'", key);
160-
errors = true;
161-
continue;
162-
}
163-
164-
if (!DBCursorDeleteEntry(cursor))
118+
// This will overwrite the entry
119+
if (!DBCursorWriteEntry(cursor, &new_value, sizeof(new_value)))
165120
{
166-
Log(LOG_LEVEL_INFO, "Unable to delete version 0 lastseen entry for '%s'", key);
167-
errors = true;
121+
Log(LOG_LEVEL_ERR,
122+
"Unable to write version 2 of entry for key '%s' during lastseen migration.",
123+
key);
124+
return false;
168125
}
169126
}
170127

171-
if (DeleteDBCursor(cursor) == false)
128+
if (!DeleteDBCursor(cursor))
172129
{
173-
Log(LOG_LEVEL_ERR, "LastseenMigrationVersion0: Unable to close cursor");
174-
errors = true;
130+
Log(LOG_LEVEL_ERR, "Unable to close cursor during lastseen migration");
131+
return false;
175132
}
176133

177-
if ((!errors) && (!WriteDB(db, "version", "1", sizeof("1"))))
134+
if (!WriteDB(db, "version", "2", sizeof("2")))
178135
{
179-
errors = true;
136+
Log(LOG_LEVEL_ERR, "Failed to update version number during lastseen migration");
137+
return false;
180138
}
181139

182-
return !errors;
140+
Log(LOG_LEVEL_INFO, "Migrated lastseen database from version 1 to 2");
141+
return true;
183142
}
184143

185144
const DBMigrationFunction dbm_migration_plan_lastseen[] =
186145
{
187146
LastseenMigrationVersion0,
147+
LastseenMigrationVersion1,
188148
NULL
189149
};

0 commit comments

Comments
 (0)