MDEV-39061 mariadb-backup compatible wrappers for BACKUP SERVER by Thirunarayanan · Pull Request #5140 · MariaDB/server

Thirunarayanan · 2026-05-28T09:52:21Z

scripts/mariabackup/mariabackup.sh: a drop-in wrapper that
lets existing mariabackup --backup invocations drive the server-side
BACKUP SERVER command without changing user scripts.

mariabackup.sh translates
--backup into "BACKUP SERVER TO '<dir>'" via the mariadb client,
forwarding connection options, and layers
--stream/--compress as tar/gzip pipelines on the result. It
validates the target directory up front and rejects the

- mbstream.sh shims the mbstream CLI onto tar, dropping
mbstream-only flags (-p/--parallel) so legacy pipelines keep working.
- README.md maps every supported --backup option to its BACKUP SERVER
equivalent and documents current limitations.

Add include/have_mariabackup_wrapper.inc redirects
$XTRABACKUP to the wrapper so a test opts in by sourcing one
file; skips when the wrapper, bash, or the mariadb client
is unavailable.

wrapper_basic.test : Exercises full backup, streaming, compression,
the ignored legacy options

CLAassistant · 2026-05-28T09:52:32Z

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

gemini-code-assist

Code Review

This pull request introduces server-side backup support (BACKUP SERVER) for MariaDB, implementing backup and log archiving mechanisms for both the InnoDB and Aria storage engines. It also includes a compatibility wrapper script (mariabackup.sh) to map legacy mariabackup CLI commands to the new server-side SQL interface. The code review identified several critical issues, including a potential server crash in backup_innodb.cc due to invalid format arguments in error reporting, null pointer dereferences and assertion failures in both engines when backup steps are executed out of order or fail to initialize, and security vulnerabilities (command injection) and argument parsing bugs in the shell wrapper script.

gemini-code-assist · 2026-05-28T09:57:25Z

