Skip to content

Preserve XFS quota options in temporary mounts #1132

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
vojtechtrefny merged 2 commits into from
Oct 8, 2025
Merged

Preserve XFS quota options in temporary mounts #1132

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6b6239e
fs: Log mount and unmount calls
vojtechtrefny Oct 7, 2025
10b3cdf
fs: Preserve XFS quota options in temporary mounts
vojtechtrefny Oct 7, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
71 changes: 70 additions & 1 deletion src/plugins/fs/generic.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -652,6 +652,57 @@ gchar* bd_fs_get_fstype (const gchar *device, GError **error) {
return fstype;
}

/* taken from kernel source: fs/xfs/libxfs/xfs_log_format.h: */
#define _XFS_UQUOTA_ACCT 0x0001 /* user quota accounting ON */
#define _XFS_UQUOTA_ENFD 0x0002 /* user quota limits enforced */
#define _XFS_GQUOTA_ACCT 0x0040 /* group quota accounting ON */
#define _XFS_GQUOTA_ENFD 0x0080 /* group quota limits enforced */
#define _XFS_PQUOTA_ACCT 0x0008 /* project quota accounting ON */
#define _XFS_PQUOTA_ENFD 0x0200 /* project quota limits enforced */

static gboolean xfs_quota_mount_options (const gchar *device, GString *options, GError **error) {
g_autofree gchar *output = NULL;
gboolean success = FALSE;
const gchar *args[8] = {"xfs_db", "-r", device, "-c", "sb 0", "-c", "p qflags", NULL};
guint64 flags = 0;

success = bd_utils_exec_and_capture_output (args, NULL, &output, error);
if (!success) {
/* error is already populated */
return FALSE;
}

if (!g_str_has_prefix (output, "qflags = ")) {
g_set_error (error, BD_FS_ERROR, BD_FS_ERROR_FAIL,
"Failed to parse XFS quota flags for %s from %s.", device, output);
return FALSE;
}

flags = g_ascii_strtoull (output + 9, NULL, 16);
if (flags & _XFS_UQUOTA_ACCT) {
if (flags & _XFS_UQUOTA_ENFD)
g_string_append_printf (options, "uquota,");
else
g_string_append_printf (options, "uqnoenforce,");
}

if (flags & _XFS_GQUOTA_ACCT) {
if (flags & _XFS_GQUOTA_ENFD)
g_string_append_printf (options, "gquota,");
else
g_string_append_printf (options, "gqnoenforce,");
}

if (flags & _XFS_PQUOTA_ACCT) {
if (flags & _XFS_PQUOTA_ENFD)
g_string_append_printf (options, "pquota,");
else
g_string_append_printf (options, "pqnoenforce,");
}

return TRUE;
}

