All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/4] More split-brain fixes for metadata uuid feature
@ 2020-01-10 12:11 Nikolay Borisov
  2020-01-10 12:11 ` [PATCH 1/4] btrfs: Call find_fsid from find_fsid_inprogress Nikolay Borisov
                   ` (4 more replies)
  0 siblings, 5 replies; 12+ messages in thread
From: Nikolay Borisov @ 2020-01-10 12:11 UTC (permalink / raw)
  To: linux-btrfs; +Cc: Nikolay Borisov

Here are 4 patches which fix a newly found split-brain scenario in the
METADATA_UUID_INCOMPAT code. They are mostly the identical with Su's
original submission but I have reworked the changelogs and some function
names. Hence, I retained his authorship of the patches.

First 2 patches factor out some code with the hopes of making find_fisd a bit
more readable and simplifying the myriad of nested 'if' in device_list_add.

Patch 3 extends find_fsid_changed to handle the case where a disk with
METADATA_UUID_INCOMPAT and FSID_CHANGING_IN_PROGRESS is scanned after a disk
which has successfully been switched to FSID == METADATA_UUID state and has
created btrfs_fs_devices.

Patch 4 handles the counterpart situation - a fully switched disk is scanned
after one which has had METADATA_UUID_INCOMPAT and FSID_CHANGING_IN_PROGRESS
set.

This series should be applied to stable branches following 5.0 when the
metadata_uuid feature got introduced.

This patchset was tested with btrfs-progs' misc 034-metadata-uuid test and a full
xfstest run with no regressions. I will also be sending an improvement to the
test case which exercises the newly added code.

  btrfs: Call find_fsid from find_fsid_inprogress
  btrfs: Factor out metadata_uuid code from find_fsid.
  btrfs: Handle another split brain scenario with metadata uuid feature
  btrfs: Fix split-brain handling when changing FSID to metadata uuid

 fs/btrfs/volumes.c | 158 ++++++++++++++++++++++++++++-----------------
 1 file changed, 100 insertions(+), 58 deletions(-)

--
2.17.1


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 1/4] btrfs: Call find_fsid from find_fsid_inprogress
  2020-01-10 12:11 [PATCH 0/4] More split-brain fixes for metadata uuid feature Nikolay Borisov
@ 2020-01-10 12:11 ` Nikolay Borisov
  2020-01-10 15:24   ` Josef Bacik
  2020-01-10 12:11 ` [PATCH 2/4] btrfs: Factor out metadata_uuid code from find_fsid Nikolay Borisov
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 12+ messages in thread
From: Nikolay Borisov @ 2020-01-10 12:11 UTC (permalink / raw)
  To: linux-btrfs; +Cc: Su Yue, Nikolay Borisov

From: Su Yue <Damenly_Su@gmx.com>

Since find_fsid_inprogress should also handle the case in which an fs
didn't change its FSID make it call find_fsid directly. This makes the
code in device_list_add simpler by eliminating a conditional call of
find_fsid. No functional changes.

Signed-off-by: Su Yue <Damenly_Su@gmx.com>
Signed-off-by: Nikolay Borisov <nborisov@suse.com>
---
 fs/btrfs/volumes.c | 19 ++++++-------------
 1 file changed, 6 insertions(+), 13 deletions(-)

diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index fa48589bd924..0bac8a31edff 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -671,7 +671,9 @@ static int btrfs_open_one_device(struct btrfs_fs_devices *fs_devices,

 /*
  * Handle scanned device having its CHANGING_FSID_V2 flag set and the fs_devices
- * being created with a disk that has already completed its fsid change.
+ * being created with a disk that has already completed its fsid change. Such
+ * disk can belong to an fs which has its FSID changed or to one which doesn't.
+ * Handle both cases here.
  */
 static struct btrfs_fs_devices *find_fsid_inprogress(
 					struct btrfs_super_block *disk_super)
@@ -687,7 +689,7 @@ static struct btrfs_fs_devices *find_fsid_inprogress(
 		}
 	}

-	return NULL;
+	return find_fsid(disk_super->fsid, NULL);
 }


@@ -736,19 +738,10 @@ static noinline struct btrfs_device *device_list_add(const char *path,
 					BTRFS_SUPER_FLAG_CHANGING_FSID_V2);

 	if (fsid_change_in_progress) {
-		if (!has_metadata_uuid) {
-			/*
-			 * When we have an image which has CHANGING_FSID_V2 set
-			 * it might belong to either a filesystem which has
-			 * disks with completed fsid change or it might belong
-			 * to fs with no UUID changes in effect, handle both.
-			 */
+		if (!has_metadata_uuid)
 			fs_devices = find_fsid_inprogress(disk_super);
-			if (!fs_devices)
-				fs_devices = find_fsid(disk_super->fsid, NULL);
-		} else {
+		else
 			fs_devices = find_fsid_changed(disk_super);
-		}
 	} else if (has_metadata_uuid) {
 		fs_devices = find_fsid(disk_super->fsid,
 				       disk_super->metadata_uuid);
--
2.17.1


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 2/4] btrfs: Factor out metadata_uuid code from find_fsid.
  2020-01-10 12:11 [PATCH 0/4] More split-brain fixes for metadata uuid feature Nikolay Borisov
  2020-01-10 12:11 ` [PATCH 1/4] btrfs: Call find_fsid from find_fsid_inprogress Nikolay Borisov
