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