Re: [PATCH cluster 03/21] pmxcfs: add live backup capability

[Wolfgang Bumiller <w.bumiller@proxmox.com>]

On Fri, Aug 28, 2026 at 03:30:12PM +0200, Shannon Sterz wrote:
> Signed-off-by: Shannon Sterz <s.sterz@proxmox.com>
> ---
>  src/PVE/Cluster.pm       |  49 ++++++++++++++-
>  src/pmxcfs/cfs-ipc-ops.h |   2 +
>  src/pmxcfs/cfs-utils.h   |   1 +
>  src/pmxcfs/database.c    | 128 +++++++++++++++++++++++++++++++++++++++
>  src/pmxcfs/memdb.c       |  30 +++++++++
>  src/pmxcfs/memdb.h       |  94 ++++++++++++++++++++++++++++
>  src/pmxcfs/server.c      |  94 ++++++++++++++++++++++++++++
>  7 files changed, 395 insertions(+), 3 deletions(-)
> 
> diff --git a/src/PVE/Cluster.pm b/src/PVE/Cluster.pm
> index 034b78c..31c06e8 100644
> --- a/src/PVE/Cluster.pm
> +++ b/src/PVE/Cluster.pm
> @@ -42,6 +42,9 @@ my $lockdir = "/etc/pve/priv/lock";
>  my $dbfile = "/var/lib/pve-cluster/config.db";
>  my $dbbackupdir = "/var/lib/pve-cluster/backup";
>  
> +# maximum number of files to keep for each db backup type (live, cluster join)
> +my $db_backup_maxfiles = 10;
> +
>  # this is just a readonly copy, the relevant one is in status.c from pmxcfs
>  # observed files are the one we can get directly through IPCC, they are cached
>  # using a computed version and only those can be used by the cfs_*_file methods
> @@ -236,6 +239,14 @@ my $ipcc_verify_token = sub {
>      die "$!\n";
>  };
>  
> +my $ipcc_init_backup = sub {
> +    my ($filename) = @_;
> +
> +    my $bindata = pack "Z*", $filename;
> +    PVE::IPCC::ipcc_send_rec(CFS_IPC_INIT_BACKUP, $bindata);
> +    warn $! if $! != 0;
> +};
> +
>  my $ccache = {};
>  
>  sub cfs_update {
> @@ -906,11 +917,10 @@ sub cfs_backup_database {
>      my $cmd = [['sqlite3', $dbfile, '.dump'], ['gzip', '-', \ ">${backup_fn}"]];
>      run_command($cmd, 'errmsg' => "cannot backup old database\n");
>  
> -    my $maxfiles = 10; # purge older backup
>      my $backups = [sort { $b cmp $a } <$dbbackupdir/config-*.sql.gz>];
>  
> -    if ((my $count = scalar(@$backups)) > $maxfiles) {
> -        foreach my $f (@$backups[$maxfiles .. $count - 1]) {
> +    if ((my $count = scalar(@$backups)) > $db_backup_maxfiles) {
> +        foreach my $f (@$backups[$db_backup_maxfiles .. $count - 1]) {
>              next if $f !~ m/^(\S+)$/; # untaint
>              print "delete old backup '$1'\n";
>              unlink $1;
> @@ -929,4 +939,37 @@ sub cfs_rename_db_unsafe {
>      rename $dbfile, "$dbfile.$ctime.bak" or warn "failed to rename old config database - $!\n";
>  }
>  
> +=head3 cfs_live_backup_database()
> +
> +Creates a new live backup of the database backing the cluster file system.
> +
> +Returns a file path to the created backup.
> +
> +=cut
> +
> +sub cfs_live_backup_database {
> +    mkdir $dbbackupdir or $!{EEXIST} or die "failed to create backup dir - $!\n";

^ While we do need to keep the `chmod` call below (due to the old code
existing), we should just pass `0700` to the `mkdir` invocation directly
as well (and also add it in `cfs_backup_database()`), to not create
unnecessary races.

> +    chmod 0700, $dbbackupdir or die "failed to change mode for backup dir - $!\n";
> +

>From here…

> +    my $backups = [sort { $b cmp $a } glob("$dbbackupdir/config-backup-*.db")];
> +
> +    if ((my $count = scalar(@$backups)) > $db_backup_maxfiles) {
> +        foreach my $f (@$backups[$db_backup_maxfiles .. $count - 1]) {
> +            next if $f !~ m/^(\S+)$/; # untaint
> +            print "cleaning up old backup '$1'\n";
> +            unlink $1;
> +        }
> +    }

…to here, this exact code already exists in the sub above it - factor it
out.

Although there is a small difference: the sub above it cleans up *after*
creating the new file.

If we only keep 1 file, we now have a time period where the last backup
was deleted and the new one doesn't exist yet.
Perhaps this one should pass `$db_backup_maxfiles + 1` to the
to-be-added cleanup helper.

> +
> +    my $ctime = time();
> +    my $backup_file = "config-backup-$ctime.db";
> +    print "starting backup of current database to \"$dbbackupdir/$backup_file\"\n";
> +
> +    eval { &$ipcc_init_backup($backup_file); };
> +
> +    warn $@ if $@;
> +
> +    return $backup_file;
> +}
> +
>  1;
> diff --git a/src/pmxcfs/cfs-ipc-ops.h b/src/pmxcfs/cfs-ipc-ops.h
> index 249308d..054ebb3 100644
> --- a/src/pmxcfs/cfs-ipc-ops.h
> +++ b/src/pmxcfs/cfs-ipc-ops.h
> @@ -45,4 +45,6 @@
>  
>  #define CFS_IPC_GET_GUEST_CONFIG_PROPERTIES 13
>  
> +#define CFS_IPC_INIT_BACKUP 14
> +
>  #endif
> diff --git a/src/pmxcfs/cfs-utils.h b/src/pmxcfs/cfs-utils.h
> index ed67264..19e3aab 100644
> --- a/src/pmxcfs/cfs-utils.h
> +++ b/src/pmxcfs/cfs-utils.h
> @@ -32,6 +32,7 @@
>  #define HOST_CLUSTER_CONF_FN "/etc/corosync/corosync.conf"
>  #define CFS_PID_FN "/var/run/pve-cluster.pid"
>  #define VARLIBDIR "/var/lib/pve-cluster"
> +#define BACKUPDIR VARLIBDIR "/backup"
>  #define RUNDIR "/run/pve-cluster"
>  
>  #define CFS_MAX(a, b) (((a) > (b)) ? (a) : (b))
> diff --git a/src/pmxcfs/database.c b/src/pmxcfs/database.c
> index 4ee2389..07b3664 100644
> --- a/src/pmxcfs/database.c
> +++ b/src/pmxcfs/database.c
> @@ -49,6 +49,8 @@ struct db_backend {
>      sqlite3_stmt *stmt_commit;
>      sqlite3_stmt *stmt_rollback;
>      sqlite3_stmt *stmt_load_all;
> +    sqlite3_backup *current_backup;
> +    sqlite3 *backup_db;
>  };
>  
>  #define VERSIONFILENAME "__version__"
> @@ -650,6 +652,10 @@ void bdb_backend_close(db_backend_t *bdb) {
>      sqlite3_finalize(bdb->stmt_rollback);
>      sqlite3_finalize(bdb->stmt_load_all);
>  
> +    // calling bdb_finish_or_abort_backup() has no effect if no backup is happening, so calling it
> +    // unconditionally here is ok



> +    (void)bdb_finish_or_abort_backup(bdb, TRUE);
> +
>      int rc;
>      if ((rc = sqlite3_close(bdb->db)) != SQLITE_OK) {
>          cfs_critical("sqlite3_close failed: %d\n", rc);
> @@ -738,3 +744,125 @@ fail:
>  
>      return NULL;
>  }
> +
> +int bdb_init_backup(db_backend_t *bdb, const char *backup_filename) {
> +    g_return_val_if_fail(bdb != NULL, BACKUP_RETURN_EPAR);
> +    g_return_val_if_fail(bdb->db != NULL, BACKUP_RETURN_EPAR);
> +    g_return_val_if_fail(backup_filename != NULL, BACKUP_RETURN_EPAR);
> +
> +    if (bdb->current_backup != NULL || bdb->backup_db != NULL) {
> +        cfs_debug("trying to start a new backup while one is already running, aborting...");
> +        return BACKUP_RETURN_EGEN;
> +    }
> +
> +    int rc;
> +    g_autofree gchar *canonical_filename = g_canonicalize_filename(backup_filename, BACKUPDIR);
> +    cfs_message("starting live backup '%s'...", canonical_filename);
> +
> +    int flags = SQLITE_OPEN_READWRITE | SQLITE_OPEN_CREATE;
> +
> +    if ((rc = sqlite3_open_v2(canonical_filename, &bdb->backup_db, flags, NULL)) != SQLITE_OK) {
> +        cfs_critical("sqlite error while opening backup file (%d): %s", rc, sqlite3_errstr(rc));
> +        (void)sqlite3_close_v2(bdb->backup_db);
> +        bdb->backup_db = NULL;
> +        return BACKUP_RETURN_EGEN;
> +    }
> +
> +    if (chmod(canonical_filename, 0600) == -1) {

(*sigh*, why don't they take a mode in the sqlite3 api - we don't want
to change the umask in threaded code -_-. Then again, the directory is
already meant to be 0700 anyway, so 🤷)

> +        cfs_critical("chmod failed for backup db: %s", strerror(errno));
> +        (void)sqlite3_close_v2(bdb->backup_db);
> +        bdb->backup_db = NULL;
> +        return BACKUP_RETURN_EGEN;
> +    }
> +
> +    bdb->current_backup = sqlite3_backup_init(bdb->backup_db, "main", bdb->db, "main");
> +
> +    if (bdb->current_backup == NULL) {
> +        rc = sqlite3_errcode(bdb->backup_db);
> +        cfs_critical("sqlite error while initializing backup (%d): %s", rc, sqlite3_errstr(rc));
> +        (void)sqlite3_close_v2(bdb->backup_db);
> +        bdb->backup_db = NULL;
> +        return BACKUP_RETURN_EGEN;
> +    }
> +
> +    return BACKUP_RETURN_CONT;
> +}
> +
> +int bdb_finish_or_abort_backup(db_backend_t *bdb, gboolean abort) {
> +    g_return_val_if_fail(bdb != NULL, BACKUP_RETURN_EPAR);
> +
> +    int rc;
> +    int to_return_code = BACKUP_RETURN_DONE;
> +    GString *db_filename = NULL;

Use
    g_autoptr(GString) db_filename = NULL;

and drop the `g_string_free` call at the end.

> +
> +    if (bdb->backup_db != NULL) {

Does it not make sense to just

    if (!bdb->backup_db) return;

?

Can `current_backup` be set without `backup_db`?

> +        sqlite3_filename fn;
> +        if ((fn = sqlite3_db_filename(bdb->backup_db, "main")) != NULL) {

IMO that's bad form - do the assignment in the line above and just
`if (fn != NULL)`

> +            // the filename is managed by the sqlite3 object, so copy it into its own string to
> +            // preserve it across the `sqlite3_close_v2()` call.
> +            db_filename = g_string_new(fn);
> +        };
> +    }
> +
> +    if ((rc = sqlite3_backup_finish(bdb->current_backup)) != SQLITE_OK) {
> +        cfs_critical("sqlite error when ending backup (%d): %s", rc, sqlite3_errstr(rc));
> +        to_return_code = BACKUP_RETURN_EGEN;
> +    }
> +
> +    bdb->current_backup = NULL;
> +
> +    if ((rc = sqlite3_close_v2(bdb->backup_db)) != SQLITE_OK) {
> +        cfs_critical("sqlite error when ending backup (%d): %s", rc, sqlite3_errstr(rc));
> +        to_return_code = BACKUP_RETURN_EGEN;
> +    }
> +
> +    bdb->backup_db = NULL;
> +
> +    // if abort is true or an error was encountered, we need to clean up the database file here
> +    if ((abort || to_return_code != BACKUP_RETURN_DONE) && db_filename != NULL) {
> +        (void)unlink(db_filename->str);

nit: should we warn if this fails?

> +    }
> +
> +    if (db_filename != NULL) {
> +        g_string_free(db_filename, TRUE);
> +    }
> +
> +    return to_return_code;
> +}
> +
> +int bdb_handle_backup(db_backend_t *bdb) {
> +    g_return_val_if_fail(bdb != NULL, BACKUP_RETURN_EPAR);
> +    g_return_val_if_fail(bdb->db != NULL, BACKUP_RETURN_EPAR);
> +
> +    if (bdb->backup_db == NULL || bdb->current_backup == NULL) {
> +        cfs_debug("no backup in progress, please initialize one first...");
> +        return BACKUP_RETURN_EGEN;
> +    }
> +
> +    // `backup_db` and `current_backup` are initialized successfully, do a back up step
> +    int rc = sqlite3_backup_step(bdb->current_backup, 10);

How is the number 10 chosen? Does it make sense to have this tuneable?

> +
> +    if (rc == SQLITE_OK || rc == SQLITE_BUSY || rc == SQLITE_LOCKED) {
> +        // backup step succeeded or encountered a non-permanent error. backup is not done.
> +        // -> signal that backup step is done and another one should be scheduled
> +        return BACKUP_RETURN_CONT;
> +    }
> +
> +    int to_return_code = BACKUP_RETURN_DONE;
> +
> +    if (rc != SQLITE_DONE) {
> +        cfs_critical("sqlite error during backup step (%d): %s", rc, sqlite3_errstr(rc));
> +        to_return_code = BACKUP_RETURN_EGEN;
> +    } else {
> +        cfs_debug("backup completed, cleaning up...");
> +    }
> +
> +    // abort the backup if we encountered an error
> +    rc = bdb_finish_or_abort_backup(bdb, to_return_code != BACKUP_RETURN_DONE);
> +
> +    if (rc != BACKUP_RETURN_DONE) {
> +        to_return_code = BACKUP_RETURN_EGEN;
> +    }
> +
> +    return to_return_code;
> +}
> diff --git a/src/pmxcfs/memdb.c b/src/pmxcfs/memdb.c
> index 77d7652..b7372f9 100644
> --- a/src/pmxcfs/memdb.c
> +++ b/src/pmxcfs/memdb.c
> @@ -1520,3 +1520,33 @@ memdb_index_t *memdb_encode_index(GHashTable *index, memdb_tree_entry_t *root) {
>  
>      return idx;
>  }
> +
> +int memdb_init_backup(memdb_t *memdb, const char *backup_filename) {
> +    g_return_val_if_fail(memdb != NULL, BACKUP_RETURN_EPAR);
> +
> +    g_mutex_lock(&memdb->mutex);
> +    int to_return = bdb_init_backup(memdb->bdb, backup_filename);
> +    g_mutex_unlock(&memdb->mutex);
> +
> +    return to_return;
> +}
> +
> +int memdb_finish_or_abort_backup(memdb_t *memdb, gboolean abort) {
> +    g_return_val_if_fail(memdb != NULL, BACKUP_RETURN_EPAR);
> +
> +    g_mutex_lock(&memdb->mutex);
> +    int to_return = bdb_finish_or_abort_backup(memdb->bdb, abort);
> +    g_mutex_unlock(&memdb->mutex);
> +
> +    return to_return;
> +}
> +
> +int memdb_handle_backup(memdb_t *memdb) {
> +    g_return_val_if_fail(memdb != NULL, BACKUP_RETURN_EPAR);
> +
> +    g_mutex_lock(&memdb->mutex);
> +    int to_return = bdb_handle_backup(memdb->bdb);
> +    g_mutex_unlock(&memdb->mutex);
> +
> +    return to_return;
> +}
> diff --git a/src/pmxcfs/memdb.h b/src/pmxcfs/memdb.h
> index bca8967..2c27959 100644
> --- a/src/pmxcfs/memdb.h
> +++ b/src/pmxcfs/memdb.h
> @@ -34,6 +34,11 @@
>  #define MEMDB_BLOCKSIZE 4096
>  #define MEMDB_BLOCKS ((MEMDB_MAX_FSSIZE + MEMDB_BLOCKSIZE - 1) / MEMDB_BLOCKSIZE)
>  
> +#define BACKUP_RETURN_CONT 0 // backup step completed, schedule next one
> +#define BACKUP_RETURN_DONE 1 // backup done
> +#define BACKUP_RETURN_EGEN 2 // encountered error while trying to back up
> +#define BACKUP_RETURN_EPAR 3 // a parameter passed to the backup helper functions was invalid

Do we have a precedent for such a numeric return code?
Otherwise, if we do such a thing, IMO the error cases should be
negative. I'm also not sure a "generic" vs "parameter" error is a useful
distinction, except for debugging, where in-place error logging with
just returning -1/EGEN would be more useful.

The exact value is actually only checked for `memdb_handle_backup` where
the `EPAR` case is AFAICT impossible anyway.

And eg. `bdb_init_backup`/`memdb_init_backup` are just checked for
`CONT`, so a boolean might be a better return type for that one. No use
in having a lot of dead cases around.

> +
>  typedef struct memdb_tree_entry memdb_tree_entry_t;
>  struct memdb_tree_entry {
>      guint64 parent;
> @@ -151,6 +156,50 @@ memdb_index_t *memdb_index_copy(memdb_index_t *idx);
>  
>  gboolean memdb_tree_entry_csum(memdb_tree_entry_t *te, guchar csum[32]);
>  
> +/**
> + * memdb_init_backup:
> + * @memdb: a memdb object that has an initialized db to back up.
> + * @backup_filename: the name for the backup file.
> + *
> + * Initializes a backup of the configuration database.
> + *
> + * Return: an integer representing the status of the backup. Either `BACKUP_RETURN_CONT` if the
> + * backup was initialized correctly or an error such as `BACKUP_RETURN_EGEN` or
> + * `BACKUP_RETURN_EPAR`.
> + */
> +int memdb_init_backup(memdb_t *memdb, const char *backup_filename);
> +
> +/**
> + * memdb_finish_or_abort_backup:
> + * @memdb: a memdb object that is currently being backed up.
> + * @abort: if true, this will be considered an abort operation and the backup database file will
> + * be removed.
> + *
> + * Will finish a backup no matter whether all backup steps have completed successfully. If they
> + * haven't, the backup is invalid and all write operations should be rolled back by sqlite. If an
> + * error occurs while finishing the backup or if abort is set, the database file will be removed.
> + *
> + * Calling this function while no backup is in progress is harmless (similar to how NULL can
> + * be passed to sqlite3_backup_finish()).
> + *
> + * Return: an integer representing the result of the operation. Either `BACKUP_RETURN_DONE` if the
> + * close operation was successful or `BACKUP_RETURN_EGEN` if finishing the backup encountered an
> + * error. If no backup is in progress, `BACKUP_RETURN_EPAR` will be returned.
> + */
> +int memdb_finish_or_abort_backup(memdb_t *memdb, gboolean abort);
> +
> +/**
> + * memdb_handle_backup:
> + * @memdb: a memdb object that has an initialized db to back up.
> + *
> + * Handles a single backup step and finalizes the backup if it is done.
> + *
> + * Return: an integer representing the status of the backup. Either `BACKUP_RETURN_CONT` if the
> + * backup was initialized correctly, `BACKUP_RETURN_DONE` if the backup is complete or an error such
> + * as `BACKUP_RETURN_EGEN` or `BACKUP_RETURN_EPAR`.
> + */
> +int memdb_handle_backup(memdb_t *memdb);
> +
>  db_backend_t *bdb_backend_open(const char *filename, memdb_tree_entry_t *root, GHashTable *index);
>  
>  void bdb_backend_close(db_backend_t *bdb);
> @@ -173,4 +222,49 @@ gboolean bdb_backend_commit_update(
>      memdb_t *memdb, memdb_index_t *master, memdb_index_t *slave, GList *inodes
>  );
>  
> +/**
> + * bdb_init_backup:
> + * @bdb: a database backend that has an initialized db to back up.
> + * @backup_filename: the name for the backup file.
> + *
> + * Initializes a backup of the configuration database.
> + *
> + * Return: an integer representing the status of the backup. Either `BACKUP_RETURN_CONT` if the
> + * backup was initialized correctly or an error such as `BACKUP_RETURN_EGEN` or
> + * `BACKUP_RETURN_EPAR`.
> + */
> +int bdb_init_backup(db_backend_t *bdb, const char *backup_filename);
> +
> +/**
> + * bdb_finish_or_abort_backup:
> + * @bdb: a database backend that is currently being backed up.
> + * @abort: if true, this will be considered an abort operation and the backup database file will
> + * be removed.
> + *
> + * Will finish a backup no matter whether all backup steps have completed successfully. If they
> + * haven't, the backup is invalid and all write operations should be rolled back by sqlite. If an
> + * error occurs while finishing the backup or if abort is set, the database file will be removed.
> + *
> + * Calling this function while no backup is in progress is harmless (similar to how NULL can
> + * be passed to sqlite3_backup_finish()).
> +
> + *
> + * Return: an integer representing the result of the operation. Either `BACKUP_RETURN_DONE` if the
> + * close operation was successful or `BACKUP_RETURN_EGEN` if finishing the backup encountered an
> + * error. If no backup is in progress, `BACKUP_RETURN_EPAR` will be returned.
> + */
> +int bdb_finish_or_abort_backup(db_backend_t *bdb, gboolean abort);
> +
> +/**
> + * bdb_handle_backup:
> + * @bdb: a database backend that has an initialized db to back up.
> + *
> + * Handles a single backup step and finalizes the backup if it is done.
> + *
> + * Return: an integer representing the status of the backup. Either `BACKUP_RETURN_CONT` if the
> + * backup was initialized correctly, `BACKUP_RETURN_DONE` if the backup is complete or an error such
> + * as `BACKUP_RETURN_EGEN` or `BACKUP_RETURN_EPAR`.
> + */
> +int bdb_handle_backup(db_backend_t *bdb);
> +
>  #endif /* _PVE_MEMDB_H_ */
> diff --git a/src/pmxcfs/server.c b/src/pmxcfs/server.c
> index 278e2c9..8149fd3 100644
> --- a/src/pmxcfs/server.c
> +++ b/src/pmxcfs/server.c
> @@ -48,6 +48,7 @@ static qb_loop_t *loop;
>  static qb_ipcs_service_t *s1;
>  static GString *outbuf;
>  static memdb_t *memdb;
> +static GRegex *filename_regex;
>  
>  static int server_started = 0;   /* protect with server_started_mutex */
>  static int terminate_server = 0; /* protect with server_started_mutex */
> @@ -100,6 +101,11 @@ typedef struct {
>      char token[];
>  } cfs_verify_token_request_header_t;
>  
> +typedef struct {
> +    struct qb_ipc_request_header req_header;
> +    char filename[];
> +} cfs_init_backup_request_header_t;
> +
>  struct s1_context {
>      int32_t client_pid;
>      uid_t uid;
> @@ -107,6 +113,37 @@ struct s1_context {
>      gboolean read_only;
>  };
>  
> +static void backup_job(void *data) {
> +    cfs_debug("handling backup step");
> +
> +    switch (memdb_handle_backup(memdb)) {
> +    case BACKUP_RETURN_CONT:
> +        cfs_debug("backup iteration done, scheduling next one");
> +
> +        int rc = qb_loop_job_add(loop, QB_LOOP_LOW, NULL, backup_job);
> +
> +        if (rc != 0) {
> +            // Abort the backup as we can't finish it without scheduling a job.
> +            cfs_critical("could not schedule next backup job (error %d), aborting backup...", rc);
> +            (void)memdb_finish_or_abort_backup(memdb, TRUE);
> +        }
> +
> +        break;
> +    case BACKUP_RETURN_DONE:
> +        cfs_message("live backup complete.");
> +        break;
> +    case BACKUP_RETURN_EGEN:
> +        cfs_debug("encountered an error while backing up.");
> +        break;
> +    case BACKUP_RETURN_EPAR:
> +        cfs_debug("a parameter passed to one of the backup functions was invalid.");
> +        break;
> +    default:
> +        cfs_debug("encountered an unknown return code while backing up.");
> +        break;
> +    }
> +}
> +
>  static int32_t s1_connection_accept_fn(qb_ipcs_connection_t *c, uid_t uid, gid_t gid) {
>      if ((uid == 0 && gid == 0) || (gid == cfs.gid)) {
>          cfs_debug("authenticated connection %d/%d", uid, gid);
> @@ -446,6 +483,45 @@ static int32_t s1_msg_process_fn(qb_ipcs_connection_t *c, void *data, size_t siz
>                  result = -ENOENT;
>              }
>          }
> +    } else if (request_id == CFS_IPC_INIT_BACKUP) {
> +        cfs_init_backup_request_header_t *rh = (cfs_init_backup_request_header_t *)data;
> +        int filename_len =
> +            request_size - G_STRUCT_OFFSET(cfs_init_backup_request_header_t, filename) - 1;
> +
> +        if (ctx->read_only) {
> +            result = -EPERM;
> +        } else if (filename_len <= 0) {
> +            cfs_debug("backup filename_len <= 0, %d", filename_len);
> +            result = -EINVAL;
> +        } else if (rh->filename[filename_len] != '\0') {
> +            cfs_debug("backup file not NULL-terminated");
> +            result = -EINVAL;
> +        } else if (strnlen(rh->filename, filename_len) != filename_len) {
> +            cfs_debug("backup file contains NULL-byte");
> +            result = -EINVAL;
> +        } else {
> +            if (filename_regex != NULL &&
> +                g_regex_match(filename_regex, rh->filename, G_REGEX_MATCH_DEFAULT, NULL)) {
> +
> +                if (memdb_init_backup(memdb, rh->filename) != BACKUP_RETURN_CONT) {
> +                    cfs_debug("could not initialize a backup");
> +                    result = -EIO;
> +                } else {
> +                    result = qb_loop_job_add(loop, QB_LOOP_LOW, NULL, backup_job);
> +                    if (result != 0) {
> +                        cfs_critical("couldn't add first backup step (error %d), abort...", result);
> +                        (void)memdb_finish_or_abort_backup(memdb, TRUE);
> +                    }
> +                }
> +
> +                if (result != 0) {
> +                    cfs_debug("could not start backup");
> +                }
> +            } else {
> +                cfs_debug("could not check backup file name or it contains forbidden characters");
> +                result = -EINVAL;
> +            }
> +        }
>      }
>  
>      cfs_debug("process result %d", result);
> @@ -577,6 +653,18 @@ gboolean server_start(memdb_t *db) {
>  
>      outbuf = g_string_sized_new(8192 * 8);
>  
> +    // set up filename regex on start
> +    filename_regex = g_regex_new(
> +        "^[A-Za-z0-9\\-_][A-Za-z0-9\\-_\\.]*$", G_REGEX_DEFAULT, G_REGEX_MATCH_DEFAULT, NULL
> +    );
> +
> +    if (filename_regex == NULL) {
> +        // Shouldn't happen since we compile a static pattern. If it does, don't abort. pmxcfs
> +        // will not allow live backups in this case, but should function fine otherwise. This is
> +        // preferable over not allowing users to run pmxcfs at all in that scenario.
> +        cfs_message("could not compile filename regex. live backups are not supported.");
> +    }
> +
>      if (!(loop = qb_loop_create())) {
>          cfs_critical("cant create event loop");
>          return FALSE;
> @@ -625,6 +713,12 @@ void server_stop(void) {
>          loop = NULL;
>      }
>  
> +    // tear down regex on server stop
> +    if (filename_regex != NULL) {
> +        g_regex_unref(filename_regex);
> +        filename_regex = NULL;
> +    }
> +
>      if (outbuf) {
>          g_string_free(outbuf, TRUE);
>          outbuf = NULL;
> -- 
> 2.47.3
> 
> 
> 
> 
> 

--