@ 2020-01-10 12:11 ` Nikolay Borisov
  2020-01-10 15:25   ` Josef Bacik
  2020-01-10 12:11 ` [PATCH 3/4] btrfs: Handle another split brain scenario with metadata uuid feature Nikolay Borisov
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 12+ messages in thread
From: Nikolay Borisov @ 2020-01-10 12:11 UTC (permalink / raw)
  To: linux-btrfs; +Cc: Su Yue, Nikolay Borisov

From: Su Yue <Damenly_Su@gmx.com>

find_fsid became rather hairy with the introduction of metadata uuid
changing feature. Alleviate this by factoring out the metadata uuid
specific code in a dedicated function which deals with finding
correct fsid for a device with changed uuid.

Signed-off-by: Su Yue <Damenly_Su@gmx.com>
Signed-off-by: Nikolay Borisov <nborisov@suse.com>
---
 fs/btrfs/volumes.c | 77 +++++++++++++++++++++++++---------------------
 1 file changed, 42 insertions(+), 35 deletions(-)

diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index 0bac8a31edff..90e5ed5f5364 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -440,39 +440,6 @@ static noinline struct btrfs_fs_devices *find_fsid(

 	ASSERT(fsid);

-	if (metadata_fsid) {
-		/*
-		 * Handle scanned device having completed its fsid change but
-		 * belonging to a fs_devices that was created by first scanning
-		 * a device which didn't have its fsid/metadata_uuid changed
-		 * at all and the CHANGING_FSID_V2 flag set.
-		 */
-		list_for_each_entry(fs_devices, &fs_uuids, fs_list) {
-			if (fs_devices->fsid_change &&
-			    memcmp(metadata_fsid, fs_devices->fsid,
-				   BTRFS_FSID_SIZE) == 0 &&
-			    memcmp(fs_devices->fsid, fs_devices->metadata_uuid,
-				   BTRFS_FSID_SIZE) == 0) {
-				return fs_devices;
-			}
-		}
-		/*
-		 * Handle scanned device having completed its fsid change but
-		 * belonging to a fs_devices that was created by a device that
-		 * has an outdated pair of fsid/metadata_uuid and
-		 * CHANGING_FSID_V2 flag set.
-		 */
-		list_for_each_entry(fs_devices, &fs_uuids, fs_list) {
-			if (fs_devices->fsid_change &&
-			    memcmp(fs_devices->metadata_uuid,
-				   fs_devices->fsid, BTRFS_FSID_SIZE) != 0 &&
-			    memcmp(metadata_fsid, fs_devices->metadata_uuid,
-				   BTRFS_FSID_SIZE) == 0) {
-				return fs_devices;
-			}
-		}
-	}
-
 	/* Handle non-split brain cases */
 	list_for_each_entry(fs_devices, &fs_uuids, fs_list) {
 		if (metadata_fsid) {
@@ -488,6 +455,47 @@ static noinline struct btrfs_fs_devices *find_fsid(
 	return NULL;
 }

+static struct btrfs_fs_devices *find_fsid_with_metadata_uuid(
+				struct btrfs_super_block *disk_super)
+{
+
+	struct btrfs_fs_devices *fs_devices;
+
+	/*
+	 * Handle scanned device having completed its fsid change but
+	 * belonging to a fs_devices that was created by first scanning
+	 * a device which didn't have its fsid/metadata_uuid changed
+	 * at all and the CHANGING_FSID_V2 flag set.
+	 */
+	list_for_each_entry(fs_devices, &fs_uuids, fs_list) {
+		if (fs_devices->fsid_change &&
+		    memcmp(disk_super->metadata_uuid, fs_devices->fsid,
+			   BTRFS_FSID_SIZE) == 0 &&
+		    memcmp(fs_devices->fsid, fs_devices->metadata_uuid,
+			   BTRFS_FSID_SIZE) == 0) {
+			return fs_devices;
+		}
+	}
+	/*
+	 * Handle scanned device having completed its fsid change but
+	 * belonging to a fs_devices that was created by a device that
+	 * has an outdated pair of fsid/metadata_uuid and
+	 * CHANGING_FSID_V2 flag set.
+	 */
+	list_for_each_entry(fs_devices, &fs_uuids, fs_list) {
+		if (fs_devices->fsid_change &&
+		    memcmp(fs_devices->metadata_uuid,
+			   fs_devices->fsid, BTRFS_FSID_SIZE) != 0 &&
+		    memcmp(disk_super->metadata_uuid, fs_devices->metadata_uuid,
+			   BTRFS_FSID_SIZE) == 0) {
+			return fs_devices;
+		}
+	}
+
+	return find_fsid(disk_super->fsid, disk_super->metadata_uuid);
+}
+
+
 static int
 btrfs_get_bdev_and_sb(const char *device_path, fmode_t flags, void *holder,
 		      int flush, struct block_device **bdev,
@@ -743,8 +751,7 @@ static noinline struct btrfs_device *device_list_add(const char *path,
 		else
 			fs_devices = find_fsid_changed(disk_super);
 	} else if (has_metadata_uuid) {
-		fs_devices = find_fsid(disk_super->fsid,
-				       disk_super->metadata_uuid);
+		fs_devices = find_fsid_with_metadata_uuid(disk_super);
 	} else {
 		fs_devices = find_fsid(disk_super->fsid, NULL);
 	}
--
2.17.1


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 3/4] btrfs: Handle another split brain scenario with metadata uuid feature
  2020-01-10 12:11 [PATCH 0/4] More split-brain fixes for metadata uuid feature Nikolay Borisov
  2020-01-10 12:11 ` [PATCH 1/4] btrfs: Call find_fsid from find_fsid_inprogress Nikolay Borisov
  2020-01-10 12:11 ` [PATCH 2/4] btrfs: Factor out metadata_uuid code from find_fsid Nikolay Borisov
@ 2020-01-10 12:11 ` Nikolay Borisov
  2020-01-10 15:29   ` Josef Bacik
  2020-01-21 15:16   ` David Sterba
  2020-01-10 12:11 ` [PATCH 4/4] btrfs: Fix split-brain handling when changing FSID to metadata uuid Nikolay Borisov
  2020-01-14 17:14 ` [PATCH 0/4] More split-brain fixes for metadata uuid feature David Sterba
  4 siblings, 2 replies; 12+ messages in thread
