Skip to contentSkip to conversation

Fix the fix for the readd bug

PATCH In review Doug Ledford 2 participants 2 messages Jul 27, 2011 – Sep 19, 2011 MD RAID

lore
Share & tools
lore

Doug Ledford

author
#0 lore raw

PATCH 1/1 Fix the fix for the readd bug

The readd bug fix didn't work due to a faulty test. We need to check raid_disks to get an upper bound of the found slots to check since nr_disks only counts good disks and we could have faulty or spares that aren't counted but amount to an occupied slot.

Signed-off-by Doug Ledford <[email protected]>
util.c renamed from util.c.readd +4 -2
@@ -364,19 +364,21 @@ int enough_fd(int fd)
 	struct mdu_disk_info_s disk;
 	int avail_disks = 0;
 	int i;
+	int raid_disks;
 	char *avail;
 
 	if (ioctl(fd, GET_ARRAY_INFO, &array) != 0 ||
 	    array.raid_disks <= 0)
 		return 0;
 	avail = calloc(array.raid_disks, 1);
-	for (i=0; i < 1024 && array.nr_disks > 0; i++) {
+	raid_disks = array.raid_disks;
+	for (i=0; i < 1024 && raid_disks > 0; i++) {
 		disk.number = i;
 		if (ioctl(fd, GET_DISK_INFO, &disk) != 0)
 			continue;
 		if (disk.major == 0 && disk.minor == 0)
 			continue;
-		array.nr_disks--;
+		raid_disks--;
 
 		if (! (disk.state & (1<<MD_DISK_SYNC)))
 			continue;
#1 lore raw
Quoted 6 lines · On Wed, 27 Jul 2011 14:30:35 -0400 Doug Ledford <[email protected]> wrote:

The readd bug fix didn't work due to a faulty test. We need to check raid_disks to get an upper bound of the found slots to check since nr_disks only counts good disks and we could have faulty or spares that aren't counted but amount to an occupied slot.

Signed-off-by Doug Ledford <[email protected]>

Sorry for late review.

I don't agree with this patch.
'nr_disks' is exactly the number of devices in the mddev->disks list in the
kernel. get_array_info() counts them. This includes failed and spare
devices.

When GET_DISK_INFO is used, these devices will return a meaningful major/minor and all others will return '0' for major and minor. So the code should work as is.

Do you remember if you had a case where this failed? Can you describe it? Can you reproduce it?
Thanks,
NeilBrown

util.c renamed from util.c.readd +4 -2
@@ -364,19 +364,21 @@ int enough_fd(int fd)
 	struct mdu_disk_info_s disk;
 	int avail_disks = 0;
 	int i;
+	int raid_disks;
 	char *avail;
 
 	if (ioctl(fd, GET_ARRAY_INFO, &array) != 0 ||
 	    array.raid_disks <= 0)
 		return 0;
 	avail = calloc(array.raid_disks, 1);
-	for (i=0; i < 1024 && array.nr_disks > 0; i++) {
+	raid_disks = array.raid_disks;
+	for (i=0; i < 1024 && raid_disks > 0; i++) {
 		disk.number = i;
 		if (ioctl(fd, GET_DISK_INFO, &disk) != 0)
 			continue;
 		if (disk.major == 0 && disk.minor == 0)
 			continue;
-		array.nr_disks--;
+		raid_disks--;
 
 		if (! (disk.state & (1<<MD_DISK_SYNC)))
 			continue;