| Message ID | 20240911085432.37828-2-xni@redhat.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | mdadm tests fix | expand |
| Context | Check | Description |
|---|---|---|
| mdraidci/vmtest-md-6_12-PR | fail | merge-conflict |
On Wed, 11 Sep 2024 16:54:23 +0800 Xiao Ni <xni@redhat.com> wrote: > + > + /* new_level is introduced in kernel 6.12 */ > + if (!err && get_linux_version() >= 6012000 && > + sysfs_set_num(sra, NULL, "new_level", > info->new_level) < 0) > + err = errno; Hi Xiao, I realized that we would do this better by checking existence of new_level sysfs file. This way, our solution is limited to kernel > 6.12 so, for example redhat 9 with kernel 5.14 will never pass the condition. I know that you fixed test issue but someone still may find this in real life. I'm not going to rework it myself, I'm fine with current approach until someone will report issue about that for older kernel. If you are going to rework this, please left a comment about kernel version that it was added, to let future maintainers know when the additional verification can be removed. Thanks, Mariusz
On Wed, Sep 25, 2024 at 3:51 PM Mariusz Tkaczyk <mariusz.tkaczyk@linux.intel.com> wrote: > > On Wed, 11 Sep 2024 16:54:23 +0800 > Xiao Ni <xni@redhat.com> wrote: > > > + > > + /* new_level is introduced in kernel 6.12 */ > > + if (!err && get_linux_version() >= 6012000 && > > + sysfs_set_num(sra, NULL, "new_level", > > info->new_level) < 0) > > + err = errno; > > Hi Xiao, > I realized that we would do this better by checking existence of new_level > sysfs file. This way, our solution is limited to kernel > 6.12 so, for example > redhat 9 with kernel 5.14 will never pass the condition. I know that you fixed > test issue but someone still may find this in real life. > > I'm not going to rework it myself, I'm fine with current approach until > someone will report issue about that for older kernel. > > If you are going to rework this, please left a comment about kernel version > that it was added, to let future maintainers know when the additional > verification can be removed. Hi Mariusz Thanks for pointing this out. You're right. I'll fix it. Regards Xiao > > Thanks, > Mariusz >
diff --git a/Grow.c b/Grow.c index 5810b128aa99..533f301468af 100644 --- a/Grow.c +++ b/Grow.c @@ -2941,15 +2941,24 @@ static int impose_reshape(struct mdinfo *sra, * persists from some earlier problem. */ int err = 0; + if (sysfs_set_num(sra, NULL, "chunk_size", info->new_chunk) < 0) err = errno; + if (!err && sysfs_set_num(sra, NULL, "layout", reshape->after.layout) < 0) err = errno; + + /* new_level is introduced in kernel 6.12 */ + if (!err && get_linux_version() >= 6012000 && + sysfs_set_num(sra, NULL, "new_level", info->new_level) < 0) + err = errno; + if (!err && subarray_set_num(container, sra, "raid_disks", reshape->after.data_disks + reshape->parity) < 0) err = errno; + if (err) { pr_err("Cannot set device shape for %s\n", devname);
Reshape needs to specify a backup file when it can't update data offset of member disks. For this situation, first, it starts reshape and then it kicks off mdadm-grow-continue service which does backup job and monitors the reshape process. The service is a new process, so it needs to read superblock from member disks to get information. But in the first step, it doesn't update new level in superblock. So it can't change level after reshape finishes, because the new level is not right. So records the new level in the first step. Signed-off-by: Xiao Ni <xni@redhat.com> --- v2: format change, add get_linux_version Grow.c | 9 +++++++++ 1 file changed, 9 insertions(+)