From: Nikolay Borisov @ 2020-01-10 12:11 UTC (permalink / raw)
  To: linux-btrfs; +Cc: Nikolay Borisov

There is one more cases which isn't handled by the original metadata
uuid work. Namely, when a filesystem has METADATA_UUID incompat bit and
the user decides to change the FSID to the original one e.g. have
metadata_uuid and fsid match. In case of power failure while this
operation is in progress we could end up in a situation where some of
the disks have the incompat bit removed and the other half have both
METADATA_UUID_INCOMPAT and FSID_CHANGING_IN_PROGRESS flags.

This patch handles the case where a disk that has successfully changed
its FSID such that it equals METADATA_UUID is scanned first.
Subsequently when a disk with both
METADATA_UUID_INCOMPAT/FSID_CHANGING_IN_PROGRESS flags is scanned
find_fsid_changed won't be able to find an appropriate btrfs_fs_devices.
This is done by extending find_fsid_changed to correctly find
btrfs_fs_devices whose metadata_uuid/fsid are the same and they match
the metadata_uuid of the currently scanned device.

Fixes: cc5de4e70256 ("btrfs: Handle final split-brain possibility during fsid change")
Reported-by: Su Yue <Damenly_Su@gmx.com>
Signed-off-by: Nikolay Borisov <nborisov@suse.com>
---
 fs/btrfs/volumes.c | 25 +++++++++++++++++--------
 1 file changed, 17 insertions(+), 8 deletions(-)

diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index 90e5ed5f5364..7739d40939bf 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -709,17 +709,26 @@ static struct btrfs_fs_devices *find_fsid_changed(
 	/*
 	 * Handles the case where scanned device is part of an fs that had
 	 * multiple successful changes of FSID but curently device didn't
-	 * observe it. Meaning our fsid will be different than theirs.
+	 * observe it. Meaning our fsid will be different than theirs. We need
+	 * to handle two subcases :
+	 *  1 - The fs still continues to have different METADATA/FSID uuids.
+	 *  2 - The fs is switched back to its original FSID (METADATA/FSID
+	 *  are equal).
 	 */
 	list_for_each_entry(fs_devices, &fs_uuids, fs_list) {
-		if (memcmp(fs_devices->metadata_uuid, fs_devices->fsid,
-			   BTRFS_FSID_SIZE) != 0 &&
-		    memcmp(fs_devices->metadata_uuid, disk_super->metadata_uuid,
-			   BTRFS_FSID_SIZE) == 0 &&
-		    memcmp(fs_devices->fsid, disk_super->fsid,
-			   BTRFS_FSID_SIZE) != 0) {
+		bool changed_fsdevices =
+			memcmp(fs_devices->metadata_uuid, fs_devices->fsid,
+			       BTRFS_FSID_SIZE) != 0 &&
+			memcmp(fs_devices->metadata_uuid,
+			       disk_super->metadata_uuid, BTRFS_FSID_SIZE) == 0 &&
+			memcmp(fs_devices->fsid, disk_super->fsid, BTRFS_FSID_SIZE) != 0;
+
+		bool unchanged_fsdevices =
+			memcmp(fs_devices->metadata_uuid, fs_devices->fsid,
+						  BTRFS_FSID_SIZE) == 0 &&
+			memcmp(fs_devices->fsid, disk_super->metadata_uuid, BTRFS_FSID_SIZE) == 0;
+		if (changed_fsdevices || unchanged_fsdevices)
 			return fs_devices;
-		}
 	}

 	return NULL;
--
2.17.1
find_fsid_changed

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 4/4] btrfs: Fix split-brain handling when changing FSID to metadata uuid
  2020-01-10 12:11 [PATCH 0/4] More split-brain fixes for metadata uuid feature Nikolay Borisov
                   ` (2 preceding siblings ...)
  2020-01-10 12:11 ` [PATCH 3/4] btrfs: Handle another split brain scenario with metadata uuid feature Nikolay Borisov
