From aa2a2754b47c6365960ae375834ac7184317e750 Mon Sep 17 00:00:00 2001 From: CentOS Sources Date: May 18 2021 06:45:56 +0000 Subject: import libblockdev-2.24-5.el8 --- diff --git a/SOURCES/0002-exec-polling-fixes.patch b/SOURCES/0002-exec-polling-fixes.patch new file mode 100644 index 0000000..5982f21 --- /dev/null +++ b/SOURCES/0002-exec-polling-fixes.patch @@ -0,0 +1,792 @@ +From 592820b7a4300cfdc4f85ecd9548f7c2321689fc Mon Sep 17 00:00:00 2001 +From: Tomas Bzatek +Date: Wed, 16 Sep 2020 17:45:07 +0200 +Subject: [PATCH 1/5] exec: Fix polling for stdout and stderr + +The condition of having both the stdout and the stderr fds ready +may never be satisfied in the case of a full stdout buffer waiting +to be read with no output on stderr yet while both fds being still +open. In such case the old code got stuck in an endless loop and +the spawned process being stuck on writing to stdout/stderr. Let's +read data from any fd once available and only react on EOF/HUP. + +This change also makes use of POSIX poll() instead of g_poll() +as it's more clear what the return values mean - Glib docs are +vague in this regard and one can only guess. +--- + src/utils/exec.c | 32 ++++++++++++++++++-------------- + 1 file changed, 18 insertions(+), 14 deletions(-) + +diff --git a/src/utils/exec.c b/src/utils/exec.c +index 37bd960..ebbcaf2 100644 +--- a/src/utils/exec.c ++++ b/src/utils/exec.c +@@ -22,6 +22,7 @@ + #include "extra_arg.h" + #include + #include ++#include + #include + #include + #include +@@ -293,7 +294,7 @@ static gboolean _utils_exec_and_report_progress (const gchar **argv, const BDExt + gint poll_status = 0; + guint i = 0; + guint8 completion = 0; +- GPollFD fds[2] = {ZERO_INIT, ZERO_INIT}; ++ struct pollfd fds[2] = { ZERO_INIT, ZERO_INIT }; + gboolean out_done = FALSE; + gboolean err_done = FALSE; + GString *stdout_data = g_string_new (NULL); +@@ -360,13 +361,16 @@ static gboolean _utils_exec_and_report_progress (const gchar **argv, const BDExt + + fds[0].fd = out_fd; + fds[1].fd = err_fd; +- fds[0].events = G_IO_IN | G_IO_HUP | G_IO_ERR; +- fds[1].events = G_IO_IN | G_IO_HUP | G_IO_ERR; ++ fds[0].events = POLLIN | POLLHUP | POLLERR; ++ fds[1].events = POLLIN | POLLHUP | POLLERR; + while (!out_done || !err_done) { +- poll_status = g_poll (fds, 2, -1); ++ poll_status = poll (fds, 2, -1); ++ g_warn_if_fail (poll_status != 0); /* no timeout specified, zero should never be returned */ + if (poll_status < 0) { ++ if (errno == EAGAIN || errno == EINTR) ++ continue; + g_set_error (error, BD_UTILS_EXEC_ERROR, BD_UTILS_EXEC_ERROR_FAILED, +- "Failed to poll output FDs."); ++ "Failed to poll output FDs: %m"); + bd_utils_report_finished (progress_id, (*error)->message); + g_io_channel_shutdown (out_pipe, FALSE, NULL); + g_io_channel_unref (out_pipe); +@@ -375,12 +379,9 @@ static gboolean _utils_exec_and_report_progress (const gchar **argv, const BDExt + g_string_free (stdout_data, TRUE); + g_string_free (stderr_data, TRUE); + return FALSE; +- } else if (poll_status != 2) +- /* both revents fields were not filled yet */ +- continue; +- if (!(fds[0].revents & G_IO_IN)) +- out_done = TRUE; +- while (!out_done) { ++ } ++ ++ while (!out_done && (fds[0].revents & POLLIN)) { + io_status = g_io_channel_read_line (out_pipe, &line, NULL, NULL, error); + if (io_status == G_IO_STATUS_NORMAL) { + if (prog_extract && prog_extract (line, &completion)) +@@ -401,9 +402,10 @@ static gboolean _utils_exec_and_report_progress (const gchar **argv, const BDExt + return FALSE; + } + } +- if (!(fds[1].revents & G_IO_IN)) +- err_done = TRUE; +- while (!err_done) { ++ if (fds[0].revents & POLLHUP || fds[0].revents & POLLERR || fds[0].revents & POLLNVAL) ++ out_done = TRUE; ++ ++ while (!err_done && (fds[1].revents & POLLIN)) { + io_status = g_io_channel_read_line (err_pipe, &line, NULL, NULL, error); + if (io_status == G_IO_STATUS_NORMAL) { + g_string_append (stderr_data, line); +@@ -421,6 +423,8 @@ static gboolean _utils_exec_and_report_progress (const gchar **argv, const BDExt + return FALSE; + } + } ++ if (fds[1].revents & POLLHUP || fds[1].revents & POLLERR || fds[1].revents & POLLNVAL) ++ err_done = TRUE; + } + + child_ret = waitpid (pid, &status, 0); +-- +2.26.2 + + +From 3025deda9ab670a454bfe373166e49f2acd1c151 Mon Sep 17 00:00:00 2001 +From: Tomas Bzatek +Date: Fri, 25 Sep 2020 18:19:46 +0200 +Subject: [PATCH 2/5] exec: Use non-blocking read and process the buffer + manually + +Waiting for data or a newline character on one fd may create a deadlock +while the other fd is being filled with data, exhausting the pipe buffer. +Setting both stdout and stderr fds non-blocking allows us to get indication +of an empty pipe, continuing with the read cycle over remaining watched fds. + +This also gets rid of GIOChannel as no extended functionality like GSource +notifications were used, degrading GIOChannel in a simple GObject wrapper +over an fd with just a convenience read_line method that we had to get rid of +anyway. Let's use standard POSIX calls and split the read buffer manually +by matching the newline character. NULL bytes should be handled gracefully +however the stack higher up is not ready for that anyway. +--- + src/utils/exec.c | 277 +++++++++++++++++++++++++++-------------------- + 1 file changed, 159 insertions(+), 118 deletions(-) + +diff --git a/src/utils/exec.c b/src/utils/exec.c +index ebbcaf2..317fb55 100644 +--- a/src/utils/exec.c ++++ b/src/utils/exec.c +@@ -23,6 +23,7 @@ + #include + #include + #include ++#include + #include + #include + #include +@@ -272,6 +273,87 @@ gboolean bd_utils_exec_and_report_status_error (const gchar **argv, const BDExtr + return TRUE; + } + ++/* buffer size in bytes used to read from stdout and stderr */ ++#define _EXEC_BUF_SIZE 64*1024 ++ ++/* similar to g_strstr_len() yet treats 'null' byte as @needle. */ ++static gchar *bd_strchr_len_null (const gchar *haystack, gssize haystack_len, const gchar needle) { ++ gchar *ret; ++ gchar *ret_null; ++ ++ ret = memchr (haystack, needle, haystack_len); ++ ret_null = memchr (haystack, 0, haystack_len); ++ if (ret && ret_null) ++ return MIN (ret, ret_null); ++ else ++ return MAX (ret, ret_null); ++} ++ ++static gboolean ++_process_fd_event (gint fd, struct pollfd *poll_fd, GString *read_buffer, GString *filtered_buffer, gsize *read_buffer_pos, gboolean *done, ++ guint64 progress_id, guint8 *progress, BDUtilsProgExtract prog_extract, GError **error) { ++ gchar buf[_EXEC_BUF_SIZE] = { 0 }; ++ ssize_t num_read; ++ gchar *line; ++ gchar *newline_pos; ++ int errno_saved; ++ gboolean eof = FALSE; ++ ++ if (! *done && (poll_fd->revents & POLLIN)) { ++ /* read until we get EOF (0) or error (-1), expecting EAGAIN */ ++ while ((num_read = read (fd, buf, _EXEC_BUF_SIZE)) > 0) ++ g_string_append_len (read_buffer, buf, num_read); ++ errno_saved = errno; ++ ++ /* process the fresh data by lines */ ++ if (read_buffer->len > *read_buffer_pos) { ++ gchar *buf_ptr; ++ gsize buf_len; ++ ++ while ((buf_ptr = read_buffer->str + *read_buffer_pos, ++ buf_len = read_buffer->len - *read_buffer_pos, ++ newline_pos = bd_strchr_len_null (buf_ptr, buf_len, '\n'))) { ++ line = g_strndup (buf_ptr, newline_pos - buf_ptr + 1); ++ if (prog_extract && prog_extract (line, progress)) ++ bd_utils_report_progress (progress_id, *progress, NULL); ++ else ++ g_string_append (filtered_buffer, line); ++ g_free (line); ++ *read_buffer_pos = newline_pos - read_buffer->str + 1; ++ } ++ } ++ ++ /* read error */ ++ if (num_read < 0 && errno_saved != EAGAIN && errno_saved != EINTR) { ++ g_set_error (error, BD_UTILS_EXEC_ERROR, BD_UTILS_EXEC_ERROR_FAILED, ++ "Error reading from pipe: %s", g_strerror (errno_saved)); ++ return FALSE; ++ } ++ ++ /* EOF */ ++ if (num_read == 0) ++ eof = TRUE; ++ } ++ ++ if (poll_fd->revents & POLLHUP || poll_fd->revents & POLLERR || poll_fd->revents & POLLNVAL) ++ eof = TRUE; ++ ++ if (eof) { ++ *done = TRUE; ++ /* process the remaining buffer */ ++ line = read_buffer->str + *read_buffer_pos; ++ /* GString guarantees the buffer is always NULL-terminated. */ ++ if (strlen (line) > 0) { ++ if (prog_extract && prog_extract (line, progress)) ++ bd_utils_report_progress (progress_id, *progress, NULL); ++ else ++ g_string_append (filtered_buffer, line); ++ } ++ } ++ ++ return TRUE; ++} ++ + static gboolean _utils_exec_and_report_progress (const gchar **argv, const BDExtraArg **extra, BDUtilsProgExtract prog_extract, gint *proc_status, gchar **stdout, gchar **stderr, GError **error) { + const gchar **args = NULL; + guint args_len = 0; +@@ -283,24 +365,26 @@ static gboolean _utils_exec_and_report_progress (const gchar **argv, const BDExt + gchar *msg = NULL; + GPid pid = 0; + gint out_fd = 0; +- GIOChannel *out_pipe = NULL; + gint err_fd = 0; +- GIOChannel *err_pipe = NULL; +- gchar *line = NULL; + gint child_ret = -1; + gint status = 0; + gboolean ret = FALSE; +- GIOStatus io_status = G_IO_STATUS_NORMAL; + gint poll_status = 0; + guint i = 0; + guint8 completion = 0; + struct pollfd fds[2] = { ZERO_INIT, ZERO_INIT }; ++ int flags; + gboolean out_done = FALSE; + gboolean err_done = FALSE; +- GString *stdout_data = g_string_new (NULL); +- GString *stderr_data = g_string_new (NULL); ++ GString *stdout_data; ++ GString *stdout_buffer; ++ GString *stderr_data; ++ GString *stderr_buffer; ++ gsize stdout_buffer_pos = 0; ++ gsize stderr_buffer_pos = 0; + gchar **old_env = NULL; + gchar **new_env = NULL; ++ gboolean success = TRUE; + + /* TODO: share this code between functions */ + if (extra) { +@@ -336,15 +420,13 @@ static gboolean _utils_exec_and_report_progress (const gchar **argv, const BDExt + G_SPAWN_DEFAULT|G_SPAWN_SEARCH_PATH|G_SPAWN_DO_NOT_REAP_CHILD, + NULL, NULL, &pid, NULL, &out_fd, &err_fd, error); + ++ g_strfreev (new_env); ++ + if (!ret) { + /* error is already populated */ +- g_string_free (stdout_data, TRUE); +- g_string_free (stderr_data, TRUE); +- g_strfreev (new_env); + g_free (args); + return FALSE; + } +- g_strfreev (new_env); + + args_str = g_strjoinv (" ", args ? (gchar **) args : (gchar **) argv); + msg = g_strdup_printf ("Started '%s'", args_str); +@@ -353,18 +435,25 @@ static gboolean _utils_exec_and_report_progress (const gchar **argv, const BDExt + g_free (args); + g_free (msg); + +- out_pipe = g_io_channel_unix_new (out_fd); +- err_pipe = g_io_channel_unix_new (err_fd); ++ /* set both fds for non-blocking read */ ++ flags = fcntl (out_fd, F_GETFL, 0); ++ if (fcntl (out_fd, F_SETFL, flags | O_NONBLOCK)) ++ g_warning ("_utils_exec_and_report_progress: Failed to set out_fd non-blocking: %m"); ++ flags = fcntl (err_fd, F_GETFL, 0); ++ if (fcntl (err_fd, F_SETFL, flags | O_NONBLOCK)) ++ g_warning ("_utils_exec_and_report_progress: Failed to set err_fd non-blocking: %m"); + +- g_io_channel_set_encoding (out_pipe, NULL, NULL); +- g_io_channel_set_encoding (err_pipe, NULL, NULL); ++ stdout_data = g_string_new (NULL); ++ stdout_buffer = g_string_new (NULL); ++ stderr_data = g_string_new (NULL); ++ stderr_buffer = g_string_new (NULL); + + fds[0].fd = out_fd; + fds[1].fd = err_fd; + fds[0].events = POLLIN | POLLHUP | POLLERR; + fds[1].events = POLLIN | POLLHUP | POLLERR; +- while (!out_done || !err_done) { +- poll_status = poll (fds, 2, -1); ++ while (! (out_done && err_done)) { ++ poll_status = poll (fds, 2, -1 /* timeout */); + g_warn_if_fail (poll_status != 0); /* no timeout specified, zero should never be returned */ + if (poll_status < 0) { + if (errno == EAGAIN || errno == EINTR) +@@ -372,140 +461,90 @@ static gboolean _utils_exec_and_report_progress (const gchar **argv, const BDExt + g_set_error (error, BD_UTILS_EXEC_ERROR, BD_UTILS_EXEC_ERROR_FAILED, + "Failed to poll output FDs: %m"); + bd_utils_report_finished (progress_id, (*error)->message); +- g_io_channel_shutdown (out_pipe, FALSE, NULL); +- g_io_channel_unref (out_pipe); +- g_io_channel_shutdown (err_pipe, FALSE, NULL); +- g_io_channel_unref (err_pipe); +- g_string_free (stdout_data, TRUE); +- g_string_free (stderr_data, TRUE); +- return FALSE; ++ success = FALSE; ++ break; + } + +- while (!out_done && (fds[0].revents & POLLIN)) { +- io_status = g_io_channel_read_line (out_pipe, &line, NULL, NULL, error); +- if (io_status == G_IO_STATUS_NORMAL) { +- if (prog_extract && prog_extract (line, &completion)) +- bd_utils_report_progress (progress_id, completion, NULL); +- else +- g_string_append (stdout_data, line); +- g_free (line); +- } else if (io_status == G_IO_STATUS_EOF) { +- out_done = TRUE; +- } else if (error && (*error)) { ++ if (!out_done) { ++ if (! _process_fd_event (out_fd, &fds[0], stdout_buffer, stdout_data, &stdout_buffer_pos, &out_done, progress_id, &completion, prog_extract, error)) { + bd_utils_report_finished (progress_id, (*error)->message); +- g_io_channel_shutdown (out_pipe, FALSE, NULL); +- g_io_channel_unref (out_pipe); +- g_io_channel_shutdown (err_pipe, FALSE, NULL); +- g_io_channel_unref (err_pipe); +- g_string_free (stdout_data, TRUE); +- g_string_free (stderr_data, TRUE); +- return FALSE; ++ success = FALSE; ++ break; + } + } +- if (fds[0].revents & POLLHUP || fds[0].revents & POLLERR || fds[0].revents & POLLNVAL) +- out_done = TRUE; + +- while (!err_done && (fds[1].revents & POLLIN)) { +- io_status = g_io_channel_read_line (err_pipe, &line, NULL, NULL, error); +- if (io_status == G_IO_STATUS_NORMAL) { +- g_string_append (stderr_data, line); +- g_free (line); +- } else if (io_status == G_IO_STATUS_EOF) { +- err_done = TRUE; +- } else if (error && (*error)) { ++ if (!err_done) { ++ if (! _process_fd_event (err_fd, &fds[1], stderr_buffer, stderr_data, &stderr_buffer_pos, &err_done, progress_id, &completion, prog_extract, error)) { + bd_utils_report_finished (progress_id, (*error)->message); +- g_io_channel_shutdown (out_pipe, FALSE, NULL); +- g_io_channel_unref (out_pipe); +- g_io_channel_shutdown (err_pipe, FALSE, NULL); +- g_io_channel_unref (err_pipe); +- g_string_free (stdout_data, TRUE); +- g_string_free (stderr_data, TRUE); +- return FALSE; ++ success = FALSE; ++ break; + } + } +- if (fds[1].revents & POLLHUP || fds[1].revents & POLLERR || fds[1].revents & POLLNVAL) +- err_done = TRUE; + } + ++ g_string_free (stdout_buffer, TRUE); ++ g_string_free (stderr_buffer, TRUE); ++ close (out_fd); ++ close (err_fd); ++ + child_ret = waitpid (pid, &status, 0); +- *proc_status = WEXITSTATUS(status); +- if (child_ret > 0) { +- if (*proc_status != 0) { +- if (stderr_data->str && (g_strcmp0 ("", stderr_data->str) != 0)) +- msg = stderr_data->str; +- else +- msg = stdout_data->str; +- g_set_error (error, BD_UTILS_EXEC_ERROR, BD_UTILS_EXEC_ERROR_FAILED, +- "Process reported exit code %d: %s", *proc_status, msg); +- bd_utils_report_finished (progress_id, (*error)->message); +- g_io_channel_shutdown (out_pipe, FALSE, NULL); +- g_io_channel_unref (out_pipe); +- g_io_channel_shutdown (err_pipe, FALSE, NULL); +- g_io_channel_unref (err_pipe); +- g_string_free (stdout_data, TRUE); +- g_string_free (stderr_data, TRUE); +- return FALSE; +- } +- if (WIFSIGNALED(status)) { +- g_set_error (error, BD_UTILS_EXEC_ERROR, BD_UTILS_EXEC_ERROR_FAILED, +- "Process killed with a signal"); +- bd_utils_report_finished (progress_id, (*error)->message); +- g_io_channel_shutdown (out_pipe, FALSE, NULL); +- g_io_channel_unref (out_pipe); +- g_io_channel_shutdown (err_pipe, FALSE, NULL); +- g_io_channel_unref (err_pipe); +- g_string_free (stdout_data, TRUE); +- g_string_free (stderr_data, TRUE); +- return FALSE; +- } +- } else if (child_ret == -1) { +- if (errno != ECHILD) { +- errno = 0; +- g_set_error (error, BD_UTILS_EXEC_ERROR, BD_UTILS_EXEC_ERROR_FAILED, +- "Failed to wait for the process"); +- bd_utils_report_finished (progress_id, (*error)->message); +- g_io_channel_shutdown (out_pipe, FALSE, NULL); +- g_io_channel_unref (out_pipe); +- g_io_channel_shutdown (err_pipe, FALSE, NULL); +- g_io_channel_unref (err_pipe); +- g_string_free (stdout_data, TRUE); +- g_string_free (stderr_data, TRUE); +- return FALSE; ++ *proc_status = WEXITSTATUS (status); ++ if (success) { ++ if (child_ret > 0) { ++ if (*proc_status != 0) { ++ msg = stderr_data->len > 0 ? stderr_data->str : stdout_data->str; ++ g_set_error (error, BD_UTILS_EXEC_ERROR, BD_UTILS_EXEC_ERROR_FAILED, ++ "Process reported exit code %d: %s", *proc_status, msg); ++ bd_utils_report_finished (progress_id, (*error)->message); ++ success = FALSE; ++ } else if (WIFSIGNALED (status)) { ++ g_set_error (error, BD_UTILS_EXEC_ERROR, BD_UTILS_EXEC_ERROR_FAILED, ++ "Process killed with a signal"); ++ bd_utils_report_finished (progress_id, (*error)->message); ++ success = FALSE; ++ } ++ } else if (child_ret == -1) { ++ if (errno != ECHILD) { ++ errno = 0; ++ g_set_error (error, BD_UTILS_EXEC_ERROR, BD_UTILS_EXEC_ERROR_FAILED, ++ "Failed to wait for the process"); ++ bd_utils_report_finished (progress_id, (*error)->message); ++ success = FALSE; ++ } else { ++ /* no such process (the child exited before we tried to wait for it) */ ++ errno = 0; ++ } + } +- /* no such process (the child exited before we tried to wait for it) */ +- errno = 0; ++ if (success) ++ bd_utils_report_finished (progress_id, "Completed"); + } +- +- bd_utils_report_finished (progress_id, "Completed"); + log_out (task_id, stdout_data->str, stderr_data->str); + log_done (task_id, *proc_status); + +- /* we don't care about the status here */ +- g_io_channel_shutdown (out_pipe, FALSE, error); +- g_io_channel_unref (out_pipe); +- g_io_channel_shutdown (err_pipe, FALSE, error); +- g_io_channel_unref (err_pipe); +- +- if (stdout) ++ if (success && stdout) + *stdout = g_string_free (stdout_data, FALSE); + else + g_string_free (stdout_data, TRUE); +- if (stderr) ++ if (success && stderr) + *stderr = g_string_free (stderr_data, FALSE); + else + g_string_free (stderr_data, TRUE); + +- return TRUE; ++ return success; + } + + /** + * bd_utils_exec_and_report_progress: + * @argv: (array zero-terminated=1): the argv array for the call + * @extra: (allow-none) (array zero-terminated=1): extra arguments +- * @prog_extract: (scope notified): function for extracting progress information ++ * @prog_extract: (scope notified) (nullable): function for extracting progress information + * @proc_status: (out): place to store the process exit status + * @error: (out): place to store error (if any) + * ++ * Note that any NULL bytes read from standard output and standard error ++ * output are treated as separators similar to newlines and @prog_extract ++ * will be called with the respective chunk. ++ * + * Returns: whether the @argv was successfully executed (no error and exit code 0) or not + */ + gboolean bd_utils_exec_and_report_progress (const gchar **argv, const BDExtraArg **extra, BDUtilsProgExtract prog_extract, gint *proc_status, GError **error) { +@@ -519,6 +558,9 @@ gboolean bd_utils_exec_and_report_progress (const gchar **argv, const BDExtraArg + * @output: (out): variable to store output to + * @error: (out): place to store error (if any) + * ++ * Note that any NULL bytes read from standard output and standard error ++ * output will be discarded. ++ * + * Returns: whether the @argv was successfully executed capturing the output or not + */ + gboolean bd_utils_exec_and_capture_output (const gchar **argv, const BDExtraArg **extra, gchar **output, GError **error) { +@@ -549,7 +591,6 @@ gboolean bd_utils_exec_and_capture_output (const gchar **argv, const BDExtraArg + g_free (stderr); + return TRUE; + } +- + } + + /** +-- +2.26.2 + + +From f5eb61c91ffc6b0d1fd709076a9579105655ff17 Mon Sep 17 00:00:00 2001 +From: Tomas Bzatek +Date: Fri, 25 Sep 2020 18:27:02 +0200 +Subject: [PATCH 3/5] exec: Clarify the BDUtilsProgExtract callback + documentation + +--- + src/utils/exec.h | 24 ++++++++++++++++++++++-- + 1 file changed, 22 insertions(+), 2 deletions(-) + +diff --git a/src/utils/exec.h b/src/utils/exec.h +index ad169e4..0e262a2 100644 +--- a/src/utils/exec.h ++++ b/src/utils/exec.h +@@ -31,10 +31,30 @@ typedef void (*BDUtilsProgFunc) (guint64 task_id, BDUtilsProgStatus status, guin + + /** + * BDUtilsProgExtract: +- * @line: line from extract progress from ++ * @line: line to extract progress from + * @completion: (out): percentage of completion + * +- * Returns: whether the line was a progress reporting line or not ++ * Callback function used to process a line captured from spawned command's standard ++ * output and standard error output. Typically used to extract completion percentage ++ * of a long-running job. ++ * ++ * Note that both outputs are read simultaneously with no guarantees of message order ++ * this function is called with. ++ * ++ * The value the @completion points to may contain value previously returned from ++ * this callback or zero when called for the first time. This is useful for extractors ++ * where only some kind of a tick mark is printed out as a progress and previous value ++ * is needed to compute an incremented value. It's important to keep in mind that this ++ * function is only called over lines, i.e. progress reporting printing out tick marks ++ * (e.g. dots) without a newline character might not work properly. ++ * ++ * The @line string usually contains trailing newline character, which may be absent ++ * however in case the spawned command exits without printing one. It's guaranteed ++ * this function is called over remaining buffer no matter what the trailing ++ * character is. ++ * ++ * Returns: whether the line was a progress reporting line and should be excluded ++ * from the collected standard output string or not. + */ + typedef gboolean (*BDUtilsProgExtract) (const gchar *line, guint8 *completion); + +-- +2.26.2 + + +From 8a7f0de5f63099a3e8bcaca005c4de04df959113 Mon Sep 17 00:00:00 2001 +From: Tomas Bzatek +Date: Fri, 25 Sep 2020 18:27:41 +0200 +Subject: [PATCH 4/5] tests: Add bufferbloat exec tests + +This should test pipe buffer exhaustion as well as potential pipe +read starvation while filling the other fd. +--- + tests/utils_test.py | 105 +++++++++++++++++++++++++++++++++++++++++++- + 1 file changed, 104 insertions(+), 1 deletion(-) + +diff --git a/tests/utils_test.py b/tests/utils_test.py +index 2bec5ed..ed13611 100644 +--- a/tests/utils_test.py ++++ b/tests/utils_test.py +@@ -1,8 +1,9 @@ + import unittest + import re + import os ++import six + import overrides_hack +-from utils import fake_utils, create_sparse_tempfile, create_lio_device, delete_lio_device, run_command, TestTags, tag_test ++from utils import fake_utils, create_sparse_tempfile, create_lio_device, delete_lio_device, run_command, TestTags, tag_test, read_file + + from gi.repository import BlockDev, GLib + +@@ -65,6 +66,9 @@ class UtilsExecLoggingTest(UtilsTestCase): + succ = BlockDev.utils_exec_and_report_error(["true"]) + self.assertTrue(succ) + ++ with six.assertRaisesRegex(self, GLib.GError, r"Process reported exit code 1"): ++ succ = BlockDev.utils_exec_and_report_error(["/bin/false"]) ++ + succ, out = BlockDev.utils_exec_and_capture_output(["echo", "hi"]) + self.assertTrue(succ) + self.assertEqual(out, "hi\n") +@@ -178,6 +182,105 @@ class UtilsExecLoggingTest(UtilsTestCase): + self.assertTrue(succ) + self.assertIn("LC_ALL=C", out) + ++ @tag_test(TestTags.NOSTORAGE, TestTags.CORE) ++ def test_exec_buffer_bloat(self): ++ """Verify that very large output from a command is handled properly""" ++ ++ # easy 64kB of data ++ cnt = 65536 ++ succ, out = BlockDev.utils_exec_and_capture_output(["bash", "-c", "for i in {1..%d}; do echo -n .; done" % cnt]) ++ self.assertTrue(succ) ++ self.assertEquals(len(out), cnt) ++ ++ succ, out = BlockDev.utils_exec_and_capture_output(["bash", "-c", "for i in {1..%d}; do echo -n .; echo -n \# >&2; done" % cnt]) ++ self.assertTrue(succ) ++ self.assertEquals(len(out), cnt) ++ ++ # now exceed the system pipe buffer size ++ # pipe(7): The maximum size (in bytes) of individual pipes that can be set by users without the CAP_SYS_RESOURCE capability. ++ cnt = int(read_file("/proc/sys/fs/pipe-max-size")) + 100 ++ self.assertGreater(cnt, 0) ++ ++ succ, out = BlockDev.utils_exec_and_capture_output(["bash", "-c", "for i in {1..%d}; do echo -n .; done" % cnt]) ++ self.assertTrue(succ) ++ self.assertEquals(len(out), cnt) ++ ++ succ, out = BlockDev.utils_exec_and_capture_output(["bash", "-c", "for i in {1..%d}; do echo -n .; echo -n \# >&2; done" % cnt]) ++ self.assertTrue(succ) ++ self.assertEquals(len(out), cnt) ++ ++ # make use of some newlines ++ succ, out = BlockDev.utils_exec_and_capture_output(["bash", "-c", "for i in {1..%d}; do echo -n .; if [ $(($i%%500)) -eq 0 ]; then echo ''; fi; done" % cnt]) ++ self.assertTrue(succ) ++ self.assertGreater(len(out), cnt) ++ ++ succ, out = BlockDev.utils_exec_and_capture_output(["bash", "-c", "for i in {1..%d}; do echo -n .; echo -n \# >&2; if [ $(($i%%500)) -eq 0 ]; then echo ''; echo '' >&2; fi; done" % cnt]) ++ self.assertTrue(succ) ++ self.assertGreater(len(out), cnt) ++ ++ ++ EXEC_PROGRESS_MSG = "Aloha, I'm the progress line you should match." ++ ++ def my_exec_progress_func_concat(self, line): ++ """Expect an concatenated string""" ++ s = "" ++ for i in range(10): ++ s += self.EXEC_PROGRESS_MSG ++ self.assertEqual(line, s) ++ self.num_matches += 1 ++ return 0 ++ ++ def my_exec_progress_func(self, line): ++ self.assertTrue(re.match(r".*%s.*" % self.EXEC_PROGRESS_MSG, line)) ++ self.num_matches += 1 ++ return 0 ++ ++ def test_exec_buffer_bloat_progress(self): ++ """Verify that progress reporting can handle large output""" ++ ++ # no newlines, should match just a single occurrence on EOF ++ cnt = 10 ++ self.num_matches = 0 ++ status = BlockDev.utils_exec_and_report_progress(["bash", "-c", "for i in {1..10}; do echo -n \"%s\"; done" % self.EXEC_PROGRESS_MSG], None, self.my_exec_progress_func_concat) ++ self.assertTrue(status) ++ self.assertEqual(self.num_matches, 1) ++ ++ # ten matches ++ self.num_matches = 0 ++ status = BlockDev.utils_exec_and_report_progress(["bash", "-c", "for i in {1..%d}; do echo \"%s\"; done" % (cnt, self.EXEC_PROGRESS_MSG)], None, self.my_exec_progress_func) ++ self.assertTrue(status) ++ self.assertEqual(self.num_matches, cnt) ++ ++ # 100k matches ++ cnt = 100000 ++ self.num_matches = 0 ++ status = BlockDev.utils_exec_and_report_progress(["bash", "-c", "for i in {1..%d}; do echo \"%s\"; done" % (cnt, self.EXEC_PROGRESS_MSG)], None, self.my_exec_progress_func) ++ self.assertTrue(status) ++ self.assertEqual(self.num_matches, cnt) ++ ++ # 100k matches on stderr ++ self.num_matches = 0 ++ status = BlockDev.utils_exec_and_report_progress(["bash", "-c", "for i in {1..%d}; do echo \"%s\" >&2; done" % (cnt, self.EXEC_PROGRESS_MSG)], None, self.my_exec_progress_func) ++ self.assertTrue(status) ++ self.assertEqual(self.num_matches, cnt) ++ ++ # 100k matches on stderr and stdout ++ self.num_matches = 0 ++ status = BlockDev.utils_exec_and_report_progress(["bash", "-c", "for i in {1..%d}; do echo \"%s\"; echo \"%s\" >&2; done" % (cnt, self.EXEC_PROGRESS_MSG, self.EXEC_PROGRESS_MSG)], None, self.my_exec_progress_func) ++ self.assertTrue(status) ++ self.assertEqual(self.num_matches, cnt * 2) ++ ++ # stdout and stderr output, non-zero return from the command and very long exception message ++ self.num_matches = 0 ++ with six.assertRaisesRegex(self, GLib.GError, r"Process reported exit code 66"): ++ status = BlockDev.utils_exec_and_report_progress(["bash", "-c", "for i in {1..%d}; do echo \"%s\"; echo \"%s\" >&2; done; exit 66" % (cnt, self.EXEC_PROGRESS_MSG, self.EXEC_PROGRESS_MSG)], None, self.my_exec_progress_func) ++ self.assertEqual(self.num_matches, cnt * 2) ++ ++ # no progress reporting callback given, tests slightly different code path ++ status = BlockDev.utils_exec_and_report_progress(["bash", "-c", "for i in {1..%d}; do echo \"%s\"; echo \"%s\" >&2; done" % (cnt, self.EXEC_PROGRESS_MSG, self.EXEC_PROGRESS_MSG)], None, None) ++ self.assertTrue(status) ++ ++ + class UtilsDevUtilsTestCase(UtilsTestCase): + @tag_test(TestTags.NOSTORAGE, TestTags.CORE) + def test_resolve_device(self): +-- +2.26.2 + + +From 7a3fd3d32dd325fb5188bcba74966e414e33c343 Mon Sep 17 00:00:00 2001 +From: Tomas Bzatek +Date: Wed, 30 Sep 2020 14:52:27 +0200 +Subject: [PATCH 5/5] tests: Add null-byte exec tests + +Some commands may print out NULL bytes in the middle of their output, +make sure everything works correctly. +--- + tests/utils_test.py | 48 +++++++++++++++++++++++++++++++++++++++++++++ + 1 file changed, 48 insertions(+) + +diff --git a/tests/utils_test.py b/tests/utils_test.py +index ed13611..1235be3 100644 +--- a/tests/utils_test.py ++++ b/tests/utils_test.py +@@ -280,6 +280,54 @@ class UtilsExecLoggingTest(UtilsTestCase): + status = BlockDev.utils_exec_and_report_progress(["bash", "-c", "for i in {1..%d}; do echo \"%s\"; echo \"%s\" >&2; done" % (cnt, self.EXEC_PROGRESS_MSG, self.EXEC_PROGRESS_MSG)], None, None) + self.assertTrue(status) + ++ def test_exec_null_bytes(self): ++ """Verify that null bytes in the output are processed properly""" ++ ++ TEST_DATA = ["First line", "Second line", "Third line"] ++ ++ status, out = BlockDev.utils_exec_and_capture_output(["bash", "-c", "echo -e \"%s\\0%s\\n%s\"" % (TEST_DATA[0], TEST_DATA[1], TEST_DATA[2])]) ++ self.assertTrue(status) ++ self.assertTrue(TEST_DATA[0] in out) ++ self.assertTrue(TEST_DATA[1] in out) ++ self.assertTrue(TEST_DATA[2] in out) ++ self.assertFalse("kuku!" in out) ++ ++ # ten matches ++ cnt = 10 ++ self.num_matches = 0 ++ status = BlockDev.utils_exec_and_report_progress(["bash", "-c", "for i in {1..%d}; do echo -e \"%s\\0%s\"; done" % (cnt, self.EXEC_PROGRESS_MSG, self.EXEC_PROGRESS_MSG)], None, self.my_exec_progress_func) ++ self.assertTrue(status) ++ self.assertEqual(self.num_matches, cnt * 2) ++ ++ # 100k matches ++ cnt = 100000 ++ self.num_matches = 0 ++ status = BlockDev.utils_exec_and_report_progress(["bash", "-c", "for i in {1..%d}; do echo -e \"%s\\0%s\"; done" % (cnt, self.EXEC_PROGRESS_MSG, self.EXEC_PROGRESS_MSG)], None, self.my_exec_progress_func) ++ self.assertTrue(status) ++ self.assertEqual(self.num_matches, cnt * 2) ++ ++ # 100k matches on stderr ++ self.num_matches = 0 ++ status = BlockDev.utils_exec_and_report_progress(["bash", "-c", "for i in {1..%d}; do echo -e \"%s\\0%s\" >&2; done" % (cnt, self.EXEC_PROGRESS_MSG, self.EXEC_PROGRESS_MSG)], None, self.my_exec_progress_func) ++ self.assertTrue(status) ++ self.assertEqual(self.num_matches, cnt * 2) ++ ++ # 100k matches on stderr and stdout ++ self.num_matches = 0 ++ status = BlockDev.utils_exec_and_report_progress(["bash", "-c", "for i in {1..%d}; do echo -e \"%s\\0%s\"; echo -e \"%s\\0%s\" >&2; done" % (cnt, self.EXEC_PROGRESS_MSG, self.EXEC_PROGRESS_MSG, self.EXEC_PROGRESS_MSG, self.EXEC_PROGRESS_MSG)], None, self.my_exec_progress_func) ++ self.assertTrue(status) ++ self.assertEqual(self.num_matches, cnt * 4) ++ ++ # stdout and stderr output, non-zero return from the command and very long exception message ++ self.num_matches = 0 ++ with six.assertRaisesRegex(self, GLib.GError, r"Process reported exit code 66"): ++ status = BlockDev.utils_exec_and_report_progress(["bash", "-c", "for i in {1..%d}; do echo -e \"%s\\0%s\"; echo -e \"%s\\0%s\" >&2; done; exit 66" % (cnt, self.EXEC_PROGRESS_MSG, self.EXEC_PROGRESS_MSG, self.EXEC_PROGRESS_MSG, self.EXEC_PROGRESS_MSG)], None, self.my_exec_progress_func) ++ self.assertEqual(self.num_matches, cnt * 4) ++ ++ # no progress reporting callback given, tests slightly different code path ++ status = BlockDev.utils_exec_and_report_progress(["bash", "-c", "for i in {1..%d}; do echo -e \"%s\\0%s\"; echo -e \"%s\\0%s\" >&2; done" % (cnt, self.EXEC_PROGRESS_MSG, self.EXEC_PROGRESS_MSG, self.EXEC_PROGRESS_MSG, self.EXEC_PROGRESS_MSG)], None, None) ++ self.assertTrue(status) ++ + + class UtilsDevUtilsTestCase(UtilsTestCase): + @tag_test(TestTags.NOSTORAGE, TestTags.CORE) +-- +2.26.2 + diff --git a/SOURCES/0003-LVM-thin-metadata-calculation-fix.patch b/SOURCES/0003-LVM-thin-metadata-calculation-fix.patch new file mode 100644 index 0000000..3eca9ae --- /dev/null +++ b/SOURCES/0003-LVM-thin-metadata-calculation-fix.patch @@ -0,0 +1,914 @@ +From 2bb371937c7ef73f26717e57a5eb78cafe90a9f7 Mon Sep 17 00:00:00 2001 +From: Vojtech Trefny +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 + + #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 + #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 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 +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 + #include ++#include + + #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 +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 + #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) { +@@ -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 + #include + #include +-#include + #include + #include + +@@ -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 +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))) { + * + * 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); + } + +@@ -2670,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 74493feb..2be1dbdb 100644 +--- a/src/plugins/lvm.c ++++ b/src/plugins/lvm.c +@@ -700,7 +700,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))); + } + +@@ -712,7 +712,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; +@@ -734,7 +734,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; + } + +@@ -754,7 +754,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) +@@ -848,7 +848,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)); + } + +@@ -862,7 +862,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)) +@@ -1983,7 +1983,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 */ + +@@ -2008,7 +2008,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); +@@ -2027,7 +2027,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); + } + +@@ -2037,7 +2037,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"; + +From f106e775d3c73e5f97512dd109627e00539b703a Mon Sep 17 00:00:00 2001 +From: Vojtech Trefny +Date: Tue, 15 Dec 2020 14:53:13 +0100 +Subject: [PATCH 5/5] Fix max size limit for LVM thinpool metadata + +DM_THIN_MAX_METADATA_SIZE is in 512 sectors, not in KiB so the +upper limit is 15.81 GiB not 31.62 GiB +--- + src/lib/plugin_apis/lvm.api | 2 +- + src/plugins/lvm.h | 10 ++++++---- + tests/lvm_dbus_tests.py | 3 ++- + tests/lvm_test.py | 3 ++- + 4 files changed, 11 insertions(+), 7 deletions(-) + +diff --git a/src/lib/plugin_apis/lvm.api b/src/lib/plugin_apis/lvm.api +index 9f25c1ed..563c1041 100644 +--- a/src/lib/plugin_apis/lvm.api ++++ b/src/lib/plugin_apis/lvm.api +@@ -20,7 +20,7 @@ + #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 (33957085184ULL) // 31.62 GiB ++#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 +diff --git a/src/plugins/lvm.h b/src/plugins/lvm.h +index 01c06ca4..2162d769 100644 +--- a/src/plugins/lvm.h ++++ b/src/plugins/lvm.h +@@ -22,11 +22,13 @@ + #define USE_DEFAULT_PE_SIZE 0 + #define RESOLVE_PE_SIZE(size) ((size) == USE_DEFAULT_PE_SIZE ? BD_LVM_DEFAULT_PE_SIZE : (size)) + +-/* 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 */ ++/* 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) +-#define BD_LVM_MAX_THPOOL_MD_SIZE (DM_THIN_MAX_METADATA_SIZE KiB) ++ ++/* 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) +diff --git a/tests/lvm_dbus_tests.py b/tests/lvm_dbus_tests.py +index c06a480c..8f2bb95d 100644 +--- a/tests/lvm_dbus_tests.py ++++ b/tests/lvm_dbus_tests.py +@@ -160,10 +160,11 @@ def test_is_valid_thpool_md_size(self): + + 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.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(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) +diff --git a/tests/lvm_test.py b/tests/lvm_test.py +index b84adece..6f80a3ba 100644 +--- a/tests/lvm_test.py ++++ b/tests/lvm_test.py +@@ -150,10 +150,11 @@ def test_is_valid_thpool_md_size(self): + + 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.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(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) diff --git a/SPECS/libblockdev.spec b/SPECS/libblockdev.spec index 647b5a6..2700cc6 100644 --- a/SPECS/libblockdev.spec +++ b/SPECS/libblockdev.spec @@ -125,12 +125,14 @@ Name: libblockdev Version: 2.24 -Release: 2%{?dist} +Release: 5%{?dist} Summary: A library for low-level manipulation with block devices License: LGPLv2+ URL: https://github.com/storaged-project/libblockdev Source0: https://github.com/storaged-project/libblockdev/releases/download/%{version}-%{release}/%{name}-%{version}.tar.gz Patch0: 0001-exec-Fix-setting-locale-for-util-calls.patch +Patch1: 0002-exec-polling-fixes.patch +Patch2: 0003-LVM-thin-metadata-calculation-fix.patch BuildRequires: glib2-devel %if %{with_gi} @@ -687,6 +689,8 @@ A meta-package that pulls all the libblockdev plugins as dependencies. %prep %setup -q -n %{name}-%{version} %patch0 -p1 +%patch1 -p1 +%patch2 -p1 %build autoreconf -ivf @@ -990,9 +994,21 @@ find %{buildroot} -type f -name "*.la" | xargs %{__rm} %files plugins-all %changelog +* Mon Jan 11 2021 Vojtech Trefny - 2.24-5 +- Fix LVM thin metadata calculation fix + Resolves: rhbz#1901714 + +* Mon Dec 14 2020 Vojtech Trefny - 2.24-4 +- LVM thin metadata calculation fix + Resolves: rhbz#1901714 + +* Wed Nov 18 2020 Vojtech Trefny - 2.24-3 +- exec: Polling fixes + Resolves: rhbz#1884689 + * Mon Nov 09 2020 Vojtech Trefny - 2.24-2 - exec: Fix setting locale for util calls - Resolves: rhbz#1895286 + Resolves: rhbz#1880031 * Fri May 22 2020 Vojtech Trefny - 2.24-1 - Rebased to the latest upstream release 2.24