[PATCH 2/7] lib-sieve: Drop binary-header resource usage tracking

With the per-user sieve-rusage file now authoritative for personal
storages, the .svbin header no longer needs to carry CPU usage. Admin
storages (sieve_before/after, default location) cannot be re-uploaded
by the user, so cross-run tracking is unnecessary there; the runtime
sieve_max_cpu_time check still enforces the per-execution limit.

Move the in-memory cumulative carry-over from sbin->header.resource_usage
to a dedicated sbin->persisted_rusage substruct. The on-disk header
struct now solely represents the wire format and is zeroed on every
save; the in-memory carry-over no longer overloads it. Drop the now-dead
"resource usage" section from sieve_binary_dumper_run() (the on-disk
fields are always zero, so the section never produced output).

Gbp-Pq: Name 0002-lib-sieve-Drop-binary-header-resource-usage-tracking.patch

diff --git a/pigeonhole/src/lib-sieve/sieve-binary-dumper.c b/pigeonhole/src/lib-sieve/sieve-binary-dumper.c
index 6ffb911f08e5f933bd6bd8bdf7fc6663cc526639..1d73147733f937e5eadb7d0b52226fedf0770050 100644 (file)
--- a/pigeonhole/src/lib-sieve/sieve-binary-dumper.c
+++ b/pigeonhole/src/lib-sieve/sieve-binary-dumper.c
@@ -6,7 +6,6 @@
 #include "ostream.h"
 #include "array.h"
 #include "buffer.h"
-#include "time-util.h"
 
 #include "sieve-common.h"
 #include "sieve-extensions.h"
@@ -117,17 +116,6 @@ bool sieve_binary_dumper_run(struct sieve_binary_dumper *dumper,
                "flags = 0x%08"PRIx32"\n",
                header->version_major, header->version_minor,
                header->flags);
-       if (header->resource_usage.update_time != 0) {
-               time_t update_time =
-                       (time_t)header->resource_usage.update_time;
-               sieve_binary_dumpf(denv,
-                       "resource usage:\n"
-                       "  update time = %s\n"
-                       "  cpu time = %"PRIu32" ms\n",
-                       t_strflocaltime("%Y-%m-%d %H:%M:%S",
-                                       update_time),
-                       header->resource_usage.cpu_time_msecs);
-       }
 
        /* Dump list of binary blocks */
 

diff --git a/pigeonhole/src/lib-sieve/sieve-binary-file.c b/pigeonhole/src/lib-sieve/sieve-binary-file.c
index 5b06ab9020ecbffd6f4b030f3db8d63fb1a3e192..db998848a47758484b947a14fb07791fc5da94f6 100644 (file)
--- a/pigeonhole/src/lib-sieve/sieve-binary-file.c
+++ b/pigeonhole/src/lib-sieve/sieve-binary-file.c
@@ -147,47 +147,15 @@ sieve_binary_file_read_header(struct sieve_binary *sbin, int fd,
        return 0;
 }
 
