Skip to content
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.

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

RFC: btrfs plugin: use libbtrfsutil #999

Draft
wants to merge 4 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions .packit.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ srpm_build_deps:
- autoconf
- autoconf-archive
- automake
- btrfs-progs-devel
- cryptsetup-devel
- device-mapper-devel
- e2fsprogs-devel
Expand Down
4 changes: 4 additions & 0 deletions configure.ac
Original file line number Diff line number Diff line change
Expand Up @@ -229,6 +229,10 @@ AS_IF([test "x$with_fs" != "xno" -o "x$with_crypto" != "xno" -o "x$with_swap" !=
[Define as neutral value if libblkid doesn't provide the definition])])]
[])

AS_IF([test "x$with_btrfs" != "xno"],
[LIBBLOCKDEV_PKG_CHECK_MODULES([LIBBTRFSUTIL], [libbtrfsutil >= 5.1])],
Copy link
Contributor Author

@jelly jelly Jan 22, 2024

Choose a reason for hiding this comment

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

This is a bit arbitrary, needs some investigation which version is really needed, then again 5.1 is from 2019.

[])
Copy link
Member

Choose a reason for hiding this comment

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

To unblock some of the CI, you'll need to libbtrfsutil dependency to few places:

  • dist/libblockdev.spec.in
  • misc/install-test-dependencies.yml
  • .packit.yaml

This will cover Packit and the GH actions checks (we'll also need to add it to our internal CI, but that's something we need to do manually).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, applied it.


AS_IF([test "x$with_btrfs" != "xno" -o "x$with_mdraid" != "xno" -o "x$with_tools" != "xno"],
[LIBBLOCKDEV_PKG_CHECK_MODULES([BYTESIZE], [bytesize >= 0.1])],
[])
Expand Down
2 changes: 1 addition & 1 deletion dist/libblockdev.spec.in
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,7 @@ with the libblockdev-utils library.

%if %{with_btrfs}
%package btrfs
BuildRequires: libbytesize-devel
BuildRequires: libbytesize-devel btrfs-progs-devel
Summary: The BTRFS plugin for the libblockdev library
Requires: %{name}-utils%{?_isa} = %{version}-%{release}
Requires: btrfs-progs
Expand Down
2 changes: 2 additions & 0 deletions misc/install-test-dependencies.yml
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
- name: Install build dependencies not covered by dnf builddep (Fedora)
package: name={{item}} state=present
with_items:
- btrfs-progs-devel
- libfdisk-devel
- keyutils-libs-devel
- libnvme-devel
Expand Down Expand Up @@ -162,6 +163,7 @@
- name: Install build dependencies not covered by apt build-dep (Debian/Ubuntu)
package: name={{item}} state=present
with_items:
- libbtrfsutil-dev
- libfdisk-dev
- libkeyutils-dev
- libext2fs-dev
Expand Down
4 changes: 2 additions & 2 deletions src/plugins/Makefile.am
Original file line number Diff line number Diff line change
Expand Up @@ -60,8 +60,8 @@ endif


if WITH_BTRFS
libbd_btrfs_la_CFLAGS = $(GLIB_CFLAGS) $(GIO_CFLAGS) $(BYTESIZE_CFLAGS) -Wall -Wextra -Werror
libbd_btrfs_la_LIBADD = ${builddir}/../utils/libbd_utils.la $(GLIB_LIBS) $(GIO_LIBS) $(BYTESIZE_LIBS)
libbd_btrfs_la_CFLAGS = $(GLIB_CFLAGS) $(GIO_CFLAGS) $(BYTESIZE_CFLAGS) $(LIBBTRFSUTIL_CFLAGS) -Wall -Wextra -Werror
libbd_btrfs_la_LIBADD = ${builddir}/../utils/libbd_utils.la $(GLIB_LIBS) $(GIO_LIBS) $(BYTESIZE_LIBS) $(LIBBTRFSUTIL_LIBS)
libbd_btrfs_la_LDFLAGS = -L${srcdir}/../utils/ -version-info 3:0:0 -Wl,--no-undefined -export-symbols-regex '^bd_.*'
libbd_btrfs_la_CPPFLAGS = -I${builddir}/../../include/
libbd_btrfs_la_SOURCES = btrfs.c btrfs.h check_deps.c check_deps.h
Expand Down
230 changes: 62 additions & 168 deletions src/plugins/btrfs.c
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,14 @@
#include <unistd.h>
#include <blockdev/utils.h>
#include <bs_size.h>
#include <btrfsutil.h>

#include "btrfs.h"
#include "check_deps.h"

#define BTRFS_MIN_VERSION "3.18.2"
// HACK: btrfsutil issue?
#define BTRFS_FSID_SIZE 16

/**
* SECTION: btrfs
Expand Down Expand Up @@ -211,23 +214,6 @@ static BDBtrfsDeviceInfo* get_device_info_from_match (GMatchInfo *match_info) {
return ret;
}

static BDBtrfsSubvolumeInfo* get_subvolume_info_from_match (GMatchInfo *match_info) {
BDBtrfsSubvolumeInfo *ret = g_new(BDBtrfsSubvolumeInfo, 1);
gchar *item = NULL;

item = g_match_info_fetch_named (match_info, "id");
ret->id = g_ascii_strtoull (item, NULL, 0);
g_free (item);

item = g_match_info_fetch_named (match_info, "parent_id");
ret->parent_id = g_ascii_strtoull (item, NULL, 0);
g_free (item);

ret->path = g_match_info_fetch_named (match_info, "path");

return ret;
}

static BDBtrfsFilesystemInfo* get_filesystem_info_from_match (GMatchInfo *match_info) {
BDBtrfsFilesystemInfo *ret = g_new(BDBtrfsFilesystemInfo, 1);
gchar *item = NULL;
Expand Down Expand Up @@ -388,22 +374,24 @@ gboolean bd_btrfs_remove_device (const gchar *mountpoint, const gchar *device, c
*
* Tech category: %BD_BTRFS_TECH_SUBVOL-%BD_BTRFS_TECH_MODE_CREATE
*/
gboolean bd_btrfs_create_subvolume (const gchar *mountpoint, const gchar *name, const BDExtraArg **extra, GError **error) {
gboolean bd_btrfs_create_subvolume (const gchar *mountpoint, const gchar *name, G_GNUC_UNUSED const BDExtraArg **extra, GError **error) {
gchar *path = NULL;
gboolean success = FALSE;
const gchar *argv[5] = {"btrfs", "subvol", "create", NULL, NULL};

if (!check_deps (&avail_deps, DEPS_BTRFS_MASK, deps, DEPS_LAST, &deps_check_lock, error) ||
!check_module_deps (&avail_module_deps, MODULE_DEPS_BTRFS_MASK, module_deps, MODULE_DEPS_LAST, &deps_check_lock, error))
return FALSE;
enum btrfs_util_error err;

if (g_str_has_suffix (mountpoint, "/"))
path = g_strdup_printf ("%s%s", mountpoint, name);
else
path = g_strdup_printf ("%s/%s", mountpoint, name);
argv[3] = path;

success = bd_utils_exec_and_report_error (argv, extra, error);

err = btrfs_util_create_subvolume (path, 0 ,NULL ,NULL);
if (err) {
g_set_error (error, BD_BTRFS_ERROR, BD_BTRFS_ERROR_DEVICE, "%s: %m", btrfs_util_strerror (err));
} else {
success = TRUE;
}

g_free (path);

return success;
Expand All @@ -421,22 +409,24 @@ gboolean bd_btrfs_create_subvolume (const gchar *mountpoint, const gchar *name,
*
* Tech category: %BD_BTRFS_TECH_SUBVOL-%BD_BTRFS_TECH_MODE_DELETE
*/
gboolean bd_btrfs_delete_subvolume (const gchar *mountpoint, const gchar *name, const BDExtraArg **extra, GError **error) {
gboolean bd_btrfs_delete_subvolume (const gchar *mountpoint, const gchar *name, G_GNUC_UNUSED const BDExtraArg **extra, GError **error) {
gchar *path = NULL;
gboolean success = FALSE;
const gchar *argv[5] = {"btrfs", "subvol", "delete", NULL, NULL};

if (!check_deps (&avail_deps, DEPS_BTRFS_MASK, deps, DEPS_LAST, &deps_check_lock, error) ||
!check_module_deps (&avail_module_deps, MODULE_DEPS_BTRFS_MASK, module_deps, MODULE_DEPS_LAST, &deps_check_lock, error))
return FALSE;
enum btrfs_util_error err;

if (g_str_has_suffix (mountpoint, "/"))
path = g_strdup_printf ("%s%s", mountpoint, name);
else
path = g_strdup_printf ("%s/%s", mountpoint, name);
argv[3] = path;

success = bd_utils_exec_and_report_error (argv, extra, error);
// TODO: support BTRFS_UTIL_DELETE_SUBVOLUME_RECURSIVE
err = btrfs_util_delete_subvolume (path, 0);
if (err) {
g_set_error (error, BD_BTRFS_ERROR, BD_BTRFS_ERROR_DEVICE, "%s: %m", btrfs_util_strerror (err));
} else {
success = TRUE;
}

g_free (path);

return success;
Expand All @@ -453,48 +443,14 @@ gboolean bd_btrfs_delete_subvolume (const gchar *mountpoint, const gchar *name,
* Tech category: %BD_BTRFS_TECH_SUBVOL-%BD_BTRFS_TECH_MODE_QUERY
*/
guint64 bd_btrfs_get_default_subvolume_id (const gchar *mountpoint, GError **error) {
GRegex *regex = NULL;
GMatchInfo *match_info = NULL;
gboolean success = FALSE;
gchar *output = NULL;
gchar *match = NULL;
enum btrfs_util_error err;
guint64 ret = 0;
const gchar *argv[5] = {"btrfs", "subvol", "get-default", mountpoint, NULL};

if (!check_deps (&avail_deps, DEPS_BTRFS_MASK, deps, DEPS_LAST, &deps_check_lock, error) ||
!check_module_deps (&avail_module_deps, MODULE_DEPS_BTRFS_MASK, module_deps, MODULE_DEPS_LAST, &deps_check_lock, error))
return 0;

regex = g_regex_new ("ID (\\d+) .*", 0, 0, error);
if (!regex) {
bd_utils_log_format (BD_UTILS_LOG_WARNING, "Failed to create new GRegex");
/* error is already populated */
return 0;
err = btrfs_util_get_default_subvolume (mountpoint, &ret);
if (err) {
g_set_error (error, BD_BTRFS_ERROR, BD_BTRFS_ERROR_NOT_FOUND, "%s: %m", btrfs_util_strerror (err));
}

success = bd_utils_exec_and_capture_output (argv, NULL, &output, error);
if (!success) {
g_regex_unref (regex);
return 0;
}

success = g_regex_match (regex, output, 0, &match_info);
if (!success) {
g_set_error (error, BD_BTRFS_ERROR, BD_BTRFS_ERROR_PARSE, "Failed to parse subvolume's ID");
g_regex_unref (regex);
g_match_info_free (match_info);
g_free (output);
return 0;
}

match = g_match_info_fetch (match_info, 1);
ret = g_ascii_strtoull (match, NULL, 0);

g_free (match);
g_match_info_free (match_info);
g_regex_unref (regex);
g_free (output);

return ret;
}

Expand All @@ -511,17 +467,15 @@ guint64 bd_btrfs_get_default_subvolume_id (const gchar *mountpoint, GError **err
*
* Tech category: %BD_BTRFS_TECH_SUBVOL-%BD_BTRFS_TECH_MODE_MODIFY
*/
gboolean bd_btrfs_set_default_subvolume (const gchar *mountpoint, guint64 subvol_id, const BDExtraArg **extra, GError **error) {
const gchar *argv[6] = {"btrfs", "subvol", "set-default", NULL, mountpoint, NULL};
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This breaks udisks, which I think we rather not? But -Werror will scream at me if I keep it around. I guess I should mark it with something __unused?

Copy link
Member

Choose a reason for hiding this comment

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

Yes, existing public API cannot be changed. Please add G_GNUC_UNUSED annotation.

If you want to keep full API compatibility, you'd need to translate any known extra args to library calls/arguments. Porting existing API is tough, creating new API from scratch is much easier.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think deprecating the argument would be the best, I am not sure what extra options can be given as btrfs subvolume set default only takes:

btrfs subvolume set-default <subvolid> <path>

So I think for backwards compatibility this would actually work out quite well in this instance. Should I add G_DEPRECATED as well to the argument and update the comment?

Copy link
Member

Choose a reason for hiding this comment

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

No need to special annotation for function arguments I guess.

gboolean bd_btrfs_set_default_subvolume (const gchar *mountpoint, guint64 subvol_id, G_GNUC_UNUSED const BDExtraArg **extra, GError **error) {
enum btrfs_util_error err;
gboolean ret = FALSE;

if (!check_deps (&avail_deps, DEPS_BTRFS_MASK, deps, DEPS_LAST, &deps_check_lock, error) ||
!check_module_deps (&avail_module_deps, MODULE_DEPS_BTRFS_MASK, module_deps, MODULE_DEPS_LAST, &deps_check_lock, error))
return FALSE;

argv[3] = g_strdup_printf ("%"G_GUINT64_FORMAT, subvol_id);
ret = bd_utils_exec_and_report_error (argv, extra, error);
g_free ((gchar *) argv[3]);
err = btrfs_util_set_default_subvolume (mountpoint, subvol_id);
if (err)
g_set_error (error, BD_BTRFS_ERROR, BD_BTRFS_ERROR_NOT_FOUND, "%s: %m", btrfs_util_strerror (err));
else
ret = TRUE;

return ret;
}
Expand Down Expand Up @@ -643,112 +597,52 @@ BDBtrfsDeviceInfo** bd_btrfs_list_devices (const gchar *device, GError **error)
* Tech category: %BD_BTRFS_TECH_SUBVOL-%BD_BTRFS_TECH_MODE_QUERY
*/
BDBtrfsSubvolumeInfo** bd_btrfs_list_subvolumes (const gchar *mountpoint, gboolean snapshots_only, GError **error) {
const gchar *argv[7] = {"btrfs", "subvol", "list", "-p", NULL, NULL, NULL};
gchar *output = NULL;
gboolean success = FALSE;
gchar **lines = NULL;
gchar **line_p = NULL;
gchar const * const pattern = "ID\\s+(?P<id>\\d+)\\s+gen\\s+\\d+\\s+(cgen\\s+\\d+\\s+)?" \
"parent\\s+(?P<parent_id>\\d+)\\s+top\\s+level\\s+\\d+\\s+" \
"(otime\\s+(\\d{4}-\\d{2}-\\d{2}\\s+\\d\\d:\\d\\d:\\d\\d|-)\\s+)?"\
"path\\s+(?P<path>\\S+)";
GRegex *regex = NULL;
GMatchInfo *match_info = NULL;
guint64 i = 0;
guint64 y = 0;
guint64 next_sorted_idx = 0;
GPtrArray *subvol_infos;
BDBtrfsSubvolumeInfo* item = NULL;
BDBtrfsSubvolumeInfo* swap_item = NULL;
struct btrfs_util_subvolume_iterator *iter;
enum btrfs_util_error err;
char *path;
char empty_uuid[BTRFS_FSID_SIZE] = {0};
struct btrfs_util_subvolume_info info;
BDBtrfsSubvolumeInfo** ret = NULL;
GError *l_error = NULL;

if (!check_deps (&avail_deps, DEPS_BTRFS_MASK, deps, DEPS_LAST, &deps_check_lock, error) ||
!check_module_deps (&avail_module_deps, MODULE_DEPS_BTRFS_MASK, module_deps, MODULE_DEPS_LAST, &deps_check_lock, error))
return NULL;

if (snapshots_only) {
argv[4] = "-s";
argv[5] = mountpoint;
} else
argv[4] = mountpoint;
GPtrArray *subvol_infos;

regex = g_regex_new (pattern, G_REGEX_EXTENDED, 0, error);
if (!regex) {
bd_utils_log_format (BD_UTILS_LOG_WARNING, "Failed to create new GRegex");
/* error is already populated */
err = btrfs_util_create_subvolume_iterator (mountpoint, 5, 0, &iter);
if (err) {
g_set_error (error, BD_BTRFS_ERROR, BD_BTRFS_ERROR_NOT_FOUND, "%s: %m", btrfs_util_strerror (err));
return NULL;
}

success = bd_utils_exec_and_capture_output (argv, NULL, &output, &l_error);
if (!success) {
g_regex_unref (regex);
if (g_error_matches (l_error, BD_UTILS_EXEC_ERROR, BD_UTILS_EXEC_ERROR_NOOUT)) {
/* no output -> no subvolumes */
g_clear_error (&l_error);
return g_new0 (BDBtrfsSubvolumeInfo*, 1);
} else {
g_propagate_error (error, l_error);
return NULL;
}
}

lines = g_strsplit (output, "\n", 0);
g_free (output);

subvol_infos = g_ptr_array_new ();
for (line_p = lines; *line_p; line_p++) {
success = g_regex_match (regex, *line_p, 0, &match_info);
if (!success) {
g_match_info_free (match_info);
while (!(err = btrfs_util_subvolume_iterator_next_info (iter, &path, &info))) {
/* See uapi/linux/btrfs.h */
if (snapshots_only && memcmp (info.parent_uuid, empty_uuid, BTRFS_FSID_SIZE) == 0)
continue;
}

g_ptr_array_add (subvol_infos, get_subvolume_info_from_match (match_info));
g_match_info_free (match_info);
BDBtrfsSubvolumeInfo *subvol = g_new (BDBtrfsSubvolumeInfo, 1);
subvol->id = info.id;
subvol->parent_id = info.parent_id;
subvol->path = g_strdup (path);
g_ptr_array_add (subvol_infos, subvol);

free (path);
}

g_strfreev (lines);
g_regex_unref (regex);
btrfs_util_destroy_subvolume_iterator (iter);

if (subvol_infos->len == 0) {
g_set_error (error, BD_BTRFS_ERROR, BD_BTRFS_ERROR_PARSE, "Failed to parse information about subvolumes");
g_ptr_array_free (subvol_infos, TRUE);
return NULL;
}
// TODO: this can be Stop iteration: Success (3)
// if (err) {
// g_set_error (error, BD_BTRFS_ERROR, BD_BTRFS_ERROR_NOT_FOUND, "%s: %m", btrfs_util_strerror (err));
// return NULL;
// }

/* now we know how much space to allocate for the result (subvols + NULL) */
ret = g_new0 (BDBtrfsSubvolumeInfo*, subvol_infos->len + 1);

/* we need to sort the subvolumes in a way that no child subvolume appears
in the list before its parent (sub)volume */

/* let's start by moving all top-level (sub)volumes to the beginning */
for (i=0; i < subvol_infos->len; i++) {
item = (BDBtrfsSubvolumeInfo*) g_ptr_array_index (subvol_infos, i);
if (item->parent_id == BD_BTRFS_MAIN_VOLUME_ID)
/* top-level (sub)volume */
ret[next_sorted_idx++] = item;
}
/* top-level (sub)volumes are now processed */
for (i=0; i < next_sorted_idx; i++)
g_ptr_array_remove_fast (subvol_infos, ret[i]);

/* now sort the rest in a way that we search for an already sorted parent or sibling */
guint64 i = 0;
BDBtrfsSubvolumeInfo* item = NULL;
for (i=0; i < subvol_infos->len; i++) {
item = (BDBtrfsSubvolumeInfo*) g_ptr_array_index (subvol_infos, i);
ret[next_sorted_idx] = item;
/* move the item towards beginning of the array checking if some parent
or sibling has been already processed/sorted before or we reached the
top-level (sub)volumes */
for (y=next_sorted_idx; (y > 0 && (ret[y-1]->id != item->parent_id) && (ret[y-1]->parent_id != item->parent_id) && (ret[y-1]->parent_id != BD_BTRFS_MAIN_VOLUME_ID)); y--) {
swap_item = ret[y-1];
ret[y-1] = ret[y];
ret[y] = swap_item;
}
next_sorted_idx++;
ret[i] = item;
}
ret[next_sorted_idx] = NULL;

/* now just free the pointer array */
g_ptr_array_free (subvol_infos, TRUE);
Expand Down
3 changes: 2 additions & 1 deletion src/plugins/btrfs.h
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ typedef enum {
BD_BTRFS_ERROR_TECH_UNAVAIL,
BD_BTRFS_ERROR_DEVICE,
BD_BTRFS_ERROR_PARSE,
BD_BTRFS_ERROR_NOT_FOUND,
} BDBtrfsError;

typedef struct BDBtrfsDeviceInfo {
Expand Down Expand Up @@ -79,7 +80,7 @@ gboolean bd_btrfs_remove_device (const gchar *mountpoint, const gchar *device, c
gboolean bd_btrfs_create_subvolume (const gchar *mountpoint, const gchar *name, const BDExtraArg **extra, GError **error);
gboolean bd_btrfs_delete_subvolume (const gchar *mountpoint, const gchar *name, const BDExtraArg **extra, GError **error);
guint64 bd_btrfs_get_default_subvolume_id (const gchar *mountpoint, GError **error);
gboolean bd_btrfs_set_default_subvolume (const gchar *mountpoint, guint64 subvol_id, const BDExtraArg **extra, GError **error);
gboolean bd_btrfs_set_default_subvolume (const gchar *mountpoint, guint64 subvol_id, G_GNUC_UNUSED const BDExtraArg **extra, GError **error);
gboolean bd_btrfs_create_snapshot (const gchar *source, const gchar *dest, gboolean ro, const BDExtraArg **extra, GError **error);
BDBtrfsDeviceInfo** bd_btrfs_list_devices (const gchar *device, GError **error);
BDBtrfsSubvolumeInfo** bd_btrfs_list_subvolumes (const gchar *mountpoint, gboolean snapshots_only, GError **error);
Expand Down
Loading
Loading