/**
* fs_mount:
* @device: the device to mount for an FS operation
Expand All @@ -672,6 +723,8 @@ static gchar* fs_mount (const gchar *device, gchar *fstype, gboolean read_only,
gchar *mountpoint = NULL;
gboolean ret = FALSE;
GError *l_error = NULL;
GString *options = NULL;
g_autofree gchar *options_str = NULL;

mountpoint = bd_fs_get_mountpoint (device, &l_error);
if (!mountpoint) {
Expand All @@ -683,7 +736,23 @@ static gchar* fs_mount (const gchar *device, gchar *fstype, gboolean read_only,
"Failed to create temporary directory for mounting '%s'.", device);
return NULL;
}
ret = bd_fs_mount (device, mountpoint, fstype, read_only ? "nosuid,nodev,ro" : "nosuid,nodev", NULL, &l_error);

options = g_string_new (NULL);
if (g_strcmp0 (fstype, "xfs") == 0) {
ret = xfs_quota_mount_options (device, options, &l_error);
if (!ret) {
bd_utils_log_format (BD_UTILS_LOG_INFO, "Failed to get XFS mount options: %s",
l_error->message);
g_clear_error (&l_error);
}
}
if (read_only)
g_string_append_printf (options, "nosuid,nodev,ro");
else
g_string_append_printf (options, "nosuid,nodev");
options_str = g_string_free (options, FALSE);

ret = bd_fs_mount (device, mountpoint, fstype, options_str, NULL, &l_error);
if (!ret) {
g_propagate_prefixed_error (error, l_error, "Failed to mount '%s': ", device);
if (g_rmdir (mountpoint) != 0)
Expand Down
10 changes: 10 additions & 0 deletions src/plugins/fs/mount.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@
#include <errno.h>
#include <string.h>

#include <blockdev/utils.h>

#include "fs.h"
#include "mount.h"

Expand Down Expand Up @@ -173,6 +175,8 @@ static gboolean do_unmount (MountArgs *args, GError **error) {
}
}

bd_utils_log_format (BD_UTILS_LOG_INFO, "Unmounting %s", args->spec);

ret = mnt_context_umount (cxt);
#ifdef LIBMOUNT_NEW_ERR_API
success = get_unmount_error_new (cxt, ret, args->spec, error);
Expand Down Expand Up @@ -424,6 +428,12 @@ static gboolean do_mount (MountArgs *args, GError **error) {
mnt_context_enable_rwonly_mount (cxt, TRUE);
#endif

bd_utils_log_format (BD_UTILS_LOG_INFO, "Mounting %s (fstype %s) to %s with options: %s",
args->device ? args->device : "unspecified device",
args->fstype ? args->fstype : "auto",
args->mountpoint ? args->mountpoint : "unspecified mountpoint",
args->options ? args->options : "none");

ret = mnt_context_mount (cxt);

/* we need to always do some libmount magic to check if the mount really
Expand Down
4 changes: 2 additions & 2 deletions tests/fs_tests/fs_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@


@contextmanager
def mounted(device, where, ro=False):
utils.mount(device, where, ro)
def mounted(device, where, ro=False, options=None):
utils.mount(device, where, ro, options)

try:
yield
Expand Down
55 changes: 47 additions & 8 deletions tests/fs_tests/generic_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -911,6 +911,15 @@ def test_udf_generic_set_uuid(self):


class GenericResize(GenericTestCase):
log = ""

def my_log_func(self, level, msg):
# not much to verify here
self.assertTrue(isinstance(level, int))
self.assertTrue(isinstance(msg, str))

self.log += msg + "\n"

def _test_generic_resize(self, mkfs_function, fstype, size_delta=0, min_size=130*1024**2):
# clean the device
succ = BlockDev.fs_clean(self.loop_devs[0])
Expand Down Expand Up @@ -1024,35 +1033,65 @@ def test_xfs_generic_resize(self):
with self.assertRaises(GLib.GError):
succ = BlockDev.fs_resize(lv, 40 * 1024**2)

self._lvresize("libbd_fs_tests", "generic_test", "400M")
# should grow to 400 MiB (full size of the LV)
self._lvresize("libbd_fs_tests", "generic_test", "380M")
# should grow to 375 MiB (full size of the LV)
with mounted(lv, self.mount_dir):
succ = BlockDev.fs_resize(lv, 0)
self.assertTrue(succ)
with mounted(lv, self.mount_dir):
fi = BlockDev.fs_xfs_get_info(lv)
self.assertTrue(fi)
self.assertEqual(fi.block_size * fi.block_count, 400 * 1024**2)
self.assertEqual(fi.block_size * fi.block_count, 380 * 1024**2)

self._lvresize("libbd_fs_tests", "generic_test", "450M")
# grow just to 430 MiB
self._lvresize("libbd_fs_tests", "generic_test", "400M")
# grow just to 390 MiB
with mounted(lv, self.mount_dir):
succ = BlockDev.fs_resize(lv, 430 * 1024**2)
succ = BlockDev.fs_resize(lv, 390 * 1024**2)
self.assertTrue(succ)
with mounted(lv, self.mount_dir):
fi = BlockDev.fs_xfs_get_info(lv)
self.assertTrue(fi)
self.assertEqual(fi.block_size * fi.block_count, 430 * 1024**2)
self.assertEqual(fi.block_size * fi.block_count, 390 * 1024**2)

# should grow to 450 MiB (full size of the LV)
# should grow to 400 MiB (full size of the LV)
with mounted(lv, self.mount_dir):
succ = BlockDev.fs_resize(lv, 0, "xfs")
self.assertTrue(succ)
with mounted(lv, self.mount_dir):
fi = BlockDev.fs_xfs_get_info(lv)
self.assertTrue(fi)
self.assertEqual(fi.block_size * fi.block_count, 400 * 1024**2)

self._lvresize("libbd_fs_tests", "generic_test", "420M")
# unmounted resize (should get automounted)
succ = BlockDev.fs_resize(lv, 0, "xfs")
self.assertTrue(succ)
with mounted(lv, self.mount_dir):
fi = BlockDev.fs_xfs_get_info(lv)
self.assertTrue(fi)
self.assertEqual(fi.block_size * fi.block_count, 420 * 1024**2)

# enable XFS quotas
with mounted(lv, self.mount_dir, options="uquota,gquota,pquota"):
pass

self._lvresize("libbd_fs_tests", "generic_test", "450M")
succ = BlockDev.utils_init_logging(self.my_log_func)
self.assertTrue(succ)
BlockDev.utils_set_log_level(BlockDev.UTILS_LOG_INFO)

succ = BlockDev.fs_resize(lv, 0, "xfs")
self.assertTrue(succ)
with mounted(lv, self.mount_dir):
fi = BlockDev.fs_xfs_get_info(lv)
self.assertTrue(fi)
self.assertEqual(fi.block_size * fi.block_count, 450 * 1024**2)

# quota options should be preserved during temp mount
self.assertIn("with options: uquota,gquota,pquota", self.log)
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot Oct 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

This assertion is likely to fail. The log message will include other mount options like nosuid and nodev appended to the quota options, so the full string will be something like ... with options: uquota,gquota,pquota,nosuid,nodev. The substring you're checking for won't be found.

To make the test more robust and accurate, you should check for the presence of each quota option individually.

Suggested change
self.assertIn("with options: uquota,gquota,pquota", self.log)
self.assertIn("with options:", self.log)
self.assertIn("uquota", self.log)
self.assertIn("gquota", self.log)
self.assertIn("pquota", self.log)

Copy link
Copy Markdown
Member

@tbzatek tbzatek Oct 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, almost ;-)


BlockDev.utils_init_logging(None)

def _can_resize_f2fs(self):
ret, out, _err = utils.run_command("resize.f2fs -V")
if ret != 0:
Expand Down
21 changes: 14 additions & 7 deletions tests/utils.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -277,12 +277,12 @@ def clean_scsi_debug(scsi_debug_dev):
def _wait_for_nvme_controllers_ready(subnqn, timeout=3):
"""
Wait for NVMe controllers with matching subsystem NQN to be in live state

:param str subnqn: subsystem nqn to match controllers against
:param int timeout: timeout in seconds (default: 3)
"""
start_time = time.time()

while time.time() - start_time < timeout:
try:
for ctrl_path in glob.glob("/sys/class/nvme/nvme*/"):
Expand All @@ -297,12 +297,12 @@ def _wait_for_nvme_controllers_ready(subnqn, timeout=3):
return
except:
continue

