diff --git a/0001-swap-error-codes.patch b/0001-swap-error-codes.patch new file mode 100644 index 0000000..b95ed83 --- /dev/null +++ b/0001-swap-error-codes.patch @@ -0,0 +1,407 @@ +From 22171c66d11597deee5726ed57e06f3b76fef7aa Mon Sep 17 00:00:00 2001 +From: Vojtech Trefny +Date: Mon, 1 Oct 2018 16:06:42 +0200 +Subject: [PATCH 1/2] Use libblkid to check swap status before swapon + +libblkid probe is more reliable than our custom check. +--- + configure.ac | 2 +- + src/plugins/Makefile.am | 4 +- + src/plugins/swap.c | 149 ++++++++++++++++++++++++++++++++-------- + 3 files changed, 124 insertions(+), 31 deletions(-) + +diff --git a/configure.ac b/configure.ac +index 4890136..be641b3 100644 +--- a/configure.ac ++++ b/configure.ac +@@ -227,7 +227,7 @@ AS_IF([test "x$with_fs" != "xno"], + AC_SUBST([PARTED_FS_CFLAGS], [])])], + []) + +-AS_IF([test "x$with_fs" != "xno" -o "x$with_crypto" != "xno"], ++AS_IF([test "x$with_fs" != "xno" -o "x$with_crypto" != "xno" -o "x$with_swap" != "xno"], + [LIBBLOCKDEV_PKG_CHECK_MODULES([BLKID], [blkid >= 2.23.0]) + # older versions of libblkid don't support BLKID_SUBLKS_BADCSUM so let's just + # define it as 0 (neutral value for bit combinations of flags) +diff --git a/src/plugins/Makefile.am b/src/plugins/Makefile.am +index 5b41822..fd15efb 100644 +--- a/src/plugins/Makefile.am ++++ b/src/plugins/Makefile.am +@@ -149,8 +149,8 @@ libbd_nvdimm_la_SOURCES = nvdimm.c nvdimm.h check_deps.c check_deps.h + endif + + if WITH_SWAP +-libbd_swap_la_CFLAGS = $(GLIB_CFLAGS) -Wall -Wextra -Werror +-libbd_swap_la_LIBADD = ${builddir}/../utils/libbd_utils.la $(GLIB_LIBS) ++libbd_swap_la_CFLAGS = $(GLIB_CFLAGS) $(BLKID_CFLAGS) -Wall -Wextra -Werror ++libbd_swap_la_LIBADD = ${builddir}/../utils/libbd_utils.la $(GLIB_LIBS) $(BLKID_LIBS) + libbd_swap_la_LDFLAGS = -L${srcdir}/../utils/ -version-info 2:0:0 -Wl,--no-undefined + libbd_swap_la_CPPFLAGS = -I${builddir}/../../include/ + libbd_swap_la_SOURCES = swap.c swap.h check_deps.c check_deps.h +diff --git a/src/plugins/swap.c b/src/plugins/swap.c +index bc52637..bfe653f 100644 +--- a/src/plugins/swap.c ++++ b/src/plugins/swap.c +@@ -21,6 +21,8 @@ + #include + #include + #include ++#include ++#include + #include + + #include "swap.h" +@@ -179,13 +181,14 @@ gboolean bd_swap_mkswap (const gchar *device, const gchar *label, const BDExtraA + * Tech category: %BD_SWAP_TECH_SWAP-%BD_SWAP_TECH_MODE_ACTIVATE_DEACTIVATE + */ + gboolean bd_swap_swapon (const gchar *device, gint priority, GError **error) { +- GIOChannel *dev_file = NULL; +- GIOStatus io_status = G_IO_STATUS_ERROR; +- GError *tmp_error = NULL; +- gsize num_read = 0; +- gchar dev_status[11]; +- dev_status[10] = '\0'; +- gint page_size; ++ blkid_probe probe = NULL; ++ gint fd = 0; ++ gint status = 0; ++ guint n_try = 0; ++ const gchar *value = NULL; ++ gint64 status_len = 0; ++ gint64 swap_pagesize = 0; ++ gint64 sys_pagesize = 0; + gint flags = 0; + gint ret = 0; + guint64 progress_id = 0; +@@ -198,53 +201,143 @@ gboolean bd_swap_swapon (const gchar *device, gint priority, GError **error) { + + bd_utils_report_progress (progress_id, 0, "Analysing the swap device"); + /* check the device if it is an activatable swap */ +- dev_file = g_io_channel_new_file (device, "r", error); +- if (!dev_file) { +- /* error is already populated */ ++ probe = blkid_new_probe (); ++ if (!probe) { ++ g_set_error (error, BD_SWAP_ERROR, BD_SWAP_ERROR_UNKNOWN_STATE, ++ "Failed to create a new probe"); + bd_utils_report_finished (progress_id, (*error)->message); + return FALSE; + } + +- page_size = getpagesize (); +- page_size = MAX (2048, page_size); +- io_status = g_io_channel_seek_position (dev_file, page_size - 10, G_SEEK_SET, &tmp_error); +- if (io_status != G_IO_STATUS_NORMAL) { ++ fd = open (device, O_RDONLY|O_CLOEXEC); ++ if (fd == -1) { + g_set_error (error, BD_SWAP_ERROR, BD_SWAP_ERROR_UNKNOWN_STATE, +- "Failed to determine device's state: %s", tmp_error->message); +- g_clear_error (&tmp_error); +- g_io_channel_shutdown (dev_file, FALSE, &tmp_error); ++ "Failed to open the device '%s'", device); + bd_utils_report_finished (progress_id, (*error)->message); ++ blkid_free_probe (probe); + return FALSE; + } + +- io_status = g_io_channel_read_chars (dev_file, dev_status, 10, &num_read, &tmp_error); +- if ((io_status != G_IO_STATUS_NORMAL) || (num_read != 10)) { ++ /* we may need to try mutliple times with some delays in case the device is ++ busy at the very moment */ ++ for (n_try=5, status=-1; (status != 0) && (n_try > 0); n_try--) { ++ status = blkid_probe_set_device (probe, fd, 0, 0); ++ if (status != 0) ++ g_usleep (100 * 1000); /* microseconds */ ++ } ++ if (status != 0) { + g_set_error (error, BD_SWAP_ERROR, BD_SWAP_ERROR_UNKNOWN_STATE, +- "Failed to determine device's state: %s", tmp_error->message); +- g_clear_error (&tmp_error); +- g_io_channel_shutdown (dev_file, FALSE, &tmp_error); +- g_clear_error (&tmp_error); ++ "Failed to create a probe for the device '%s'", device); + bd_utils_report_finished (progress_id, (*error)->message); ++ blkid_free_probe (probe); ++ close (fd); + return FALSE; + } + +- g_io_channel_shutdown (dev_file, FALSE, &tmp_error); +- g_clear_error (&tmp_error); ++ blkid_probe_enable_superblocks (probe, 1); ++ blkid_probe_set_superblocks_flags (probe, BLKID_SUBLKS_TYPE | BLKID_SUBLKS_MAGIC); + +- if (g_str_has_prefix (dev_status, "SWAP-SPACE")) { ++ /* we may need to try mutliple times with some delays in case the device is ++ busy at the very moment */ ++ for (n_try=5, status=-1; !(status == 0 || status == 1) && (n_try > 0); n_try--) { ++ status = blkid_do_safeprobe (probe); ++ if (status < 0) ++ g_usleep (100 * 1000); /* microseconds */ ++ } ++ if (status < 0) { ++ /* -1 or -2 = error during probing*/ ++ g_set_error (error, BD_SWAP_ERROR, BD_SWAP_ERROR_UNKNOWN_STATE, ++ "Failed to probe the device '%s'", device); ++ bd_utils_report_finished (progress_id, (*error)->message); ++ blkid_free_probe (probe); ++ close (fd); ++ return FALSE; ++ } else if (status == 1) { ++ /* 1 = nothing detected */ ++ g_set_error (error, BD_SWAP_ERROR, BD_SWAP_ERROR_UNKNOWN_STATE, ++ "No superblock detected on the device '%s'", device); ++ bd_utils_report_finished (progress_id, (*error)->message); ++ blkid_free_probe (probe); ++ close (fd); ++ return FALSE; ++ } ++ ++ status = blkid_probe_lookup_value (probe, "TYPE", &value, NULL); ++ if (status != 0) { ++ g_set_error (error, BD_SWAP_ERROR, BD_SWAP_ERROR_UNKNOWN_STATE, ++ "Failed to get format type for the device '%s'", device); ++ bd_utils_report_finished (progress_id, (*error)->message); ++ blkid_free_probe (probe); ++ close (fd); ++ return FALSE; ++ } ++ ++ if (g_strcmp0 (value, "swap") != 0) { ++ g_set_error (error, BD_SWAP_ERROR, BD_SWAP_ERROR_UNKNOWN_STATE, ++ "Device '%s' is not formatted as swap", device); ++ bd_utils_report_finished (progress_id, (*error)->message); ++ blkid_free_probe (probe); ++ close (fd); ++ return FALSE; ++ } ++ ++ status = blkid_probe_lookup_value (probe, "SBMAGIC", &value, NULL); ++ if (status != 0) { ++ g_set_error (error, BD_SWAP_ERROR, BD_SWAP_ERROR_UNKNOWN_STATE, ++ "Failed to get swap status on the device '%s'", device); ++ bd_utils_report_finished (progress_id, (*error)->message); ++ blkid_free_probe (probe); ++ close (fd); ++ return FALSE; ++ } ++ ++ if (g_strcmp0 (value, "SWAP-SPACE") == 0) { + g_set_error (error, BD_SWAP_ERROR, BD_SWAP_ERROR_ACTIVATE, + "Old swap format, cannot activate."); + bd_utils_report_finished (progress_id, (*error)->message); ++ blkid_free_probe (probe); ++ close (fd); + return FALSE; +- } else if (g_str_has_prefix (dev_status, "S1SUSPEND") || g_str_has_prefix (dev_status, "S2SUSPEND")) { ++ } else if (g_strcmp0 (value, "S1SUSPEND") == 0 || g_strcmp0 (value, "S2SUSPEND") == 0) { + g_set_error (error, BD_SWAP_ERROR, BD_SWAP_ERROR_ACTIVATE, + "Suspended system on the swap device, cannot activate."); + bd_utils_report_finished (progress_id, (*error)->message); ++ blkid_free_probe (probe); ++ close (fd); + return FALSE; +- } else if (!g_str_has_prefix (dev_status, "SWAPSPACE2")) { ++ } else if (g_strcmp0 (value, "SWAPSPACE2") != 0) { + g_set_error (error, BD_SWAP_ERROR, BD_SWAP_ERROR_ACTIVATE, + "Unknown swap space format, cannot activate."); + bd_utils_report_finished (progress_id, (*error)->message); ++ blkid_free_probe (probe); ++ close (fd); ++ return FALSE; ++ } ++ ++ status_len = (gint64) strlen (value); ++ ++ status = blkid_probe_lookup_value (probe, "SBMAGIC_OFFSET", &value, NULL); ++ if (status != 0 || !value) { ++ g_set_error (error, BD_SWAP_ERROR, BD_SWAP_ERROR_UNKNOWN_STATE, ++ "Failed to get swap status on the device '%s'", device); ++ bd_utils_report_finished (progress_id, (*error)->message); ++ blkid_free_probe (probe); ++ close (fd); ++ return FALSE; ++ } ++ ++ swap_pagesize = status_len + g_ascii_strtoll (value, (char **)NULL, 10); ++ ++ blkid_free_probe (probe); ++ close (fd); ++ ++ sys_pagesize = getpagesize (); ++ ++ if (swap_pagesize != sys_pagesize) { ++ g_set_error (error, BD_SWAP_ERROR, BD_SWAP_ERROR_UNKNOWN_STATE, ++ "Swap format pagesize (%"G_GINT64_FORMAT") and system pagesize (%"G_GINT64_FORMAT") don't match", ++ swap_pagesize, sys_pagesize); ++ bd_utils_report_finished (progress_id, (*error)->message); + return FALSE; + } + +-- +2.17.1 + + +From bef564b7525fc97dc1f4e4383768bd8f22f4f0b5 Mon Sep 17 00:00:00 2001 +From: Vojtech Trefny +Date: Thu, 4 Oct 2018 08:07:55 +0200 +Subject: [PATCH 2/2] Add error codes and Python exceptions for swapon fails + +We need to be able to tell why swapon failed so our users can +decide what to do. +--- + src/lib/plugin_apis/swap.api | 4 ++++ + src/plugins/swap.c | 10 +++++----- + src/plugins/swap.h | 4 ++++ + src/python/gi/overrides/BlockDev.py | 19 +++++++++++++++++-- + tests/swap_test.py | 13 +++++++++++++ + 5 files changed, 43 insertions(+), 7 deletions(-) + +diff --git a/src/lib/plugin_apis/swap.api b/src/lib/plugin_apis/swap.api +index d0906fe..3fcc0e5 100644 +--- a/src/lib/plugin_apis/swap.api ++++ b/src/lib/plugin_apis/swap.api +@@ -10,6 +10,10 @@ typedef enum { + BD_SWAP_ERROR_UNKNOWN_STATE, + BD_SWAP_ERROR_ACTIVATE, + BD_SWAP_ERROR_TECH_UNAVAIL, ++ BD_SWAP_ERROR_ACTIVATE_OLD, ++ BD_SWAP_ERROR_ACTIVATE_SUSPEND, ++ BD_SWAP_ERROR_ACTIVATE_UNKNOWN, ++ BD_SWAP_ERROR_ACTIVATE_PAGESIZE, + } BDSwapError; + + typedef enum { +diff --git a/src/plugins/swap.c b/src/plugins/swap.c +index bfe653f..28db6f3 100644 +--- a/src/plugins/swap.c ++++ b/src/plugins/swap.c +@@ -292,21 +292,21 @@ gboolean bd_swap_swapon (const gchar *device, gint priority, GError **error) { + } + + if (g_strcmp0 (value, "SWAP-SPACE") == 0) { +- g_set_error (error, BD_SWAP_ERROR, BD_SWAP_ERROR_ACTIVATE, ++ g_set_error (error, BD_SWAP_ERROR, BD_SWAP_ERROR_ACTIVATE_OLD, + "Old swap format, cannot activate."); + bd_utils_report_finished (progress_id, (*error)->message); + blkid_free_probe (probe); + close (fd); + return FALSE; + } else if (g_strcmp0 (value, "S1SUSPEND") == 0 || g_strcmp0 (value, "S2SUSPEND") == 0) { +- g_set_error (error, BD_SWAP_ERROR, BD_SWAP_ERROR_ACTIVATE, ++ g_set_error (error, BD_SWAP_ERROR, BD_SWAP_ERROR_ACTIVATE_SUSPEND, + "Suspended system on the swap device, cannot activate."); + bd_utils_report_finished (progress_id, (*error)->message); + blkid_free_probe (probe); + close (fd); + return FALSE; + } else if (g_strcmp0 (value, "SWAPSPACE2") != 0) { +- g_set_error (error, BD_SWAP_ERROR, BD_SWAP_ERROR_ACTIVATE, ++ g_set_error (error, BD_SWAP_ERROR, BD_SWAP_ERROR_ACTIVATE_UNKNOWN, + "Unknown swap space format, cannot activate."); + bd_utils_report_finished (progress_id, (*error)->message); + blkid_free_probe (probe); +@@ -318,7 +318,7 @@ gboolean bd_swap_swapon (const gchar *device, gint priority, GError **error) { + + status = blkid_probe_lookup_value (probe, "SBMAGIC_OFFSET", &value, NULL); + if (status != 0 || !value) { +- g_set_error (error, BD_SWAP_ERROR, BD_SWAP_ERROR_UNKNOWN_STATE, ++ g_set_error (error, BD_SWAP_ERROR, BD_SWAP_ERROR_ACTIVATE_PAGESIZE, + "Failed to get swap status on the device '%s'", device); + bd_utils_report_finished (progress_id, (*error)->message); + blkid_free_probe (probe); +@@ -334,7 +334,7 @@ gboolean bd_swap_swapon (const gchar *device, gint priority, GError **error) { + sys_pagesize = getpagesize (); + + if (swap_pagesize != sys_pagesize) { +- g_set_error (error, BD_SWAP_ERROR, BD_SWAP_ERROR_UNKNOWN_STATE, ++ g_set_error (error, BD_SWAP_ERROR, BD_SWAP_ERROR_ACTIVATE_PAGESIZE, + "Swap format pagesize (%"G_GINT64_FORMAT") and system pagesize (%"G_GINT64_FORMAT") don't match", + swap_pagesize, sys_pagesize); + bd_utils_report_finished (progress_id, (*error)->message); +diff --git a/src/plugins/swap.h b/src/plugins/swap.h +index a01c873..9947bad 100644 +--- a/src/plugins/swap.h ++++ b/src/plugins/swap.h +@@ -12,6 +12,10 @@ typedef enum { + BD_SWAP_ERROR_UNKNOWN_STATE, + BD_SWAP_ERROR_ACTIVATE, + BD_SWAP_ERROR_TECH_UNAVAIL, ++ BD_SWAP_ERROR_ACTIVATE_OLD, ++ BD_SWAP_ERROR_ACTIVATE_SUSPEND, ++ BD_SWAP_ERROR_ACTIVATE_UNKNOWN, ++ BD_SWAP_ERROR_ACTIVATE_PAGESIZE, + } BDSwapError; + + typedef enum { +diff --git a/src/python/gi/overrides/BlockDev.py b/src/python/gi/overrides/BlockDev.py +index c2ef2f4..e608887 100644 +--- a/src/python/gi/overrides/BlockDev.py ++++ b/src/python/gi/overrides/BlockDev.py +@@ -1031,7 +1031,17 @@ __all__.append("MpathError") + + class SwapError(BlockDevError): + pass +-__all__.append("SwapError") ++class SwapActivateError(SwapError): ++ pass ++class SwapOldError(SwapActivateError): ++ pass ++class SwapSuspendError(SwapActivateError): ++ pass ++class SwapUnknownError(SwapActivateError): ++ pass ++class SwapPagesizeError(SwapActivateError): ++ pass ++__all__.extend(("SwapError", "SwapActivateError", "SwapOldError", "SwapSuspendError", "SwapUnknownError", "SwapPagesizeError")) + + class KbdError(BlockDevError): + pass +@@ -1070,6 +1080,11 @@ __all__.append("BlockDevNotImplementedError") + not_implemented_rule = XRule(GLib.Error, re.compile(r".*The function '.*' called, but not implemented!"), None, BlockDevNotImplementedError) + + fs_nofs_rule = XRule(GLib.Error, None, 3, FSNoFSError) ++swap_activate_rule = XRule(GLib.Error, None, 1, SwapActivateError) ++swap_old_rule = XRule(GLib.Error, None, 3, SwapOldError) ++swap_suspend_rule = XRule(GLib.Error, None, 4, SwapSuspendError) ++swap_unknown_rule = XRule(GLib.Error, None, 5, SwapUnknownError) ++swap_pagesize_rule = XRule(GLib.Error, None, 6, SwapPagesizeError) + + btrfs = ErrorProxy("btrfs", BlockDev, [(GLib.Error, BtrfsError)], [not_implemented_rule]) + __all__.append("btrfs") +@@ -1092,7 +1107,7 @@ __all__.append("md") + mpath = ErrorProxy("mpath", BlockDev, [(GLib.Error, MpathError)], [not_implemented_rule]) + __all__.append("mpath") + +-swap = ErrorProxy("swap", BlockDev, [(GLib.Error, SwapError)], [not_implemented_rule]) ++swap = ErrorProxy("swap", BlockDev, [(GLib.Error, SwapError)], [not_implemented_rule, swap_activate_rule, swap_old_rule, swap_suspend_rule, swap_unknown_rule, swap_pagesize_rule]) + __all__.append("swap") + + kbd = ErrorProxy("kbd", BlockDev, [(GLib.Error, KbdError)], [not_implemented_rule]) +diff --git a/tests/swap_test.py b/tests/swap_test.py +index 05d0c19..395fdf5 100644 +--- a/tests/swap_test.py ++++ b/tests/swap_test.py +@@ -97,6 +97,19 @@ class SwapTestCase(SwapTest): + _ret, out, _err = run_command("blkid -ovalue -sLABEL -p %s" % self.loop_dev) + self.assertEqual(out, "BlockDevSwap") + ++ def test_swapon_pagesize(self): ++ """Verify that activating swap with different pagesize fails""" ++ ++ # create swap with 64k pagesize ++ ret, out, err = run_command("mkswap --pagesize 65536 %s" % self.loop_dev) ++ if ret != 0: ++ self.fail("Failed to prepare swap for pagesize test: %s %s" % (out, err)) ++ ++ # activation should fail because swap has different pagesize ++ with self.assertRaises(BlockDev.SwapPagesizeError): ++ BlockDev.swap.swapon(self.loop_dev) ++ ++ + class SwapUnloadTest(SwapTest): + def setUp(self): + # make sure the library is initialized with all plugins loaded for other +-- +2.17.1 + diff --git a/libblockdev.spec b/libblockdev.spec index f037503..f3cfd52 100644 --- a/libblockdev.spec +++ b/libblockdev.spec @@ -121,11 +121,12 @@ Name: libblockdev Version: 2.20 -Release: 1%{?dist} +Release: 2%{?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-swap-error-codes.patch BuildRequires: glib2-devel %if %{with_gi} @@ -659,8 +660,10 @@ A meta-package that pulls all the libblockdev plugins as dependencies. %prep %setup -q -n %{name}-%{version} +%patch0 -p1 %build +autoreconf -ivf %configure %{?configure_opts} %{__make} %{?_smp_mflags} @@ -957,6 +960,10 @@ find %{buildroot} -type f -name "*.la" | xargs %{__rm} %files plugins-all %changelog +* Mon Oct 08 2018 Vojtech Trefny - 2.20-2 +- Use libblkid to check swap status before swapon (vtrefny) +- Add error codes and Python exceptions for swapon fails (vtrefny) + * Wed Sep 26 2018 Vojtech Trefny - 2.20-1 - Use unsafe caching for storage for devel/testing VMs (v.podzimek) - Require newer version of cryptsetup for LUKS2 tests (vtrefny)