[PATCH 0/3] [mdadm] fixes and feature for journal device

[PATCH 0/3] [mdadm] fixes and feature for journal device

[Song Liu]

As we give journal device disk role of 0
(http://marc.info/?l=linux-raid&m=145030044008753), we need 2 fixes
in mdadm (patch 1 and 2).

Patch 3 tries to add journal to an existing array that does not have
journal before.

Song Liu (3):
  [mdadm] move journal to end of --detail list
  [mdadm] in --add assign raid_disk of 0 to journal
  Add new journal to array that does not have journal

 Assemble.c |  5 +----
 Detail.c   |  5 +++--
 Manage.c   | 34 +++++++++++++++++++++++++++-------
 3 files changed, 31 insertions(+), 13 deletions(-)

--
2.4.6

[PATCH 1/3] [mdadm] move journal to end of --detail list

[Song Liu]

As we give journal device raid_disk of 0, the output of --detail is:

    Number   Major   Minor   RaidDevice State
       0       8       17        0      active sync   /dev/sdb1
       5       8       24        0      journal   /dev/sdb8
       1       8       18        1      active sync   /dev/sdb2
       2       8       19        2      active sync   /dev/sdb3
       3       8       21        3      active sync   /dev/sdb5

       4       8       23        -      spare   /dev/sdb7

This patch makes it back to:
    Number   Major   Minor   RaidDevice State
       0       8       17        0      active sync   /dev/sdb1
       1       8       18        1      active sync   /dev/sdb2
       2       8       19        2      active sync   /dev/sdb3
       3       8       21        3      active sync   /dev/sdb5

       4       8       23        -      spare   /dev/sdb7
       5       8       24        -      journal   /dev/sdb8

Signed-off-by: Song Liu <songliubraving@fb.com>
Signed-off-by: Shaohua Li <shli@fb.com>
---
 Detail.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/Detail.c b/Detail.c
index 5bd2dc6..37403d6 100644
--- a/Detail.c
+++ b/Detail.c
@@ -326,7 +326,8 @@ int Detail(char *dev, struct context *c)
 		    && disks[disk.raid_disk*2].state == (1<<MD_DISK_REMOVED))
 			disks[disk.raid_disk*2] = disk;
 		else if (disk.raid_disk >= 0 && disk.raid_disk < array.raid_disks
-			 && disks[disk.raid_disk*2+1].state == (1<<MD_DISK_REMOVED))
+			 && disks[disk.raid_disk*2+1].state == (1<<MD_DISK_REMOVED)
+			 && !(disk.state & (1<<MD_DISK_JOURNAL)))
 			disks[disk.raid_disk*2+1] = disk;
 		else if (next < max_disks*2)
 			disks[next++] = disk;
@@ -621,7 +622,7 @@ This is pretty boring
 			if (disk.number < 0)
 				printf("       -   %5d    %5d        -     ",
 				       disk.major, disk.minor);
-			else if (disk.raid_disk < 0)
+			else if (disk.raid_disk < 0 || disk.state & (1<<MD_DISK_JOURNAL))
 				printf("   %5d   %5d    %5d        -     ",
 				       disk.number, disk.major, disk.minor);
 			else if (disk.number < 0)
-- 
2.4.6

[PATCH 2/3] [mdadm] in --add assign raid_disk of 0 to journal

[Song Liu]

Signed-off-by: Song Liu <songliubraving@fb.com>
Signed-off-by: Shaohua Li <shli@fb.com>
---
 Manage.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Manage.c b/Manage.c
index 4540fac..7e1b94b 100644
--- a/Manage.c
+++ b/Manage.c
@@ -948,7 +948,7 @@ int Manage_add(int fd, int tfd, struct mddev_dev *dv,
 			pr_err("%s does not support journal device.\n", devname);
 			return -1;
 		}
