From 2bb371937c7ef73f26717e57a5eb78cafe90a9f7 Mon Sep 17 00:00:00 2001

From: Vojtech Trefny <vtrefny@redhat.com>

Date: Fri, 20 Sep 2019 09:58:01 +0200

Subject: [PATCH 1/5] Mark all GIR file constants as guint64

See 9676585e65f69ec1c309f45ba139035408d59b8e for more information.

---

 src/lib/plugin_apis/btrfs.api  |  2 +-

 src/lib/plugin_apis/crypto.api |  2 +-

 src/lib/plugin_apis/lvm.api    | 20 ++++++++++----------

 src/lib/plugin_apis/mdraid.api |  4 ++--

 4 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/src/lib/plugin_apis/btrfs.api b/src/lib/plugin_apis/btrfs.api

index b52fb4ce..ef0f6c28 100644

--- a/src/lib/plugin_apis/btrfs.api

+++ b/src/lib/plugin_apis/btrfs.api

@@ -3,7 +3,7 @@

 #include <blockdev/utils.h>

 #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 e3d69986..ef0217fe 100644

--- a/src/lib/plugin_apis/crypto.api

+++ b/src/lib/plugin_apis/crypto.api

@@ -1,7 +1,7 @@

 #include <glib.h>

 #include <blockdev/utils.h>

-#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 21c8f74d..ea52263b 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 (2097152ULL) // 2 MiB

+#define BD_LVM_MAX_THPOOL_MD_SIZE G_GUINT64_CONSTANT (17179869184ULL) // 16 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");

diff --git a/src/lib/plugin_apis/mdraid.api b/src/lib/plugin_apis/mdraid.api

index 3bd9eaf2..02ca9530 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");

From aeedce3bcaa8182c9878cc51d3f85a6c6eb6a01f Mon Sep 17 00:00:00 2001

From: Vojtech Trefny <vtrefny@redhat.com>

Date: Thu, 3 Dec 2020 14:41:25 +0100

Subject: [PATCH 2/5] lvm: Set thin metadata limits to match limits LVM uses in

 lvcreate

---

 src/lib/plugin_apis/lvm.api | 4 ++--

 src/plugins/lvm.h           | 9 +++++++--

 tests/lvm_dbus_tests.py     | 7 ++++---

 tests/lvm_test.py           | 7 ++++---

 4 files changed, 17 insertions(+), 10 deletions(-)

diff --git a/src/lib/plugin_apis/lvm.api b/src/lib/plugin_apis/lvm.api

index ea52263b..e843c916 100644

--- a/src/lib/plugin_apis/lvm.api

+++ b/src/lib/plugin_apis/lvm.api

@@ -19,8 +19,8 @@

 #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 (2097152ULL) // 2 MiB

-#define BD_LVM_MAX_THPOOL_MD_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 (33957085184ULL) // 31.62 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

diff --git a/src/plugins/lvm.h b/src/plugins/lvm.h

index 4b970f2e..01c06ca4 100644

--- a/src/plugins/lvm.h

+++ b/src/plugins/lvm.h

@@ -1,5 +1,6 @@

 #include <glib.h>

 #include <blockdev/utils.h>

+#include <libdevmapper.h>

 #ifndef BD_LVM

 #define BD_LVM

@@ -21,8 +22,12 @@

 #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 constants for thin pool metadata size are actually half of these

+   but when they calculate the actual metadata size they double the limits

+   so lets just double the limits here too */

+#define BD_LVM_MIN_THPOOL_MD_SIZE (4 MiB)

+#define BD_LVM_MAX_THPOOL_MD_SIZE (DM_THIN_MAX_METADATA_SIZE KiB)

+

 #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/tests/lvm_dbus_tests.py b/tests/lvm_dbus_tests.py

index 47402fc3..b517aae9 100644

--- a/tests/lvm_dbus_tests.py

+++ b/tests/lvm_dbus_tests.py

@@ -160,12 +160,13 @@ def test_get_thpool_meta_size(self):

     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(4 * 1024**2))

