[PATCH 0/2] --detail show messy container name

[PATCH 0/2] --detail show messy container name

[Xiao Ni]

This patch set tries to fix the bug that --detail show messy container
name. It adds a new method to check if one device is alive in first
patch.

V2:
use access rather than devid2kname

V3:
define a new function to check if disk is alive
include <stdbool.h> to use bool

Xiao Ni (2):
  mdadm/lib: Define a new helper function is_dev_alived
  mdadm/Detail: Can't show container name correctly when unpluging disks

 Detail.c | 16 ++++++++--------
 lib.c    | 11 +++++++++++
 mdadm.h  |  2 ++
 3 files changed, 21 insertions(+), 8 deletions(-)

-- 
2.7.5

[PATCH 1/2] mdadm/lib: Define a new helper function is_dev_alived

[Xiao Ni]

The function is used to check if one member disk is alive.

Signed-off-by: Xiao Ni <xni@redhat.com>
Acked-by: Mariusz Tkaczyk <mariusz.tkaczyk@linux.intel.com>
---
 lib.c   | 11 +++++++++++
 mdadm.h |  2 ++
 2 files changed, 13 insertions(+)

diff --git a/lib.c b/lib.c
index 76d1fbb..7e3e3d4 100644
--- a/lib.c
+++ b/lib.c
@@ -27,6 +27,17 @@
 #include	<ctype.h>
 #include	<limits.h>
 
+bool is_dev_alive(char *path)
+{
+	if (!path)
+		return false;
+
+	if (access(path, R_OK) == 0)
+		return true;
+
+	return false;
+}
+
 /* This fill contains various 'library' style function.  They
  * have no dependency on anything outside this file.
  */
diff --git a/mdadm.h b/mdadm.h
index 53ea0de..4e483bf 100644
--- a/mdadm.h
+++ b/mdadm.h
@@ -45,6 +45,7 @@ extern __off64_t lseek64 __P ((int __fd, __off64_t __offset, int __whence));
 #include	<errno.h>
 #include	<string.h>
 #include	<syslog.h>
+#include	<stdbool.h>
 /* Newer glibc requires sys/sysmacros.h directly for makedev() */
 #include	<sys/sysmacros.h>
 #ifdef __dietlibc__
@@ -1499,6 +1500,7 @@ extern int check_partitions(int fd, char *dname,
 extern int fstat_is_blkdev(int fd, char *devname, dev_t *rdev);
 extern int stat_is_blkdev(char *devname, dev_t *rdev);
 
+extern bool is_dev_alive(char *path);
 extern int get_mdp_major(void);
 extern int get_maj_min(char *dev, int *major, int *minor);
 extern int dev_open(char *dev, int flags);
-- 
2.7.5

[PATCH 2/2] mdadm/Detail: Can't show container name correctly when unpluging disks

[Xiao Ni]

The test case is:
1. create one imsm container
2. create a raid5 device from the container
3. unplug two disks
4. mdadm --detail /dev/md126
[root@rhel85 ~]# mdadm -D /dev/md126
/dev/md126:
         Container : ��, member 0

The Detail function first gets container name by function
map_dev_preferred. Then it tries to find which disks are
available. In patch db5377883fef(It should be FAILED..)
uses map_dev_preferred to find which disks are under /dev.

But now, the major/minor information comes from kernel space.
map_dev_preferred malloc memory and init a device list when
first be called by Detail. It can't find the device in the
list by the major/minor. It free the memory and reinit the
list.

The container name now points to an area tha has been freed.
So the containt is a mess.

This patch replaces map_dev_preferred with access.

Fixes: db5377883fef (It should be FAILED when raid has)
Signed-off-by: Xiao Ni <xni@redhat.com>
Reported-by: Fine Fan <ffan@redhat.com>
Acked-by: Mariusz Tkaczyk <mariusz.tkaczyk@linux.intel.com>
---
 Detail.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/Detail.c b/Detail.c
index d3af0ab..95d4cc7 100644
--- a/Detail.c
+++ b/Detail.c
@@ -351,14 +351,14 @@ int Detail(char *dev, struct context *c)
 	avail = xcalloc(array.raid_disks, 1);
 
 	for (d = 0; d < array.raid_disks; d++) {
-		char *dv, *dv_rep;
-		dv = map_dev_preferred(disks[d*2].major,
-				disks[d*2].minor, 0, c->prefer);
-		dv_rep = map_dev_preferred(disks[d*2+1].major,
-				disks[d*2+1].minor, 0, c->prefer);
-
-		if ((dv && (disks[d*2].state & (1<<MD_DISK_SYNC))) ||
-		    (dv_rep && (disks[d*2+1].state & (1<<MD_DISK_SYNC)))) {
+		char dv[PATH_MAX], dv_rep[PATH_MAX];
+		snprintf(dv, PATH_MAX, "/sys/dev/block/%d:%d",
+			disks[d*2].major, disks[d*2].minor);
+		snprintf(dv_rep, PATH_MAX, "/sys/dev/block/%d:%d",
+			disks[d*2+1].major, disks[d*2+1].minor);
+
+		if ((is_dev_alive(dv) && (disks[d*2].state & (1<<MD_DISK_SYNC))) ||
+		    (is_dev_alive(dv_rep) && (disks[d*2+1].state & (1<<MD_DISK_SYNC)))) {
 			avail_disks ++;
 			avail[d] = 1;
 		} else
-- 
2.7.5

Re: [PATCH 0/2] --detail show messy container name

[Jes Sorensen]

On 10/27/21 8:23 AM, Xiao Ni wrote:
> This patch set tries to fix the bug that --detail show messy container
> name. It adds a new method to check if one device is alive in first
> patch.
> 
> V2:
> use access rather than devid2kname
> 
> V3:
> define a new function to check if disk is alive
> include <stdbool.h> to use bool
> 
> Xiao Ni (2):
>   mdadm/lib: Define a new helper function is_dev_alived
>   mdadm/Detail: Can't show container name correctly when unpluging disks

Applied!

Thanks,
Jes