+        {
+        fail:
+          fail= 1;
+          my_error(ER_ERROR_ON_RENAME, MYF(ME_ERROR_LOG), s, d, errno);


Passing s and d (which are int file descriptors on Unix) to my_error with the ER_ERROR_ON_RENAME format specifier will cause a segmentation fault/crash because ER_ERROR_ON_RENAME expects string arguments (const char*). They should be replaced with the actual file names.

Suggested change

my_error(ER_ERROR_ON_RENAME, MYF(ME_ERROR_LOG), s, d, errno);

my_error(ER_ERROR_ON_RENAME, MYF(ME_ERROR_LOG), "ib_logfile101", dst.c_str(), errno);

gemini-code-assist · 2026-05-28T09:57:26Z

+int aria_backup_end(THD *thd, bool abort) noexcept
+{
+  int ret_val= aria_backup->end(thd, abort);
+  aria_backup.reset();
+  return ret_val;
+}


If aria_backup_start was never called (e.g., because another plugin's backup_start failed earlier in plugin_foreach_with_mask), aria_backup will be nullptr. When plugin_foreach_with_mask subsequently calls backup_end for all plugins, aria_backup_end will dereference aria_backup without a null check, causing a server crash. A null check should be added.

int aria_backup_end(THD *thd, bool abort) noexcept { if (!aria_backup) return 0; int ret_val= aria_backup->end(thd, abort); aria_backup.reset(); return ret_val; }

gemini-code-assist · 2026-05-28T09:57:26Z

+  int end(THD *thd, bool abort) noexcept
+  {
+    int fail= 0;
+    log_sys.latch.wr_lock();


If backup_start was never called or failed, max_first_lsn is 0. In this case, calling InnoDB_backup::end will trigger assertions like ut_ad(log_sys.archive) which can fail and crash the debug server. We should return early if max_first_lsn is 0.

int end(THD *thd, bool abort) noexcept { int fail= 0; log_sys.latch.wr_lock(); if (!max_first_lsn) { log_sys.latch.wr_unlock(); return 0; }

gemini-code-assist · 2026-05-28T09:57:26Z

+    if (!max_first_lsn)
+    {
+      ut_ad(queue.empty());
+      if (old_size)
+        delete_logs();
+      logs.clear();
+    }
+    log_sys.latch.wr_unlock();


If backup_start was never called or failed, max_first_lsn is 0. In this case, calling InnoDB_backup::fini will check the transaction state and trigger the assertion ut_ad("invalid state" == 0) because the transaction state is not TRX_STATE_BACKUP. We should return early if max_first_lsn is 0.

Suggested change

if (!max_first_lsn)

{

ut_ad(queue.empty());

if (old_size)

delete_logs();

logs.clear();

}

log_sys.latch.wr_unlock();

if (!max_first_lsn)

{

ut_ad(queue.empty());

if (old_size)

delete_logs();

logs.clear();

log_sys.latch.wr_unlock();

return 0;

}

log_sys.latch.wr_unlock();

gemini-code-assist · 2026-05-28T09:57:26Z

+if [[ -n "$FINAL_INCLUDE" ]]; then
+    echo "Setting backup_include='$FINAL_INCLUDE'" >&2
+    mariadb $MARIADB_OPTS -e "SET GLOBAL backup_include='$FINAL_INCLUDE'"
+fi
+
+if [[ -n "$FINAL_EXCLUDE" ]]; then
+    echo "Setting backup_exclude='$FINAL_EXCLUDE'" >&2
+    mariadb $MARIADB_OPTS -e "SET GLOBAL backup_exclude='$FINAL_EXCLUDE'"
+fi
+
+# Execute BACKUP SERVER
+SQL="BACKUP SERVER TO '$TARGET_DIR'"
+echo "Executing: $SQL" >&2
+mariadb $MARIADB_OPTS -e "$SQL"


The parameters FINAL_INCLUDE, FINAL_EXCLUDE, and TARGET_DIR are interpolated into shell commands. To effectively prevent command injection, we should validate these inputs using a strict whitelist of allowed characters (such as alphanumeric characters and a limited set of safe symbols like '-', '_', '.') instead of relying on a blacklist or simple escaping.

Suggested change

if [[ -n "$FINAL_INCLUDE" ]]; then

echo "Setting backup_include='$FINAL_INCLUDE'" >&2

mariadb $MARIADB_OPTS -e "SET GLOBAL backup_include='$FINAL_INCLUDE'"

fi

if [[ -n "$FINAL_EXCLUDE" ]]; then

echo "Setting backup_exclude='$FINAL_EXCLUDE'" >&2

mariadb $MARIADB_OPTS -e "SET GLOBAL backup_exclude='$FINAL_EXCLUDE'"

fi

# Execute BACKUP SERVER

SQL="BACKUP SERVER TO '$TARGET_DIR'"

echo "Executing: $SQL" >&2

mariadb $MARIADB_OPTS -e "$SQL"

if [[ -n "$FINAL_INCLUDE" && ! "$FINAL_INCLUDE" =~ ^[a-zA-Z0-9_.-]+$ ]]; then

echo "Error: Invalid characters in FINAL_INCLUDE" >&2

exit 1

fi

References

When validating input that will be interpolated into shell commands, use a strict whitelist of allowed characters (e.g., alphanumeric and a limited set of safe symbols like '-', '_', '.') instead of a blacklist to effectively prevent command injection.

gemini-code-assist · 2026-05-28T09:57:26Z

+TABLES_PATTERN=""
+TABLES_EXCLUDE_PATTERN=""
+TABLES_FILE=""
+MARIADB_OPTS=""


Handling MARIADB_OPTS as a flat string will break if any option contains spaces or special characters (e.g., --password="my secret pass"). We should use a bash array instead.

Suggested change

MARIADB_OPTS=""

MARIADB_OPTS=()

gemini-code-assist · 2026-05-28T09:57:26Z

+        --user=*|--password=*|--host=*|--port=*|--socket=*)
+            # Connection options (long form) - pass through to mariadb client
+            MARIADB_OPTS="$MARIADB_OPTS $1"
+            shift
+            ;;
+        -u|-p|-h|-P|-S)
+            # Short form. If the next argv looks like a value (not another
+            # flag), consume it as well — supports both `-u USER` and bare
+            # `-p` (which mariadb client treats as a password prompt).
+            if [[ -n "${2-}" && "$2" != -* ]]; then
+                MARIADB_OPTS="$MARIADB_OPTS $1 $2"
+                shift 2
+            else
+                MARIADB_OPTS="$MARIADB_OPTS $1"
+                shift
+            fi
+            ;;
+        -u*|-p*|-h*|-P*|-S*)
+            # Bundled short form: -uUSER, -pSECRET, -hHOST, ...
+            MARIADB_OPTS="$MARIADB_OPTS $1"
+            shift
+            ;;
+        --defaults-file=*|--defaults-extra-file=*)
+            # Config file options - pass through to mariadb client
+            MARIADB_OPTS="$MARIADB_OPTS $1"
+            shift
+            ;;


