[PATCH v3] mdadm: change in add-journal support

[PATCH v3] mdadm: change in add-journal support

[Song Liu]

I have made some confusion with previous versions of the change set, so
I would like to resend and clarify.

The very first version of this set has two patches, one for changes
in --add-journal the other for new option of --remove-journal:

http://marc.info/?l=linux-raid&m=148960265129793
http://marc.info/?l=linux-raid&m=148960267329810

After getting feedback from Shaohua, I realized that there is issue
with the add-journal patch, and the remove-journal patch is not
necessary. Therefore, I sent the second version of this set:

http://marc.info/?l=linux-raid&m=148978754820615

However, I forgot to make it clear that the first set should be discarded,
which caused confusion.

To make it clear, please discard all the patches above, and only apply the
following patch.

Thanks,
Song

Song Liu (1):
  mdadm/r5cache: allow adding journal to array without journal

 Manage.c   | 9 ---------
 mdadm.8.in | 5 ++---
 2 files changed, 2 insertions(+), 12 deletions(-)

--
2.9.3

[PATCH v3] mdadm/r5cache: allow adding journal to array without journal

[Song Liu]

Currently, --add-journal can be only used to recreate broken journal
for arrays with journal since  creation. As the kernel code getting
more mature, this constraint is no longer necessary.

This patch allows --add-journal to add journal to array without
journal.

Signed-off-by: Song Liu <songliubraving@fb.com>
---
 Manage.c   | 9 ---------
 mdadm.8.in | 5 ++---
 2 files changed, 2 insertions(+), 12 deletions(-)

diff --git a/Manage.c b/Manage.c
index 5c3d2b9..d038b68 100644
--- a/Manage.c
+++ b/Manage.c
@@ -946,7 +946,6 @@ int Manage_add(int fd, int tfd, struct mddev_dev *dv,
 
 	/* only add journal to array that supports journaling */
 	if (dv->disposition == 'j') {
-		struct mdinfo mdi;
 		struct mdinfo *mdp;
 
 		mdp = sysfs_read(fd, NULL, GET_ARRAY_STATE);
@@ -960,14 +959,6 @@ int Manage_add(int fd, int tfd, struct mddev_dev *dv,
 			pr_err("%s is not readonly, cannot add journal.\n", devname);
 			return -1;
 		}
-
-		sysfs_free(mdp);
-
-		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;
-		}
 		disc.raid_disk = 0;
 	}
 
diff --git a/mdadm.8.in b/mdadm.8.in
index df1d460..9e5e896 100644
--- a/mdadm.8.in
+++ b/mdadm.8.in
@@ -1474,9 +1474,8 @@ the device is found or <slot>:missing in case the device is not found.
 
 .TP
 .BR \-\-add-journal
-Recreate journal for RAID-4/5/6 array that lost a journal device. In the
-current implementation, this command cannot add a journal to an array
-that had a failed journal. To avoid interrupting on-going write opertions,
+Add journal to an existing array, or recreate journal for RAID-4/5/6 array
+that lost a journal device. To avoid interrupting on-going write opertions,
 .B \-\-add-journal
 only works for array in Read-Only state.
 
-- 
2.9.3

Re: [PATCH v3] mdadm/r5cache: allow adding journal to array without journal

[jes.sorensen]

Song Liu <songliubraving@fb.com> writes:
> Currently, --add-journal can be only used to recreate broken journal
> for arrays with journal since  creation. As the kernel code getting
> more mature, this constraint is no longer necessary.
>
> This patch allows --add-journal to add journal to array without
> journal.
>
> Signed-off-by: Song Liu <songliubraving@fb.com>
> ---
>  Manage.c   | 9 ---------
>  mdadm.8.in | 5 ++---
>  2 files changed, 2 insertions(+), 12 deletions(-)
>
> diff --git a/Manage.c b/Manage.c
> index 5c3d2b9..d038b68 100644
> --- a/Manage.c
> +++ b/Manage.c
> @@ -946,7 +946,6 @@ int Manage_add(int fd, int tfd, struct mddev_dev *dv,
>  
>  	/* only add journal to array that supports journaling */
>  	if (dv->disposition == 'j') {
> -		struct mdinfo mdi;
>  		struct mdinfo *mdp;
>  
>  		mdp = sysfs_read(fd, NULL, GET_ARRAY_STATE);
> @@ -960,14 +959,6 @@ int Manage_add(int fd, int tfd, struct mddev_dev *dv,
>  			pr_err("%s is not readonly, cannot add journal.\n", devname);
>  			return -1;
>  		}
> -
> -		sysfs_free(mdp);
> -

Song,

Sorry for nagging you again on this one, however removing
sysfs_free(mdp) means you are leaking mbp here.

Cheers,
Jes

Re: [PATCH v3] mdadm/r5cache: allow adding journal to array without journal

[Song Liu]

On Mar 28, 2017, at 10:52 AM, jes.sorensen@gmail.com wrote:
> 
> Song Liu <songliubraving@fb.com> writes:
>> Currently, --add-journal can be only used to recreate broken journal
>> for arrays with journal since  creation. As the kernel code getting
>> more mature, this constraint is no longer necessary.
>> 
>> This patch allows --add-journal to add journal to array without
>> journal.
>> 
>> Signed-off-by: Song Liu <songliubraving@fb.com>
>> ---
>> Manage.c   | 9 ---------
>> mdadm.8.in | 5 ++---
>> 2 files changed, 2 insertions(+), 12 deletions(-)
>> 
>> diff --git a/Manage.c b/Manage.c
>> index 5c3d2b9..d038b68 100644
>> --- a/Manage.c
>> +++ b/Manage.c
>> @@ -946,7 +946,6 @@ int Manage_add(int fd, int tfd, struct mddev_dev *dv,
>> 
>> 	/* only add journal to array that supports journaling */
>> 	if (dv->disposition == 'j') {
>> -		struct mdinfo mdi;
>> 		struct mdinfo *mdp;
>> 
>> 		mdp = sysfs_read(fd, NULL, GET_ARRAY_STATE);
>> @@ -960,14 +959,6 @@ int Manage_add(int fd, int tfd, struct mddev_dev *dv,
>> 			pr_err("%s is not readonly, cannot add journal.\n", devname);
>> 			return -1;
>> 		}
>> -
>> -		sysfs_free(mdp);
>> -
> 
> Song,
> 
> Sorry for nagging you again on this one, however removing
> sysfs_free(mdp) means you are leaking mbp here.
> 
> Cheers,
> Jes

Oops... new patch coming...

Thanks,
Song