[PATCH] md: allow changing set_name of running array

Subsystems: software raid (multiple disks) support, the rest

STALE3254d

7 messages, 3 authors, 2017-09-05 · open the first message on its own page

[PATCH] md: allow changing set_name of running array

From: Michał Mirosław <mirq-linux@rere.qmqm.pl>
Date: 2017-08-29 22:02:45 

Allow changing active array's set_name. This is the only way to
safely update superblock on an array which carries a mounted fs.

Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
---
 drivers/md/md.c | 79 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 79 insertions(+)

diff --git a/drivers/md/md.c b/drivers/md/md.c
index caca5d689cdc..59aa10669bef 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -5051,6 +5051,84 @@ static struct md_sysfs_entry md_consistency_policy =
 __ATTR(consistency_policy, S_IRUGO | S_IWUSR, consistency_policy_show,
        consistency_policy_store);
 
+static ssize_t
+set_name_show(struct mddev *mddev, char *page)
+{
+	struct mdp_superblock_1 *sb;
+	struct md_rdev *rdev;
+	int err;
+
+	err = mddev_lock(mddev);
+	if (err)
+		return err;
+
+	/* only version-1 superblocks carry a MD set's name */
+	err = -ENXIO;
+	if (mddev->major_version != 1)
+		goto out_unlock;
+
+	err = -EINVAL;
+	rdev_for_each(rdev, mddev) {
+		if (WARN_ON(!rdev->sb_loaded))
+			continue;
+
+		sb = page_address(rdev->sb_page);
+		err = sprintf(page, "%.32s\n", sb->set_name);
+		break;
+	}
+
+out_unlock:
+	mddev_unlock(mddev);
+	return err;
+}
+
+static ssize_t
+set_name_store(struct mddev *mddev, const char *buf, size_t buflen)
+{
+	struct mdp_superblock_1 *sb;
+	struct md_rdev *rdev;
+	size_t len = buflen;
+	int err;
+
+	if (len && buf[len - 1] == '\n')
+		--len;
+
+	if (len > sizeof(sb->set_name))
+		return -ENOSPC;
+
+	err = mddev_lock(mddev);
+	if (err)
+		return err;
+
+	/* only version-1 superblocks carry a MD set's name */
+	err = -ENXIO;
+	if (mddev->major_version != 1)
+		goto out_unlock;
+
+	err = -EROFS;
+	if (mddev->ro)
+		goto out_unlock;
+
+	rdev_for_each(rdev, mddev) {
+		if (WARN_ON(!rdev->sb_loaded))
+			continue;
+
+		sb = page_address(rdev->sb_page);
+		memcpy(sb->set_name, buf, len);
+		memset(&sb->set_name[len], 0, sizeof(sb->set_name) - len);
+	}
+
+	md_update_sb(mddev, 1);
+	err = buflen;
+
+out_unlock:
+	mddev_unlock(mddev);
+	return err;
+}
+
+static struct md_sysfs_entry md_set_name =
+__ATTR(set_name, S_IRUGO | S_IWUSR, set_name_show, set_name_store);
+
 static struct attribute *md_default_attrs[] = {
 	&md_level.attr,
 	&md_layout.attr,
@@ -5067,6 +5145,7 @@ static struct attribute *md_default_attrs[] = {
 	&md_array_size.attr,
 	&max_corr_read_errors.attr,
 	&md_consistency_policy.attr,
+	&md_set_name.attr,
 	NULL,
 };
 
-- 
2.11.0

Re: [PATCH] md: allow changing set_name of running array

From: NeilBrown <hidden>
Date: 2017-09-01 00:07:29 

On Wed, Aug 30 2017, Michał Mirosław wrote:

Allow changing active array's set_name. This is the only way to
safely update superblock on an array which carries a mounted fs.
Do you really need to change the set_name of an active array?

The name is only used when the array is actived, so wait until the next
time the array is stopped, and change the name then.

You can boot with a rescue CD or similar and use "--assemble
--update=name", or with a bit of effort you could get the normal boot
sequence to change the name.

I wouldn't object to adding something to mdadm so that it would read
something from mdadm.conf, and update the set name at boot time.

What is the underlying problem that you are trying to solve here?

Thanks,
NeilBrown
quoted hunk
Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
---
 drivers/md/md.c | 79 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 79 insertions(+)

diff --git a/drivers/md/md.c b/drivers/md/md.c
index caca5d689cdc..59aa10669bef 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -5051,6 +5051,84 @@ static struct md_sysfs_entry md_consistency_policy =
 __ATTR(consistency_policy, S_IRUGO | S_IWUSR, consistency_policy_show,
        consistency_policy_store);
 
+static ssize_t
+set_name_show(struct mddev *mddev, char *page)
+{
+	struct mdp_superblock_1 *sb;
+	struct md_rdev *rdev;
+	int err;
+
+	err = mddev_lock(mddev);
+	if (err)
+		return err;
+
+	/* only version-1 superblocks carry a MD set's name */
+	err = -ENXIO;
+	if (mddev->major_version != 1)
+		goto out_unlock;
+
+	err = -EINVAL;
+	rdev_for_each(rdev, mddev) {
+		if (WARN_ON(!rdev->sb_loaded))
+			continue;
+
+		sb = page_address(rdev->sb_page);
+		err = sprintf(page, "%.32s\n", sb->set_name);
+		break;
+	}
+
+out_unlock:
+	mddev_unlock(mddev);
+	return err;
+}
+
+static ssize_t
+set_name_store(struct mddev *mddev, const char *buf, size_t buflen)
+{
+	struct mdp_superblock_1 *sb;
+	struct md_rdev *rdev;
+	size_t len = buflen;
+	int err;
+
+	if (len && buf[len - 1] == '\n')
+		--len;
+
+	if (len > sizeof(sb->set_name))
+		return -ENOSPC;
+
+	err = mddev_lock(mddev);
+	if (err)
+		return err;
+
+	/* only version-1 superblocks carry a MD set's name */
+	err = -ENXIO;
+	if (mddev->major_version != 1)
+		goto out_unlock;
+
+	err = -EROFS;
+	if (mddev->ro)
+		goto out_unlock;
+
+	rdev_for_each(rdev, mddev) {
+		if (WARN_ON(!rdev->sb_loaded))
+			continue;
+
+		sb = page_address(rdev->sb_page);
+		memcpy(sb->set_name, buf, len);
+		memset(&sb->set_name[len], 0, sizeof(sb->set_name) - len);
+	}
+
+	md_update_sb(mddev, 1);
+	err = buflen;
+
+out_unlock:
+	mddev_unlock(mddev);
+	return err;
+}
+
+static struct md_sysfs_entry md_set_name =
+__ATTR(set_name, S_IRUGO | S_IWUSR, set_name_show, set_name_store);
+
 static struct attribute *md_default_attrs[] = {
 	&md_level.attr,
 	&md_layout.attr,
@@ -5067,6 +5145,7 @@ static struct attribute *md_default_attrs[] = {
 	&md_array_size.attr,
 	&max_corr_read_errors.attr,
 	&md_consistency_policy.attr,
+	&md_set_name.attr,
 	NULL,
 };
 
-- 
2.11.0

--
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

Re: [PATCH] md: allow changing set_name of running array

From: Michał Mirosław <mirq-linux@rere.qmqm.pl>
Date: 2017-09-01 16:41:10 

On Fri, Sep 01, 2017 at 10:07:29AM +1000, NeilBrown wrote:
On Wed, Aug 30 2017, Michał Mirosław wrote:
quoted
Allow changing active array's set_name. This is the only way to
safely update superblock on an array which carries a mounted fs.
Do you really need to change the set_name of an active array?

The name is only used when the array is actived, so wait until the next
time the array is stopped, and change the name then.

You can boot with a rescue CD or similar and use "--assemble
--update=name", or with a bit of effort you could get the normal boot
sequence to change the name.

I wouldn't object to adding something to mdadm so that it would read
something from mdadm.conf, and update the set name at boot time.

What is the underlying problem that you are trying to solve here?
I had to fix /dev/md* naming on a system with no physical access.

The problem was that despite matching mdadm.conf entries, arrays started
with random 127-i indexes (because superblocks' set_names didn't match
hostname, I guess).

Best Regards,
Michał Mirosław

Re: [PATCH] md: allow changing set_name of running array

From: NeilBrown <hidden>
Date: 2017-09-04 02:22:33 

On Fri, Sep 01 2017, Michał Mirosław wrote:

On Fri, Sep 01, 2017 at 10:07:29AM +1000, NeilBrown wrote:
quoted
On Wed, Aug 30 2017, Michał Mirosław wrote:
quoted
Allow changing active array's set_name. This is the only way to
safely update superblock on an array which carries a mounted fs.
Do you really need to change the set_name of an active array?

The name is only used when the array is actived, so wait until the next
time the array is stopped, and change the name then.

You can boot with a rescue CD or similar and use "--assemble
--update=name", or with a bit of effort you could get the normal boot
sequence to change the name.

I wouldn't object to adding something to mdadm so that it would read
something from mdadm.conf, and update the set name at boot time.

What is the underlying problem that you are trying to solve here?
I had to fix /dev/md* naming on a system with no physical access.

The problem was that despite matching mdadm.conf entries, arrays started
with random 127-i indexes (because superblocks' set_names didn't match
hostname, I guess).
That's odd.  If an array is listed in mdadm.conf, that is enough to tell
mdadm that it is "local" so that it doesn't need the hostname to match.
Can you show me exactly what was in your mdadm.conf?

Thanks,
NeilBrown

Re: [PATCH] md: allow changing set_name of running array

From: Michał Mirosław <mirq-linux@rere.qmqm.pl>
Date: 2017-09-04 19:36:39 

On Mon, Sep 04, 2017 at 12:22:33PM +1000, NeilBrown wrote:
On Fri, Sep 01 2017, Michał Mirosław wrote:
quoted
On Fri, Sep 01, 2017 at 10:07:29AM +1000, NeilBrown wrote:
quoted
On Wed, Aug 30 2017, Michał Mirosław wrote:
quoted
Allow changing active array's set_name. This is the only way to
safely update superblock on an array which carries a mounted fs.
Do you really need to change the set_name of an active array?

The name is only used when the array is actived, so wait until the next
time the array is stopped, and change the name then.

You can boot with a rescue CD or similar and use "--assemble
--update=name", or with a bit of effort you could get the normal boot
sequence to change the name.

I wouldn't object to adding something to mdadm so that it would read
something from mdadm.conf, and update the set name at boot time.

What is the underlying problem that you are trying to solve here?
I had to fix /dev/md* naming on a system with no physical access.

The problem was that despite matching mdadm.conf entries, arrays started
with random 127-i indexes (because superblocks' set_names didn't match
hostname, I guess).
That's odd.  If an array is listed in mdadm.conf, that is enough to tell
mdadm that it is "local" so that it doesn't need the hostname to match.
Can you show me exactly what was in your mdadm.conf?
This is what was I had when it would reassign arrays on boot. System
hostname is 'rere' - it was renamed some time after creating arrays.

---8<---
HOMEHOST <system>
MAILADDR root
ARRAY /dev/md/0  metadata=1.2 UUID=d5ea39a9:5ec27c36:c83296d4:70f879d4 name=rere2:0
ARRAY /dev/md/1  metadata=1.2 UUID=9bf69b92:2e7ba905:5f959c09:c8fecb99 name=rere2:1
---8<---

I don't have old versions of mdadm.conf, so had to recreate this one.
IIRC I verified UUIDs back then, but don't remember whether I modified
names or not. Thinking of it, how would mdadm behave if it had matching
UUIDs and mismatching array names in mdadm.conf?

Best Regards,
Michał Mirosław

Re: [PATCH] md: allow changing set_name of running array

From: NeilBrown <hidden>
Date: 2017-09-05 01:00:40 

On Mon, Sep 04 2017, Michał Mirosław wrote:

On Mon, Sep 04, 2017 at 12:22:33PM +1000, NeilBrown wrote:
quoted
On Fri, Sep 01 2017, Michał Mirosław wrote:
quoted
On Fri, Sep 01, 2017 at 10:07:29AM +1000, NeilBrown wrote:
quoted
On Wed, Aug 30 2017, Michał Mirosław wrote:
quoted
Allow changing active array's set_name. This is the only way to
safely update superblock on an array which carries a mounted fs.
Do you really need to change the set_name of an active array?

The name is only used when the array is actived, so wait until the next
time the array is stopped, and change the name then.

You can boot with a rescue CD or similar and use "--assemble
--update=name", or with a bit of effort you could get the normal boot
sequence to change the name.

I wouldn't object to adding something to mdadm so that it would read
something from mdadm.conf, and update the set name at boot time.

What is the underlying problem that you are trying to solve here?
I had to fix /dev/md* naming on a system with no physical access.

The problem was that despite matching mdadm.conf entries, arrays started
with random 127-i indexes (because superblocks' set_names didn't match
hostname, I guess).
That's odd.  If an array is listed in mdadm.conf, that is enough to tell
mdadm that it is "local" so that it doesn't need the hostname to match.
Can you show me exactly what was in your mdadm.conf?
This is what was I had when it would reassign arrays on boot. System
hostname is 'rere' - it was renamed some time after creating arrays.

---8<---
HOMEHOST <system>
MAILADDR root
ARRAY /dev/md/0  metadata=1.2 UUID=d5ea39a9:5ec27c36:c83296d4:70f879d4 name=rere2:0
ARRAY /dev/md/1  metadata=1.2 UUID=9bf69b92:2e7ba905:5f959c09:c8fecb99 name=rere2:1
---8<---

I don't have old versions of mdadm.conf, so had to recreate this one.
IIRC I verified UUIDs back then, but don't remember whether I modified
names or not. Thinking of it, how would mdadm behave if it had matching
UUIDs and mismatching array names in mdadm.conf?
That's what I was wondering and part of why I asked to see the
mdadm.conf.

If a name is given in mdadm.conf and it doesn't match the name in the
metadata, that line will be ignored.  If mdadm doesn't find a match in
mdadm.conf and the hostname doesn't match, the the array will be treated
as "foreign" and will be assembled as e.g. /dev/md127 with a symlink
from /dev/md/0_1.
If the name and uuid in mdadm.conf do match the metadata, it will be
treat as "local" and will be given the name that you would expect
(/dev/md0, /dev/md/0).

So my guess is that the mdadm.conf you had at the time doesn't match
what you have provided above.

It would be nice to add an option to "mdadm --incremental" to tell it to
update various details like you can with "mdadm --assemble"....

NeilBrown

Re: [PATCH] md: allow changing set_name of running array

From: Phil Turmel <hidden>
Date: 2017-09-05 03:06:29 

On 09/04/2017 09:00 PM, NeilBrown wrote:
If a name is given in mdadm.conf and it doesn't match the name in the
metadata, that line will be ignored.  If mdadm doesn't find a match in
mdadm.conf and the hostname doesn't match, the the array will be treated
as "foreign" and will be assembled as e.g. /dev/md127 with a symlink
from /dev/md/0_1.
If the name and uuid in mdadm.conf do match the metadata, it will be
treat as "local" and will be given the name that you would expect
(/dev/md0, /dev/md/0).

So my guess is that the mdadm.conf you had at the time doesn't match
what you have provided above.

It would be nice to add an option to "mdadm --incremental" to tell it to
update various details like you can with "mdadm --assemble"....
I just delete the name and metadata version from mdadm.conf.  The above
would become:

ARRAY /dev/md/0 UUID=d5ea39a9:5ec27c36:c83296d4:70f879d4
ARRAY /dev/md/1 UUID=9bf69b92:2e7ba905:5f959c09:c8fecb99

The default output of mdadm -Es is overspecified for use as mdadm.conf,
though it works on precisely the setup that produced it.

Phil
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help