@ 2020-01-10 12:11 ` Nikolay Borisov
  2020-01-10 15:58   ` Josef Bacik
  2020-01-14 17:14 ` [PATCH 0/4] More split-brain fixes for metadata uuid feature David Sterba
  4 siblings, 1 reply; 12+ messages in thread
From: Nikolay Borisov @ 2020-01-10 12:11 UTC (permalink / raw)
  To: linux-btrfs; +Cc: Nikolay Borisov

Current code doesn't correctly handle the situation which arises when
a file system that has METADATA_UUID_INCOMPAT flag set  has its FSID
changed to the one in metadata uuid. This causes the incompat flag to
disappear. In case of a power failure we could end up in a situation
where part of the disks in a multi-disk filesystem are correctly
reverted to METADATA_UUID_INCOMPAT flag unset state, while others have
METADATA_UUID_INCOMPAT set and CHANGING_FSID_V2_IN_PROGRESS.

This patch corrects the behavior required to handle the case where a
disk of the second type is scanned first, creating the necessary
btrfs_fs_devices. Subsequently, when a disk which has already completed
the transition is scanned it should overwrite the data in
btrfs_fs_devices.

Reported-by: Su Yue <Damenly_Su@gmx.com>
Signed-off-by: Nikolay Borisov <nborisov@suse.com>
---
 fs/btrfs/volumes.c | 41 +++++++++++++++++++++++++++++++++++++----
 1 file changed, 37 insertions(+), 4 deletions(-)

diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index 7739d40939bf..871e163d1252 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -733,6 +733,32 @@ static struct btrfs_fs_devices *find_fsid_changed(

 	return NULL;
 }
+
+static struct btrfs_fs_devices *find_fsid_reverted_metadata(
+				struct btrfs_super_block *disk_super)
+{
+	struct btrfs_fs_devices *fs_devices;
+
+	/*
+	 * Handles the case where the scanned device is part of an fs whose last
+	 * metadata uuid change reverted it to the original FSID. At the same time
+	 * fs_devices was first created by another constitutent device which didn't
+	 * fully observer the operation. This results in an btrfs_fs_devices
+	 * created with metadata/fsid different AND btrfs_fs_devices::fsid_change
+	 * set AND the metadata_uuid of the fs_devices equal to the FSID of the
+	 * disk.
+	 */
+	list_for_each_entry(fs_devices, &fs_uuids, fs_list) {
+		if (memcmp(fs_devices->fsid, fs_devices->metadata_uuid,
+			   BTRFS_FSID_SIZE) != 0 &&
+		    memcmp(fs_devices->metadata_uuid, disk_super->fsid,
+			   BTRFS_FSID_SIZE) == 0 &&
+		    fs_devices->fsid_change)
+			return fs_devices;
+	}
+
+	return NULL;
+}
 /*
  * Add new device to list of registered devices
  *
@@ -762,7 +788,9 @@ static noinline struct btrfs_device *device_list_add(const char *path,
 	} else if (has_metadata_uuid) {
 		fs_devices = find_fsid_with_metadata_uuid(disk_super);
 	} else {
-		fs_devices = find_fsid(disk_super->fsid, NULL);
+		fs_devices = find_fsid_reverted_metadata(disk_super);
+		if (!fs_devices)
+			fs_devices = find_fsid(disk_super->fsid, NULL);
 	}


@@ -792,12 +820,17 @@ static noinline struct btrfs_device *device_list_add(const char *path,
 		 * a device which had the CHANGING_FSID_V2 flag then replace the
 		 * metadata_uuid/fsid values of the fs_devices.
 		 */
-		if (has_metadata_uuid && fs_devices->fsid_change &&
+		if (fs_devices->fsid_change &&
 		    found_transid > fs_devices->latest_generation) {
 			memcpy(fs_devices->fsid, disk_super->fsid,
 					BTRFS_FSID_SIZE);
-			memcpy(fs_devices->metadata_uuid,
-					disk_super->metadata_uuid, BTRFS_FSID_SIZE);
+
+			if (has_metadata_uuid)
+				memcpy(fs_devices->metadata_uuid,
+				       disk_super->metadata_uuid, BTRFS_FSID_SIZE);
+			else
+				memcpy(fs_devices->metadata_uuid,
+				       disk_super->fsid, BTRFS_FSID_SIZE);

 			fs_devices->fsid_change = false;
 		}
--
2.17.1


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/4] btrfs: Call find_fsid from find_fsid_inprogress
  2020-01-10 12:11 ` [PATCH 1/4] btrfs: Call find_fsid from find_fsid_inprogress Nikolay Borisov
@ 2020-01-10 15:24   ` Josef Bacik
  0 siblings, 0 replies; 12+ messages in thread
From: Josef Bacik @ 2020-01-10 15:24 UTC (permalink / raw)
  To: Nikolay Borisov, linux-btrfs; +Cc: Su Yue

On 1/10/20 7:11 AM, Nikolay Borisov wrote:
> From: Su Yue <Damenly_Su@gmx.com>
> 
> Since find_fsid_inprogress should also handle the case in which an fs
> didn't change its FSID make it call find_fsid directly. This makes the
> code in device_list_add simpler by eliminating a conditional call of
> find_fsid. No functional changes.
> 
> Signed-off-by: Su Yue <Damenly_Su@gmx.com>
> Signed-off-by: Nikolay Borisov <nborisov@suse.com>

Reviewed-by: Josef Bacik <josef@toxicpanda.com>

Thanks,

Josef

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/4] btrfs: Factor out metadata_uuid code from find_fsid.
  2020-01-10 12:11 ` [PATCH 2/4] btrfs: Factor out metadata_uuid code from find_fsid Nikolay Borisov