We should append connection options to the MARIADB_OPTS array instead of concatenating them into a flat string to correctly handle arguments with spaces or special characters.

--user=*|--password=*|--host=*|--port=*|--socket=*) # Connection options (long form) - pass through to mariadb client MARIADB_OPTS+=("$1") shift ;; -u|-p|-h|-P|-S) # Short form. If the next argv looks like a value (not another # flag), consume it as well — supports both `-u USER` and bare # `-p` (which mariadb client treats as a password prompt). if [[ -n "${2-}" && "$2" != -* ]]; then MARIADB_OPTS+=("$1" "$2") shift 2 else MARIADB_OPTS+=("$1") shift fi ;; -u*|-p*|-h*|-P*|-S*) # Bundled short form: -uUSER, -pSECRET, -hHOST, ... MARIADB_OPTS+=("$1") shift ;; --defaults-file=*|--defaults-extra-file=*) # Config file options - pass through to mariadb client MARIADB_OPTS+=("$1") shift ;;

gemini-code-assist · 2026-05-28T09:57:26Z

+  }
+
+#ifndef _WIN32
+  int dir= open(target.str, O_DIRECTORY);


To be fully standard and portable, open with O_DIRECTORY should explicitly include the access mode O_RDONLY.

Suggested change

int dir= open(target.str, O_DIRECTORY);

int dir= open(target.str, O_RDONLY | O_DIRECTORY);

gemini-code-assist · 2026-05-28T09:57:26Z

+    explicit Aria_backup(THD *thd, Target target) noexcept
+    : target(target)
+#ifndef _WIN32
+    , datadir_fd(open(maria_data_root, O_DIRECTORY))


To be fully standard and portable, open with O_DIRECTORY should explicitly include the access mode O_RDONLY.

, datadir_fd(open(maria_data_root, O_RDONLY | O_DIRECTORY))

grooverdan · 2026-05-29T05:50:51Z

Hey @Thirunarayanan , have you thought of how this is going to end up in packaging? It is replacing the original mariadb-backup?

It needs some INSTALL/INSTALL_SCRIPT cmake directives around this to give it a install location and a cmake component. debian installation/packaging would need the relevant debian/{package}.install to include te script.

