MDEV-39061 mariadb-backup compatible wrappers for BACKUP SERVER

[Thirunarayanan]

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]

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
0 out of 3 committers have signed the CLA.

❌ mariadb-andrzejjarzabek
❌ dr-m
❌ Thirunarayanan
_{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]

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]

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]

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]

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]

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

1. 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]

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]

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]

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]

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]

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.