@ 2020-01-10 15:25   ` Josef Bacik
  0 siblings, 0 replies; 12+ messages in thread
From: Josef Bacik @ 2020-01-10 15:25 UTC (permalink / raw)
  To: Nikolay Borisov, linux-btrfs; +Cc: Su Yue

On 1/10/20 7:11 AM, Nikolay Borisov wrote:
> From: Su Yue <Damenly_Su@gmx.com>
> 
> find_fsid became rather hairy with the introduction of metadata uuid
> changing feature. Alleviate this by factoring out the metadata uuid
> specific code in a dedicated function which deals with finding
> correct fsid for a device with changed uuid.
> 
> Signed-off-by: Su Yue <Damenly_Su@gmx.com>
> Signed-off-by: Nikolay Borisov <nborisov@suse.com>

Reviewed-by: Josef Bacik <josef@toxicpanda.com>

Thanks,

Josef

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 3/4] btrfs: Handle another split brain scenario with metadata uuid feature
  2020-01-10 12:11 ` [PATCH 3/4] btrfs: Handle another split brain scenario with metadata uuid feature Nikolay Borisov
@ 2020-01-10 15:29   ` Josef Bacik
  2020-01-21 15:16   ` David Sterba
  1 sibling, 0 replies; 12+ messages in thread
From: Josef Bacik @ 2020-01-10 15:29 UTC (permalink / raw)
  To: Nikolay Borisov, linux-btrfs

On 1/10/20 7:11 AM, Nikolay Borisov wrote:
> There is one more cases which isn't handled by the original metadata
> uuid work. Namely, when a filesystem has METADATA_UUID incompat bit and
> the user decides to change the FSID to the original one e.g. have
> metadata_uuid and fsid match. In case of power failure while this
> operation is in progress we could end up in a situation where some of
> the disks have the incompat bit removed and the other half have both
> METADATA_UUID_INCOMPAT and FSID_CHANGING_IN_PROGRESS flags.
> 
> This patch handles the case where a disk that has successfully changed
> its FSID such that it equals METADATA_UUID is scanned first.
> Subsequently when a disk with both
> METADATA_UUID_INCOMPAT/FSID_CHANGING_IN_PROGRESS flags is scanned
> find_fsid_changed won't be able to find an appropriate btrfs_fs_devices.
> This is done by extending find_fsid_changed to correctly find
> btrfs_fs_devices whose metadata_uuid/fsid are the same and they match
> the metadata_uuid of the currently scanned device.
> 
> Fixes: cc5de4e70256 ("btrfs: Handle final split-brain possibility during fsid change")
> Reported-by: Su Yue <Damenly_Su@gmx.com>
> Signed-off-by: Nikolay Borisov <nborisov@suse.com>

Reviewed-by: Josef Bacik <josef@toxicpanda.com>

Thanks,

Josef

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 4/4] btrfs: Fix split-brain handling when changing FSID to metadata uuid
  2020-01-10 12:11 ` [PATCH 4/4] btrfs: Fix split-brain handling when changing FSID to metadata uuid Nikolay Borisov
@ 2020-01-10 15:58   ` Josef Bacik
  0 siblings, 0 replies; 12+ messages in thread
From: Josef Bacik @ 2020-01-10 15:58 UTC (permalink / raw)
  To: Nikolay Borisov, linux-btrfs