scripts/mariabackup/mariabackup.sh: a drop-in wrapper that lets existing mariabackup invocations drive the server-side BACKUP SERVER command without changing user scripts. mariabackup.sh covers all four mariabackup modes. --backup translates into "BACKUP SERVER TO '<dir>'" via the mariadb client, forwarding connection options, and layers --stream/--compress as tar/gzip pipelines on the result. --prepare runs mariadbd --bootstrap on backup.cnf so InnoDB applies the archived redo log. --copy-back / --move-back drop a prepared backup into the datadir via cp -r / mv. --prepare --incremental-dir copies the incremental's ib_logfile* into the base and advances innodb_log_recovery_target; innodb_log_recovery_start stays pinned to the base checkpoint. --apply-log-only maps to --innodb-force-recovery=3 to skip rollback between incrementals. --rollback-xa runs two passes: normal recovery, then a second bootstrap with --tc-heuristic-recover=ROLLBACK. --copy-back / --move-back refuse a non-empty datadir unless --force-non-empty-directories is set, and print the post-action chown / systemctl start commands. For incremental --backup, innodb_log_archive_start is treated as a startup-only, read-only server invariant: the wrapper reads @@global.innodb_log_archive_start and fails fast if the archive floor exceeds the base backup's end LSN. Limitations: --export is accepted but not yet implemented; the wrapper prints a warning and runs plain recovery without producing the per-table .cfg files needed for ALTER TABLE ... IMPORT TABLESPACE. mbstream.sh shims the mbstream CLI onto tar, dropping mbstream-only flags (-p/--parallel) so legacy pipelines keep working. README.md maps every supported option per mode to its BACKUP SERVER equivalent and documents the backup.cnf format. Add include/have_mariabackup_wrapper.inc redirects $XTRABACKUP to the wrapper so a test opts in by sourcing one file; skips when the wrapper, bash, or the mariadb client is unavailable. wrapper_basic.test: exercises full backup, streaming, compression, the ignored legacy options.

Thirunarayanan requested a review from dr-m May 28, 2026 09:52

Thirunarayanan added the MariaDB Corporation label May 28, 2026

gemini-code-assist Bot reviewed May 28, 2026

View reviewed changes

dr-m force-pushed the MDEV-14992 branch from d595ce0 to c0e48fc Compare May 29, 2026 14:17

Thirunarayanan force-pushed the MDEV-39061 branch from 8d0dc79 to 051f681 Compare June 2, 2026 14:34

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

MDEV-39061 mariadb-backup compatible wrappers for BACKUP SERVER#5140

MDEV-39061 mariadb-backup compatible wrappers for BACKUP SERVER#5140
Thirunarayanan wants to merge 1 commit into
MDEV-14992from
MDEV-39061

Thirunarayanan commented May 28, 2026

Uh oh!

CLAassistant commented May 28, 2026 •

edited

Loading

Uh oh!

gemini-code-assist Bot left a comment

Uh oh!

gemini-code-assist Bot May 28, 2026

Uh oh!

gemini-code-assist Bot May 28, 2026

Uh oh!

gemini-code-assist Bot May 28, 2026

Uh oh!

gemini-code-assist Bot May 28, 2026

Uh oh!

gemini-code-assist Bot May 28, 2026

Uh oh!

gemini-code-assist Bot May 28, 2026

Uh oh!

gemini-code-assist Bot May 28, 2026

Uh oh!

gemini-code-assist Bot May 28, 2026

Uh oh!

gemini-code-assist Bot May 28, 2026

Uh oh!

grooverdan commented May 29, 2026

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

3 participants

	my_error(ER_ERROR_ON_RENAME, MYF(ME_ERROR_LOG), s, d, errno);
	my_error(ER_ERROR_ON_RENAME, MYF(ME_ERROR_LOG), "ib_logfile101", dst.c_str(), errno);

	int dir= open(target.str, O_DIRECTORY);
	int dir= open(target.str, O_RDONLY \| O_DIRECTORY);

Uh oh!

Conversation

Thirunarayanan commented May 28, 2026

Uh oh!

CLAassistant commented May 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist Bot May 28, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot May 28, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot May 28, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot May 28, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot May 28, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot May 28, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot May 28, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot May 28, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot May 28, 2026

Choose a reason for hiding this comment

Uh oh!

grooverdan commented May 29, 2026

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

3 participants

CLAassistant commented May 28, 2026 •

edited

Loading