New version

- Open the device file as RDWR when committing parts (vpodzime)
- Handle mdadm --examine output during migration (adamw)
This commit is contained in:
Vratislav Podzimek 2016-10-27 11:45:45 +02:00
parent 9463583b61
commit d4b36b4c9e
3 changed files with 256 additions and 1 deletions

View File

@ -1,6 +1,6 @@
Name: libblockdev
Version: 1.9
Release: 6%{?dist}
Release: 7%{?dist}
Summary: A library for low-level manipulation with block devices
License: LGPLv2+
URL: https://github.com/rhinstaller/libblockdev
@ -14,6 +14,8 @@ Patch4: mdadm_examine_uuid.patch
Patch5: mdadm_fw_raid_device.patch
Patch6: obj_path_signatures.patch
Patch7: parted_udev_issues.patch
Patch8: md_examine_migrated.patch
Patch9: part_rdwr_commit.patch
BuildRequires: glib2-devel
BuildRequires: gobject-introspection-devel
@ -392,6 +394,8 @@ A meta-package that pulls all the libblockdev plugins as dependencies.
%patch5 -p1
%patch6 -p1
%patch7 -p1
%patch8 -p1
%patch9 -p1
%build
%configure
@ -589,6 +593,11 @@ find %{buildroot} -type f -name "*.la" | xargs %{__rm}
%files plugins-all
%changelog
* Thu Oct 27 2016 Vratislav Podzimek <vpodzime@redhat.com> - 1.9-7
- Open the device file as RDWR when committing parts (vpodzime)
- Handle mdadm --examine output during migration (adamw)
Resolves: rhbz#1381996
* Mon Oct 24 2016 Vratislav Podzimek <vpodzime@redhat.com> - 1.9-6
- Prevent issues between libparted and udev (vpodzime)

160
md_examine_migrated.patch Normal file
View File