On 1/10/20 7:11 AM, Nikolay Borisov wrote:
> Current code doesn't correctly handle the situation which arises when
> a file system that has METADATA_UUID_INCOMPAT flag set  has its FSID
> changed to the one in metadata uuid. This causes the incompat flag to
> disappear. In case of a power failure we could end up in a situation
> where part of the disks in a multi-disk filesystem are correctly
> reverted to METADATA_UUID_INCOMPAT flag unset state, while others have
> METADATA_UUID_INCOMPAT set and CHANGING_FSID_V2_IN_PROGRESS.
> 
> This patch corrects the behavior required to handle the case where a
> disk of the second type is scanned first, creating the necessary
> btrfs_fs_devices. Subsequently, when a disk which has already completed
> the transition is scanned it should overwrite the data in
> btrfs_fs_devices.
> 
> Reported-by: Su Yue <Damenly_Su@gmx.com>
> Signed-off-by: Nikolay Borisov <nborisov@suse.com>

Reviewed-by: Josef Bacik <josef@toxicpanda.com>

Thanks,

Josef

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 0/4] More split-brain fixes for metadata uuid feature
  2020-01-10 12:11 [PATCH 0/4] More split-brain fixes for metadata uuid feature Nikolay Borisov
                   ` (3 preceding siblings ...)
  2020-01-10 12:11 ` [PATCH 4/4] btrfs: Fix split-brain handling when changing FSID to metadata uuid Nikolay Borisov
@ 2020-01-14 17:14 ` David Sterba
  2020-01-21 15:23   ` David Sterba
  4 siblings, 1 reply; 12+ messages in thread
From: David Sterba @ 2020-01-14 17:14 UTC (permalink / raw)
  To: Nikolay Borisov; +Cc: linux-btrfs

On Fri, Jan 10, 2020 at 02:11:31PM +0200, Nikolay Borisov wrote:
> Here are 4 patches which fix a newly found split-brain scenario in the
> METADATA_UUID_INCOMPAT code. They are mostly the identical with Su's
> original submission but I have reworked the changelogs and some function
> names. Hence, I retained his authorship of the patches.
> 
> First 2 patches factor out some code with the hopes of making find_fisd a bit
> more readable and simplifying the myriad of nested 'if' in device_list_add.
> 
> Patch 3 extends find_fsid_changed to handle the case where a disk with
> METADATA_UUID_INCOMPAT and FSID_CHANGING_IN_PROGRESS is scanned after a disk
> which has successfully been switched to FSID == METADATA_UUID state and has
> created btrfs_fs_devices.
> 
> Patch 4 handles the counterpart situation - a fully switched disk is scanned
> after one which has had METADATA_UUID_INCOMPAT and FSID_CHANGING_IN_PROGRESS
> set.
> 
> This series should be applied to stable branches following 5.0 when the
> metadata_uuid feature got introduced.
> 
> This patchset was tested with btrfs-progs' misc 034-metadata-uuid test and a full
> xfstest run with no regressions. I will also be sending an improvement to the
> test case which exercises the newly added code.
> 
>   btrfs: Call find_fsid from find_fsid_inprogress
>   btrfs: Factor out metadata_uuid code from find_fsid.
>   btrfs: Handle another split brain scenario with metadata uuid feature
>   btrfs: Fix split-brain handling when changing FSID to metadata uuid

I'm adding it to for-next, thanks.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 3/4] btrfs: Handle another split brain scenario with metadata uuid feature
  2020-01-10 12:11 ` [PATCH 3/4] btrfs: Handle another split brain scenario with metadata uuid feature Nikolay Borisov
  2020-01-10 15:29   ` Josef Bacik
@ 2020-01-21 15:16   ` David Sterba
  1 sibling, 0 replies; 12+ messages in thread
From: David Sterba @ 2020-01-21 15:16 UTC (permalink / raw)
  To: Nikolay Borisov; +Cc: linux-btrfs

On Fri, Jan 10, 2020 at 02:11:34PM +0200, Nikolay Borisov wrote:
>  	list_for_each_entry(fs_devices, &fs_uuids, fs_list) {
> -		if (memcmp(fs_devices->metadata_uuid, fs_devices->fsid,
> -			   BTRFS_FSID_SIZE) != 0 &&
> -		    memcmp(fs_devices->metadata_uuid, disk_super->metadata_uuid,
> -			   BTRFS_FSID_SIZE) == 0 &&
> -		    memcmp(fs_devices->fsid, disk_super->fsid,
> -			   BTRFS_FSID_SIZE) != 0) {
> +		bool changed_fsdevices =
> +			memcmp(fs_devices->metadata_uuid, fs_devices->fsid,
> +			       BTRFS_FSID_SIZE) != 0 &&
> +			memcmp(fs_devices->metadata_uuid,
> +			       disk_super->metadata_uuid, BTRFS_FSID_SIZE) == 0 &&
> +			memcmp(fs_devices->fsid, disk_super->fsid, BTRFS_FSID_SIZE) != 0;
> +
> +		bool unchanged_fsdevices =
> +			memcmp(fs_devices->metadata_uuid, fs_devices->fsid,
> +						  BTRFS_FSID_SIZE) == 0 &&
> +			memcmp(fs_devices->fsid, disk_super->metadata_uuid, BTRFS_FSID_SIZE) == 0;
> +		if (changed_fsdevices || unchanged_fsdevices)
>  			return fs_devices;

This is ugly, I've converted it to if (memcmp) and dropped the
variables.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 0/4] More split-brain fixes for metadata uuid feature
  2020-01-14 17:14 ` [PATCH 0/4] More split-brain fixes for metadata uuid feature David Sterba
