Re: [PATCH] second attempt at DDF1 support for dmraid

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]


Heinz Mauelshagen wrote:
> Darrick,
> 
> this is the result of my first quick go through your code:
> 
> o streamlined various functions, e.g.
>   - introduced array for state definitions in disk_status()
>   - avoided dual find_or_alloc_set calls in ddf1_group()
>   - ...
> 
> o s/free/dbg_free/
> 
> o s/malloc/dbg_malloc/
> 
> o pretty printing

Looks good!  ddf1_group will likely change when I get my hands on a
RAID10 setup, but I'm (mostly) happy with the ddf1 driver for now.

> Please check if my patch doesn't break stuff for you and if you ike it ;-)
> 
> BTW: your code doesn't work with DELL privided DDV metadata
>      (goes into endless loop). Need to investogate more next week...

The loop that you put into disk_status() needs to increment s at the end
of the loop.  The attached patch corrects that error and adds a sanity
check in case name() returns NULL.

Can you send me a copy of this DDV metadata?  I'll send you some (big)
tarballs shortly, and off-list.

--D
diff -Naurp v8.hm/lib/format/ataraid/ddf1.c v9/lib/format/ataraid/ddf1.c
--- v8.hm/lib/format/ataraid/ddf1.c	2006-05-12 14:53:21.000000000 -0700
+++ v9/lib/format/ataraid/ddf1.c	2006-05-12 14:55:07.000000000 -0700
@@ -109,6 +109,7 @@ static enum status disk_status(struct dd
 		do {
 			if (disk->state & s->flag)
 				return s->status;
+			s++;
 		} while (s->status != s_ok);
 	}
 
@@ -1129,11 +1130,17 @@ static struct raid_set *ddf1_group(struc
 	struct ddf1_phys_drive *pd;
 	struct ddf1_config_record *cr;
 	struct raid_set *rs;
+	char *set_name;
 
 	if (!(pd = get_this_phys_drive(ddf1)))
 		LOG_ERR(lc, NULL, "Cannot find physical drive description!\n");
 
-	if ((rs = find_or_alloc_raid_set(lc, name(lc, ddf1), FIND_TOP, rd,
+	set_name = name(lc, ddf1);
+	if (!set_name)
+		LOG_ERR(lc, NULL, "%s: Could not find RAID array name.\n",
+			rd->di->path);
+
+	if ((rs = find_or_alloc_raid_set(lc, set_name, FIND_TOP, rd,
 					 LC_RS(lc), NO_CREATE,
 					 NO_CREATE_ARG))) {
 		if ((cr = get_config(ddf1, pd))) {

Attachment: signature.asc
Description: OpenPGP digital signature

_______________________________________________

Ataraid-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/ataraid-list

[Linux RAID]     [Linux IDE]     [Linux SCSI]     [Kernel]     [Linux Books]     [Linux Admin]     [GFS]     [RPM]     [Photos]     [Yosemite Photos]     [Yosemite News]     [AMD 64]
  Powered by Linux