-static int
-sieve_binary_file_write_header(struct sieve_binary *sbin, int fd,
-                              struct sieve_binary_header *header,
-                              enum sieve_error *error_code_r)
-{
-       ssize_t wret;
-
-       wret = pwrite(fd, header, sizeof(*header), 0);
-       if (wret < 0) {
-               e_error(sbin->event, "update: "
-                       "failed to write to binary: %m");
-               *error_code_r = SIEVE_ERROR_TEMP_FAILURE;
-               return -1;
-       } else if (wret != sizeof(*header)) {
-               e_error(sbin->event, "update: "
-                       "header written partially %zd/%zu",
-                       wret, sizeof(*header));
-               *error_code_r = SIEVE_ERROR_TEMP_FAILURE;
-               return -1;
-       }
-       return 0;
-}
-
-static void sieve_binary_file_update_header(struct sieve_binary *sbin)
+/* The on-disk binary header carries an unused resource_usage struct for
+   backward compatibility. Tracking lives in the per-user sieve-rusage file. */
+static void sieve_binary_file_zero_header_rusage(struct sieve_binary *sbin)
 {
        struct sieve_binary_header *header = &sbin->header;
-       struct sieve_resource_usage rusage;
-
-       sieve_binary_get_resource_usage(sbin, &rusage);
 
        i_zero(&header->resource_usage);
-       if (HAS_ALL_BITS(header->flags, SIEVE_BINARY_FLAG_RESOURCE_LIMIT) ||
-           sieve_resource_usage_is_high(sbin->svinst, &rusage)) {
-               header->resource_usage.update_time = ioloop_time;
-               header->resource_usage.cpu_time_msecs = rusage.cpu_time_msecs;
-       }
-
        sieve_resource_usage_init(&sbin->rusage);
        sbin->rusage_updated = FALSE;
-
-       (void)sieve_binary_check_resource_usage(sbin);
 }
 
 /*
@@ -347,7 +315,7 @@ sieve_binary_save_to_stream(struct sieve_binary *sbin, struct ostream *stream)
        header->hdr_size = sizeof(*header);
 
        header->flags &= ENUM_NEGATE(SIEVE_BINARY_FLAG_RESOURCE_LIMIT);
-       sieve_binary_file_update_header(sbin);
+       sieve_binary_file_zero_header_rusage(sbin);
 
        if (!_save_aligned(sbin, stream, header, sizeof(*header), NULL)) {
                e_error(sbin->event, "save: failed to save header");
@@ -864,6 +832,11 @@ _sieve_binary_open(struct sieve_binary *sbin, enum sieve_error *error_code_r)
                return FALSE;
        offset = sbin->header.hdr_size;
 
+       /* Resource usage tracking is no longer kept in the binary file;
+          discard any data from older versions. */
+       i_zero(&sbin->header.resource_usage);
+       sbin->header.flags &= ENUM_NEGATE(SIEVE_BINARY_FLAG_RESOURCE_LIMIT);
+
        /* Load block index */
 
        for (i = 0; i < sbin->header.blocks && result; i++) {
@@ -979,47 +952,27 @@ int sieve_binary_check_executable(struct sieve_binary *sbin,
  * Resource usage
  */
 
-static int
-sieve_binary_file_do_update_resource_usage(
-       struct sieve_binary *sbin, int fd, enum sieve_error *error_code_r)
-{
-       struct sieve_binary_header *header = &sbin->header;
-       struct file_lock *lock;
-       const char *error;
-       int ret;
-
-       struct file_lock_settings lock_set = {
-               .lock_method = FILE_LOCK_METHOD_FCNTL,
-       };
-       ret = file_wait_lock(fd, sbin->path, F_WRLCK, &lock_set,
-                            SIEVE_BINARY_FILE_LOCK_TIMEOUT, &lock, &error);
-       if (ret <= 0) {
-               e_error(sbin->event, "%s", error);
-               *error_code_r = SIEVE_ERROR_TEMP_FAILURE;
-               return -1;
-       }
-
-       ret = sieve_binary_file_read_header(sbin, fd, header, error_code_r);
-       if (ret == 0) {
-               sieve_binary_file_update_header(sbin);
-               ret = sieve_binary_file_write_header(sbin, fd, header,
-                                                    error_code_r);
-       }
-
-       file_lock_free(&lock);
-
-       return ret;
-}
-
-static int
-sieve_binary_storage_update_resource_usage(struct sieve_binary *sbin,
-                                          struct sieve_storage *storage,
-                                          enum sieve_error *error_code_r)
+int sieve_binary_file_update_resource_usage(struct sieve_binary *sbin,
+                                           enum sieve_error *error_code_r)
 {
+       struct sieve_storage *storage;
        struct sieve_resource_usage delta;
        struct sieve_resource_usage total;
        uint32_t flags;
 
+       sieve_error_args_init(&error_code_r, NULL);
+
+       sieve_binary_file_close(&sbin->file);
+
+       if (sbin->script == NULL || sbin->script->storage == NULL ||
+           sbin->script->storage->rusage_path == NULL) {
+               /* No per-user file: tracking discarded. */
+               sieve_resource_usage_init(&sbin->rusage);
+               sbin->rusage_updated = FALSE;
+               return 0;
+       }
+       storage = sbin->script->storage;
+
        delta = sbin->rusage;
        sieve_binary_get_resource_usage(sbin, &total);
        flags = sbin->header.flags & SIEVE_BINARY_FLAG_RESOURCE_LIMIT;
@@ -1041,47 +994,3 @@ sieve_binary_storage_update_resource_usage(struct sieve_binary *sbin,
        sbin->rusage_updated = FALSE;
        return 0;
 }
-
-int sieve_binary_file_update_resource_usage(struct sieve_binary *sbin,
-                                           enum sieve_error *error_code_r)
-{
-       struct sieve_storage *storage = NULL;
-       int fd, ret = 0;
-
-       sieve_error_args_init(&error_code_r, NULL);
-
-       sieve_binary_file_close(&sbin->file);
-
-       if (sbin->script != NULL && sbin->script->storage != NULL &&
-           sbin->script->storage->rusage_path != NULL)
-               storage = sbin->script->storage;
-
-       if (storage != NULL) {
-               return sieve_binary_storage_update_resource_usage(
-                       sbin, storage, error_code_r);
-       }
-
-       if (sbin->path == NULL)
-               return 0;
-       if (sbin->header.version_major != SIEVE_BINARY_VERSION_MAJOR) {
-               return sieve_binary_save(sbin, sbin->path, TRUE, 0600,
-                                        error_code_r);
-       }
-
-       fd = sieve_binary_fd_open(sbin, sbin->path, O_RDWR, error_code_r);
-       if (fd < 0) {
-               i_assert(*error_code_r != SIEVE_ERROR_NONE);
-               return -1;
-       }
-
-       ret = sieve_binary_file_do_update_resource_usage(sbin, fd,
-                                                        error_code_r);
-       i_assert(ret == 0 || *error_code_r != SIEVE_ERROR_NONE);
-
-       if (close(fd) < 0) {
-               e_error(sbin->event, "update: "
-                       "failed to close: close() failed: %m");
-       }
-
-       return ret;
-}

diff --git a/pigeonhole/src/lib-sieve/sieve-binary-private.h b/pigeonhole/src/lib-sieve/sieve-binary-private.h
index 6f66437ab9b99fa06acfafc5d4739911c6136fc3..695d0a2b1f445ec4f33cfc2a67726c268649b2d5 100644 (file)
--- a/pigeonhole/src/lib-sieve/sieve-binary-private.h
+++ b/pigeonhole/src/lib-sieve/sieve-binary-private.h
@@ -98,6 +98,12 @@ struct sieve_binary {
        struct sieve_binary_header header;
        struct sieve_resource_usage rusage;
 
+       /* Cumulative CPU usage loaded from per-user sieve-rusage file */
+       struct {
+               time_t update_time;
+               uint32_t cpu_time_msecs;
+       } persisted_rusage;
+
        /* When the binary is loaded into memory or when it is being constructed
           by the generator, extensions can be associated to the binary. The
           extensions array is a sequential list of all linked extensions. The

diff --git a/pigeonhole/src/lib-sieve/sieve-binary.c b/pigeonhole/src/lib-sieve/sieve-binary.c
index cb248338d7b0108be8f0945fbad240966d40df13..f6aedf027095a29cfe058b2b34457b80510573fe 100644 (file)
--- a/pigeonhole/src/lib-sieve/sieve-binary.c
+++ b/pigeonhole/src/lib-sieve/sieve-binary.c
@@ -218,8 +218,8 @@ void sieve_binary_apply_persisted_rusage(struct sieve_binary *sbin)
        if (sieve_rusage_storage_load(storage, &rusage, &flags) <= 0)
                return;
 
-       header->resource_usage.cpu_time_msecs = rusage.cpu_time_msecs;
-       header->resource_usage.update_time = ioloop_time;
+       sbin->persisted_rusage.cpu_time_msecs = rusage.cpu_time_msecs;
+       sbin->persisted_rusage.update_time = ioloop_time;
        header->flags |= (flags & SIEVE_BINARY_FLAG_RESOURCE_LIMIT);
        sieve_resource_usage_init(&sbin->rusage);
        sbin->rusage_updated = FALSE;
@@ -228,15 +228,14 @@ void sieve_binary_apply_persisted_rusage(struct sieve_binary *sbin)
 void sieve_binary_get_resource_usage(struct sieve_binary *sbin,
                                     struct sieve_resource_usage *rusage_r)
 {
-       struct sieve_binary_header *header = &sbin->header;
-       time_t update_time = header->resource_usage.update_time;
+       time_t update_time = sbin->persisted_rusage.update_time;
        unsigned int timeout = sbin->svinst->set->resource_usage_timeout;
 
        if (update_time != 0 && (ioloop_time - update_time) > (time_t)timeout)
-               i_zero(&header->resource_usage);
+               i_zero(&sbin->persisted_rusage);
 
        sieve_resource_usage_init(rusage_r);
-       rusage_r->cpu_time_msecs = header->resource_usage.cpu_time_msecs;
+       rusage_r->cpu_time_msecs = sbin->persisted_rusage.cpu_time_msecs;
        sieve_resource_usage_add(rusage_r, &sbin->rusage);
 }
 
@@ -278,9 +277,7 @@ bool sieve_binary_record_resource_usage(
 void sieve_binary_set_resource_usage(struct sieve_binary *sbin,
                                     const struct sieve_resource_usage *rusage)
 {
-       struct sieve_binary_header *header = &sbin->header;
-
-       i_zero(&header->resource_usage);
+       i_zero(&sbin->persisted_rusage);
        sbin->rusage = *rusage;
        sbin->rusage_updated = TRUE;