@ 2020-01-21 15:23   ` David Sterba
  0 siblings, 0 replies; 12+ messages in thread
From: David Sterba @ 2020-01-21 15:23 UTC (permalink / raw)
  To: dsterba, Nikolay Borisov, linux-btrfs

On Tue, Jan 14, 2020 at 06:14:03PM +0100, David Sterba wrote:
> On Fri, Jan 10, 2020 at 02:11:31PM +0200, Nikolay Borisov wrote:
> > Here are 4 patches which fix a newly found split-brain scenario in the
> > METADATA_UUID_INCOMPAT code. They are mostly the identical with Su's
> > original submission but I have reworked the changelogs and some function
> > names. Hence, I retained his authorship of the patches.
> > 
> > First 2 patches factor out some code with the hopes of making find_fisd a bit
> > more readable and simplifying the myriad of nested 'if' in device_list_add.
> > 
> > Patch 3 extends find_fsid_changed to handle the case where a disk with
> > METADATA_UUID_INCOMPAT and FSID_CHANGING_IN_PROGRESS is scanned after a disk
> > which has successfully been switched to FSID == METADATA_UUID state and has
> > created btrfs_fs_devices.
> > 
> > Patch 4 handles the counterpart situation - a fully switched disk is scanned
> > after one which has had METADATA_UUID_INCOMPAT and FSID_CHANGING_IN_PROGRESS
> > set.
> > 
> > This series should be applied to stable branches following 5.0 when the
> > metadata_uuid feature got introduced.
> > 
> > This patchset was tested with btrfs-progs' misc 034-metadata-uuid test and a full
> > xfstest run with no regressions. I will also be sending an improvement to the
> > test case which exercises the newly added code.
> > 
> >   btrfs: Call find_fsid from find_fsid_inprogress
> >   btrfs: Factor out metadata_uuid code from find_fsid.
> >   btrfs: Handle another split brain scenario with metadata uuid feature
> >   btrfs: Fix split-brain handling when changing FSID to metadata uuid
> 
> I'm adding it to for-next, thanks.

Moving to misc-next.

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2020-01-21 15:24 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-01-10 12:11 [PATCH 0/4] More split-brain fixes for metadata uuid feature Nikolay Borisov
2020-01-10 12:11 ` [PATCH 1/4] btrfs: Call find_fsid from find_fsid_inprogress Nikolay Borisov
2020-01-10 15:24   ` Josef Bacik
2020-01-10 12:11 ` [PATCH 2/4] btrfs: Factor out metadata_uuid code from find_fsid Nikolay Borisov
2020-01-10 15:25   ` Josef Bacik
2020-01-10 12:11 ` [PATCH 3/4] btrfs: Handle another split brain scenario with metadata uuid feature Nikolay Borisov
2020-01-10 15:29   ` Josef Bacik
2020-01-21 15:16   ` David Sterba
2020-01-10 12:11 ` [PATCH 4/4] btrfs: Fix split-brain handling when changing FSID to metadata uuid Nikolay Borisov
2020-01-10 15:58   ` Josef Bacik
2020-01-14 17:14 ` [PATCH 0/4] More split-brain fixes for metadata uuid feature David Sterba
2020-01-21 15:23   ` David Sterba

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.