From c47daaed6fee13fc5915bdd7ebc6304d0f806e4a Mon Sep 17 00:00:00 2001 From: Packit Service Date: Jan 13 2021 14:11:26 +0000 Subject: Apply patch 0003-LVM-thin-metadata-calculation-fix.patch patch_name: 0003-LVM-thin-metadata-calculation-fix.patch present_in_specfile: true --- diff --git a/dist/libblockdev.spec.in b/dist/libblockdev.spec.in index 7c77517..3e0a53c 100644 --- a/dist/libblockdev.spec.in +++ b/dist/libblockdev.spec.in @@ -387,8 +387,6 @@ BuildRequires: device-mapper-devel Summary: The LVM plugin for the libblockdev library Requires: %{name}-utils%{?_isa} >= 0.11 Requires: lvm2 -# for thin_metadata_size -Requires: device-mapper-persistent-data %description lvm The libblockdev library plugin (and in the same time a standalone library) @@ -411,8 +409,6 @@ BuildRequires: device-mapper-devel Summary: The LVM plugin for the libblockdev library Requires: %{name}-utils%{?_isa} >= 1.4 Requires: lvm2-dbusd >= 2.02.156 -# for thin_metadata_size -Requires: device-mapper-persistent-data %description lvm-dbus The libblockdev library plugin (and in the same time a standalone library) diff --git a/src/lib/plugin_apis/btrfs.api b/src/lib/plugin_apis/btrfs.api index b52fb4c..ef0f6c2 100644 --- a/src/lib/plugin_apis/btrfs.api +++ b/src/lib/plugin_apis/btrfs.api @@ -3,7 +3,7 @@ #include #define BD_BTRFS_MAIN_VOLUME_ID 5 -#define BD_BTRFS_MIN_MEMBER_SIZE (128 MiB) +#define BD_BTRFS_MIN_MEMBER_SIZE G_GUINT64_CONSTANT (134217728ULL) // 128 MiB GQuark bd_btrfs_error_quark (void) { return g_quark_from_static_string ("g-bd-btrfs-error-quark"); diff --git a/src/lib/plugin_apis/crypto.api b/src/lib/plugin_apis/crypto.api index e3d6998..ef0217f 100644 --- a/src/lib/plugin_apis/crypto.api +++ b/src/lib/plugin_apis/crypto.api @@ -1,7 +1,7 @@ #include #include -#define BD_CRYPTO_LUKS_METADATA_SIZE (2 MiB) +#define BD_CRYPTO_LUKS_METADATA_SIZE G_GUINT64_CONSTANT (2097152ULL) // 2 MiB GQuark bd_crypto_error_quark (void) { return g_quark_from_static_string ("g-bd-crypto-error-quark"); diff --git a/src/lib/plugin_apis/lvm.api b/src/lib/plugin_apis/lvm.api index ce669fd..dc8c134 100644 --- a/src/lib/plugin_apis/lvm.api +++ b/src/lib/plugin_apis/lvm.api @@ -15,18 +15,18 @@ #define BD_LVM_MAX_LV_SIZE G_GUINT64_CONSTANT (9223372036854775808ULL) -#define BD_LVM_DEFAULT_PE_START (1 MiB) -#define BD_LVM_DEFAULT_PE_SIZE (4 MiB) -#define BD_LVM_MIN_PE_SIZE (1 KiB) -#define BD_LVM_MAX_PE_SIZE (16 GiB) -#define BD_LVM_MIN_THPOOL_MD_SIZE (2 MiB) -#define BD_LVM_MAX_THPOOL_MD_SIZE (16 GiB) -#define BD_LVM_MIN_THPOOL_CHUNK_SIZE (64 KiB) -#define BD_LVM_MAX_THPOOL_CHUNK_SIZE (1 GiB) -#define BD_LVM_DEFAULT_CHUNK_SIZE (64 KiB) +#define BD_LVM_DEFAULT_PE_START G_GUINT64_CONSTANT (1048576ULL) // 1 MiB +#define BD_LVM_DEFAULT_PE_SIZE G_GUINT64_CONSTANT (4194304ULL) // 4 MiB +#define BD_LVM_MIN_PE_SIZE G_GUINT64_CONSTANT (1024ULL) // 1 KiB +#define BD_LVM_MAX_PE_SIZE G_GUINT64_CONSTANT (17179869184ULL) // 16 GiB +#define BD_LVM_MIN_THPOOL_MD_SIZE G_GUINT64_CONSTANT (4194304ULL) // 4 MiB +#define BD_LVM_MAX_THPOOL_MD_SIZE G_GUINT64_CONSTANT (16978542592ULL) // 15.81 GiB +#define BD_LVM_MIN_THPOOL_CHUNK_SIZE G_GUINT64_CONSTANT (65536ULL) // 64 KiB +#define BD_LVM_MAX_THPOOL_CHUNK_SIZE G_GUINT64_CONSTANT (1073741824ULL) // 1 GiB +#define BD_LVM_DEFAULT_CHUNK_SIZE G_GUINT64_CONSTANT (65536ULL) // 64 KiB /* according to lvmcache (7) */ -#define BD_LVM_MIN_CACHE_MD_SIZE (8 MiB) +#define BD_LVM_MIN_CACHE_MD_SIZE (8388608ULL) // 8 MiB GQuark bd_lvm_error_quark (void) { return g_quark_from_static_string ("g-bd-lvm-error-quark"); @@ -704,11 +704,13 @@ guint64 bd_lvm_get_thpool_padding (guint64 size, guint64 pe_size, gboolean inclu * bd_lvm_get_thpool_meta_size: * @size: size of the thin pool * @chunk_size: chunk size of the thin pool or 0 to use the default (%BD_LVM_DEFAULT_CHUNK_SIZE) - * @n_snapshots: number of snapshots that will be created in the pool + * @n_snapshots: ignored * @error: (out): place to store error (if any) * - * Returns: recommended size of the metadata space for the specified pool or 0 - * in case of error + * Note: This function will be changed in 3.0: the @n_snapshots parameter + * is currently not used and will be removed. + * + * Returns: recommended size of the metadata space for the specified pool * * Tech category: %BD_LVM_TECH_THIN_CALCS no mode (it is ignored) */ diff --git a/src/lib/plugin_apis/mdraid.api b/src/lib/plugin_apis/mdraid.api index 3bd9eaf..02ca953 100644 --- a/src/lib/plugin_apis/mdraid.api +++ b/src/lib/plugin_apis/mdraid.api @@ -7,8 +7,8 @@ /* taken from blivet */ // these defaults were determined empirically -#define BD_MD_SUPERBLOCK_SIZE (2 MiB) -#define BD_MD_CHUNK_SIZE (512 KiB) +#define BD_MD_SUPERBLOCK_SIZE G_GUINT64_CONSTANT (2097152ULL) // 2 MiB +#define BD_MD_CHUNK_SIZE G_GUINT64_CONSTANT (524288ULL) // 512 KiB GQuark bd_md_error_quark (void) { return g_quark_from_static_string ("g-bd-md-error-quark"); diff --git a/src/plugins/lvm-dbus.c b/src/plugins/lvm-dbus.c index 9f821a9..b7b4480 100644 --- a/src/plugins/lvm-dbus.c +++ b/src/plugins/lvm-dbus.c @@ -20,7 +20,6 @@ #include #include #include -#include #include #include #include @@ -248,14 +247,6 @@ static volatile guint avail_features = 0; static volatile guint avail_module_deps = 0; static GMutex deps_check_lock; -#define DEPS_THMS 0 -#define DEPS_THMS_MASK (1 << DEPS_THMS) -#define DEPS_LAST 1 - -static const UtilDep deps[DEPS_LAST] = { - {"thin_metadata_size", NULL, NULL, NULL}, -}; - #define DBUS_DEPS_LVMDBUSD 0 #define DBUS_DEPS_LVMDBUSD_MASK (1 << DBUS_DEPS_LVMDBUSD) #define DBUS_DEPS_LAST 1 @@ -301,17 +292,6 @@ gboolean bd_lvm_check_deps (void) { check_ret = check_ret && success; } - for (i=0; i < DEPS_LAST; i++) { - success = bd_utils_check_util_version (deps[i].name, deps[i].version, - deps[i].ver_arg, deps[i].ver_regexp, &error); - if (!success) - g_warning ("%s", error->message); - else - g_atomic_int_or (&avail_deps, 1 << i); - g_clear_error (&error); - check_ret = check_ret && success; - } - if (!check_ret) g_warning("Cannot load the LVM plugin"); @@ -386,10 +366,7 @@ gboolean bd_lvm_is_tech_avail (BDLVMTech tech, guint64 mode, GError **error) { g_set_error (error, BD_LVM_ERROR, BD_LVM_ERROR_TECH_UNAVAIL, "Only 'query' supported for thin calculations"); return FALSE; - } else if ((mode & BD_LVM_TECH_MODE_QUERY) && - !check_deps (&avail_deps, DEPS_THMS_MASK, deps, DEPS_LAST, &deps_check_lock, error)) - return FALSE; - else + } else return TRUE; case BD_LVM_TECH_CALCS: if (mode & ~BD_LVM_TECH_MODE_QUERY) { @@ -982,7 +959,7 @@ static BDLVMPVdata* get_pv_data_from_props (GVariant *props, GError **error) { return data; } -static BDLVMVGdata* get_vg_data_from_props (GVariant *props, GError **error __attribute__((unused))) { +static BDLVMVGdata* get_vg_data_from_props (GVariant *props, GError **error UNUSED) { BDLVMVGdata *data = g_new0 (BDLVMVGdata, 1); GVariantDict dict; @@ -1084,7 +1061,7 @@ static BDLVMLVdata* get_lv_data_from_props (GVariant *props, GError **error) { return data; } -static BDLVMVDOPooldata* get_vdo_data_from_props (GVariant *props, GError **error __attribute__((unused))) { +static BDLVMVDOPooldata* get_vdo_data_from_props (GVariant *props, GError **error UNUSED) { BDLVMVDOPooldata *data = g_new0 (BDLVMVDOPooldata, 1); GVariantDict dict; gchar *value = NULL; @@ -1188,7 +1165,7 @@ static GVariant* create_size_str_param (guint64 size, const gchar *unit) { * * Tech category: %BD_LVM_TECH_CALCS no mode (it is ignored) */ -gboolean bd_lvm_is_supported_pe_size (guint64 size, GError **error __attribute__((unused))) { +gboolean bd_lvm_is_supported_pe_size (guint64 size, GError **error UNUSED) { return (((size % 2) == 0) && (size >= (BD_LVM_MIN_PE_SIZE)) && (size <= (BD_LVM_MAX_PE_SIZE))); } @@ -1200,7 +1177,7 @@ gboolean bd_lvm_is_supported_pe_size (guint64 size, GError **error __attribute__ * * Tech category: %BD_LVM_TECH_CALCS no mode (it is ignored) */ -guint64 *bd_lvm_get_supported_pe_sizes (GError **error __attribute__((unused))) { +guint64 *bd_lvm_get_supported_pe_sizes (GError **error UNUSED) { guint8 i; guint64 val = BD_LVM_MIN_PE_SIZE; guint8 num_items = ((guint8) round (log2 ((double) BD_LVM_MAX_PE_SIZE))) - ((guint8) round (log2 ((double) BD_LVM_MIN_PE_SIZE))) + 2; @@ -1222,7 +1199,7 @@ guint64 *bd_lvm_get_supported_pe_sizes (GError **error __attribute__((unused))) * * Tech category: %BD_LVM_TECH_CALCS no mode (it is ignored) */ -guint64 bd_lvm_get_max_lv_size (GError **error __attribute__((unused))) { +guint64 bd_lvm_get_max_lv_size (GError **error UNUSED) { return BD_LVM_MAX_LV_SIZE; } @@ -1242,7 +1219,7 @@ guint64 bd_lvm_get_max_lv_size (GError **error __attribute__((unused))) { * * Tech category: %BD_LVM_TECH_CALCS no mode (it is ignored) */ -guint64 bd_lvm_round_size_to_pe (guint64 size, guint64 pe_size, gboolean roundup, GError **error __attribute__((unused))) { +guint64 bd_lvm_round_size_to_pe (guint64 size, guint64 pe_size, gboolean roundup, GError **error UNUSED) { pe_size = RESOLVE_PE_SIZE(pe_size); guint64 delta = size % pe_size; if (delta == 0) @@ -1303,53 +1280,28 @@ guint64 bd_lvm_get_thpool_padding (guint64 size, guint64 pe_size, gboolean inclu * bd_lvm_get_thpool_meta_size: * @size: size of the thin pool * @chunk_size: chunk size of the thin pool or 0 to use the default (%BD_LVM_DEFAULT_CHUNK_SIZE) - * @n_snapshots: number of snapshots that will be created in the pool + * @n_snapshots: ignored * @error: (out): place to store error (if any) * - * Returns: recommended size of the metadata space for the specified pool or 0 - * in case of error + * Note: This function will be changed in 3.0: the @n_snapshots parameter + * is currently not used and will be removed. + * + * Returns: recommended size of the metadata space for the specified pool * * Tech category: %BD_LVM_TECH_THIN_CALCS no mode (it is ignored) */ -guint64 bd_lvm_get_thpool_meta_size (guint64 size, guint64 chunk_size, guint64 n_snapshots, GError **error) { - /* ub - output in bytes, n - output just the number */ - const gchar* args[7] = {"thin_metadata_size", "-ub", "-n", NULL, NULL, NULL, NULL}; - gchar *output = NULL; - gboolean success = FALSE; - guint64 ret = 0; - - if (!check_deps (&avail_deps, DEPS_THMS_MASK, deps, DEPS_LAST, &deps_check_lock, error)) - return 0; +guint64 bd_lvm_get_thpool_meta_size (guint64 size, guint64 chunk_size, guint64 n_snapshots UNUSED, GError **error UNUSED) { + guint64 md_size = 0; - /* s - total size, b - chunk size, m - number of snapshots */ - args[3] = g_strdup_printf ("-s%"G_GUINT64_FORMAT, size); - args[4] = g_strdup_printf ("-b%"G_GUINT64_FORMAT, - chunk_size != 0 ? chunk_size : (guint64) BD_LVM_DEFAULT_CHUNK_SIZE); - args[5] = g_strdup_printf ("-m%"G_GUINT64_FORMAT, n_snapshots); - - success = bd_utils_exec_and_capture_output (args, NULL, &output, error); - g_free ((gchar*) args[3]); - g_free ((gchar*) args[4]); - g_free ((gchar*) args[5]); - - if (!success) { - /* error is already set */ - g_free (output); - return 0; - } - - ret = g_ascii_strtoull (output, NULL, 0); - if (ret == 0) { - g_set_error (error, BD_LVM_ERROR, BD_LVM_ERROR_PARSE, - "Failed to parse number from thin_metadata_size's output: '%s'", - output); - g_free (output); - return 0; - } + /* based on lvcreate metadata size calculation */ + md_size = UINT64_C(64) * size / (chunk_size ? chunk_size : BD_LVM_DEFAULT_CHUNK_SIZE); - g_free (output); + if (md_size > BD_LVM_MAX_THPOOL_MD_SIZE) + md_size = BD_LVM_MAX_THPOOL_MD_SIZE; + else if (md_size < BD_LVM_MIN_THPOOL_MD_SIZE) + md_size = BD_LVM_MIN_THPOOL_MD_SIZE; - return MAX (ret, BD_LVM_MIN_THPOOL_MD_SIZE); + return md_size; } /** @@ -1361,7 +1313,7 @@ guint64 bd_lvm_get_thpool_meta_size (guint64 size, guint64 chunk_size, guint64 n * * Tech category: %BD_LVM_TECH_THIN_CALCS no mode (it is ignored) */ -gboolean bd_lvm_is_valid_thpool_md_size (guint64 size, GError **error __attribute__((unused))) { +gboolean bd_lvm_is_valid_thpool_md_size (guint64 size, GError **error UNUSED) { return ((BD_LVM_MIN_THPOOL_MD_SIZE <= size) && (size <= BD_LVM_MAX_THPOOL_MD_SIZE)); } @@ -1375,7 +1327,7 @@ gboolean bd_lvm_is_valid_thpool_md_size (guint64 size, GError **error __attribut * * Tech category: %BD_LVM_TECH_THIN_CALCS no mode (it is ignored) */ -gboolean bd_lvm_is_valid_thpool_chunk_size (guint64 size, gboolean discard, GError **error __attribute__((unused))) { +gboolean bd_lvm_is_valid_thpool_chunk_size (guint64 size, gboolean discard, GError **error UNUSED) { gdouble size_log2 = 0.0; if ((size < BD_LVM_MIN_THPOOL_CHUNK_SIZE) || (size > BD_LVM_MAX_THPOOL_CHUNK_SIZE)) @@ -2664,7 +2616,7 @@ gboolean bd_lvm_thsnapshotcreate (const gchar *vg_name, const gchar *origin_name * * Tech category: %BD_LVM_TECH_GLOB_CONF no mode (it is ignored) */ -gboolean bd_lvm_set_global_config (const gchar *new_config, GError **error __attribute__((unused))) { +gboolean bd_lvm_set_global_config (const gchar *new_config, GError **error UNUSED) { /* XXX: the error attribute will likely be used in the future when some validation comes into the game */ @@ -2689,7 +2641,7 @@ gboolean bd_lvm_set_global_config (const gchar *new_config, GError **error __att * * Tech category: %BD_LVM_TECH_GLOB_CONF no mode (it is ignored) */ -gchar* bd_lvm_get_global_config (GError **error __attribute__((unused))) { +gchar* bd_lvm_get_global_config (GError **error UNUSED) { gchar *ret = NULL; g_mutex_lock (&global_config_lock); @@ -2708,7 +2660,7 @@ gchar* bd_lvm_get_global_config (GError **error __attribute__((unused))) { * * Tech category: %BD_LVM_TECH_CACHE_CALCS no mode (it is ignored) */ -guint64 bd_lvm_cache_get_default_md_size (guint64 cache_size, GError **error __attribute__((unused))) { +guint64 bd_lvm_cache_get_default_md_size (guint64 cache_size, GError **error UNUSED) { return MAX ((guint64) cache_size / 1000, BD_LVM_MIN_CACHE_MD_SIZE); } @@ -2718,7 +2670,7 @@ guint64 bd_lvm_cache_get_default_md_size (guint64 cache_size, GError **error __a * * Get LV type string from flags. */ -static const gchar* get_lv_type_from_flags (BDLVMCachePoolFlags flags, gboolean meta, GError **error __attribute__((unused))) { +static const gchar* get_lv_type_from_flags (BDLVMCachePoolFlags flags, gboolean meta, GError **error UNUSED) { if (!meta) { if (flags & BD_LVM_CACHE_POOL_STRIPED) return "striped"; diff --git a/src/plugins/lvm.c b/src/plugins/lvm.c index 32ad55c..50da656 100644 --- a/src/plugins/lvm.c +++ b/src/plugins/lvm.c @@ -20,7 +20,6 @@ #include #include #include -#include #include #include @@ -211,13 +210,10 @@ static GMutex deps_check_lock; #define DEPS_LVM 0 #define DEPS_LVM_MASK (1 << DEPS_LVM) -#define DEPS_THMS 1 -#define DEPS_THMS_MASK (1 << DEPS_THMS) -#define DEPS_LAST 2 +#define DEPS_LAST 1 static const UtilDep deps[DEPS_LAST] = { {"lvm", LVM_MIN_VERSION, "version", "LVM version:\\s+([\\d\\.]+)"}, - {"thin_metadata_size", NULL, NULL, NULL}, }; #define FEATURES_VDO 0 @@ -234,6 +230,8 @@ static const UtilFeatureDep features[FEATURES_LAST] = { static const gchar*const module_deps[MODULE_DEPS_LAST] = { "kvdo" }; +#define UNUSED __attribute__((unused)) + /** * bd_lvm_check_deps: * @@ -315,10 +313,7 @@ gboolean bd_lvm_is_tech_avail (BDLVMTech tech, guint64 mode, GError **error) { g_set_error (error, BD_LVM_ERROR, BD_LVM_ERROR_TECH_UNAVAIL, "Only 'query' supported for thin calculations"); return FALSE; - } else if ((mode & BD_LVM_TECH_MODE_QUERY) && - !check_deps (&avail_deps, DEPS_THMS_MASK, deps, DEPS_LAST, &deps_check_lock, error)) - return FALSE; - else + } else return TRUE; case BD_LVM_TECH_CALCS: if (mode & ~BD_LVM_TECH_MODE_QUERY) { @@ -703,7 +698,7 @@ static BDLVMVDOPooldata* get_vdo_data_from_table (GHashTable *table, gboolean fr * * Tech category: %BD_LVM_TECH_CALCS no mode (it is ignored) */ -gboolean bd_lvm_is_supported_pe_size (guint64 size, GError **error __attribute__((unused))) { +gboolean bd_lvm_is_supported_pe_size (guint64 size, GError **error UNUSED) { return (((size % 2) == 0) && (size >= (BD_LVM_MIN_PE_SIZE)) && (size <= (BD_LVM_MAX_PE_SIZE))); } @@ -715,7 +710,7 @@ gboolean bd_lvm_is_supported_pe_size (guint64 size, GError **error __attribute__ * * Tech category: %BD_LVM_TECH_CALCS no mode (it is ignored) */ -guint64 *bd_lvm_get_supported_pe_sizes (GError **error __attribute__((unused))) { +guint64 *bd_lvm_get_supported_pe_sizes (GError **error UNUSED) { guint8 i; guint64 val = BD_LVM_MIN_PE_SIZE; guint8 num_items = ((guint8) round (log2 ((double) BD_LVM_MAX_PE_SIZE))) - ((guint8) round (log2 ((double) BD_LVM_MIN_PE_SIZE))) + 2; @@ -737,7 +732,7 @@ guint64 *bd_lvm_get_supported_pe_sizes (GError **error __attribute__((unused))) * * Tech category: %BD_LVM_TECH_CALCS no mode (it is ignored) */ -guint64 bd_lvm_get_max_lv_size (GError **error __attribute__((unused))) { +guint64 bd_lvm_get_max_lv_size (GError **error UNUSED) { return BD_LVM_MAX_LV_SIZE; } @@ -757,7 +752,7 @@ guint64 bd_lvm_get_max_lv_size (GError **error __attribute__((unused))) { * * Tech category: %BD_LVM_TECH_CALCS no mode (it is ignored) */ -guint64 bd_lvm_round_size_to_pe (guint64 size, guint64 pe_size, gboolean roundup, GError **error __attribute__((unused))) { +guint64 bd_lvm_round_size_to_pe (guint64 size, guint64 pe_size, gboolean roundup, GError **error UNUSED) { pe_size = RESOLVE_PE_SIZE(pe_size); guint64 delta = size % pe_size; if (delta == 0) @@ -818,53 +813,28 @@ guint64 bd_lvm_get_thpool_padding (guint64 size, guint64 pe_size, gboolean inclu * bd_lvm_get_thpool_meta_size: * @size: size of the thin pool * @chunk_size: chunk size of the thin pool or 0 to use the default (%BD_LVM_DEFAULT_CHUNK_SIZE) - * @n_snapshots: number of snapshots that will be created in the pool + * @n_snapshots: ignored * @error: (out): place to store error (if any) * - * Returns: recommended size of the metadata space for the specified pool or 0 - * in case of error + * Note: This function will be changed in 3.0: the @n_snapshots parameter + * is currently not used and will be removed. + * + * Returns: recommended size of the metadata space for the specified pool * * Tech category: %BD_LVM_TECH_THIN_CALCS no mode (it is ignored) */ -guint64 bd_lvm_get_thpool_meta_size (guint64 size, guint64 chunk_size, guint64 n_snapshots, GError **error) { - /* ub - output in bytes, n - output just the number */ - const gchar* args[7] = {"thin_metadata_size", "-ub", "-n", NULL, NULL, NULL, NULL}; - gchar *output = NULL; - gboolean success = FALSE; - guint64 ret = 0; - - if (!check_deps (&avail_deps, DEPS_THMS_MASK, deps, DEPS_LAST, &deps_check_lock, error)) - return 0; +guint64 bd_lvm_get_thpool_meta_size (guint64 size, guint64 chunk_size, guint64 n_snapshots UNUSED, GError **error UNUSED) { + guint64 md_size = 0; - /* s - total size, b - chunk size, m - number of snapshots */ - args[3] = g_strdup_printf ("-s%"G_GUINT64_FORMAT, size); - args[4] = g_strdup_printf ("-b%"G_GUINT64_FORMAT, - chunk_size != 0 ? chunk_size : (guint64) BD_LVM_DEFAULT_CHUNK_SIZE); - args[5] = g_strdup_printf ("-m%"G_GUINT64_FORMAT, n_snapshots); + /* based on lvcreate metadata size calculation */ + md_size = UINT64_C(64) * size / (chunk_size ? chunk_size : BD_LVM_DEFAULT_CHUNK_SIZE); - success = bd_utils_exec_and_capture_output (args, NULL, &output, error); - g_free ((gchar*) args[3]); - g_free ((gchar*) args[4]); - g_free ((gchar*) args[5]); - - if (!success) { - /* error is already set */ - g_free (output); - return 0; - } - - ret = g_ascii_strtoull (output, NULL, 0); - if (ret == 0) { - g_set_error (error, BD_LVM_ERROR, BD_LVM_ERROR_PARSE, - "Failed to parse number from thin_metadata_size's output: '%s'", - output); - g_free (output); - return 0; - } - - g_free (output); + if (md_size > BD_LVM_MAX_THPOOL_MD_SIZE) + md_size = BD_LVM_MAX_THPOOL_MD_SIZE; + else if (md_size < BD_LVM_MIN_THPOOL_MD_SIZE) + md_size = BD_LVM_MIN_THPOOL_MD_SIZE; - return MAX (ret, BD_LVM_MIN_THPOOL_MD_SIZE); + return md_size; } /** @@ -876,7 +846,7 @@ guint64 bd_lvm_get_thpool_meta_size (guint64 size, guint64 chunk_size, guint64 n * * Tech category: %BD_LVM_TECH_THIN_CALCS no mode (it is ignored) */ -gboolean bd_lvm_is_valid_thpool_md_size (guint64 size, GError **error __attribute__((unused))) { +gboolean bd_lvm_is_valid_thpool_md_size (guint64 size, GError **error UNUSED) { return ((BD_LVM_MIN_THPOOL_MD_SIZE <= size) && (size <= BD_LVM_MAX_THPOOL_MD_SIZE)); } @@ -890,7 +860,7 @@ gboolean bd_lvm_is_valid_thpool_md_size (guint64 size, GError **error __attribut * * Tech category: %BD_LVM_TECH_THIN_CALCS no mode (it is ignored) */ -gboolean bd_lvm_is_valid_thpool_chunk_size (guint64 size, gboolean discard, GError **error __attribute__((unused))) { +gboolean bd_lvm_is_valid_thpool_chunk_size (guint64 size, gboolean discard, GError **error UNUSED) { gdouble size_log2 = 0.0; if ((size < BD_LVM_MIN_THPOOL_CHUNK_SIZE) || (size > BD_LVM_MAX_THPOOL_CHUNK_SIZE)) @@ -2011,7 +1981,7 @@ gboolean bd_lvm_thsnapshotcreate (const gchar *vg_name, const gchar *origin_name * * Tech category: %BD_LVM_TECH_GLOB_CONF no mode (it is ignored) */ -gboolean bd_lvm_set_global_config (const gchar *new_config, GError **error __attribute__((unused))) { +gboolean bd_lvm_set_global_config (const gchar *new_config, GError **error UNUSED) { /* XXX: the error attribute will likely be used in the future when some validation comes into the game */ @@ -2036,7 +2006,7 @@ gboolean bd_lvm_set_global_config (const gchar *new_config, GError **error __att * * Tech category: %BD_LVM_TECH_GLOB_CONF no mode (it is ignored) */ -gchar* bd_lvm_get_global_config (GError **error __attribute__((unused))) { +gchar* bd_lvm_get_global_config (GError **error UNUSED) { gchar *ret = NULL; g_mutex_lock (&global_config_lock); @@ -2055,7 +2025,7 @@ gchar* bd_lvm_get_global_config (GError **error __attribute__((unused))) { * * Tech category: %BD_LVM_TECH_CACHE_CALCS no mode (it is ignored) */ -guint64 bd_lvm_cache_get_default_md_size (guint64 cache_size, GError **error __attribute__((unused))) { +guint64 bd_lvm_cache_get_default_md_size (guint64 cache_size, GError **error UNUSED) { return MAX ((guint64) cache_size / 1000, BD_LVM_MIN_CACHE_MD_SIZE); } @@ -2065,7 +2035,7 @@ guint64 bd_lvm_cache_get_default_md_size (guint64 cache_size, GError **error __a * * Get LV type string from flags. */ -static const gchar* get_lv_type_from_flags (BDLVMCachePoolFlags flags, gboolean meta, GError **error __attribute__((unused))) { +static const gchar* get_lv_type_from_flags (BDLVMCachePoolFlags flags, gboolean meta, GError **error UNUSED) { if (!meta) { if (flags & BD_LVM_CACHE_POOL_STRIPED) return "striped"; diff --git a/src/plugins/lvm.h b/src/plugins/lvm.h index 4b970f2..2162d76 100644 --- a/src/plugins/lvm.h +++ b/src/plugins/lvm.h @@ -1,5 +1,6 @@ #include #include +#include #ifndef BD_LVM #define BD_LVM @@ -21,8 +22,14 @@ #define USE_DEFAULT_PE_SIZE 0 #define RESOLVE_PE_SIZE(size) ((size) == USE_DEFAULT_PE_SIZE ? BD_LVM_DEFAULT_PE_SIZE : (size)) -#define BD_LVM_MIN_THPOOL_MD_SIZE (2 MiB) -#define BD_LVM_MAX_THPOOL_MD_SIZE (16 GiB) +/* lvm constant for thin pool metadata size is actually half of this + but when they calculate the actual metadata size they double the limit + so lets just double the limit here too */ +#define BD_LVM_MIN_THPOOL_MD_SIZE (4 MiB) + +/* DM_THIN_MAX_METADATA_SIZE is in 512 sectors */ +#define BD_LVM_MAX_THPOOL_MD_SIZE (DM_THIN_MAX_METADATA_SIZE * 512) + #define BD_LVM_MIN_THPOOL_CHUNK_SIZE (64 KiB) #define BD_LVM_MAX_THPOOL_CHUNK_SIZE (1 GiB) #define BD_LVM_DEFAULT_CHUNK_SIZE (64 KiB) diff --git a/src/python/gi/overrides/BlockDev.py b/src/python/gi/overrides/BlockDev.py index d78bfaa..f768c8b 100644 --- a/src/python/gi/overrides/BlockDev.py +++ b/src/python/gi/overrides/BlockDev.py @@ -462,6 +462,12 @@ def lvm_get_thpool_padding(size, pe_size=0, included=False): return _lvm_get_thpool_padding(size, pe_size, included) __all__.append("lvm_get_thpool_padding") +_lvm_get_thpool_meta_size = BlockDev.lvm_get_thpool_meta_size +@override(BlockDev.lvm_get_thpool_meta_size) +def lvm_get_thpool_meta_size(size, chunk_size=0, n_snapshots=0): + return _lvm_get_thpool_meta_size(size, chunk_size, n_snapshots) +__all__.append("lvm_get_thpool_meta_size") + _lvm_pvcreate = BlockDev.lvm_pvcreate @override(BlockDev.lvm_pvcreate) def lvm_pvcreate(device, data_alignment=0, metadata_size=0, extra=None, **kwargs): diff --git a/tests/library_test.py b/tests/library_test.py index e8bb175..08e44fd 100644 --- a/tests/library_test.py +++ b/tests/library_test.py @@ -349,8 +349,7 @@ class LibraryOpsTestCase(unittest.TestCase): # try reinitializing with only some utilities being available and thus # only some plugins able to load - with fake_path("tests/lib_missing_utils", keep_utils=["swapon", "swapoff", "mkswap", "lvm", - "thin_metadata_size", "swaplabel"]): + with fake_path("tests/lib_missing_utils", keep_utils=["swapon", "swapoff", "mkswap", "lvm", "swaplabel"]): succ, loaded = BlockDev.try_reinit(self.requested_plugins, True, None) self.assertFalse(succ) for plug_name in ("swap", "lvm", "crypto"): @@ -361,8 +360,7 @@ class LibraryOpsTestCase(unittest.TestCase): # now the same with a subset of plugins requested plugins = BlockDev.plugin_specs_from_names(["lvm", "swap", "crypto"]) - with fake_path("tests/lib_missing_utils", keep_utils=["swapon", "swapoff", "mkswap", "lvm", - "thin_metadata_size", "swaplabel"]): + with fake_path("tests/lib_missing_utils", keep_utils=["swapon", "swapoff", "mkswap", "lvm","swaplabel"]): succ, loaded = BlockDev.try_reinit(plugins, True, None) self.assertTrue(succ) self.assertEqual(set(loaded), set(["swap", "lvm", "crypto"])) diff --git a/tests/lvm_dbus_tests.py b/tests/lvm_dbus_tests.py index 47402fc..8f2bb95 100644 --- a/tests/lvm_dbus_tests.py +++ b/tests/lvm_dbus_tests.py @@ -141,31 +141,31 @@ class LvmNoDevTestCase(LVMTestCase): def test_get_thpool_meta_size(self): """Verify that getting recommended thin pool metadata size works as expected""" - # no idea how thin_metadata_size works, but let's at least check that - # the function works and returns what thin_metadata_size says - out1 = subprocess.check_output(["thin_metadata_size", "-ub", "-n", "-b64k", "-s1t", "-m100"]) - self.assertEqual(int(out1), BlockDev.lvm_get_thpool_meta_size (1 * 1024**4, 64 * 1024, 100)) + # metadata size is calculated as 64 * pool_size / chunk_size + self.assertEqual(BlockDev.lvm_get_thpool_meta_size(1 * 1024**4, 64 * 1024), 1 * 1024**3) - out2 = subprocess.check_output(["thin_metadata_size", "-ub", "-n", "-b128k", "-s1t", "-m100"]) - self.assertEqual(int(out2), BlockDev.lvm_get_thpool_meta_size (1 * 1024**4, 128 * 1024, 100)) + self.assertEqual(BlockDev.lvm_get_thpool_meta_size(1 * 1024**4, 128 * 1024), 512 * 1024**2) - # twice the chunk_size -> roughly half the metadata needed - self.assertAlmostEqual(float(out1) / float(out2), 2, places=2) - - # unless thin_metadata_size gives a value that is not valid (too small) - self.assertEqual(BlockDev.lvm_get_thpool_meta_size (100 * 1024**2, 128 * 1024, 100), + # lower limit is 4 MiB + self.assertEqual(BlockDev.lvm_get_thpool_meta_size(100 * 1024**2, 128 * 1024), BlockDev.LVM_MIN_THPOOL_MD_SIZE) + # upper limit is 31.62 GiB + self.assertEqual(BlockDev.lvm_get_thpool_meta_size(100 * 1024**4, 64 * 1024), + BlockDev.LVM_MAX_THPOOL_MD_SIZE) + @tag_test(TestTags.NOSTORAGE) def test_is_valid_thpool_md_size(self): """Verify that is_valid_thpool_md_size works as expected""" - self.assertTrue(BlockDev.lvm_is_valid_thpool_md_size(2 * 1024**2)) - self.assertTrue(BlockDev.lvm_is_valid_thpool_md_size(3 * 1024**2)) - self.assertTrue(BlockDev.lvm_is_valid_thpool_md_size(16 * 1024**3)) + self.assertTrue(BlockDev.lvm_is_valid_thpool_md_size(4 * 1024**2)) + self.assertTrue(BlockDev.lvm_is_valid_thpool_md_size(5 * 1024**2)) + self.assertTrue(BlockDev.lvm_is_valid_thpool_md_size(15 * 1024**3)) self.assertFalse(BlockDev.lvm_is_valid_thpool_md_size(1 * 1024**2)) - self.assertFalse(BlockDev.lvm_is_valid_thpool_md_size(17 * 1024**3)) + self.assertFalse(BlockDev.lvm_is_valid_thpool_md_size(3 * 1024**2)) + self.assertFalse(BlockDev.lvm_is_valid_thpool_md_size(16 * 1024**3)) + self.assertFalse(BlockDev.lvm_is_valid_thpool_md_size(32 * 1024**3)) @tag_test(TestTags.NOSTORAGE) def test_is_valid_thpool_chunk_size(self): diff --git a/tests/lvm_test.py b/tests/lvm_test.py index 149cf54..6f80a3b 100644 --- a/tests/lvm_test.py +++ b/tests/lvm_test.py @@ -134,31 +134,28 @@ class LvmNoDevTestCase(LVMTestCase): def test_get_thpool_meta_size(self): """Verify that getting recommended thin pool metadata size works as expected""" - # no idea how thin_metadata_size works, but let's at least check that - # the function works and returns what thin_metadata_size says - out1 = subprocess.check_output(["thin_metadata_size", "-ub", "-n", "-b64k", "-s1t", "-m100"]) - self.assertEqual(int(out1), BlockDev.lvm_get_thpool_meta_size (1 * 1024**4, 64 * 1024, 100)) - out2 = subprocess.check_output(["thin_metadata_size", "-ub", "-n", "-b128k", "-s1t", "-m100"]) - self.assertEqual(int(out2), BlockDev.lvm_get_thpool_meta_size (1 * 1024**4, 128 * 1024, 100)) + self.assertEqual(BlockDev.lvm_get_thpool_meta_size(1 * 1024**4, 64 * 1024), + 1 * 1024**3) - # twice the chunk_size -> roughly half the metadata needed - self.assertAlmostEqual(float(out1) / float(out2), 2, places=2) + self.assertEqual(BlockDev.lvm_get_thpool_meta_size(1 * 1024**4, 128 * 1024), + 512 * 1024**2) - # unless thin_metadata_size gives a value that is not valid (too small) - self.assertEqual(BlockDev.lvm_get_thpool_meta_size (100 * 1024**2, 128 * 1024, 100), + self.assertEqual(BlockDev.lvm_get_thpool_meta_size(100 * 1024**2, 128 * 1024), BlockDev.LVM_MIN_THPOOL_MD_SIZE) @tag_test(TestTags.NOSTORAGE) def test_is_valid_thpool_md_size(self): """Verify that is_valid_thpool_md_size works as expected""" - self.assertTrue(BlockDev.lvm_is_valid_thpool_md_size(2 * 1024**2)) - self.assertTrue(BlockDev.lvm_is_valid_thpool_md_size(3 * 1024**2)) - self.assertTrue(BlockDev.lvm_is_valid_thpool_md_size(16 * 1024**3)) + self.assertTrue(BlockDev.lvm_is_valid_thpool_md_size(4 * 1024**2)) + self.assertTrue(BlockDev.lvm_is_valid_thpool_md_size(5 * 1024**2)) + self.assertTrue(BlockDev.lvm_is_valid_thpool_md_size(15 * 1024**3)) self.assertFalse(BlockDev.lvm_is_valid_thpool_md_size(1 * 1024**2)) - self.assertFalse(BlockDev.lvm_is_valid_thpool_md_size(17 * 1024**3)) + self.assertFalse(BlockDev.lvm_is_valid_thpool_md_size(3 * 1024**2)) + self.assertFalse(BlockDev.lvm_is_valid_thpool_md_size(16 * 1024**3)) + self.assertFalse(BlockDev.lvm_is_valid_thpool_md_size(32 * 1024**3)) @tag_test(TestTags.NOSTORAGE) def test_is_valid_thpool_chunk_size(self): diff --git a/tests/utils.py b/tests/utils.py index 182eda6..584fde5 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -70,7 +70,7 @@ def fake_utils(path="."): finally: os.environ["PATH"] = old_path -ALL_UTILS = {"lvm", "thin_metadata_size", "btrfs", "mkswap", "swaplabel", "multipath", "mpathconf", "dmsetup", "mdadm", "make-bcache", "sgdisk", "sfdisk"} +ALL_UTILS = {"lvm", "btrfs", "mkswap", "swaplabel", "multipath", "mpathconf", "dmsetup", "mdadm", "make-bcache", "sgdisk", "sfdisk"} @contextmanager def fake_path(path=None, keep_utils=None, all_but=None):