except:
pass

time.sleep(1)

os.system("udevadm settle")

def find_nvme_ctrl_devs_for_subnqn(subnqn, wait_for_ready=True):
Expand Down Expand Up @@ -756,11 +756,18 @@ def run(cmd_string):
return subprocess.call(cmd_string, close_fds=True, shell=True)


def mount(device, where, ro=False):
def mount(device, where, ro=False, options=None):
if not os.path.isdir(where):
os.makedirs(where)

if ro:
os.system("mount -oro %s %s" % (device, where))
if options:
options += ",ro"
else:
options = "ro"

if options:
os.system("mount -o %s %s %s" % (options, device, where))
else:
os.system("mount %s %s" % (device, where))
Comment on lines +759 to 772
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot Oct 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Using os.system with string formatting is vulnerable to shell injection. While this is test code, it's a good practice to use safer alternatives. I recommend refactoring this function to use subprocess.run with a list of arguments, which avoids the shell and is more secure.

def mount(device, where, ro=False, options=None):
    if not os.path.isdir(where):
        os.makedirs(where)

    mount_cmd = ["mount"]
    if ro:
        if options:
            options += ",ro"
        else:
            options = "ro"

    if options:
        mount_cmd.extend(["-o", options])

    mount_cmd.extend([device, where])
    # Using subprocess.run is safer than os.system with string formatting.
    # The original implementation ignored the return code, so we do the same.
    subprocess.run(mount_cmd)


Expand Down