@ -0,0 +1,160 @@
From 673422da37ce9f96b44a182addd1aa922a9b047f Mon Sep 17 00:00:00 2001
From: Adam Williamson <awilliam@redhat.com>
Date: Mon, 24 Oct 2016 14:41:47 -0700
Subject: [PATCH] Handle mdadm --examine output during migration
If a RAID set is undergoing any operation considered to be a
'migration' - which includes initialization of a new set at any
RAID level that includes redundancy - mdadm --examine will have
lines like this:
RAID Level : 5 <-- 5
Members : 3 <-- 3
Map State : normal <-- uninitialized
At present we don't understand this at all. This is a fairly
minimal fix which just has parse_mdadm_vars notice such lines,
split the 'value' on "<--", and stuff the first value (the one
being migrated *to*, which is usually what we care about, I
think) into the table. We could do something more elaborate
like store both values and make `get_examine_data_from_table`
understand that, but it's more complicated and this should be
all we truly need for now.
(cherry picked from commit 07aec87385e2fdbe3f0855d697facf04b9ea6521)
Signed-off-by: Vratislav Podzimek <vpodzime@redhat.com>
---
src/plugins/mdraid.c | 13 +++++-
tests/md_test.py | 8 ++++
tests/mdadm_fw_RAID_examine_migrate/mdadm | 69 +++++++++++++++++++++++++++++++
3 files changed, 88 insertions(+), 2 deletions(-)
create mode 100755 tests/mdadm_fw_RAID_examine_migrate/mdadm
diff --git a/src/plugins/mdraid.c b/src/plugins/mdraid.c
index 9cb0cc7..c4780eb 100644
--- a/src/plugins/mdraid.c
+++ b/src/plugins/mdraid.c
@@ -155,6 +155,7 @@ static GHashTable* parse_mdadm_vars (gchar *str, gchar *item_sep, gchar *key_val
gchar **items = NULL;
gchar **item_p = NULL;
gchar **key_val = NULL;
+ gchar **vals = NULL;
table = g_hash_table_new_full (g_str_hash, g_str_equal, g_free, g_free);
*num_items = 0;
@@ -165,8 +166,16 @@ static GHashTable* parse_mdadm_vars (gchar *str, gchar *item_sep, gchar *key_val
if (g_strv_length (key_val) == 2) {
/* we only want to process valid lines (with the separator) */
/* only use the first value for the given key */
- if (!g_hash_table_contains (table, g_strstrip (key_val[0])))
- g_hash_table_insert (table, g_strstrip (key_val[0]), g_strstrip (key_val[1]));
+ if (!g_hash_table_contains (table, g_strstrip (key_val[0]))) {
+ if (strstr (key_val[1], "<--")) {
+ /* mdadm --examine output for a set being migrated */
+ vals = g_strsplit (key_val[1], "<--", 2);
+ g_hash_table_insert (table, g_strstrip (key_val[0]), g_strstrip (vals[0]));
+ g_free (vals[1]);
+ } else {
+ g_hash_table_insert (table, g_strstrip (key_val[0]), g_strstrip (key_val[1]));
+ }
+ }
(*num_items)++;
} else
/* invalid line, just free key_val */
diff --git a/tests/md_test.py b/tests/md_test.py
index d7e007a..926d0c5 100644
--- a/tests/md_test.py
+++ b/tests/md_test.py
@@ -415,6 +415,14 @@ class FakeMDADMutilTest(unittest.TestCase):
self.assertIs(ex_data.metadata, None)
+ def test_fw_raid_migrating(self):
+ """Verify that md_examine works when array is migrating ("foo <-- bar" values in output) """
+
+ with fake_utils("tests/mdadm_fw_RAID_examine_migrate"):
+ ex_data = BlockDev.md_examine("fake_dev")
+
+ self.assertEqual(ex_data.chunk_size, 128 * 1024)
+
class MDUnloadTest(unittest.TestCase):
def setUp(self):
diff --git a/tests/mdadm_fw_RAID_examine_migrate/mdadm b/tests/mdadm_fw_RAID_examine_migrate/mdadm
new file mode 100755
index 0000000..36ab0ca
--- /dev/null
+++ b/tests/mdadm_fw_RAID_examine_migrate/mdadm
@@ -0,0 +1,69 @@
+#!/bin/bash
+
+echo "$@"|grep -- "--brief" &>/dev/null
+is_brief=$?
+
+echo "$@"|grep -- "--export" &>/dev/null
+is_export=$?
+
+if [ $is_brief -eq 0 ]; then
+ cat <<EOF
+ARRAY metadata=imsm UUID=83c77249:90fb26a2:882bfb52:87f71b59
+ARRAY /dev/md/Volume0 container=83c77249:90fb26a2:882bfb52:87f71b59 member=0 UUID=76508998:aea211a0:b4fa4eda:78e41fbb
+EOF
+elif [ $is_export -eq 0 ]; then
+ cat <<EOF
+MD_METADATA=imsm
+MD_LEVEL=container
+MD_UUID=83c77249:90fb26a2:882bfb52:87f71b59
+MD_DEVICES=3
+EOF
+else
+ cat <<EOF
+/dev/sda:
+ Magic : Intel Raid ISM Cfg Sig.
+ Version : 1.2.02
+ Orig Family : 56330de5
+ Family : 56330de5
+ Generation : 00000005
+ Attributes : All supported
+ UUID : 83c77249:90fb26a2:882bfb52:87f71b59
+ Checksum : 359f2642 correct
+ MPB Sectors : 2
+ Disks : 3
+ RAID Devices : 1
+
+ Disk00 Serial : 50026B723702E422
+ State : active
+ Id : 00000000
+ Usable Size : 234436872 (111.79 GiB 120.03 GB)
+
+[Volume0]:
+ UUID : 76508998:aea211a0:b4fa4eda:78e41fbb
+ RAID Level : 5 <-- 5
+ Members : 3 <-- 3
+ Slots : [UUU] <-- [UUU]
+ Failed disk : none
+ This Slot : 0
+ Array Size : 468873216 (223.58 GiB 240.06 GB)
+ Per Dev Size : 234436872 (111.79 GiB 120.03 GB)
+ Sector Offset : 0
+ Num Stripes : 915768
+ Chunk Size : 128 KiB <-- 128 KiB
+ Reserved : 0
+ Migrate State : initialize
+ Map State : normal <-- uninitialized
+ Checkpoint : 0 (768)
+ Dirty State : clean
+
+ Disk01 Serial : 2327B37FS
+ State : active
+ Id : 00000002
+ Usable Size : 976768392 (465.76 GiB 500.11 GB)
+
+ Disk02 Serial : 232K7DDFS
+ State : active
+ Id : 00000003
+ Usable Size : 976768392 (465.76 GiB 500.11 GB)
+EOF
+fi
--
2.7.4

86
part_rdwr_commit.patch Normal file
View File

@ -0,0 +1,86 @@
From 1edbfcdb4a973120048b5c04e0eb92b05ad3cbd2 Mon Sep 17 00:00:00 2001
From: Vratislav Podzimek <vpodzime@redhat.com>
Date: Wed, 26 Oct 2016 16:13:58 +0200
Subject: [PATCH] Open the device file as RDWR when committing parts
We block udev from generating events for the device while we are between
committing changes to disk and informing kernel about the changes. If we close
the file descriptor later, no udev changes are generated either unless we open
is as RDWR. Here's the difference:
RDONLY:
# udevadm monitor
monitor will print the received events for:
UDEV - the event which udev sends out after rule processing
KERNEL - the kernel uevent
KERNEL[68221.911864] change /devices/virtual/block/loop0 (block)
KERNEL[68221.913442] change /devices/virtual/block/loop0 (block)
KERNEL[68221.913794] add /devices/virtual/block/loop0/loop0p1 (block)
KERNEL[68221.915019] change /devices/virtual/block/loop0 (block)
KERNEL[68221.915217] change /devices/virtual/block/loop0/loop0p1 (block)
UDEV [68221.941293] change /devices/virtual/block/loop0 (block)
UDEV [68221.954214] change /devices/virtual/block/loop0/loop0p1 (block)
RDWR:
# udevadm monitor
monitor will print the received events for:
UDEV - the event which udev sends out after rule processing
KERNEL - the kernel uevent
KERNEL[68161.533114] change /devices/virtual/block/loop0 (block)
KERNEL[68161.533165] change /devices/virtual/block/loop0 (block)
UDEV [68161.533219] change /devices/virtual/block/loop0 (block)
KERNEL[68161.533262] add /devices/virtual/block/loop0/loop0p1 (block)
KERNEL[68161.533292] change /devices/virtual/block/loop0 (block)
KERNEL[68161.533326] change /devices/virtual/block/loop0/loop0p1 (block)
UDEV [68161.539255] add /devices/virtual/block/loop0/loop0p1 (block)
UDEV [68161.560215] change /devices/virtual/block/loop0 (block)
UDEV [68161.572345] change /devices/virtual/block/loop0/loop0p1 (block)
As seen above, there's no 'add' udev event in case we open the device file as
RDONLY.
(cherry picked from commit 44fdb3953eb05ff4f43a576f15d508928dca3eb7)
Signed-off-by: Vratislav Podzimek <vpodzime@redhat.com>
---
src/plugins/part.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/src/plugins/part.c b/src/plugins/part.c
index d5239e3..0f8e678 100644
--- a/src/plugins/part.c
+++ b/src/plugins/part.c
@@ -157,7 +157,7 @@ static gboolean disk_commit (PedDisk *disk, gchar *path, GError **error) {
/* XXX: try to grab an exclusive lock for the device so that udev doesn't
trigger events for it in between the two operations we need to perform
(see below) */
- dev_fd = open (disk->dev->path, O_RDONLY|O_CLOEXEC);
+ dev_fd = open (disk->dev->path, O_RDWR|O_CLOEXEC);
if (dev_fd >= 0)
/* if this fails, we can do no better anyway, so just ignore the return
value */
@@ -167,6 +167,7 @@ static gboolean disk_commit (PedDisk *disk, gchar *path, GError **error) {
if (ret == 0) {
set_parted_error (error, BD_PART_ERROR_FAIL);
g_prefix_error (error, "Failed to commit changes to device '%s'", path);
+ flock (dev_fd, LOCK_UN);
close (dev_fd);
return FALSE;
}
@@ -175,10 +176,12 @@ static gboolean disk_commit (PedDisk *disk, gchar *path, GError **error) {
if (ret == 0) {
set_parted_error (error, BD_PART_ERROR_FAIL);
g_prefix_error (error, "Failed to inform OS about changes on the '%s' device", path);
+ flock (dev_fd, LOCK_UN);
close (dev_fd);
return FALSE;
}
+ flock (dev_fd, LOCK_UN);
close (dev_fd);
return TRUE;
}
--
2.7.4