From 3f9f1fa90a087186dcc96060537543d2685616d8 Mon Sep 17 00:00:00 2001
From: Vojtech Trefny <vtrefny@redhat.com>
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 6285a48..7ac5089 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 e7b4bf0..6219a40 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 = $(GLIB_LIBS) ${builddir}/../utils/libbd_utils.la
+libbd_swap_la_CFLAGS = $(GLIB_CFLAGS) $(BLKID_CFLAGS) -Wall -Wextra -Werror
+libbd_swap_la_LIBADD = $(GLIB_LIBS) $(BLKID_LIBS) ${builddir}/../utils/libbd_utils.la
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 <string.h>
#include <unistd.h>
#include <sys/swap.h>
+#include <fcntl.h>
+#include <blkid.h>
#include <blockdev/utils.h>
#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 f6508829e7cac138e4961a1c3ef6170d6f67bfd9 Mon Sep 17 00:00:00 2001
From: Vojtech Trefny <vtrefny@redhat.com>
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