+        self.assertTrue(BlockDev.lvm_is_valid_thpool_md_size(5 * 1024**2))

         self.assertTrue(BlockDev.lvm_is_valid_thpool_md_size(16 * 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(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 149cf54a..d0085651 100644

--- a/tests/lvm_test.py

+++ b/tests/lvm_test.py

@@ -153,12 +153,13 @@ def test_get_thpool_meta_size(self):

     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(4 * 1024**2))

+        self.assertTrue(BlockDev.lvm_is_valid_thpool_md_size(5 * 1024**2))

         self.assertTrue(BlockDev.lvm_is_valid_thpool_md_size(16 * 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(32 * 1024**3))

     @tag_test(TestTags.NOSTORAGE)

     def test_is_valid_thpool_chunk_size(self):

From 15fcf1bb62865083a3483fc51f45e11cdc2d7386 Mon Sep 17 00:00:00 2001

From: Vojtech Trefny <vtrefny@redhat.com>

Date: Thu, 3 Dec 2020 14:46:00 +0100

Subject: [PATCH 3/5] lvm: Do not use thin_metadata_size to recommend thin

 metadata size

thin_metadata_size calculates the metadata size differently and

recommends smaller metadata than lvcreate so we should use the

lvcreate formula instead.

Resolves: rhbz#1898668

---

 dist/libblockdev.spec.in            |  4 --

 src/lib/plugin_apis/lvm.api         |  8 +--

 src/plugins/lvm-dbus.c              | 78 ++++++-----------------------

 src/plugins/lvm.c                   | 66 +++++++-----------------

 src/python/gi/overrides/BlockDev.py |  6 +++

 tests/library_test.py               |  6 +--

 tests/lvm_dbus_tests.py             | 20 ++++----

 tests/lvm_test.py                   | 15 ++----

 tests/utils.py                      |  2 +-

 9 files changed, 61 insertions(+), 144 deletions(-)

diff --git a/dist/libblockdev.spec.in b/dist/libblockdev.spec.in

index 7c775174..3e0a53c6 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/lvm.api b/src/lib/plugin_apis/lvm.api

index e843c916..9f25c1ed 100644

--- a/src/lib/plugin_apis/lvm.api

+++ b/src/lib/plugin_apis/lvm.api

@@ -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/plugins/lvm-dbus.c b/src/plugins/lvm-dbus.c

index 9f821a99..24d54426 100644

--- a/src/plugins/lvm-dbus.c

+++ b/src/plugins/lvm-dbus.c

@@ -20,7 +20,6 @@

 #include <glib.h>

 #include <math.h>

 #include <string.h>

-#include <libdevmapper.h>

 #include <unistd.h>

 #include <blockdev/utils.h>

 #include <gio/gio.h>

@@ -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) {

@@ -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;

 }

 /**

diff --git a/src/plugins/lvm.c b/src/plugins/lvm.c

index 6bfaa338..74493feb 100644

--- a/src/plugins/lvm.c

+++ b/src/plugins/lvm.c

@@ -20,7 +20,6 @@

 #include <glib.h>

 #include <math.h>

 #include <string.h>

-#include <libdevmapper.h>

 #include <unistd.h>

 #include <blockdev/utils.h>

@@ -213,13 +212,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

@@ -236,6 +232,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:

  *

@@ -317,10 +315,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) {

@@ -820,53 +815,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;

 }

 /**

diff --git a/src/python/gi/overrides/BlockDev.py b/src/python/gi/overrides/BlockDev.py

index d78bfaab..f768c8bd 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 e8bb175a..08e44fdc 100644

--- a/tests/library_test.py

+++ b/tests/library_test.py

@@ -349,8 +349,7 @@ def test_try_reinit(self):

         # 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 @@ def test_try_reinit(self):

         # 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 b517aae9..c06a480c 100644

--- a/tests/lvm_dbus_tests.py

+++ b/tests/lvm_dbus_tests.py

@@ -141,21 +141,19 @@ def test_get_thpool_padding(self):

     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"""

diff --git a/tests/lvm_test.py b/tests/lvm_test.py

index d0085651..b84adece 100644

--- a/tests/lvm_test.py

+++ b/tests/lvm_test.py

@@ -134,19 +134,14 @@ def test_get_thpool_padding(self):

     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)

diff --git a/tests/utils.py b/tests/utils.py

index 182eda6a..584fde5c 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):

From 27961a3fcb205ea51f40668d68765dd8d388777b Mon Sep 17 00:00:00 2001

From: Vojtech Trefny <vtrefny@redhat.com>

Date: Thu, 3 Dec 2020 14:48:08 +0100

Subject: [PATCH 4/5] lvm: Use the UNUSED macro instead of

 __attribute__((unused))

for unused attributes. It makes the function definition a little

bit more readable.

---

 src/plugins/lvm-dbus.c | 24 ++++++++++++------------

 src/plugins/lvm.c      | 20 ++++++++++----------

 2 files changed, 22 insertions(+), 22 deletions(-)

diff --git a/src/plugins/lvm-dbus.c b/src/plugins/lvm-dbus.c

index 24d54426..b7b4480e 100644

--- a/src/plugins/lvm-dbus.c

+++ b/src/plugins/lvm-dbus.c

@@ -959,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;

@@ -1061,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;

@@ -1165,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)));

 }

@@ -1177,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;

@@ -1199,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;

 }

@@ -1219,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)

@@ -1313,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));

 }

@@ -1327,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))

@@ -2616,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 */

@@ -2641,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);

@@ -2660,7 +2660,7 @@ gchar* bd_lvm_get_global_config (GError **error __attribute__((unused))) {