-		disc.raid_disk = array->raid_disks;
+		disc.raid_disk = 0;
 	}
 
 	if (array->not_persistent==0) {
-- 
2.4.6

[PATCH 3/3] Add new journal to array that does not have journal

[Song Liu]

This patch enables adding journal to an array that does not have journal before.

To add the journal, the array should finish resync. Otherwise, mdadm complains:

[root] mdadm --add-journal /dev/md0 /dev/sdb8 -vvv
mdadm: /dev/md0 has active resync, please retry after resync is done.

The array need to be restarted for the journal to work:

[root] mdadm --add-journal /dev/md0 /dev/sdb8
mdadm: Making /dev/md0 readonly before adding journal...
mdadm: Added new journal to /dev/md0.
mdadm: Please restart the array to make it live.

[root] mdadm --stop /dev/md*
mdadm: stopped /dev/md0

[root] mdadm -A /dev/md0 /dev/sdb[12358]
mdadm: device 8 in /dev/md0 has wrong state in superblock, but /dev/sdb8 seems ok
mdadm: /dev/md0 has been started with 4 drives and 1 journal.

Signed-off-by: Song Liu <songliubraving@fb.com>
Signed-off-by: Shaohua Li <shli@fb.com>
---
 Assemble.c |  5 +----
 Manage.c   | 32 ++++++++++++++++++++++++++------
 2 files changed, 27 insertions(+), 10 deletions(-)

diff --git a/Assemble.c b/Assemble.c
index a7cd163..4ddd650 100644
--- a/Assemble.c
+++ b/Assemble.c
@@ -1556,10 +1556,7 @@ try_again:
 		 */
 		if (content->array.level != LEVEL_MULTIPATH) {
 			if (devices[j].i.disk.state & (1<<MD_DISK_JOURNAL)) {
-				if (content->journal_device_required)
-					journalcnt++;
-				else	/* unexpected journal, mark as faulty */
-					devices[j].i.disk.state |= (1<<MD_DISK_FAULTY);
+				journalcnt++;
 			} else if (!(devices[j].i.disk.state & (1<<MD_DISK_ACTIVE))) {
 				if (!(devices[j].i.disk.state
 				      & (1<<MD_DISK_FAULTY))) {
diff --git a/Manage.c b/Manage.c
index 7e1b94b..da14b99 100644
--- a/Manage.c
+++ b/Manage.c
@@ -740,6 +740,7 @@ int Manage_add(int fd, int tfd, struct mddev_dev *dv,
 	struct supertype *dev_st = NULL;
 	int j;
 	mdu_disk_info_t disc;
+	int new_journal = 0;
 
 	if (!get_dev_size(tfd, dv->devname, &ldsize)) {
 		if (dv->disposition == 'M')
@@ -935,19 +936,33 @@ int Manage_add(int fd, int tfd, struct mddev_dev *dv,
 	if (dv->disposition == 'j') {
 		struct mdinfo mdi;
 		struct mdinfo *mdp;
+		struct mdstat_ent *mds, *m;
+		int percent = -1;
+
+		mds = mdstat_read(0, 0);
+		for (m = mds; m; m = m->next)
+			if (strcmp(m->devnm, fd2devnm(fd)) == 0)
+				percent = m->percent;
+		free_mdstat(mds);
+
+		if (percent > 0) {
+			pr_err("%s has active resync, please retry after resync is done.\n", devname);
+			return -1;
+		}
 
 		mdp = sysfs_read(fd, NULL, GET_ARRAY_STATE);
 
 		if (strncmp(mdp->sysfs_array_state, "readonly", 8) != 0) {
-			pr_err("%s is not readonly, cannot add journal.\n", devname);
-			return -1;
+			pr_err("Making %s readonly before adding journal...\n", devname);
+			if (Manage_ro(devname, fd, 1)) {
+				pr_err("Please retry.\n");
+				return -1;
+			}
 		}
 
 		tst->ss->getinfo_super(tst, &mdi, NULL);
-		if (mdi.journal_device_required == 0) {
-			pr_err("%s does not support journal device.\n", devname);
-			return -1;
-		}
+		if (mdi.journal_device_required == 0)
+			new_journal = 1;
 		disc.raid_disk = 0;
 	}
 
@@ -1064,6 +1079,11 @@ int Manage_add(int fd, int tfd, struct mddev_dev *dv,
 		close(container_fd);
 	} else {
 		tst->ss->free_super(tst);
+		if (new_journal) {
+			pr_err("Added new journal to %s. \n", devname);
+			pr_err("Please restart the array to make it live.\n");
+			return 1;
+		}
 		if (ioctl(fd, ADD_NEW_DISK, &disc)) {
 			if (dv->disposition == 'j')
 				pr_err("Failed to hot add %s as journal, "
-- 
2.4.6

Re: [PATCH 3/3] Add new journal to array that does not have journal

[NeilBrown]

[-- Attachment #1: Type: text/plain, Size: 4971 bytes --]

On Tue, Dec 22 2015, Song Liu wrote:

I've applied the first two patches - thanks.

This one bothers me.

> This patch enables adding journal to an array that does not have journal before.
>
> To add the journal, the array should finish resync. Otherwise, mdadm complains:
>
> [root] mdadm --add-journal /dev/md0 /dev/sdb8 -vvv
> mdadm: /dev/md0 has active resync, please retry after resync is done.

I'm reasonably happy with not adding a journal while a resync is
happening (be nice if you could though).

However this should be an option in the --grow subcommand, for consistency.

>
> The array need to be restarted for the journal to work:
>
> [root] mdadm --add-journal /dev/md0 /dev/sdb8
> mdadm: Making /dev/md0 readonly before adding journal...
> mdadm: Added new journal to /dev/md0.
> mdadm: Please restart the array to make it live.

I'm not at all happy with adding a journal and it not really being
added.

The best case is to add the journal and have it be live straight away.
Why can't we do that.

The second best option is to add the journal as part of --assemble,
e.g. with --update=journal

>
> [root] mdadm --stop /dev/md*
> mdadm: stopped /dev/md0
>
> [root] mdadm -A /dev/md0 /dev/sdb[12358]
> mdadm: device 8 in /dev/md0 has wrong state in superblock, but /dev/sdb8 seems ok
> mdadm: /dev/md0 has been started with 4 drives and 1 journal.
>
> Signed-off-by: Song Liu <songliubraving@fb.com>
> Signed-off-by: Shaohua Li <shli@fb.com>
> ---
>  Assemble.c |  5 +----
>  Manage.c   | 32 ++++++++++++++++++++++++++------
>  2 files changed, 27 insertions(+), 10 deletions(-)
>
> diff --git a/Assemble.c b/Assemble.c
> index a7cd163..4ddd650 100644
> --- a/Assemble.c
> +++ b/Assemble.c
> @@ -1556,10 +1556,7 @@ try_again:
>  		 */
>  		if (content->array.level != LEVEL_MULTIPATH) {
>  			if (devices[j].i.disk.state & (1<<MD_DISK_JOURNAL)) {
> -				if (content->journal_device_required)
> -					journalcnt++;
> -				else	/* unexpected journal, mark as faulty */
> -					devices[j].i.disk.state |= (1<<MD_DISK_FAULTY);
> +				journalcnt++;
>  			} else if (!(devices[j].i.disk.state & (1<<MD_DISK_ACTIVE))) {
>  				if (!(devices[j].i.disk.state
>  				      & (1<<MD_DISK_FAULTY))) {
> diff --git a/Manage.c b/Manage.c
> index 7e1b94b..da14b99 100644
> --- a/Manage.c
> +++ b/Manage.c
> @@ -740,6 +740,7 @@ int Manage_add(int fd, int tfd, struct mddev_dev *dv,
>  	struct supertype *dev_st = NULL;
>  	int j;
>  	mdu_disk_info_t disc;
> +	int new_journal = 0;
>  
>  	if (!get_dev_size(tfd, dv->devname, &ldsize)) {
>  		if (dv->disposition == 'M')
> @@ -935,19 +936,33 @@ int Manage_add(int fd, int tfd, struct mddev_dev *dv,
>  	if (dv->disposition == 'j') {
>  		struct mdinfo mdi;
>  		struct mdinfo *mdp;
> +		struct mdstat_ent *mds, *m;
> +		int percent = -1;
> +
> +		mds = mdstat_read(0, 0);
> +		for (m = mds; m; m = m->next)
> +			if (strcmp(m->devnm, fd2devnm(fd)) == 0)
> +				percent = m->percent;
> +		free_mdstat(mds);
> +
> +		if (percent > 0) {
> +			pr_err("%s has active resync, please retry after resync is done.\n", devname);
> +			return -1;
> +		}

This is a rather clumsy way to test if a resync is happening.  It is all
we could manage years ago, but today we can just read the sync_action
file from sysfs.  If that isn't "idle", then assume some
resync/recovery/reshape is happening.

>  
>  		mdp = sysfs_read(fd, NULL, GET_ARRAY_STATE);
>  
>  		if (strncmp(mdp->sysfs_array_state, "readonly", 8) != 0) {
> -			pr_err("%s is not readonly, cannot add journal.\n", devname);
> -			return -1;
> +			pr_err("Making %s readonly before adding journal...\n", devname);
> +			if (Manage_ro(devname, fd, 1)) {
> +				pr_err("Please retry.\n");
> +				return -1;
> +			}

??  Why does the array have to be read-only to add a journal?
That would prevent you adding a journal to an array with a mounted
filesystem.


Thanks,
NeilBrown

>  		}
>  
>  		tst->ss->getinfo_super(tst, &mdi, NULL);
> -		if (mdi.journal_device_required == 0) {
> -			pr_err("%s does not support journal device.\n", devname);
> -			return -1;
> -		}
> +		if (mdi.journal_device_required == 0)
> +			new_journal = 1;
>  		disc.raid_disk = 0;
>  	}
>  
> @@ -1064,6 +1079,11 @@ int Manage_add(int fd, int tfd, struct mddev_dev *dv,
>  		close(container_fd);
>  	} else {
>  		tst->ss->free_super(tst);
> +		if (new_journal) {
> +			pr_err("Added new journal to %s. \n", devname);
> +			pr_err("Please restart the array to make it live.\n");
> +			return 1;
> +		}
>  		if (ioctl(fd, ADD_NEW_DISK, &disc)) {
>  			if (dv->disposition == 'j')
>  				pr_err("Failed to hot add %s as journal, "
> -- 
> 2.4.6
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-raid" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 818 bytes --]