DDF/RAID10 patch series

DDF/RAID10 patch series

[mwilck]

This is the DDF/RAID10 patch series I announced.

The whole patch set is aimed at making --Create work with DDF/RAID10.
When --Create eventually works, RAID10 can be added to the DDF
regression test.

With this patch set, DDF is still far from the level of functionality
present in IMSM, but getting a little closer.

The patch set also includes some cleanup patches and debug code which
I needed to find the bugs in earlier versions, and which might be useful
later as well.

Please review and test.

Regards
Martin

Martin Wilck (27):
  DDF (cleanup): use a common macro for failed searches
  DDF: check_secondary: fix treatment of missing BVDs
  DDF: load_ddf_headers: use secondary header as fallback
  DDF: handle "open flag" according to spec
  DDF: Implement store_super_ddf
  DDF: ddf_open_new: implement minimal consistency check
  DDF: find_vdcr: account for secondary RAID level
  DDF: ddf_set_disk: move status logic to separate function
  DDF: get_svd_state: Status logic for secondary RAID level
  DDF: allow empty slots in virt disk table
  DDF: layout_ddf2md: new DDF->md RAID layout conversion
  DDF: layout_md2ddf: new md->DDF layout conversion
  DDF: Simplify allocation of "other BVDs"
  DDF: init_super_ddf_bvd: initialize other bvds
  DDF: validate_geometry_ddf: support RAID10
  DDF: use LBA_OFFSET macro instead of lba_offset field
  DDF: get_extents: support secondary RAID level
  DDF: add_to_super_ddf: allow empty slots in phys disk table
  DDF: add_to_super_ddf: Use same amount of workspace as other disks
  DDF: add_to_super_ddf: RAID10 changes
  DDF: add_to_super_ddf_bvd: use get_svd_state()
  DDF: getinfo_super_ddf_bvd: lba_offset calculation for RAID10
  DDF: guid_str: convenience function to print GUID for debugging
  DDF: ddf_set_array_state: more meaningful output
  DDF: ddf_process_update: handle update of conf records for SVD
  DDF: ddf_process_update: Fix vlist treatment for SVDs
  tests/10ddf-create: add RAID 10 array

 super-ddf.c        | 1134 +++++++++++++++++++++++++++++++++-------------------
 tests/10ddf-create |   15 +-
 2 files changed, 739 insertions(+), 410 deletions(-)

[PATCH 01/27] DDF (cleanup): use a common macro for failed searches

[mwilck]

Use DDF_NOTFOUND instead of NO_SUCH_REFNUM.

Signed-off-by: Martin Wilck <mwilck@arcor.de>
---
 super-ddf.c |    5 ++---
 1 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/super-ddf.c b/super-ddf.c
index 56aad68..91feb87 100644
--- a/super-ddf.c
+++ b/super-ddf.c
@@ -3257,7 +3257,6 @@ static int check_secondary(const struct vcl *vc)
 	return 0;
 }
 
-#define NO_SUCH_REFNUM (0xFFFFFFFF)
 static unsigned int get_pd_index_from_refnum(const struct vcl *vc,
 					     __u32 refnum, unsigned int nmax,
 					     const struct vd_config **bvd,
@@ -3298,7 +3297,7 @@ static unsigned int get_pd_index_from_refnum(const struct vcl *vc,
 	}
 bad:
 	*bvd = NULL;
-	return NO_SUCH_REFNUM;
+	return DDF_NOTFOUND;
 }
 
 static struct mdinfo *container_content_ddf(struct supertype *st, char *subarray)
@@ -3414,7 +3413,7 @@ static struct mdinfo *container_content_ddf(struct supertype *st, char *subarray
 			i = get_pd_index_from_refnum(
 				vc, ddf->phys->entries[pd].refnum,
 				ddf->mppe, &bvd, &iphys);
-			if (i == NO_SUCH_REFNUM)
+			if (i == DDF_NOTFOUND)
 				continue;
 
 			this->array.working_disks++;
-- 
1.7.1

[PATCH 02/27] DDF: check_secondary: fix treatment of missing BVDs

[mwilck]

Unused BVDs should just be skipped instead of bailing out.

Signed-off-by: Martin Wilck <mwilck@arcor.de>
---
 super-ddf.c |    6 ++----
 1 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/super-ddf.c b/super-ddf.c
index 91feb87..c445df2 100644
--- a/super-ddf.c
+++ b/super-ddf.c
@@ -3222,10 +3222,8 @@ static int check_secondary(const struct vcl *vc)
 	__set_sec_seen(conf->sec_elmnt_seq);
 	for (i = 0; i < conf->sec_elmnt_count-1; i++) {
 		const struct vd_config *bvd = vc->other_bvds[i];
-		if (bvd == NULL) {
-			pr_err("BVD %d is missing\n", i+1);
-			return -1;
-		}
+		if (bvd == NULL)
+			continue;
 		if (bvd->srl != conf->srl) {
 			pr_err("Inconsistent secondary RAID level across BVDs\n");
 			return -1;
-- 
1.7.1

[PATCH 03/27] DDF: load_ddf_headers: use secondary header as fallback

[mwilck]

When the primary header can't be read, use the secondary header
as fallback.

Signed-off-by: Martin Wilck <mwilck@arcor.de>
---
 super-ddf.c |   13 +++++++++----
 1 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/super-ddf.c b/super-ddf.c
index c445df2..45522f2 100644
--- a/super-ddf.c
+++ b/super-ddf.c
@@ -601,15 +601,15 @@ static int load_ddf_headers(int fd, struct ddf_super *super, char *devname)
 				DDF_REVISION_2, super->anchor.revision,devname);
 		return 2;
 	}
+	super->active = NULL;
 	if (load_ddf_header(fd, __be64_to_cpu(super->anchor.primary_lba),
 			    dsize >> 9,  1,
 			    &super->primary, &super->anchor) == 0) {
 		if (devname)
 			pr_err("Failed to load primary DDF header "
 			       "on %s\n", devname);
-		return 2;
-	}
-	super->active = &super->primary;
+	} else
+		super->active = &super->primary;
 	if (load_ddf_header(fd, __be64_to_cpu(super->anchor.secondary_lba),
 			    dsize >> 9,  2,
 			    &super->secondary, &super->anchor)) {
@@ -619,9 +619,14 @@ static int load_ddf_headers(int fd, struct ddf_super *super, char *devname)
 		    || (__be32_to_cpu(super->primary.seq)
 			== __be32_to_cpu(super->secondary.seq) &&
 			super->primary.openflag && !super->secondary.openflag)
+		    || super->active == NULL
 			)
 			super->active = &super->secondary;
-	}
+	} else if (devname)
+		pr_err("Failed to load secondary DDF header on %s\n",
+		       devname);
+	if (super->active == NULL)
+		return 2;
 	return 0;
 }
 
-- 
1.7.1

[PATCH 04/27] DDF: handle "open flag" according to spec

[mwilck]

The DDF spec mandates that the "open flag" be set to non-0 before
writing a configuration, and reset to 0 when finished to indicate
success.

Signed-off-by: Martin Wilck <mwilck@arcor.de>
---
 super-ddf.c |   26 ++++++++++++++++++--------
 1 files changed, 18 insertions(+), 8 deletions(-)

diff --git a/super-ddf.c b/super-ddf.c
index 45522f2..b806949 100644
--- a/super-ddf.c
+++ b/super-ddf.c
@@ -2518,6 +2518,7 @@ static int __write_ddf_structure(struct dl *d, struct ddf_super *ddf, __u8 type,
 	unsigned long long sector;
 	struct ddf_header *header;
 	int fd, i, n_config, conf_size;
+	int ret = 0;
 
 	fd = d->fd;
 
@@ -2535,23 +2536,23 @@ static int __write_ddf_structure(struct dl *d, struct ddf_super *ddf, __u8 type,
 	}
 
 	header->type = type;
-	header->openflag = 0;
+	header->openflag = 1;
 	header->crc = calc_crc(header, 512);
 
 	lseek64(fd, sector<<9, 0);
 	if (write(fd, header, 512) < 0)
-		return 0;
+		goto out;
 
 	ddf->controller.crc = calc_crc(&ddf->controller, 512);
 	if (write(fd, &ddf->controller, 512) < 0)
-		return 0;
+		goto out;
 
 	ddf->phys->crc = calc_crc(ddf->phys, ddf->pdsize);
 	if (write(fd, ddf->phys, ddf->pdsize) < 0)
-		return 0;
+		goto out;
 	ddf->virt->crc = calc_crc(ddf->virt, ddf->vdsize);
 	if (write(fd, ddf->virt, ddf->vdsize) < 0)
-		return 0;
+		goto out;
 
 	/* Now write lots of config records. */
 	n_config = ddf->max_part;
@@ -2590,13 +2591,22 @@ static int __write_ddf_structure(struct dl *d, struct ddf_super *ddf, __u8 type,
 		}
 	}
 	if (i <= n_config)
-		return 0;
+		goto out;
 
 	d->disk.crc = calc_crc(&d->disk, 512);
 	if (write(fd, &d->disk, 512) < 0)
-		return 0;
+		goto out;
 
-	return 1;
+	ret = 1;
+out:
+	header->openflag = 0;
+	header->crc = calc_crc(header, 512);
+
+	lseek64(fd, sector<<9, 0);
+	if (write(fd, header, 512) < 0)
+		ret = 0;
+
+	return ret;
 }
 
 static int __write_init_super_ddf(struct supertype *st)
-- 
1.7.1

[PATCH 05/27] DDF: Implement store_super_ddf

[mwilck]

This patch implements the previously unsupported case where
store_super_ddf is called with a non-empty superblock.

For DDF, writing meta data to just one disk makes no sense.
We would run the risk of writing inconsistent meta data
to the devices. So just call __write_init_super_ddf and
write to all devices, including the one passed by the caller.

This patch assumes that the device to store the superblock on
has already been added to the DDF structure. Otherwise, an
error message will be emitted.

Signed-off-by: Martin Wilck <mwilck@arcor.de>
---
 super-ddf.c |   41 +++++++++++++++++++++++++++++++++++------
 1 files changed, 35 insertions(+), 6 deletions(-)

diff --git a/super-ddf.c b/super-ddf.c
index b806949..b3c846d 100644
--- a/super-ddf.c
+++ b/super-ddf.c
@@ -3471,15 +3471,44 @@ static int store_super_ddf(struct supertype *st, int fd)
 	if (!ddf)
 		return 1;
 
-	/* ->dlist and ->conflist will be set for updates, currently not
-	 * supported
-	 */
-	if (ddf->dlist || ddf->conflist)
-		return 1;
-
 	if (!get_dev_size(fd, NULL, &dsize))
 		return 1;
 
+	if (ddf->dlist || ddf->conflist) {
+		struct stat sta;
+		struct dl *dl;
+		int ofd, ret;
+
+		if (fstat(fd, &sta) == -1 || !S_ISBLK(sta.st_mode)) {
+			pr_err("%s: file descriptor for invalid device\n",
+			       __func__);
+			return 1;
+		}
+		for (dl = ddf->dlist; dl; dl = dl->next)
+			if (dl->major == (int)major(sta.st_rdev) &&
+			    dl->minor == (int)minor(sta.st_rdev))
+				break;
+		if (!dl) {
+			pr_err("%s: couldn't find disk %d/%d\n", __func__,
+			       (int)major(sta.st_rdev),
+			       (int)minor(sta.st_rdev));
+			return 1;
+		}
+		/*
+		   For DDF, writing to just one disk makes no sense.
+		   We would run the risk of writing inconsistent meta data
+		   to the devices. So just call __write_init_super_ddf and
+		   write to all devices, including this one.
+		   Use the fd passed to this function, just in case dl->fd
+		   is invalid.
+		 */
+		ofd = dl->fd;
+		dl->fd = fd;
+		ret =  __write_init_super_ddf(st);
+		dl->fd = ofd;
+		return ret;
+	}
+
 	if (posix_memalign(&buf, 512, 512) != 0)
 		return 1;
 	memset(buf, 0, 512);
-- 
1.7.1

[PATCH 06/27] DDF: ddf_open_new: implement minimal consistency check

[mwilck]

Added a minimal consitency check as in imsm_open_new().

Signed-off-by: Martin Wilck <mwilck@arcor.de>
---
 super-ddf.c |   10 ++++++++--
 1 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/super-ddf.c b/super-ddf.c
index b3c846d..ae24bb9 100644
--- a/super-ddf.c
+++ b/super-ddf.c
@@ -3695,8 +3695,14 @@ static int compare_super_ddf(struct supertype *st, struct supertype *tst)
  */
 static int ddf_open_new(struct supertype *c, struct active_array *a, char *inst)
 {
-	dprintf("ddf: open_new %s\n", inst);
-	a->info.container_member = atoi(inst);
+	struct ddf_super *ddf = c->sb;
+	int n = atoi(inst);
+	if (n >= (int)__be16_to_cpu(ddf->virt->populated_vdes)) {
+		pr_err("%s: subarray index %d out of range\n", __func__, n);
+		return -ENODEV;
+	}
+	dprintf("ddf: open_new %d\n", n);
+	a->info.container_member = n;
 	return 0;
 }
 
-- 
1.7.1

[PATCH 07/27] DDF: find_vdcr: account for secondary RAID level

[mwilck]

If secondary RAID level is taken into account, translation between
the md RAID member (raid_disk) and the index of a physical disk
in a BVD becomes more complex.

Also, take into account that the member list can have unused entries
(this is independent of secondary RAID level).

Adapt usage of find_vdcr() accordingly

Signed-off-by: Martin Wilck <mwilck@arcor.de>
---
 super-ddf.c |  100 +++++++++++++++++++++++++++++++++++++++++++++++++++--------
 1 files changed, 87 insertions(+), 13 deletions(-)

diff --git a/super-ddf.c b/super-ddf.c
index ae24bb9..c448bff 100644
--- a/super-ddf.c
+++ b/super-ddf.c
@@ -1441,13 +1441,76 @@ static int match_home_ddf(struct supertype *st, char *homehost)
 }
 
 #ifndef MDASSEMBLE
-static struct vd_config *find_vdcr(struct ddf_super *ddf, unsigned int inst)
+static int find_index_in_bvd(const struct ddf_super *ddf,
+			     const struct vd_config *conf, unsigned int n,
+			     unsigned int *n_bvd)
+{
+	/*
+	 * Find the index of the n-th valid physical disk in this BVD
+	 */
+	unsigned int i, j;
+	for (i = 0, j = 0; i < ddf->mppe &&
+		     j < __be16_to_cpu(conf->prim_elmnt_count); i++) {
+		if (conf->phys_refnum[i] != 0xffffffff) {
+			if (n == j) {
+				*n_bvd = i;
+				return 1;
+			}
+			j++;
+		}
+	}
+	dprintf("%s: couldn't find BVD member %u (total %u)\n",
+		__func__, n, __be16_to_cpu(conf->prim_elmnt_count));
+	return 0;
+}
+
+static struct vd_config *find_vdcr(struct ddf_super *ddf, unsigned int inst,
+				   unsigned int n,
+				   unsigned int *n_bvd, struct vcl **vcl)
 {
 	struct vcl *v;
 
-	for (v = ddf->conflist; v; v = v->next)
-		if (inst == v->vcnum)
-			return &v->conf;
+	for (v = ddf->conflist; v; v = v->next) {
+		unsigned int nsec, ibvd;
+		struct vd_config *conf;
+		if (inst != v->vcnum)
+			continue;
+		conf = &v->conf;
+		if (conf->sec_elmnt_count == 1) {
+			if (find_index_in_bvd(ddf, conf, n, n_bvd)) {
+				*vcl = v;
+				return conf;
+			} else
+				goto bad;
+		}
+		if (v->other_bvds == NULL) {
+			pr_err("%s: BUG: other_bvds is NULL, nsec=%u\n",
+			       __func__, conf->sec_elmnt_count);
+			goto bad;
+		}
+		nsec = n / __be16_to_cpu(conf->prim_elmnt_count);
+		if (conf->sec_elmnt_seq != nsec) {
+			for (ibvd = 1; ibvd < conf->sec_elmnt_count; ibvd++) {
+				if (v->other_bvds[ibvd-1] == NULL)
+					continue;
+				if (v->other_bvds[ibvd-1]->sec_elmnt_seq
+				    == nsec)
+					break;
+			}
+			if (ibvd == conf->sec_elmnt_count)
+				goto bad;
+			conf = v->other_bvds[ibvd-1];
+		}
+		if (!find_index_in_bvd(ddf, conf,
+				       n - nsec*conf->sec_elmnt_count, n_bvd))
+			goto bad;
+		dprintf("%s: found disk %u as member %u in bvd %d of array %u\n"
+			, __func__, n, *n_bvd, ibvd-1, inst);
+		*vcl = v;
+		return conf;
+	}
+bad:
+	pr_err("%s: Could't find disk %d in array %u\n", __func__, n, inst);
 	return NULL;
 }
 #endif
@@ -3769,9 +3832,11 @@ static int ddf_set_array_state(struct active_array *a, int consistent)
 static void ddf_set_disk(struct active_array *a, int n, int state)
 {
 	struct ddf_super *ddf = a->container->sb;
-	unsigned int inst = a->info.container_member;
-	struct vd_config *vc = find_vdcr(ddf, inst);
-	int pd = find_phys(ddf, vc->phys_refnum[n]);
+	unsigned int inst = a->info.container_member, n_bvd;
+	struct vcl *vcl;
+	struct vd_config *vc = find_vdcr(ddf, inst, (unsigned int)n,
+					 &n_bvd, &vcl);
+	int pd;
 	int i, st, working;
 	struct mdinfo *mdi;
 	struct dl *dl;
@@ -3796,15 +3861,21 @@ static void ddf_set_disk(struct active_array *a, int n, int state)
 	if (!dl)
 		return;
 
+	pd = find_phys(ddf, vc->phys_refnum[n_bvd]);
 	if (pd < 0 || pd != dl->pdnum) {
 		/* disk doesn't currently exist or has changed.
 		 * If it is now in_sync, insert it. */
+		dprintf("%s: phys disk not found for %d: %d/%d ref %08x\n",
+			__func__, dl->pdnum, dl->major, dl->minor,
+			dl->disk.refnum);
+		dprintf("%s: array %u disk %u ref %08x pd %d\n",
+			__func__, inst, n_bvd, vc->phys_refnum[n_bvd], pd);
 		if ((state & DS_INSYNC) && ! (state & DS_FAULTY)) {
-			struct vcl *vcl;
-			pd = dl->pdnum;
-			vc->phys_refnum[n] = dl->disk.refnum;
-			vcl = container_of(vc, struct vcl, conf);
-			vcl->lba_offset[n] = mdi->data_offset;
+			__u64 *lba_offset;
+			pd = dl->pdnum; /* FIXME: is this really correct ? */
+			vc->phys_refnum[n_bvd] = dl->disk.refnum;
+			lba_offset = (__u64 *)&vc->phys_refnum[ddf->mppe];
+			lba_offset[n_bvd] = mdi->data_offset;
 			ddf->phys->entries[pd].type &=
 				~__cpu_to_be16(DDF_Global_Spare);
 			ddf->phys->entries[pd].type |=
@@ -4187,8 +4258,10 @@ static struct mdinfo *ddf_activate_spare(struct active_array *a,
 	struct metadata_update *mu;
 	struct dl *dl;
 	int i;
+	struct vcl *vcl;
 	struct vd_config *vc;
 	__u64 *lba;
+	unsigned int n_bvd;
 
 	for (d = a->info.devs ; d ; d = d->next) {
 		if ((d->curr_state & DS_FAULTY) &&
@@ -4353,7 +4426,8 @@ static struct mdinfo *ddf_activate_spare(struct active_array *a,
 	mu->space = NULL;
 	mu->space_list = NULL;
 	mu->next = *updates;
-	vc = find_vdcr(ddf, a->info.container_member);
+	vc = find_vdcr(ddf, a->info.container_member, di->disk.raid_disk,
+		       &n_bvd, &vcl);
 	memcpy(mu->buf, vc, ddf->conf_rec_len * 512);
 
 	vc = (struct vd_config*)mu->buf;
-- 
1.7.1

[PATCH 08/27] DDF: ddf_set_disk: move status logic to separate function

[mwilck]

Moved code to determine RAID status to a separate function
get_bvd_status(). I need this to account for secondary RAID level.

Signed-off-by: Martin Wilck <mwilck@arcor.de>
---
 super-ddf.c |   94 +++++++++++++++++++++++++++++++++-------------------------
 1 files changed, 53 insertions(+), 41 deletions(-)

diff --git a/super-ddf.c b/super-ddf.c
index c448bff..322ad5e 100644
--- a/super-ddf.c
+++ b/super-ddf.c
@@ -1515,7 +1515,7 @@ bad:
 }
 #endif
 
-static int find_phys(struct ddf_super *ddf, __u32 phys_refnum)
+static int find_phys(const struct ddf_super *ddf, __u32 phys_refnum)
 {
 	/* Find the entry in phys_disk which has the given refnum
 	 * and return it's index
@@ -3812,9 +3812,55 @@ static int ddf_set_array_state(struct active_array *a, int consistent)
 	return consistent;
 }
 
-#define container_of(ptr, type, member) ({                      \
-	const typeof( ((type *)0)->member ) *__mptr = (ptr); \
-	(type *)( (char *)__mptr - offsetof(type,member) );})
+static int get_bvd_state(const struct ddf_super *ddf,
+			 const struct vd_config *vc)
+{
+	unsigned int i, n_bvd, working = 0;
+	unsigned int n_prim = __be16_to_cpu(vc->prim_elmnt_count);
+	int pd, st, state;
+	for (i = 0; i < n_prim; i++) {
+		if (!find_index_in_bvd(ddf, vc, i, &n_bvd))
+			continue;
+		pd = find_phys(ddf, vc->phys_refnum[n_bvd]);
+		if (pd < 0)
+			continue;
+		st = __be16_to_cpu(ddf->phys->entries[pd].state);
+		if ((st & (DDF_Online|DDF_Failed|DDF_Rebuilding))
+		    == DDF_Online)
+			working++;
+	}
+
+	state = DDF_state_degraded;
+	if (working == n_prim)
+		state = DDF_state_optimal;
+	else
+		switch (vc->prl) {
+		case DDF_RAID0:
+		case DDF_CONCAT:
+		case DDF_JBOD:
+			state = DDF_state_failed;
+			break;
+		case DDF_RAID1:
+			if (working == 0)
+				state = DDF_state_failed;
+			else if (working >= 2)
+				state = DDF_state_part_optimal;
+			break;
+		case DDF_RAID4:
+		case DDF_RAID5:
+			if (working < n_prim - 1)
+				state = DDF_state_failed;
+			break;
+		case DDF_RAID6:
+			if (working < n_prim - 2)
+				state = DDF_state_failed;
+			else if (working == n_prim - 1)
+				state = DDF_state_part_optimal;
+			break;
+		}
+	return state;
+}
+
 /*
  * The state of each disk is stored in the global phys_disk structure
  * in phys_disk.entries[n].state.
@@ -3837,7 +3883,6 @@ static void ddf_set_disk(struct active_array *a, int n, int state)
 	struct vd_config *vc = find_vdcr(ddf, inst, (unsigned int)n,
 					 &n_bvd, &vcl);
 	int pd;
-	int i, st, working;
 	struct mdinfo *mdi;
 	struct dl *dl;
 
@@ -3901,43 +3946,10 @@ static void ddf_set_disk(struct active_array *a, int n, int state)
 	 * It needs to be one of "optimal", "degraded", "failed".
 	 * I don't understand 'deleted' or 'missing'.
 	 */
-	working = 0;
-	for (i=0; i < a->info.array.raid_disks; i++) {
-		pd = find_phys(ddf, vc->phys_refnum[i]);
-		if (pd < 0)
-			continue;
-		st = __be16_to_cpu(ddf->phys->entries[pd].state);
-		if ((st & (DDF_Online|DDF_Failed|DDF_Rebuilding))
-		    == DDF_Online)
-			working++;
+	state = get_bvd_state(ddf, vc);
+	if (vc->sec_elmnt_count > 1) {
+		/* treat secondary level */
 	}
-	state = DDF_state_degraded;
-	if (working == a->info.array.raid_disks)
-		state = DDF_state_optimal;
-	else switch(vc->prl) {
-		case DDF_RAID0:
-		case DDF_CONCAT:
-		case DDF_JBOD:
-			state = DDF_state_failed;
-			break;
-		case DDF_RAID1:
-			if (working == 0)
-				state = DDF_state_failed;
-			else if (working == 2 && state == DDF_state_degraded)
-				state = DDF_state_part_optimal;
-			break;
-		case DDF_RAID4:
-		case DDF_RAID5:
-			if (working < a->info.array.raid_disks-1)
-				state = DDF_state_failed;
-			break;
-		case DDF_RAID6:
-			if (working < a->info.array.raid_disks-2)
-				state = DDF_state_failed;
-			else if (working == a->info.array.raid_disks-1)
-				state = DDF_state_part_optimal;
-			break;
-		}
 
 	if (ddf->virt->entries[inst].state !=
 	    ((ddf->virt->entries[inst].state & ~DDF_state_mask)
-- 
1.7.1

[PATCH 09/27] DDF: get_svd_state: Status logic for secondary RAID level

[mwilck]

Implement logic to derive the status of a secondary RAID
from its members. Use it in ddf_set_disk.

Signed-off-by: Martin Wilck <mwilck@arcor.de>
---
 super-ddf.c |   37 +++++++++++++++++++++++++++++++++----
 1 files changed, 33 insertions(+), 4 deletions(-)

diff --git a/super-ddf.c b/super-ddf.c
index 322ad5e..8b7a621 100644
--- a/super-ddf.c
+++ b/super-ddf.c
@@ -3861,6 +3861,38 @@ static int get_bvd_state(const struct ddf_super *ddf,
 	return state;
 }
 
+static int secondary_state(int state, int other, int seclevel)
+{
+	if (state == DDF_state_optimal && other == DDF_state_optimal)
+		return DDF_state_optimal;
+	if (seclevel == DDF_2MIRRORED) {
+		if (state == DDF_state_optimal || other == DDF_state_optimal)
+			return DDF_state_part_optimal;
+		if (state == DDF_state_failed && other == DDF_state_failed)
+			return DDF_state_failed;
+		return DDF_state_degraded;
+	} else {
+		if (state == DDF_state_failed || other == DDF_state_failed)
+			return DDF_state_failed;
+		if (state == DDF_state_degraded || other == DDF_state_degraded)
+			return DDF_state_degraded;
+		return DDF_state_part_optimal;
+	}
+}
+
+static int get_svd_state(const struct ddf_super *ddf, const struct vcl *vcl)
+{
+	int state = get_bvd_state(ddf, &vcl->conf);
+	unsigned int i;
+	for (i = 1; i < vcl->conf.sec_elmnt_count; i++) {
+		state = secondary_state(
+			state,
+			get_bvd_state(ddf, vcl->other_bvds[i-1]),
+			vcl->conf.srl);
+	}
+	return state;
+}
+
 /*
  * The state of each disk is stored in the global phys_disk structure
  * in phys_disk.entries[n].state.
@@ -3946,10 +3978,7 @@ static void ddf_set_disk(struct active_array *a, int n, int state)
 	 * It needs to be one of "optimal", "degraded", "failed".
 	 * I don't understand 'deleted' or 'missing'.
 	 */
-	state = get_bvd_state(ddf, vc);
-	if (vc->sec_elmnt_count > 1) {
-		/* treat secondary level */
-	}
+	state = get_svd_state(ddf, vcl);
 
 	if (ddf->virt->entries[inst].state !=
 	    ((ddf->virt->entries[inst].state & ~DDF_state_mask)
-- 
1.7.1

[PATCH 10/27] DDF: allow empty slots in virt disk table

[mwilck]

The DDF code was assuming that the VD slots 0..populated_vdes
were used and the rest was unused. Remove this assumption and
deal with empty slots instead.

Signed-off-by: Martin Wilck <mwilck@arcor.de>
---
 super-ddf.c |  100 +++++++++++++++++++++++++++++++++-------------------------
 1 files changed, 57 insertions(+), 43 deletions(-)

diff --git a/super-ddf.c b/super-ddf.c
index 8b7a621..0f955be 100644
--- a/super-ddf.c
+++ b/super-ddf.c
@@ -447,7 +447,7 @@ struct ddf_super {
 #endif
 
 #if DEBUG
-static int all_ff(char *guid);
+static int all_ff(const char *guid);
 static void pr_state(struct ddf_super *ddf, const char *msg)
 {
 	unsigned int i;
@@ -1059,7 +1059,7 @@ static int map_num1(struct num_mapping *map, int num)
 	return map[i].num2;
 }
 
-static int all_ff(char *guid)
+static int all_ff(const char *guid)
 {
 	int i;
 	for (i = 0; i < DDF_GUID_LEN; i++)
@@ -1156,11 +1156,13 @@ static void examine_vd(int n, struct ddf_super *sb, char *guid)
 static void examine_vds(struct ddf_super *sb)
 {
 	int cnt = __be16_to_cpu(sb->virt->populated_vdes);
-	int i;
+	unsigned int i;
 	printf("  Virtual Disks : %d\n", cnt);
 
-	for (i=0; i<cnt; i++) {
+	for (i = 0; i < __be16_to_cpu(sb->virt->max_vdes); i++) {
 		struct virtual_entry *ve = &sb->virt->entries[i];
+		if (all_ff(ve->guid))
+			continue;
 		printf("\n");
 		printf("      VD GUID[%d] : ", i); print_guid(ve->guid, 1);
 		printf("\n");
@@ -1816,6 +1818,44 @@ static void make_header_guid(char *guid)
 	memcpy(guid+20, &stamp, 4);
 }
 
+static unsigned int find_unused_vde(const struct ddf_super *ddf)
+{
+	unsigned int i;
+	for (i = 0; i < __be16_to_cpu(ddf->virt->max_vdes); i++) {
+		if (all_ff(ddf->virt->entries[i].guid))
+			return i;
+	}
+	return DDF_NOTFOUND;
+}
+
+static unsigned int find_vde_by_name(const struct ddf_super *ddf,
+				     const char *name)
+{
+	unsigned int i;
+	if (name == NULL)
+		return DDF_NOTFOUND;
+	for (i = 0; i < __be16_to_cpu(ddf->virt->max_vdes); i++) {
+		if (all_ff(ddf->virt->entries[i].guid))
+			continue;
+		if (!strncmp(name, ddf->virt->entries[i].name,
+			     sizeof(ddf->virt->entries[i].name)))
+			return i;
+	}
+	return DDF_NOTFOUND;
+}
+
+static unsigned int find_vde_by_guid(const struct ddf_super *ddf,
+				     const char *guid)
+{
+	unsigned int i;
+	if (guid == NULL || all_ff(guid))
+		return DDF_NOTFOUND;
+	for (i = 0; i < __be16_to_cpu(ddf->virt->max_vdes); i++)
+		if (!memcmp(ddf->virt->entries[i].guid, guid, DDF_GUID_LEN))
+			return i;
+	return DDF_NOTFOUND;
+}
+
 static int init_super_ddf_bvd(struct supertype *st,
 			      mdu_array_info_t *info,
 			      unsigned long long size,
@@ -2209,33 +2249,13 @@ static int init_super_ddf_bvd(struct supertype *st,
 	struct vcl *vcl;
 	struct vd_config *vc;
 
-	if (__be16_to_cpu(ddf->virt->populated_vdes)
-	    >= __be16_to_cpu(ddf->virt->max_vdes)) {
-		pr_err("This ddf already has the "
-		       "maximum of %d virtual devices\n",
-		       __be16_to_cpu(ddf->virt->max_vdes));
+	if (find_vde_by_name(ddf, name) != DDF_NOTFOUND) {
+		pr_err("This ddf already has an array called %s\n", name);
 		return 0;
 	}
-
-	if (name)
-		for (venum = 0; venum < __be16_to_cpu(ddf->virt->max_vdes); venum++)
-			if (!all_ff(ddf->virt->entries[venum].guid)) {
-				char *n = ddf->virt->entries[venum].name;
-
-				if (strncmp(name, n, 16) == 0) {
-					pr_err("This ddf already"
-					       " has an array called %s\n",
-					       name);
-					return 0;
-				}
-			}
-
-	for (venum = 0; venum < __be16_to_cpu(ddf->virt->max_vdes); venum++)
-		if (all_ff(ddf->virt->entries[venum].guid))
-			break;
-	if (venum == __be16_to_cpu(ddf->virt->max_vdes)) {
-		pr_err("Cannot find spare slot for "
-		       "virtual disk - DDF is corrupt\n");
+	venum = find_unused_vde(ddf);
+	if (venum == DDF_NOTFOUND) {
+		pr_err("Cannot find spare slot for virtual disk\n");
 		return 0;
 	}
 	ve = &ddf->virt->entries[venum];
@@ -3760,8 +3780,8 @@ static int ddf_open_new(struct supertype *c, struct active_array *a, char *inst)
 {
 	struct ddf_super *ddf = c->sb;
 	int n = atoi(inst);
-	if (n >= (int)__be16_to_cpu(ddf->virt->populated_vdes)) {
-		pr_err("%s: subarray index %d out of range\n", __func__, n);
+	if (all_ff(ddf->virt->entries[n].guid)) {
+		pr_err("%s: subarray %d doesn't exist\n", __func__, n);
 		return -ENODEV;
 	}
 	dprintf("ddf: open_new %d\n", n);
@@ -4112,10 +4132,8 @@ static void ddf_process_update(struct supertype *st,
 			return;
 		vd = (struct virtual_disk*)update->buf;
 
-		ent = __be16_to_cpu(vd->populated_vdes);
-		if (ent >= __be16_to_cpu(ddf->virt->max_vdes))
-			return;
-		if (!all_ff(ddf->virt->entries[ent].guid))
+		ent = find_unused_vde(ddf);
+		if (ent == DDF_NOTFOUND)
 			return;
 		ddf->virt->entries[ent] = vd->entries[0];
 		ddf->virt->populated_vdes = __cpu_to_be16(1 +
@@ -4150,14 +4168,10 @@ static void ddf_process_update(struct supertype *st,
 			memcpy(&vcl->conf, vc, update->len);
 			vcl->lba_offset = (__u64*)
 				&vcl->conf.phys_refnum[mppe];
-			for (ent = 0;
-			     ent < __be16_to_cpu(ddf->virt->populated_vdes);
-			     ent++)
-				if (memcmp(vc->guid, ddf->virt->entries[ent].guid,
-					   DDF_GUID_LEN) == 0) {
-					vcl->vcnum = ent;
-					break;
-				}
+			ent = find_vde_by_guid(ddf, vc->guid);
+			if (ent == DDF_NOTFOUND)
+				return;
+			vcl->vcnum = ent;
 			ddf->conflist = vcl;
 		}
 		/* Set DDF_Transition on all Failed devices - to help
-- 
1.7.1

[PATCH 11/27] DDF: layout_ddf2md: new DDF->md RAID layout conversion

[mwilck]

layout_ddf2md() is a new RAID layout conversion routine.
It obsoletes the previous separate routines for obtaining
md level and layout (map_num1, rlq_to_layout).

Signed-off-by: Martin Wilck <mwilck@arcor.de>
---
 super-ddf.c |  197 ++++++++++++++++++++++++++++++-----------------------------
 1 files changed, 100 insertions(+), 97 deletions(-)

diff --git a/super-ddf.c b/super-ddf.c
index 0f955be..a43401e 100644
--- a/super-ddf.c
+++ b/super-ddf.c
@@ -484,6 +484,102 @@ static unsigned int calc_crc(void *buf, int len)
 	return __cpu_to_be32(newcrc);
 }
 
+static int err_bad_ddf_layout(const struct vd_config *conf)
+{
+	pr_err("DDF RAID %u qualifier %u with %u disks is unsupported\n",
+	       conf->prl, conf->rlq, __be16_to_cpu(conf->prim_elmnt_count));
+	return -1;
+}
+
+static int layout_ddf2md(const struct vd_config *conf,
+			 mdu_array_info_t *array)
+{
+	int level = LEVEL_UNSUPPORTED;
+	int layout = 0;
+	int raiddisks = __be16_to_cpu(conf->prim_elmnt_count);
+
+	if (conf->sec_elmnt_count > 1) {
+		/* see also check_secondary() */
+		if (conf->prl != DDF_RAID1 ||
+		    (conf->srl != DDF_2STRIPED && conf->srl != DDF_2SPANNED)) {
+			pr_err("Unsupported secondary RAID level %u/%u\n",
+			       conf->prl, conf->srl);
+			return -1;
+		}
+		if (raiddisks == 2 && conf->rlq == DDF_RAID1_SIMPLE)
+			layout = 0x102;
+		else if  (raiddisks == 3 && conf->rlq == DDF_RAID1_MULTI)
+			layout = 0x103;
+		else
+			return err_bad_ddf_layout(conf);
+		raiddisks *= conf->sec_elmnt_count;
+		level = 10;
+		goto good;
+	}
+
+	switch (conf->prl) {
+	case DDF_CONCAT:
+		level = LEVEL_LINEAR;
+		break;
+	case DDF_RAID0:
+		if (conf->rlq != DDF_RAID0_SIMPLE)
+			return err_bad_ddf_layout(conf);
+		level = 0;
+		break;
+	case DDF_RAID1:
+		if (!((conf->rlq == DDF_RAID1_SIMPLE && raiddisks == 2) ||
+		      (conf->rlq == DDF_RAID1_MULTI && raiddisks == 3)))
+			return err_bad_ddf_layout(conf);
+		level = 1;
+		break;
+	case DDF_RAID4:
+		if (conf->rlq != DDF_RAID4_N)
+			return err_bad_ddf_layout(conf);
+		level = 4;
+		break;
+	case DDF_RAID5:
+		switch (conf->rlq) {
+		case DDF_RAID5_N_RESTART:
+			layout = ALGORITHM_LEFT_ASYMMETRIC;
+			break;
+		case DDF_RAID5_0_RESTART:
+			layout = ALGORITHM_RIGHT_ASYMMETRIC;
+			break;
+		case DDF_RAID5_N_CONTINUE:
+			layout = ALGORITHM_LEFT_SYMMETRIC;
+			break;
+		default:
+			return err_bad_ddf_layout(conf);
+		}
+		level = 5;
+		break;
+	case DDF_RAID6:
+		switch (conf->rlq) {
+		case DDF_RAID5_N_RESTART:
+			layout = ALGORITHM_ROTATING_N_RESTART;
+			break;
+		case DDF_RAID6_0_RESTART:
+			layout = ALGORITHM_ROTATING_ZERO_RESTART;
+			break;
+		case DDF_RAID5_N_CONTINUE:
+			layout = ALGORITHM_ROTATING_N_CONTINUE;
+			break;
+		default:
+			return err_bad_ddf_layout(conf);
+		}
+		level = 6;
+		break;
+	default:
+		return err_bad_ddf_layout(conf);
+	};
+
+good:
+	array->level = level;
+	array->layout = layout;
+	array->raid_disks = raiddisks;
+	return 0;
+}
+
 static int load_ddf_header(int fd, unsigned long long lba,
 			   unsigned long long size,
 			   int type,
@@ -1032,33 +1128,6 @@ static mapping_t ddf_sec_level[] = {
 };
 #endif
 
-struct num_mapping {
-	int num1, num2;
-};
-static struct num_mapping ddf_level_num[] = {
-	{ DDF_RAID0, 0 },
-	{ DDF_RAID1, 1 },
-	{ DDF_RAID3, LEVEL_UNSUPPORTED },
-	{ DDF_RAID4, 4 },
-	{ DDF_RAID5, 5 },
-	{ DDF_RAID1E, LEVEL_UNSUPPORTED },
-	{ DDF_JBOD, LEVEL_UNSUPPORTED },
-	{ DDF_CONCAT, LEVEL_LINEAR },
-	{ DDF_RAID5E, LEVEL_UNSUPPORTED },
-	{ DDF_RAID5EE, LEVEL_UNSUPPORTED },
-	{ DDF_RAID6, 6},
-	{ MAXINT, MAXINT }
-};
-
-static int map_num1(struct num_mapping *map, int num)
-{
-	int i;
-	for (i=0 ; map[i].num1 != MAXINT; i++)
-		if (map[i].num1 == num)
-			break;
-	return map[i].num2;
-}
-
 static int all_ff(const char *guid)
 {
 	int i;
@@ -1638,8 +1707,6 @@ static void getinfo_super_ddf(struct supertype *st, struct mdinfo *info, char *m
 	}
 }
 
-static int rlq_to_layout(int rlq, int prl, int raiddisks);
-
 static void getinfo_super_ddf_bvd(struct supertype *st, struct mdinfo *info, char *map)
 {
 	struct ddf_super *ddf = st->sb;
@@ -1651,12 +1718,8 @@ static void getinfo_super_ddf_bvd(struct supertype *st, struct mdinfo *info, cha
 	__u32 *cptr;
 
 	memset(info, 0, sizeof(*info));
-	/* FIXME this returns BVD info - what if we want SVD ?? */
-
-	info->array.raid_disks    = __be16_to_cpu(vc->conf.prim_elmnt_count);
-	info->array.level	  = map_num1(ddf_level_num, vc->conf.prl);
-	info->array.layout	  = rlq_to_layout(vc->conf.rlq, vc->conf.prl,
-						  info->array.raid_disks);
+	if (layout_ddf2md(&vc->conf, &info->array) == -1)
+		return;
 	info->array.md_minor	  = -1;
 	cptr = (__u32 *)(vc->conf.guid + 16);
 	info->array.ctime	  = DECADE + __be32_to_cpu(*cptr);
@@ -2140,48 +2203,6 @@ static int layout_to_rlq(int level, int layout, int raiddisks)
 	return -1;
 }
 
-static int rlq_to_layout(int rlq, int prl, int raiddisks)
-{
-	switch(prl) {
-	case DDF_RAID0:
-		return 0; /* hopefully rlq == DDF_RAID0_SIMPLE */
-	case DDF_RAID1:
-		return 0; /* hopefully rlq == SIMPLE or MULTI depending
-			     on raiddisks*/
-	case DDF_RAID4:
-		switch(rlq) {
-		case DDF_RAID4_N:
-			return 0;
-		default:
-			/* not supported */
-			return -1; /* FIXME this isn't checked */
-		}
-	case DDF_RAID5:
-		switch(rlq) {
-		case DDF_RAID5_N_RESTART:
-			return ALGORITHM_LEFT_ASYMMETRIC;
-		case DDF_RAID5_0_RESTART:
-			return ALGORITHM_RIGHT_ASYMMETRIC;
-		case DDF_RAID5_N_CONTINUE:
-			return ALGORITHM_LEFT_SYMMETRIC;
-		default:
-			return -1;
-		}
-	case DDF_RAID6:
-		switch(rlq) {
-		case DDF_RAID5_N_RESTART:
-			return ALGORITHM_ROTATING_N_RESTART;
-		case DDF_RAID6_0_RESTART:
-			return ALGORITHM_ROTATING_ZERO_RESTART;
-		case DDF_RAID5_N_CONTINUE:
-			return ALGORITHM_ROTATING_N_CONTINUE;
-		default:
-			return -1;
-		}
-	}
-	return -1;
-}
-
 #ifndef MDASSEMBLE
 struct extent {
 	unsigned long long start, size;
@@ -3433,26 +3454,8 @@ static struct mdinfo *container_content_ddf(struct supertype *st, char *subarray
 		this->next = rest;
 		rest = this;
 
-		if (vc->conf.sec_elmnt_count == 1) {
-			this->array.level = map_num1(ddf_level_num,
-						     vc->conf.prl);
-			this->array.raid_disks =
-				__be16_to_cpu(vc->conf.prim_elmnt_count);
-			this->array.layout =
-				rlq_to_layout(vc->conf.rlq, vc->conf.prl,
-					      this->array.raid_disks);
-		} else {
-			/* The only supported layout is RAID 10.
-			 * Compatibility has been checked in check_secondary()
-			 * above.
-			 */
-			this->array.level = 10;
-			this->array.raid_disks =
-				__be16_to_cpu(vc->conf.prim_elmnt_count)
-				* vc->conf.sec_elmnt_count;
-			this->array.layout = 0x100 |
-				__be16_to_cpu(vc->conf.prim_elmnt_count);
-		}
+		if (layout_ddf2md(&vc->conf, &this->array))
+			continue;
 		this->array.md_minor      = -1;
 		this->array.major_version = -1;
 		this->array.minor_version = -2;
-- 
1.7.1

[PATCH 12/27] DDF: layout_md2ddf: new md->DDF layout conversion

[mwilck]

Support for RAID 10 makes it necessary to rewrite the algorithm
for deriving DDF layout from MD layout. The functions level_to_prl
and layout_to_rlq are combined in a single function that takes
md layout parameters and converts them to DDF.

Signed-off-by: Martin Wilck <mwilck@arcor.de>
---
 super-ddf.c |  178 ++++++++++++++++++++++++++++++++++++++---------------------
 1 files changed, 114 insertions(+), 64 deletions(-)

diff --git a/super-ddf.c b/super-ddf.c
index a43401e..933485d 100644
--- a/super-ddf.c
+++ b/super-ddf.c
@@ -484,6 +484,107 @@ static unsigned int calc_crc(void *buf, int len)
 	return __cpu_to_be32(newcrc);
 }
 
+#define DDF_INVALID_LEVEL 0xff
+#define DDF_NO_SECONDARY 0xff
+static int err_bad_md_layout(const mdu_array_info_t *array)
+{
+	pr_err("RAID%d layout %x with %d disks is unsupported for DDF\n",
+	       array->level, array->layout, array->raid_disks);
+	return DDF_INVALID_LEVEL;
+}
+
+static int layout_md2ddf(const mdu_array_info_t *array,
+			 struct vd_config *conf)
+{
+	__u16 prim_elmnt_count = __cpu_to_be16(array->raid_disks);
+	__u8 prl = DDF_INVALID_LEVEL, rlq = 0;
+	__u8 sec_elmnt_count = 1;
+	__u8 srl = DDF_NO_SECONDARY;
+
+	switch (array->level) {
+	case LEVEL_LINEAR:
+		prl = DDF_CONCAT;
+		break;
+	case 0:
+		rlq = DDF_RAID0_SIMPLE;
+		prl = DDF_RAID0;
+		break;
+	case 1:
+		switch (array->raid_disks) {
+		case 2:
+			rlq = DDF_RAID1_SIMPLE;
+			break;
+		case 3:
+			rlq = DDF_RAID1_MULTI;
+			break;
+		default:
+			return err_bad_md_layout(array);
+		}
+		prl = DDF_RAID1;
+		break;
+	case 4:
+		if (array->layout != 0)
+			return err_bad_md_layout(array);
+		rlq = DDF_RAID4_N;
+		prl = DDF_RAID4;
+		break;
+	case 5:
+		switch (array->layout) {
+		case ALGORITHM_LEFT_ASYMMETRIC:
+			rlq = DDF_RAID5_N_RESTART;
+			break;
+		case ALGORITHM_RIGHT_ASYMMETRIC:
+			rlq = DDF_RAID5_0_RESTART;
+			break;
+		case ALGORITHM_LEFT_SYMMETRIC:
+			rlq = DDF_RAID5_N_CONTINUE;
+			break;
+		case ALGORITHM_RIGHT_SYMMETRIC:
+			/* not mentioned in standard */
+			return err_bad_md_layout(array);
+		}
+		prl = DDF_RAID5;
+		break;
+	case 6:
+		switch (array->layout) {
+		case ALGORITHM_ROTATING_N_RESTART:
+			rlq = DDF_RAID5_N_RESTART;
+			break;
+		case ALGORITHM_ROTATING_ZERO_RESTART:
+			rlq = DDF_RAID6_0_RESTART;
+			break;
+		case ALGORITHM_ROTATING_N_CONTINUE:
+			rlq = DDF_RAID5_N_CONTINUE;
+			break;
+		}
+		prl = DDF_RAID6;
+		break;
+	case 10:
+		if (array->raid_disks % 2 == 0 && array->layout == 0x102) {
+			rlq = DDF_RAID1_SIMPLE;
+			prim_elmnt_count =  __cpu_to_be16(2);
+			sec_elmnt_count = array->raid_disks / 2;
+		} else if (array->raid_disks % 3 == 0
+			   && array->layout == 0x103) {
+			rlq = DDF_RAID1_MULTI;
+			prim_elmnt_count =  __cpu_to_be16(3);
+			sec_elmnt_count = array->raid_disks / 3;
+		} else
+			return err_bad_md_layout(array);
+		srl = DDF_2SPANNED;
+		prl = DDF_RAID1;
+		break;
+	default:
+		return err_bad_md_layout(array);
+	}
+	conf->prl = prl;
+	conf->prim_elmnt_count = prim_elmnt_count;
+	conf->rlq = rlq;
+	conf->srl = srl;
+	conf->sec_elmnt_count = sec_elmnt_count;
+	return 0;
+}
+
 static int err_bad_ddf_layout(const struct vd_config *conf)
 {
 	pr_err("DDF RAID %u qualifier %u with %u disks is unsupported\n",
@@ -2150,59 +2251,6 @@ static int chunk_to_shift(int chunksize)
 	return ffs(chunksize/512)-1;
 }
 
-static int level_to_prl(int level)
-{
-	switch (level) {
-	case LEVEL_LINEAR: return DDF_CONCAT;
-	case 0: return DDF_RAID0;
-	case 1: return DDF_RAID1;
-	case 4: return DDF_RAID4;
-	case 5: return DDF_RAID5;
-	case 6: return DDF_RAID6;
-	default: return -1;
-	}
-}
-
-static int layout_to_rlq(int level, int layout, int raiddisks)
-{
-	switch(level) {
-	case 0:
-		return DDF_RAID0_SIMPLE;
-	case 1:
-		switch(raiddisks) {
-		case 2: return DDF_RAID1_SIMPLE;
-		case 3: return DDF_RAID1_MULTI;
-		default: return -1;
-		}
-	case 4:
-		switch(layout) {
-		case 0: return DDF_RAID4_N;
-		}
-		break;
-	case 5:
-		switch(layout) {
-		case ALGORITHM_LEFT_ASYMMETRIC:
-			return DDF_RAID5_N_RESTART;
-		case ALGORITHM_RIGHT_ASYMMETRIC:
-			return DDF_RAID5_0_RESTART;
-		case ALGORITHM_LEFT_SYMMETRIC:
-			return DDF_RAID5_N_CONTINUE;
-		case ALGORITHM_RIGHT_SYMMETRIC:
-			return -1; /* not mentioned in standard */
-		}
-	case 6:
-		switch(layout) {
-		case ALGORITHM_ROTATING_N_RESTART:
-			return DDF_RAID5_N_RESTART;
-		case ALGORITHM_ROTATING_ZERO_RESTART:
-			return DDF_RAID6_0_RESTART;
-		case ALGORITHM_ROTATING_N_CONTINUE:
-			return DDF_RAID5_N_CONTINUE;
-		}
-	}
-	return -1;
-}
-
 #ifndef MDASSEMBLE
 struct extent {
 	unsigned long long start, size;
@@ -2320,13 +2368,15 @@ static int init_super_ddf_bvd(struct supertype *st,
 	vc->timestamp = __cpu_to_be32(time(0)-DECADE);
 	vc->seqnum = __cpu_to_be32(1);
 	memset(vc->pad0, 0xff, 24);
-	vc->prim_elmnt_count = __cpu_to_be16(info->raid_disks);
 	vc->chunk_shift = chunk_to_shift(info->chunk_size);
-	vc->prl = level_to_prl(info->level);
-	vc->rlq = layout_to_rlq(info->level, info->layout, info->raid_disks);
-	vc->sec_elmnt_count = 1;
+	if (layout_md2ddf(info, vc) == -1 ||
+		__be16_to_cpu(vc->prim_elmnt_count) > ddf->mppe) {
+		pr_err("%s: unsupported RAID level/layout %d/%d with %d disks\n",
+		       __func__, info->level, info->layout, info->raid_disks);
+		free(vcl);
+		return 0;
+	}
 	vc->sec_elmnt_seq = 0;
-	vc->srl = 0;
 	vc->blocks = __cpu_to_be64(info->size * 2);
 	vc->array_blocks = __cpu_to_be64(
 		calc_array_size(info->level, info->raid_disks, info->layout,
@@ -2990,12 +3040,12 @@ static int validate_geometry_ddf(struct supertype *st,
 	}
 
 	if (!dev) {
-		/* Initial sanity check.  Exclude illegal levels. */
-		int i;
-		for (i=0; ddf_level_num[i].num1 != MAXINT; i++)
-			if (ddf_level_num[i].num2 == level)
-				break;
-		if (ddf_level_num[i].num1 == MAXINT) {
+		mdu_array_info_t array = {
+			.level = level, .layout = layout,
+			.raid_disks = raiddisks
+		};
+		struct vd_config conf;
+		if (layout_md2ddf(&array, &conf) == -1) {
 			if (verbose)
 				pr_err("DDF does not support level %d arrays\n",
 				       level);
-- 
1.7.1

[PATCH 13/27] DDF: Simplify allocation of "other BVDs"

[mwilck]

Instead of allocating the other_bvds array and every element
separately, allocate all in a single chunk. Also, move allocation
in a subroutine as it's used in several places.

Signed-off-by: Martin Wilck <mwilck@arcor.de>
---
 super-ddf.c |   86 ++++++++++++++++++++++++++++++++++++----------------------
 1 files changed, 53 insertions(+), 33 deletions(-)

diff --git a/super-ddf.c b/super-ddf.c
index 933485d..1ff8a28 100644
--- a/super-ddf.c
+++ b/super-ddf.c
@@ -863,13 +863,35 @@ static int load_ddf_global(int fd, struct ddf_super *super, char *devname)
 	return 0;
 }
 
+#define DDF_UNUSED_BVD 0xff
+static int alloc_other_bvds(const struct ddf_super *ddf, struct vcl *vcl)
+{
+	unsigned int n_vds = vcl->conf.sec_elmnt_count - 1;
+	unsigned int i, vdsize;
+	void *p;
+	if (n_vds == 0) {
+		vcl->other_bvds = NULL;
+		return 0;
+	}
+	vdsize = ddf->conf_rec_len * 512;
+	if (posix_memalign(&p, 512, n_vds *
+			   (vdsize +  sizeof(struct vd_config *))) != 0)
+		return -1;
+	vcl->other_bvds = (struct vd_config **) (p + n_vds * vdsize);
+	for (i = 0; i < n_vds; i++) {
+		vcl->other_bvds[i] = p + i * vdsize;
+		memset(vcl->other_bvds[i], 0, vdsize);
+		vcl->other_bvds[i]->sec_elmnt_seq = DDF_UNUSED_BVD;
+	}
+	return 0;
+}
+
 static void add_other_bvd(struct vcl *vcl, struct vd_config *vd,
 			  unsigned int len)
 {
 	int i;
 	for (i = 0; i < vcl->conf.sec_elmnt_count-1; i++)
-		if (vcl->other_bvds[i] != NULL &&
-		    vcl->other_bvds[i]->sec_elmnt_seq == vd->sec_elmnt_seq)
+		if (vcl->other_bvds[i]->sec_elmnt_seq == vd->sec_elmnt_seq)
 			break;
 
 	if (i < vcl->conf.sec_elmnt_count-1) {
@@ -877,18 +899,13 @@ static void add_other_bvd(struct vcl *vcl, struct vd_config *vd,
 			return;
 	} else {
 		for (i = 0; i < vcl->conf.sec_elmnt_count-1; i++)
-			if (vcl->other_bvds[i] == NULL)
+			if (vcl->other_bvds[i]->sec_elmnt_seq == DDF_UNUSED_BVD)
 				break;
 		if (i == vcl->conf.sec_elmnt_count-1) {
 			pr_err("no space for sec level config %u, count is %u\n",
 			       vd->sec_elmnt_seq, vcl->conf.sec_elmnt_count);
 			return;
 		}
-		if (posix_memalign((void **)&vcl->other_bvds[i], 512, len)
-		    != 0) {
-			pr_err("%s could not allocate vd buf\n", __func__);
-			return;
-		}
 	}
 	memcpy(vcl->other_bvds[i], vd, len);
 }
@@ -1006,12 +1023,13 @@ static int load_ddf_local(int fd, struct ddf_super *super,
 			}
 			vcl->next = super->conflist;
 			vcl->block_sizes = NULL; /* FIXME not for CONCAT */
-			if (vd->sec_elmnt_count > 1)
-				vcl->other_bvds =
-					xcalloc(vd->sec_elmnt_count - 1,
-						sizeof(struct vd_config *));
-			else
-				vcl->other_bvds = NULL;
+			vcl->conf.sec_elmnt_count = vd->sec_elmnt_count;
+			if (alloc_other_bvds(super, vcl) != 0) {
+				pr_err("%s could not allocate other bvds\n",
+				       __func__);
+				free(vcl);
+				return 1;
+			};
 			super->conflist = vcl;
 			dl->vlist[vnum++] = vcl;
 		}
@@ -1129,13 +1147,12 @@ static void free_super_ddf(struct supertype *st)
 		ddf->conflist = v->next;
 		if (v->block_sizes)
 			free(v->block_sizes);
-		if (v->other_bvds) {
-			int i;
-			for (i = 0; i < v->conf.sec_elmnt_count-1; i++)
-				if (v->other_bvds[i] != NULL)
-					free(v->other_bvds[i]);
-			free(v->other_bvds);
-		}
+		if (v->other_bvds)
+			/*
+			   v->other_bvds[0] points to beginning of buffer,
+			   see alloc_other_bvds()
+			*/
+			free(v->other_bvds[0]);
 		free(v);
 	}
 	while (ddf->dlist) {
@@ -1663,8 +1680,6 @@ static struct vd_config *find_vdcr(struct ddf_super *ddf, unsigned int inst,
 		nsec = n / __be16_to_cpu(conf->prim_elmnt_count);
 		if (conf->sec_elmnt_seq != nsec) {
 			for (ibvd = 1; ibvd < conf->sec_elmnt_count; ibvd++) {
-				if (v->other_bvds[ibvd-1] == NULL)
-					continue;
 				if (v->other_bvds[ibvd-1]->sec_elmnt_seq
 				    == nsec)
 					break;
@@ -2359,8 +2374,6 @@ static int init_super_ddf_bvd(struct supertype *st,
 	vcl->lba_offset = (__u64*) &vcl->conf.phys_refnum[ddf->mppe];
 	vcl->vcnum = venum;
 	vcl->block_sizes = NULL; /* FIXME not for CONCAT */
-	vcl->other_bvds = NULL;
-
 	vc = &vcl->conf;
 
 	vc->magic = DDF_VD_CONF_MAGIC;
@@ -2377,6 +2390,12 @@ static int init_super_ddf_bvd(struct supertype *st,
 		return 0;
 	}
 	vc->sec_elmnt_seq = 0;
+	if (alloc_other_bvds(ddf, vcl) != 0) {
+		pr_err("%s could not allocate other bvds\n",
+		       __func__);
+		free(vcl);
+		return 0;
+	}
 	vc->blocks = __cpu_to_be64(info->size * 2);
 	vc->array_blocks = __cpu_to_be64(
 		calc_array_size(info->level, info->raid_disks, info->layout,
@@ -3391,7 +3410,7 @@ static int check_secondary(const struct vcl *vc)
 	__set_sec_seen(conf->sec_elmnt_seq);
 	for (i = 0; i < conf->sec_elmnt_count-1; i++) {
 		const struct vd_config *bvd = vc->other_bvds[i];
-		if (bvd == NULL)
+		if (bvd->sec_elmnt_seq == DDF_UNUSED_BVD)
 			continue;
 		if (bvd->srl != conf->srl) {
 			pr_err("Inconsistent secondary RAID level across BVDs\n");
@@ -3449,9 +3468,9 @@ static unsigned int get_pd_index_from_refnum(const struct vcl *vc,
 
 	for (n = 1; n < vc->conf.sec_elmnt_count; n++) {
 		struct vd_config *vd = vc->other_bvds[n-1];
-		if (vd == NULL)
-			continue;
 		sec = vd->sec_elmnt_seq;
+		if (sec == DDF_UNUSED_BVD)
+			continue;
 		for (i = 0, j = 0 ; i < nmax ; i++) {
 			if (vd->phys_refnum[i] != 0xffffffff)
 				j++;
@@ -3753,12 +3772,13 @@ static int compare_super_ddf(struct supertype *st, struct supertype *tst)
 
 		vl1->next = first->conflist;
 		vl1->block_sizes = NULL;
-		if (vl2->conf.sec_elmnt_count > 1) {
-			vl1->other_bvds = xcalloc(vl2->conf.sec_elmnt_count - 1,
-						  sizeof(struct vd_config *));
-		} else
-			vl1->other_bvds = NULL;
 		memcpy(&vl1->conf, &vl2->conf, first->conf_rec_len*512);
+		if (alloc_other_bvds(first, vl1) != 0) {
+			pr_err("%s could not allocate other bvds\n",
+			       __func__);
+			free(vl1);
+			return 3;
+		}
 		vl1->lba_offset = (__u64 *)
 			&vl1->conf.phys_refnum[first->mppe];
 		for (vd = 0; vd < max_vds; vd++)
-- 
1.7.1

[PATCH 14/27] DDF: init_super_ddf_bvd: initialize other bvds

[mwilck]

Properly initialize the data structures of the other BVDs
in Create().

Signed-off-by: Martin Wilck <mwilck@arcor.de>
---
 super-ddf.c |    7 ++++++-
 1 files changed, 6 insertions(+), 1 deletions(-)

diff --git a/super-ddf.c b/super-ddf.c
index 1ff8a28..f42bb16 100644
--- a/super-ddf.c
+++ b/super-ddf.c
@@ -2328,7 +2328,7 @@ static int init_super_ddf_bvd(struct supertype *st,
 	 * We need to create a new vd_config and a new virtual_entry
 	 */
 	struct ddf_super *ddf = st->sb;
-	unsigned int venum;
+	unsigned int venum, i;
 	struct virtual_entry *ve;
 	struct vcl *vcl;
 	struct vd_config *vc;
@@ -2423,6 +2423,11 @@ static int init_super_ddf_bvd(struct supertype *st,
 	memset(vc->phys_refnum, 0xff, 4*ddf->mppe);
 	memset(vc->phys_refnum+ddf->mppe, 0x00, 8*ddf->mppe);
 
+	for (i = 1; i < vc->sec_elmnt_count; i++) {
+		memcpy(vcl->other_bvds[i-1], vc, ddf->conf_rec_len * 512);
+		vcl->other_bvds[i-1]->sec_elmnt_seq = i;
+	}
+
 	vcl->next = ddf->conflist;
 	ddf->conflist = vcl;
 	ddf->currentconf = vcl;
-- 
1.7.1

[PATCH 15/27] DDF: validate_geometry_ddf: support RAID10

[mwilck]

Make validate_geometry_ddf() use the same logic to check supported
RAID levels that init_super_ddf_bvd() uses.

Signed-off-by: Martin Wilck <mwilck@arcor.de>
---
 super-ddf.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/super-ddf.c b/super-ddf.c
index f42bb16..1893f45 100644
--- a/super-ddf.c
+++ b/super-ddf.c
@@ -3071,8 +3071,8 @@ static int validate_geometry_ddf(struct supertype *st,
 		struct vd_config conf;
 		if (layout_md2ddf(&array, &conf) == -1) {
 			if (verbose)
-				pr_err("DDF does not support level %d arrays\n",
-				       level);
+				pr_err("DDF does not support level %d /layout %d arrays with %d disks\n",
+				       level, layout, raiddisks);
 			return 0;
 		}
 		/* Should check layout? etc */
-- 
1.7.1

[PATCH 16/27] DDF: use LBA_OFFSET macro instead of lba_offset field

[mwilck]

Remove the lba_offset field from struct vcl. This field acted as
a "cache" for the address of the lba_offset field in the vd_config
structure. This isn't useful any more if there are multiple
vd_configs in a vcl.

This patch also adds __cpu_to_be64 in two places where it has been
quite obviously forgotten (ddf_set_disk, ddf_activate_spare).

Signed-off-by: Martin Wilck <mwilck@arcor.de>
---
 super-ddf.c |   32 ++++++++++++--------------------
 1 files changed, 12 insertions(+), 20 deletions(-)

diff --git a/super-ddf.c b/super-ddf.c
index 1893f45..f22c0e3 100644
--- a/super-ddf.c
+++ b/super-ddf.c
@@ -311,6 +311,7 @@ struct vd_config {
       /*__u64	lba_offset[0];  LBA offset in each phys.  Note extents in a
 				bvd are always the same size */
 };
+#define LBA_OFFSET(ddf, vd) ((__u64 *) &(vd)->phys_refnum[(ddf)->mppe])
 
 /* vd_config.cache_pol[7] is a bitmap */
 #define	DDF_cache_writeback	1	/* else writethrough */
@@ -407,8 +408,6 @@ struct ddf_super {
 			char space[512];
 			struct {
 				struct vcl	*next;
-				__u64		*lba_offset; /* location in 'conf' of
-							      * the lba table */
 				unsigned int	vcnum; /* index into ->virt */
 				struct vd_config **other_bvds;
 				__u64		*block_sizes; /* NULL if all the same */
@@ -1034,9 +1033,6 @@ static int load_ddf_local(int fd, struct ddf_super *super,
 			dl->vlist[vnum++] = vcl;
 		}
 		memcpy(&vcl->conf, vd, super->conf_rec_len*512);
-		vcl->lba_offset = (__u64*)
-			&vcl->conf.phys_refnum[super->mppe];
-
 		for (i=0; i < max_virt_disks ; i++)
 			if (memcmp(super->virt->entries[i].guid,
 				   vcl->conf.guid, DDF_GUID_LEN)==0)
@@ -1844,7 +1840,8 @@ static void getinfo_super_ddf_bvd(struct supertype *st, struct mdinfo *info, cha
 	info->custom_array_size	  = 0;
 
 	if (cd >= 0 && (unsigned)cd < ddf->mppe) {
-		info->data_offset	  = __be64_to_cpu(vc->lba_offset[cd]);
+		info->data_offset =
+			__be64_to_cpu(LBA_OFFSET(ddf, &vc->conf)[cd]);
 		if (vc->block_sizes)
 			info->component_size = vc->block_sizes[cd];
 		else
@@ -2303,7 +2300,8 @@ static struct extent *get_extents(struct ddf_super *ddf, struct dl *dl)
 		for (j = 0; j < v->conf.prim_elmnt_count; j++)
 			if (v->conf.phys_refnum[j] == dl->disk.refnum) {
 				/* This device plays role 'j' in  'v'. */
-				rv[n].start = __be64_to_cpu(v->lba_offset[j]);
+				rv[n].start = __be64_to_cpu(
+					LBA_OFFSET(ddf, &v->conf)[j]);
 				rv[n].size = __be64_to_cpu(v->conf.blocks);
 				n++;
 				break;
@@ -2371,7 +2369,6 @@ static int init_super_ddf_bvd(struct supertype *st,
 		pr_err("%s could not allocate vd_config\n", __func__);
 		return 0;
 	}
-	vcl->lba_offset = (__u64*) &vcl->conf.phys_refnum[ddf->mppe];
 	vcl->vcnum = venum;
 	vcl->block_sizes = NULL; /* FIXME not for CONCAT */
 	vc = &vcl->conf;
@@ -2472,7 +2469,7 @@ static void add_to_super_ddf_bvd(struct supertype *st,
 		return;
 
 	vc = &ddf->currentconf->conf;
-	lba_offset = ddf->currentconf->lba_offset;
+	lba_offset = LBA_OFFSET(ddf, &ddf->currentconf->conf);
 
 	ex = get_extents(ddf, dl);
 	if (!ex)
@@ -3572,7 +3569,6 @@ static struct mdinfo *container_content_ddf(struct supertype *st, char *subarray
 			struct dl *d;
 			const struct vd_config *bvd;
 			unsigned int iphys;
-			__u64 *lba_offset;
 			int stt;
 
 			if (ddf->phys->entries[pd].refnum == 0xFFFFFFFF)
@@ -3611,8 +3607,8 @@ static struct mdinfo *container_content_ddf(struct supertype *st, char *subarray
 			dev->recovery_start = MaxSector;
 
 			dev->events = __be32_to_cpu(ddf->primary.seq);
-			lba_offset =  (__u64 *)&bvd->phys_refnum[ddf->mppe];
-			dev->data_offset = __be64_to_cpu(lba_offset[iphys]);
+			dev->data_offset =
+				__be64_to_cpu(LBA_OFFSET(ddf, bvd)[iphys]);
 			dev->component_size = __be64_to_cpu(bvd->blocks);
 			if (d->devname)
 				strcpy(dev->name, d->devname);
@@ -3784,8 +3780,6 @@ static int compare_super_ddf(struct supertype *st, struct supertype *tst)
 			free(vl1);
 			return 3;
 		}
-		vl1->lba_offset = (__u64 *)
-			&vl1->conf.phys_refnum[first->mppe];
 		for (vd = 0; vd < max_vds; vd++)
 			if (!memcmp(first->virt->entries[vd].guid,
 				    vl1->conf.guid, DDF_GUID_LEN))
@@ -4046,11 +4040,10 @@ static void ddf_set_disk(struct active_array *a, int n, int state)
 		dprintf("%s: array %u disk %u ref %08x pd %d\n",
 			__func__, inst, n_bvd, vc->phys_refnum[n_bvd], pd);
 		if ((state & DS_INSYNC) && ! (state & DS_FAULTY)) {
-			__u64 *lba_offset;
 			pd = dl->pdnum; /* FIXME: is this really correct ? */
 			vc->phys_refnum[n_bvd] = dl->disk.refnum;
-			lba_offset = (__u64 *)&vc->phys_refnum[ddf->mppe];
-			lba_offset[n_bvd] = mdi->data_offset;
+			LBA_OFFSET(ddf, vc)[n_bvd] =
+				__cpu_to_be64(mdi->data_offset);
 			ddf->phys->entries[pd].type &=
 				~__cpu_to_be16(DDF_Global_Spare);
 			ddf->phys->entries[pd].type |=
@@ -4244,8 +4237,6 @@ static void ddf_process_update(struct supertype *st,
 			update->space = NULL;
 			vcl->next = ddf->conflist;
 			memcpy(&vcl->conf, vc, update->len);
-			vcl->lba_offset = (__u64*)
-				&vcl->conf.phys_refnum[mppe];
 			ent = find_vde_by_guid(ddf, vc->guid);
 			if (ent == DDF_NOTFOUND)
 				return;
@@ -4568,7 +4559,8 @@ static struct mdinfo *ddf_activate_spare(struct active_array *a,
 	for (di = rv ; di ; di = di->next) {
 		vc->phys_refnum[di->disk.raid_disk] =
 			ddf->phys->entries[dl->pdnum].refnum;
-		lba[di->disk.raid_disk] = di->data_offset;
+		LBA_OFFSET(ddf, vc)[di->disk.raid_disk]
+			= __cpu_to_be64(di->data_offset);
 	}
 	*updates = mu;
 	return rv;
-- 
1.7.1

[PATCH 17/27] DDF: get_extents: support secondary RAID level

[mwilck]

Use get_pd_index_from_refnum() in get_extents to determine
matching VD. This will ensure RAID 10 (secondary RAID level)
support, too.

This also fixes a bug in the previous get_extents() code (missing
__be16_to_cpu for conf.prim_elmnt_count).

DDF test case (10ddf-create) verified.

Signed-off-by: Martin Wilck <mwilck@arcor.de>
---
 super-ddf.c |   30 ++++++++++++++----------------
 1 files changed, 14 insertions(+), 16 deletions(-)

diff --git a/super-ddf.c b/super-ddf.c
index f22c0e3..e7aa0ad 100644
--- a/super-ddf.c
+++ b/super-ddf.c
@@ -467,6 +467,11 @@ static void pr_state(const struct ddf_super *ddf, const char *msg) {}
 #define ddf_set_updates_pending(x) \
 	do { (x)->updates_pending = 1; pr_state(x, __func__); } while (0)
 
+static unsigned int get_pd_index_from_refnum(const struct vcl *vc,
+					     __u32 refnum, unsigned int nmax,
+					     const struct vd_config **bvd,
+					     unsigned int *idx);
+
 static unsigned int calc_crc(void *buf, int len)
 {
 	/* crcs are always at the same place as in the ddf_header */
@@ -2289,23 +2294,21 @@ static struct extent *get_extents(struct ddf_super *ddf, struct dl *dl)
 	 */
 	struct extent *rv;
 	int n = 0;
-	unsigned int i, j;
+	unsigned int i;
 
 	rv = xmalloc(sizeof(struct extent) * (ddf->max_part + 2));
 
 	for (i = 0; i < ddf->max_part; i++) {
+		const struct vd_config *bvd;
+		unsigned int ibvd;
 		struct vcl *v = dl->vlist[i];
-		if (v == NULL)
+		if (v == NULL ||
+		    get_pd_index_from_refnum(v, dl->disk.refnum, ddf->mppe,
+					     &bvd, &ibvd) == DDF_NOTFOUND)
 			continue;
-		for (j = 0; j < v->conf.prim_elmnt_count; j++)
-			if (v->conf.phys_refnum[j] == dl->disk.refnum) {
-				/* This device plays role 'j' in  'v'. */
-				rv[n].start = __be64_to_cpu(
-					LBA_OFFSET(ddf, &v->conf)[j]);
-				rv[n].size = __be64_to_cpu(v->conf.blocks);
-				n++;
-				break;
-			}
+		rv[n].start = __be64_to_cpu(LBA_OFFSET(ddf, bvd)[ibvd]);
+		rv[n].size = __be64_to_cpu(bvd->blocks);
+		n++;
 	}
 	qsort(rv, n, sizeof(*rv), cmp_extent);
 
@@ -2682,11 +2685,6 @@ static int remove_from_super_ddf(struct supertype *st, mdu_disk_info_t *dk)
  */
 #define NULL_CONF_SZ	4096
 
-static unsigned int get_pd_index_from_refnum(const struct vcl *vc,
-					     __u32 refnum, unsigned int nmax,
-					     const struct vd_config **bvd,
-					     unsigned int *idx);
-
 static int __write_ddf_structure(struct dl *d, struct ddf_super *ddf, __u8 type,
 				 char *null_aligned)
 {
-- 
1.7.1

[PATCH 18/27] DDF: add_to_super_ddf: allow empty slots in phys disk table

[mwilck]

The used slots in the phys disk table aren't necessarily the
first ones. Rather, unused entries are represented by entries
where the GUID is all 0xff.

Signed-off-by: Martin Wilck <mwilck@arcor.de>
---
 super-ddf.c |   29 +++++++++++++++++++++++------
 1 files changed, 23 insertions(+), 6 deletions(-)

diff --git a/super-ddf.c b/super-ddf.c
index e7aa0ad..093f2dc 100644
--- a/super-ddf.c
+++ b/super-ddf.c
@@ -2242,6 +2242,8 @@ static int init_super_ddf(struct supertype *st,
 	pd->used_pdes = __cpu_to_be16(0);
 	pd->max_pdes = __cpu_to_be16(max_phys_disks);
 	memset(pd->pad, 0xff, 52);
+	for (i = 0; i < max_phys_disks; i++)
+		memset(pd->entries[i].guid, 0xff, DDF_GUID_LEN);
 
 	if (posix_memalign((void**)&vd, 512, vdsize) != 0) {
 		pr_err("%s could not allocate vd\n", __func__);
@@ -2538,6 +2540,16 @@ static void add_to_super_ddf_bvd(struct supertype *st,
 	ddf_set_updates_pending(ddf);
 }
 
+static unsigned int find_unused_pde(const struct ddf_super *ddf)
+{
+	unsigned int i;
+	for (i = 0; i < __be16_to_cpu(ddf->phys->max_pdes); i++) {
+		if (all_ff(ddf->phys->entries[i].guid))
+			return i;
+	}
+	return DDF_NOTFOUND;
+}
+
 /* add a device to a container, either while creating it or while
  * expanding a pre-existing container
  */
@@ -2564,6 +2576,14 @@ static int add_to_super_ddf(struct supertype *st,
 	 * a phys_disk entry and a more detailed disk_data entry.
 	 */
 	fstat(fd, &stb);
+	n = find_unused_pde(ddf);
+	if (n == DDF_NOTFOUND) {
+		pr_err("%s: No free slot in array, cannot add disk\n",
+		       __func__);
+		return 1;
+	}
+	pde = &ddf->phys->entries[n];
+
 	if (posix_memalign((void**)&dd, 512,
 		           sizeof(*dd) + sizeof(dd->vlist[0]) * ddf->max_part) != 0) {
 		pr_err("%s could allocate buffer for new disk, aborting\n",
@@ -2602,8 +2622,6 @@ static int add_to_super_ddf(struct supertype *st,
 	for (i = 0; i < ddf->max_part ; i++)
 		dd->vlist[i] = NULL;
 
-	n = __be16_to_cpu(ddf->phys->used_pdes);
-	pde = &ddf->phys->entries[n];
 	dd->pdnum = n;
 
 	if (st->update_tail) {
@@ -2616,10 +2634,9 @@ static int add_to_super_ddf(struct supertype *st,
 		pd->used_pdes = __cpu_to_be16(n);
 		pde = &pd->entries[0];
 		dd->mdupdate = pd;
-	} else {
-		n++;
-		ddf->phys->used_pdes = __cpu_to_be16(n);
-	}
+	} else
+		ddf->phys->used_pdes = __cpu_to_be16(
+			1 + __be16_to_cpu(ddf->phys->used_pdes));
 
 	memcpy(pde->guid, dd->disk.guid, DDF_GUID_LEN);
 	pde->refnum = dd->disk.refnum;
-- 
1.7.1

[PATCH 19/27] DDF: add_to_super_ddf: Use same amount of workspace as other disks

[mwilck]

If there are already disks in the container, reserve the same amount
of workspace as the first disk. Fill in the primary/secondary/
workspace LBA values.

Signed-off-by: Martin Wilck <mwilck@arcor.de>
---
 super-ddf.c |   33 +++++++++++++++++++++++++++++----
 1 files changed, 29 insertions(+), 4 deletions(-)

diff --git a/super-ddf.c b/super-ddf.c
index 093f2dc..040a965 100644
--- a/super-ddf.c
+++ b/super-ddf.c
@@ -2583,6 +2583,13 @@ static int add_to_super_ddf(struct supertype *st,
 		return 1;
 	}
 	pde = &ddf->phys->entries[n];
+	get_dev_size(fd, NULL, &size);
+	if (size <= 32*1024*1024) {
+		pr_err("%s: device size must be at least 32MB\n",
+		       __func__);
+		return 1;
+	}
+	size >>= 9;
 
 	if (posix_memalign((void**)&dd, 512,
 		           sizeof(*dd) + sizeof(dd->vlist[0]) * ddf->max_part) != 0) {
@@ -2642,13 +2649,31 @@ static int add_to_super_ddf(struct supertype *st,
 	pde->refnum = dd->disk.refnum;
 	pde->type = __cpu_to_be16(DDF_Forced_PD_GUID | DDF_Global_Spare);
 	pde->state = __cpu_to_be16(DDF_Online);
-	get_dev_size(fd, NULL, &size);
-	/* We are required to reserve 32Meg, and record the size in sectors */
-	pde->config_size = __cpu_to_be64( (size - 32*1024*1024) / 512);
+	dd->size = size;
+	/*
+	 * If there is already a device in dlist, try to reserve the same
+	 * amount of workspace. Otherwise, use 32MB.
+	 * We checked disk size above already.
+	 */
+#define __calc_lba(new, old, lba, mb) do { \
+		unsigned long long dif; \
+		if ((old) != NULL) \
+			dif = (old)->size - __be64_to_cpu((old)->lba); \
+		else \
+			dif = (new)->size; \
+		if ((new)->size > dif) \
+			(new)->lba = __cpu_to_be64((new)->size - dif); \
+		else \
+			(new)->lba = __cpu_to_be64((new)->size - (mb*1024*2)); \
+	} while (0)
+	__calc_lba(dd, ddf->dlist, workspace_lba, 32);
+	__calc_lba(dd, ddf->dlist, primary_lba, 16);
+	__calc_lba(dd, ddf->dlist, secondary_lba, 32);
+	pde->config_size = dd->workspace_lba;
+
 	sprintf(pde->path, "%17.17s","Information: nil") ;
 	memset(pde->pad, 0xff, 6);
 
-	dd->size = size >> 9;
 	if (st->update_tail) {
 		dd->next = ddf->add_list;
 		ddf->add_list = dd;
-- 
1.7.1

[PATCH 20/27] DDF: add_to_super_ddf: RAID10 changes

[mwilck]

The index in the BVD may be different in the RAID10 case.

Signed-off-by: Martin Wilck <mwilck@arcor.de>
---
 super-ddf.c |   13 +++++++++----
 1 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/super-ddf.c b/super-ddf.c
index 040a965..96d13ae 100644
--- a/super-ddf.c
+++ b/super-ddf.c
@@ -2454,11 +2454,11 @@ static void add_to_super_ddf_bvd(struct supertype *st,
 	struct dl *dl;
 	struct ddf_super *ddf = st->sb;
 	struct vd_config *vc;
-	__u64 *lba_offset;
 	unsigned int working;
 	unsigned int i;
 	unsigned long long blocks, pos, esize;
 	struct extent *ex;
+	unsigned int raid_disk = dk->raid_disk;
 
 	if (fd == -1) {
 		for (dl = ddf->dlist; dl ; dl = dl->next)
@@ -2474,7 +2474,12 @@ static void add_to_super_ddf_bvd(struct supertype *st,
 		return;
 
 	vc = &ddf->currentconf->conf;
-	lba_offset = LBA_OFFSET(ddf, &ddf->currentconf->conf);
+	if (vc->sec_elmnt_count > 1) {
+		int n = __be16_to_cpu(vc->prim_elmnt_count);
+		if (raid_disk >= n)
+			vc = ddf->currentconf->other_bvds[raid_disk / n - 1];
+		raid_disk %= n;
+	}
 
 	ex = get_extents(ddf, dl);
 	if (!ex)
@@ -2498,8 +2503,8 @@ static void add_to_super_ddf_bvd(struct supertype *st,
 		return;
 
 	ddf->currentdev = dk->raid_disk;
-	vc->phys_refnum[dk->raid_disk] = dl->disk.refnum;
-	lba_offset[dk->raid_disk] = __cpu_to_be64(pos);
+	vc->phys_refnum[raid_disk] = dl->disk.refnum;
+	LBA_OFFSET(ddf, vc)[raid_disk] = __cpu_to_be64(pos);
 
 	for (i = 0; i < ddf->max_part ; i++)
 		if (dl->vlist[i] == NULL)
-- 
1.7.1

[PATCH 21/27] DDF: add_to_super_ddf_bvd: use get_svd_state()

[mwilck]

Reuse get_svd_state() rather than own state algorithm.

Signed-off-by: Martin Wilck <mwilck@arcor.de>
---
 super-ddf.c |   30 +++++++-----------------------
 1 files changed, 7 insertions(+), 23 deletions(-)

diff --git a/super-ddf.c b/super-ddf.c
index 96d13ae..0247ca5 100644
--- a/super-ddf.c
+++ b/super-ddf.c
@@ -2437,6 +2437,8 @@ static int init_super_ddf_bvd(struct supertype *st,
 	return 1;
 }
 
+static int get_svd_state(const struct ddf_super *, const struct vcl *);
+
 #ifndef MDASSEMBLE
 static void add_to_super_ddf_bvd(struct supertype *st,
 				 mdu_disk_info_t *dk, int fd, char *devname)
@@ -2454,7 +2456,6 @@ static void add_to_super_ddf_bvd(struct supertype *st,
 	struct dl *dl;
 	struct ddf_super *ddf = st->sb;
 	struct vd_config *vc;
-	unsigned int working;
 	unsigned int i;
 	unsigned long long blocks, pos, esize;
 	struct extent *ex;
@@ -2475,7 +2476,7 @@ static void add_to_super_ddf_bvd(struct supertype *st,
 
 	vc = &ddf->currentconf->conf;
 	if (vc->sec_elmnt_count > 1) {
-		int n = __be16_to_cpu(vc->prim_elmnt_count);
+		unsigned int n = __be16_to_cpu(vc->prim_elmnt_count);
 		if (raid_disk >= n)
 			vc = ddf->currentconf->other_bvds[raid_disk / n - 1];
 		raid_disk %= n;
@@ -2518,28 +2519,11 @@ static void add_to_super_ddf_bvd(struct supertype *st,
 	if (devname)
 		dl->devname = devname;
 
-	/* Check how many working raid_disks, and if we can mark
-	 * array as optimal yet
-	 */
-	working = 0;
-
-	for (i = 0; i < __be16_to_cpu(vc->prim_elmnt_count); i++)
-		if (vc->phys_refnum[i] != 0xffffffff)
-			working++;
-
-	/* Find which virtual_entry */
+	/* Check if we can mark array as optimal yet */
 	i = ddf->currentconf->vcnum;
-	if (working == __be16_to_cpu(vc->prim_elmnt_count))
-		ddf->virt->entries[i].state =
-			(ddf->virt->entries[i].state & ~DDF_state_mask)
-			| DDF_state_optimal;
-
-	if (vc->prl == DDF_RAID6 &&
-	    working+1 == __be16_to_cpu(vc->prim_elmnt_count))
-		ddf->virt->entries[i].state =
-			(ddf->virt->entries[i].state & ~DDF_state_mask)
-			| DDF_state_part_optimal;
-
+	ddf->virt->entries[i].state =
+		(ddf->virt->entries[i].state & ~DDF_state_mask)
+		| get_svd_state(ddf, ddf->currentconf);
 	ddf->phys->entries[dl->pdnum].type &= ~__cpu_to_be16(DDF_Global_Spare);
 	ddf->phys->entries[dl->pdnum].type |= __cpu_to_be16(DDF_Active_in_VD);
 	ddf_set_updates_pending(ddf);
-- 
1.7.1

[PATCH 22/27] DDF: getinfo_super_ddf_bvd: lba_offset calculation for RAID10

[mwilck]

Secondary RAID needs some extra logic here, too.

Signed-off-by: Martin Wilck <mwilck@arcor.de>
---
 super-ddf.c |   10 ++++++++++
 1 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/super-ddf.c b/super-ddf.c
index 0247ca5..cd6f298 100644
--- a/super-ddf.c
+++ b/super-ddf.c
@@ -1829,10 +1829,12 @@ static void getinfo_super_ddf_bvd(struct supertype *st, struct mdinfo *info, cha
 	struct ddf_super *ddf = st->sb;
 	struct vcl *vc = ddf->currentconf;
 	int cd = ddf->currentdev;
+	int n_prim;
 	int j;
 	struct dl *dl;
 	int map_disks = info->array.raid_disks;
 	__u32 *cptr;
+	struct vd_config *conf;
 
 	memset(info, 0, sizeof(*info));
 	if (layout_ddf2md(&vc->conf, &info->array) == -1)
@@ -1844,6 +1846,14 @@ static void getinfo_super_ddf_bvd(struct supertype *st, struct mdinfo *info, cha
 	info->array.chunk_size	  = 512 << vc->conf.chunk_shift;
 	info->custom_array_size	  = 0;
 
+	conf = &vc->conf;
+	n_prim = __be16_to_cpu(conf->prim_elmnt_count);
+	if (conf->sec_elmnt_count > 1 && cd >= n_prim) {
+		int ibvd = cd / n_prim - 1;
+		cd %= n_prim;
+		conf = vc->other_bvds[ibvd];
+	}
+
 	if (cd >= 0 && (unsigned)cd < ddf->mppe) {
 		info->data_offset =
 			__be64_to_cpu(LBA_OFFSET(ddf, &vc->conf)[cd]);
-- 
1.7.1

[PATCH 23/27] DDF: guid_str: convenience function to print GUID for debugging

[mwilck]

For debugging DDF structure changes, it is important to be able
to identify VCs by their GUIDs.

Signed-off-by: Martin Wilck <mwilck@arcor.de>
---
 super-ddf.c |   14 ++++++++++++++
 1 files changed, 14 insertions(+), 0 deletions(-)

diff --git a/super-ddf.c b/super-ddf.c
index cd6f298..bfb7f22 100644
--- a/super-ddf.c
+++ b/super-ddf.c
@@ -1293,6 +1293,17 @@ static void print_guid(char *guid, int tstamp)
 	printf(")");
 }
 
+static const char *guid_str(const char *guid)
+{
+	static char buf[DDF_GUID_LEN*2+1];
+	int i;
+	char *p = buf;
+	for (i = 0; i < DDF_GUID_LEN; i++)
+		p += sprintf(p, "%02x", (unsigned char)guid[i]);
+	*p = '\0';
+	return (const char *) buf;
+}
+
 static void examine_vd(int n, struct ddf_super *sb, char *guid)
 {
 	int crl = sb->conf_rec_len;
@@ -2789,6 +2800,9 @@ static int __write_ddf_structure(struct dl *d, struct ddf_super *ddf, __u8 type,
 					&dummy);
 		}
 		if (c) {
+			dprintf("writing conf record %i on disk %08x for %s/%u\n",
+				i, d->disk.refnum, guid_str(vdc->guid),
+				vdc->sec_elmnt_seq);
 			vdc->seqnum = header->seq;
 			vdc->crc = calc_crc(vdc, conf_size);
 			if (write(fd, vdc, conf_size) < 0)
-- 
1.7.1

[PATCH 24/27] DDF: ddf_set_array_state: more meaningful output

[mwilck]

Print the array GUID and the array state.

Signed-off-by: Martin Wilck <mwilck@arcor.de>
---
 super-ddf.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/super-ddf.c b/super-ddf.c
index bfb7f22..8e1d502 100644
--- a/super-ddf.c
+++ b/super-ddf.c
@@ -3952,7 +3952,9 @@ static int ddf_set_array_state(struct active_array *a, int consistent)
 	if (old != ddf->virt->entries[inst].init_state)
 		ddf_set_updates_pending(ddf);
 
-	dprintf("ddf mark %d %s %llu\n", inst, consistent?"clean":"dirty",
+	dprintf("ddf mark %d/%s (%d) %s %llu\n", inst,
+		guid_str(ddf->virt->entries[inst].guid), a->curr_state,
+		consistent?"clean":"dirty",
 		a->info.resync_start);
 	return consistent;
 }
-- 
1.7.1

[PATCH 25/27] DDF: ddf_process_update: handle update of conf records for SVD

[mwilck]

For secondary RAID, we need to check which BVD needs to be updated.

Signed-off-by: Martin Wilck <mwilck@arcor.de>
---
 super-ddf.c |   17 ++++++++++++++++-
 1 files changed, 16 insertions(+), 1 deletions(-)

diff --git a/super-ddf.c b/super-ddf.c
index 8e1d502..77c00b2 100644
--- a/super-ddf.c
+++ b/super-ddf.c
@@ -4282,7 +4282,22 @@ static void ddf_process_update(struct supertype *st,
 			/* An update, just copy the phys_refnum and lba_offset
 			 * fields
 			 */
-			memcpy(vcl->conf.phys_refnum, vc->phys_refnum,
+			struct vd_config *conf = &vcl->conf;
+			if (vcl->other_bvds != NULL &&
+			    conf->sec_elmnt_seq != vc->sec_elmnt_seq) {
+				unsigned int i;
+				for (i = 1; i < conf->sec_elmnt_count; i++)
+					if (vcl->other_bvds[i-1]->sec_elmnt_seq
+					    == vc->sec_elmnt_seq)
+						break;
+				if (i == conf->sec_elmnt_count) {
+					pr_err("%s/DDF_VD_CONF_MAGIC: BVD %u not found\n",
+					       __func__, vc->sec_elmnt_seq);
+					return;
+				}
+				conf = vcl->other_bvds[i-1];
+			}
+			memcpy(conf->phys_refnum, vc->phys_refnum,
 			       mppe * (sizeof(__u32) + sizeof(__u64)));
 		} else {
 			/* A new VD_CONF */
-- 
1.7.1

[PATCH 26/27] DDF: ddf_process_update: Fix vlist treatment for SVDs

[mwilck]

With secondary RAID level, disks may belong to other BVDs in
a given conf record. This needs to be taken into account
when fixing the vlist.

Signed-off-by: Martin Wilck <mwilck@arcor.de>
---
 super-ddf.c |   48 ++++++++++++++++++++++++++----------------------
 1 files changed, 26 insertions(+), 22 deletions(-)

diff --git a/super-ddf.c b/super-ddf.c
index 77c00b2..ebb5e0b 100644
--- a/super-ddf.c
+++ b/super-ddf.c
@@ -4323,30 +4323,34 @@ static void ddf_process_update(struct supertype *st,
 					|= __be16_to_cpu(DDF_Transition);
 		/* Now make sure vlist is correct for each dl. */
 		for (dl = ddf->dlist; dl; dl = dl->next) {
-			unsigned int dn;
 			unsigned int vn = 0;
 			int in_degraded = 0;
-			for (vcl = ddf->conflist; vcl ; vcl = vcl->next)
-				for (dn=0; dn < ddf->mppe ; dn++)
-					if (vcl->conf.phys_refnum[dn] ==
-					    dl->disk.refnum) {
-						int vstate;
-						dprintf("dev %d has %p at %d\n",
-							dl->pdnum, vcl, vn);
-						/* Clear the Transition flag */
-						if (ddf->phys->entries[dl->pdnum].state
-						    & __be16_to_cpu(DDF_Failed))
-							ddf->phys->entries[dl->pdnum].state &=
-								~__be16_to_cpu(DDF_Transition);
-
-						dl->vlist[vn++] = vcl;
-						vstate = ddf->virt->entries[vcl->vcnum].state
-							& DDF_state_mask;
-						if (vstate == DDF_state_degraded ||
-						    vstate == DDF_state_part_optimal)
-							in_degraded = 1;
-						break;
-					}
+			for (vcl = ddf->conflist; vcl ; vcl = vcl->next) {
+				unsigned int dn, ibvd;
+				const struct vd_config *conf;
+				int vstate;
+				dn = get_pd_index_from_refnum(vcl,
+							      dl->disk.refnum,
+							      ddf->mppe,
+							      &conf, &ibvd);
+				if (dn == DDF_NOTFOUND)
+					continue;
+				dprintf("dev %d/%08x has %s (sec=%u) at %d\n",
+					dl->pdnum, dl->disk.refnum,
+					guid_str(conf->guid),
+					conf->sec_elmnt_seq, vn);
+				/* Clear the Transition flag */
+				if (ddf->phys->entries[dl->pdnum].state
+				    & __be16_to_cpu(DDF_Failed))
+					ddf->phys->entries[dl->pdnum].state &=
+						~__be16_to_cpu(DDF_Transition);
+				dl->vlist[vn++] = vcl;
+				vstate = ddf->virt->entries[vcl->vcnum].state
+					& DDF_state_mask;
+				if (vstate == DDF_state_degraded ||
+					vstate == DDF_state_part_optimal)
+					in_degraded = 1;
+			}
 			while (vn < ddf->max_part)
 				dl->vlist[vn++] = NULL;
 			if (dl->vlist[0]) {
-- 
1.7.1

[PATCH 27/27] tests/10ddf-create: add RAID 10 array

[mwilck]

This patch adds RAID10 support to the DDF test script.
It actually passes!

Signed-off-by: Martin Wilck <mwilck@arcor.de>
---
 tests/10ddf-create |   15 ++++++++++++---
 1 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/tests/10ddf-create b/tests/10ddf-create
index 3055a5d..6f1f51d 100644
--- a/tests/10ddf-create
+++ b/tests/10ddf-create
@@ -14,17 +14,21 @@ mdadm -CR r0 -l0 -n5 /dev/md/ddf0 -z 5000
 if mdadm -CR r0 -l1 -n2 /dev/md/ddf0 -z 5000
 then echo >&2 create with same name should fail ; exit 1
 fi
+mdadm -CR r10 -l10 -n4 -pn2 /dev/md/ddf0 -z 5000
 mdadm -CR r1 -l1 -n2 /dev/md/ddf0
 mdadm -CR r5 -l5 -n3 /dev/md/ddf0
 testdev /dev/md/r0 5 5000 512
-# r0 will use 4608 due to chunk size, so that leaves 28160 for the rest
-testdev /dev/md/r1 1 28160 64
-testdev /dev/md/r5 2 28160 512
+testdev /dev/md/r10 2 5000 512
+# r0/r10 will use 4608 due to chunk size, so that leaves 23552 for the rest
+testdev /dev/md/r1 1 23552 64
+testdev /dev/md/r5 2 23552 512
 dd if=/dev/sda of=/dev/md/r0 || true
+dd if=/dev/sda of=/dev/md/r10 || true
 dd if=/dev/sda of=/dev/md/r1 || true
 dd if=/dev/sda of=/dev/md/r5 || true
 
 s0=`sha1sum /dev/md/r0`
+s10=`sha1sum /dev/md/r10`
 s1=`sha1sum /dev/md/r1`
 s5=`sha1sum /dev/md/r5`
 
@@ -35,12 +39,16 @@ mdadm -I /dev/md/ddf0
 
 udevadm settle
 s0a=`sha1sum /dev/md/r0`
+s10a=`sha1sum /dev/md/r10`
 s1a=`sha1sum /dev/md/r1`
 s5a=`sha1sum /dev/md/r5`
 
 if [ "$s0" != "$s0a" ]; then
    echo r0 did not match ; exit 1;
 fi
+if [ "$s10" != "$s10a" ]; then
+   echo r10 did not match ; exit 1;
+fi
 if [ "$s1" != "$s1a" ]; then
    echo r1 did not match ; exit 1;
 fi
@@ -50,6 +58,7 @@ fi
 
 # failure status just means it has completed already, so ignore it.
 mdadm --wait /dev/md/r1 || true
+mdadm --wait /dev/md/r10 || true
 mdadm --wait /dev/md/r5 || true
 
 mdadm -Dbs > /var/tmp/mdadm.conf
-- 
1.7.1

Re: DDF/RAID10 patch series

[NeilBrown]

[-- Attachment #1: Type: text/plain, Size: 682 bytes --]

On Wed,  3 Jul 2013 22:27:40 +0200 mwilck@arcor.de wrote:

> This is the DDF/RAID10 patch series I announced.
> 
> The whole patch set is aimed at making --Create work with DDF/RAID10.
> When --Create eventually works, RAID10 can be added to the DDF
> regression test.
> 
> With this patch set, DDF is still far from the level of functionality
> present in IMSM, but getting a little closer.
> 
> The patch set also includes some cleanup patches and debug code which
> I needed to find the bugs in earlier versions, and which might be useful
> later as well.
> 
> Please review and test.

Thanks Martin.  I hope to look over these Monday next week.

NeilBrown


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

Re: [PATCH 03/27] DDF: load_ddf_headers: use secondary header as fallback

[NeilBrown]

[-- Attachment #1: Type: text/plain, Size: 2109 bytes --]

On Wed,  3 Jul 2013 22:27:43 +0200 mwilck@arcor.de wrote:

> When the primary header can't be read, use the secondary header
> as fallback.
> 
> Signed-off-by: Martin Wilck <mwilck@arcor.de>
> ---
>  super-ddf.c |   13 +++++++++----
>  1 files changed, 9 insertions(+), 4 deletions(-)
> 
> diff --git a/super-ddf.c b/super-ddf.c
> index c445df2..45522f2 100644
> --- a/super-ddf.c
> +++ b/super-ddf.c
> @@ -601,15 +601,15 @@ static int load_ddf_headers(int fd, struct ddf_super *super, char *devname)
>  				DDF_REVISION_2, super->anchor.revision,devname);
>  		return 2;
>  	}
> +	super->active = NULL;
>  	if (load_ddf_header(fd, __be64_to_cpu(super->anchor.primary_lba),
>  			    dsize >> 9,  1,
>  			    &super->primary, &super->anchor) == 0) {
>  		if (devname)
>  			pr_err("Failed to load primary DDF header "
>  			       "on %s\n", devname);
> -		return 2;
> -	}
> -	super->active = &super->primary;
> +	} else
> +		super->active = &super->primary;
>  	if (load_ddf_header(fd, __be64_to_cpu(super->anchor.secondary_lba),
>  			    dsize >> 9,  2,
>  			    &super->secondary, &super->anchor)) {
> @@ -619,9 +619,14 @@ static int load_ddf_headers(int fd, struct ddf_super *super, char *devname)
>  		    || (__be32_to_cpu(super->primary.seq)
>  			== __be32_to_cpu(super->secondary.seq) &&
>  			super->primary.openflag && !super->secondary.openflag)
> +		    || super->active == NULL
>  			)
>  			super->active = &super->secondary;
> -	}
> +	} else if (devname)
> +		pr_err("Failed to load secondary DDF header on %s\n",
> +		       devname);
> +	if (super->active == NULL)
> +		return 2;
>  	return 0;
>  }
>  

Should that "|| super->active == NULL" test go at the beginning of the 'if'
rather than the set?  Currently it comparing sequence numbers between the
successfully read secondary and the (possibly) badly read primary.

Changing wouldn't actually affect the outcome so I'll apply the patch as-is,
but if you agree that it makes sense earlier, then I'd happily access a
follow-up patch which moves the test.

Thanks,
NeilBrown

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

Re: [PATCH 05/27] DDF: Implement store_super_ddf

[NeilBrown]

[-- Attachment #1: Type: text/plain, Size: 2841 bytes --]

On Wed,  3 Jul 2013 22:27:45 +0200 mwilck@arcor.de wrote:

> This patch implements the previously unsupported case where
> store_super_ddf is called with a non-empty superblock.
> 
> For DDF, writing meta data to just one disk makes no sense.
> We would run the risk of writing inconsistent meta data
> to the devices. So just call __write_init_super_ddf and
> write to all devices, including the one passed by the caller.
> 
> This patch assumes that the device to store the superblock on
> has already been added to the DDF structure. Otherwise, an
> error message will be emitted.
> 
> Signed-off-by: Martin Wilck <mwilck@arcor.de>

I'm not sure I really like this.  If mdadm is calling ->store_super, then it
wants to write to just one device.  It will quite possibly call
->store_super on all devices, and with your change that will write to all
devices multiple times.
So maybe a different interface is needed?

What is the big-picture effect of this patch?  What mdadm function now works
that didn't work before?

Thanks,
NeilBrown



> ---
>  super-ddf.c |   41 +++++++++++++++++++++++++++++++++++------
>  1 files changed, 35 insertions(+), 6 deletions(-)
> 
> diff --git a/super-ddf.c b/super-ddf.c
> index b806949..b3c846d 100644
> --- a/super-ddf.c
> +++ b/super-ddf.c
> @@ -3471,15 +3471,44 @@ static int store_super_ddf(struct supertype *st, int fd)
>  	if (!ddf)
>  		return 1;
>  
> -	/* ->dlist and ->conflist will be set for updates, currently not
> -	 * supported
> -	 */
> -	if (ddf->dlist || ddf->conflist)
> -		return 1;
> -
>  	if (!get_dev_size(fd, NULL, &dsize))
>  		return 1;
>  
> +	if (ddf->dlist || ddf->conflist) {
> +		struct stat sta;
> +		struct dl *dl;
> +		int ofd, ret;
> +
> +		if (fstat(fd, &sta) == -1 || !S_ISBLK(sta.st_mode)) {
> +			pr_err("%s: file descriptor for invalid device\n",
> +			       __func__);
> +			return 1;
> +		}
> +		for (dl = ddf->dlist; dl; dl = dl->next)
> +			if (dl->major == (int)major(sta.st_rdev) &&
> +			    dl->minor == (int)minor(sta.st_rdev))
> +				break;
> +		if (!dl) {
> +			pr_err("%s: couldn't find disk %d/%d\n", __func__,
> +			       (int)major(sta.st_rdev),
> +			       (int)minor(sta.st_rdev));
> +			return 1;
> +		}
> +		/*
> +		   For DDF, writing to just one disk makes no sense.
> +		   We would run the risk of writing inconsistent meta data
> +		   to the devices. So just call __write_init_super_ddf and
> +		   write to all devices, including this one.
> +		   Use the fd passed to this function, just in case dl->fd
> +		   is invalid.
> +		 */
> +		ofd = dl->fd;
> +		dl->fd = fd;
> +		ret =  __write_init_super_ddf(st);
> +		dl->fd = ofd;
> +		return ret;
> +	}
> +
>  	if (posix_memalign(&buf, 512, 512) != 0)
>  		return 1;
>  	memset(buf, 0, 512);


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

Re: [PATCH 07/27] DDF: find_vdcr: account for secondary RAID level

[NeilBrown]

[-- Attachment #1: Type: text/plain, Size: 5901 bytes --]

On Wed,  3 Jul 2013 22:27:47 +0200 mwilck@arcor.de wrote:

> If secondary RAID level is taken into account, translation between
> the md RAID member (raid_disk) and the index of a physical disk
> in a BVD becomes more complex.
> 
> Also, take into account that the member list can have unused entries
> (this is independent of secondary RAID level).
> 
> Adapt usage of find_vdcr() accordingly
> 
> Signed-off-by: Martin Wilck <mwilck@arcor.de>
> ---
>  super-ddf.c |  100 +++++++++++++++++++++++++++++++++++++++++++++++++++--------
>  1 files changed, 87 insertions(+), 13 deletions(-)
> 
> diff --git a/super-ddf.c b/super-ddf.c
> index ae24bb9..c448bff 100644
> --- a/super-ddf.c
> +++ b/super-ddf.c
> @@ -1441,13 +1441,76 @@ static int match_home_ddf(struct supertype *st, char *homehost)
>  }
>  
>  #ifndef MDASSEMBLE
> -static struct vd_config *find_vdcr(struct ddf_super *ddf, unsigned int inst)
> +static int find_index_in_bvd(const struct ddf_super *ddf,
> +			     const struct vd_config *conf, unsigned int n,
> +			     unsigned int *n_bvd)
> +{
> +	/*
> +	 * Find the index of the n-th valid physical disk in this BVD
> +	 */
> +	unsigned int i, j;
> +	for (i = 0, j = 0; i < ddf->mppe &&
> +		     j < __be16_to_cpu(conf->prim_elmnt_count); i++) {
> +		if (conf->phys_refnum[i] != 0xffffffff) {

Should that 0xffffffff be DDF_NOTFOUND?

(I glazed over reading the rest of this patch .. sorry.  Maybe I'll try again
another day)

NeilBrown


> +			if (n == j) {
> +				*n_bvd = i;
> +				return 1;
> +			}
> +			j++;
> +		}
> +	}
> +	dprintf("%s: couldn't find BVD member %u (total %u)\n",
> +		__func__, n, __be16_to_cpu(conf->prim_elmnt_count));
> +	return 0;
> +}
> +
> +static struct vd_config *find_vdcr(struct ddf_super *ddf, unsigned int inst,
> +				   unsigned int n,
> +				   unsigned int *n_bvd, struct vcl **vcl)
>  {
>  	struct vcl *v;
>  
> -	for (v = ddf->conflist; v; v = v->next)
> -		if (inst == v->vcnum)
> -			return &v->conf;
> +	for (v = ddf->conflist; v; v = v->next) {
> +		unsigned int nsec, ibvd;
> +		struct vd_config *conf;
> +		if (inst != v->vcnum)
> +			continue;
> +		conf = &v->conf;
> +		if (conf->sec_elmnt_count == 1) {
> +			if (find_index_in_bvd(ddf, conf, n, n_bvd)) {
> +				*vcl = v;
> +				return conf;
> +			} else
> +				goto bad;
> +		}
> +		if (v->other_bvds == NULL) {
> +			pr_err("%s: BUG: other_bvds is NULL, nsec=%u\n",
> +			       __func__, conf->sec_elmnt_count);
> +			goto bad;
> +		}
> +		nsec = n / __be16_to_cpu(conf->prim_elmnt_count);
> +		if (conf->sec_elmnt_seq != nsec) {
> +			for (ibvd = 1; ibvd < conf->sec_elmnt_count; ibvd++) {
> +				if (v->other_bvds[ibvd-1] == NULL)
> +					continue;
> +				if (v->other_bvds[ibvd-1]->sec_elmnt_seq
> +				    == nsec)
> +					break;
> +			}
> +			if (ibvd == conf->sec_elmnt_count)
> +				goto bad;
> +			conf = v->other_bvds[ibvd-1];
> +		}
> +		if (!find_index_in_bvd(ddf, conf,
> +				       n - nsec*conf->sec_elmnt_count, n_bvd))
> +			goto bad;
> +		dprintf("%s: found disk %u as member %u in bvd %d of array %u\n"
> +			, __func__, n, *n_bvd, ibvd-1, inst);
> +		*vcl = v;
> +		return conf;
> +	}
> +bad:
> +	pr_err("%s: Could't find disk %d in array %u\n", __func__, n, inst);
>  	return NULL;
>  }
>  #endif
> @@ -3769,9 +3832,11 @@ static int ddf_set_array_state(struct active_array *a, int consistent)
>  static void ddf_set_disk(struct active_array *a, int n, int state)
>  {
>  	struct ddf_super *ddf = a->container->sb;
> -	unsigned int inst = a->info.container_member;
> -	struct vd_config *vc = find_vdcr(ddf, inst);
> -	int pd = find_phys(ddf, vc->phys_refnum[n]);
> +	unsigned int inst = a->info.container_member, n_bvd;
> +	struct vcl *vcl;
> +	struct vd_config *vc = find_vdcr(ddf, inst, (unsigned int)n,
> +					 &n_bvd, &vcl);
> +	int pd;
>  	int i, st, working;
>  	struct mdinfo *mdi;
>  	struct dl *dl;
> @@ -3796,15 +3861,21 @@ static void ddf_set_disk(struct active_array *a, int n, int state)
>  	if (!dl)
>  		return;
>  
> +	pd = find_phys(ddf, vc->phys_refnum[n_bvd]);
>  	if (pd < 0 || pd != dl->pdnum) {
>  		/* disk doesn't currently exist or has changed.
>  		 * If it is now in_sync, insert it. */
> +		dprintf("%s: phys disk not found for %d: %d/%d ref %08x\n",
> +			__func__, dl->pdnum, dl->major, dl->minor,
> +			dl->disk.refnum);
> +		dprintf("%s: array %u disk %u ref %08x pd %d\n",
> +			__func__, inst, n_bvd, vc->phys_refnum[n_bvd], pd);
>  		if ((state & DS_INSYNC) && ! (state & DS_FAULTY)) {
> -			struct vcl *vcl;
> -			pd = dl->pdnum;
> -			vc->phys_refnum[n] = dl->disk.refnum;
> -			vcl = container_of(vc, struct vcl, conf);
> -			vcl->lba_offset[n] = mdi->data_offset;
> +			__u64 *lba_offset;
> +			pd = dl->pdnum; /* FIXME: is this really correct ? */
> +			vc->phys_refnum[n_bvd] = dl->disk.refnum;
> +			lba_offset = (__u64 *)&vc->phys_refnum[ddf->mppe];
> +			lba_offset[n_bvd] = mdi->data_offset;
>  			ddf->phys->entries[pd].type &=
>  				~__cpu_to_be16(DDF_Global_Spare);
>  			ddf->phys->entries[pd].type |=
> @@ -4187,8 +4258,10 @@ static struct mdinfo *ddf_activate_spare(struct active_array *a,
>  	struct metadata_update *mu;
>  	struct dl *dl;
>  	int i;
> +	struct vcl *vcl;
>  	struct vd_config *vc;
>  	__u64 *lba;
> +	unsigned int n_bvd;
>  
>  	for (d = a->info.devs ; d ; d = d->next) {
>  		if ((d->curr_state & DS_FAULTY) &&
> @@ -4353,7 +4426,8 @@ static struct mdinfo *ddf_activate_spare(struct active_array *a,
>  	mu->space = NULL;
>  	mu->space_list = NULL;
>  	mu->next = *updates;
> -	vc = find_vdcr(ddf, a->info.container_member);
> +	vc = find_vdcr(ddf, a->info.container_member, di->disk.raid_disk,
> +		       &n_bvd, &vcl);
>  	memcpy(mu->buf, vc, ddf->conf_rec_len * 512);
>  
>  	vc = (struct vd_config*)mu->buf;


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

Re: [PATCH 12/27] DDF: layout_md2ddf: new md->DDF layout conversion

[NeilBrown]

[-- Attachment #1: Type: text/plain, Size: 6910 bytes --]

On Wed,  3 Jul 2013 22:27:52 +0200 mwilck@arcor.de wrote:

> Support for RAID 10 makes it necessary to rewrite the algorithm
> for deriving DDF layout from MD layout. The functions level_to_prl
> and layout_to_rlq are combined in a single function that takes
> md layout parameters and converts them to DDF.
> 
> Signed-off-by: Martin Wilck <mwilck@arcor.de>
> ---
>  super-ddf.c |  178 ++++++++++++++++++++++++++++++++++++++---------------------
>  1 files changed, 114 insertions(+), 64 deletions(-)
> 
> diff --git a/super-ddf.c b/super-ddf.c
> index a43401e..933485d 100644
> --- a/super-ddf.c
> +++ b/super-ddf.c
> @@ -484,6 +484,107 @@ static unsigned int calc_crc(void *buf, int len)
>  	return __cpu_to_be32(newcrc);
>  }
>  
> +#define DDF_INVALID_LEVEL 0xff
> +#define DDF_NO_SECONDARY 0xff
> +static int err_bad_md_layout(const mdu_array_info_t *array)
> +{
> +	pr_err("RAID%d layout %x with %d disks is unsupported for DDF\n",
> +	       array->level, array->layout, array->raid_disks);
> +	return DDF_INVALID_LEVEL;
> +}
> +
> +static int layout_md2ddf(const mdu_array_info_t *array,
> +			 struct vd_config *conf)
> +{
> +	__u16 prim_elmnt_count = __cpu_to_be16(array->raid_disks);
> +	__u8 prl = DDF_INVALID_LEVEL, rlq = 0;
> +	__u8 sec_elmnt_count = 1;
> +	__u8 srl = DDF_NO_SECONDARY;
> +
> +	switch (array->level) {
> +	case LEVEL_LINEAR:
> +		prl = DDF_CONCAT;
> +		break;
> +	case 0:
> +		rlq = DDF_RAID0_SIMPLE;
> +		prl = DDF_RAID0;
> +		break;
> +	case 1:
> +		switch (array->raid_disks) {
> +		case 2:
> +			rlq = DDF_RAID1_SIMPLE;
> +			break;
> +		case 3:
> +			rlq = DDF_RAID1_MULTI;
> +			break;
> +		default:
> +			return err_bad_md_layout(array);
> +		}
> +		prl = DDF_RAID1;
> +		break;
> +	case 4:
> +		if (array->layout != 0)
> +			return err_bad_md_layout(array);
> +		rlq = DDF_RAID4_N;
> +		prl = DDF_RAID4;
> +		break;
> +	case 5:
> +		switch (array->layout) {
> +		case ALGORITHM_LEFT_ASYMMETRIC:
> +			rlq = DDF_RAID5_N_RESTART;
> +			break;
> +		case ALGORITHM_RIGHT_ASYMMETRIC:
> +			rlq = DDF_RAID5_0_RESTART;
> +			break;
> +		case ALGORITHM_LEFT_SYMMETRIC:
> +			rlq = DDF_RAID5_N_CONTINUE;
> +			break;
> +		case ALGORITHM_RIGHT_SYMMETRIC:
> +			/* not mentioned in standard */
> +			return err_bad_md_layout(array);

You need a 'default:' case here to throw an error.

> +		}
> +		prl = DDF_RAID5;
> +		break;
> +	case 6:
> +		switch (array->layout) {
> +		case ALGORITHM_ROTATING_N_RESTART:
> +			rlq = DDF_RAID5_N_RESTART;
> +			break;
> +		case ALGORITHM_ROTATING_ZERO_RESTART:
> +			rlq = DDF_RAID6_0_RESTART;
> +			break;
> +		case ALGORITHM_ROTATING_N_CONTINUE:
> +			rlq = DDF_RAID5_N_CONTINUE;
> +			break;
> +		}

Ditto here.

I've made those two changes to the patch.

NeilBrown


> +		prl = DDF_RAID6;
> +		break;
> +	case 10:
> +		if (array->raid_disks % 2 == 0 && array->layout == 0x102) {
> +			rlq = DDF_RAID1_SIMPLE;
> +			prim_elmnt_count =  __cpu_to_be16(2);
> +			sec_elmnt_count = array->raid_disks / 2;
> +		} else if (array->raid_disks % 3 == 0
> +			   && array->layout == 0x103) {
> +			rlq = DDF_RAID1_MULTI;
> +			prim_elmnt_count =  __cpu_to_be16(3);
> +			sec_elmnt_count = array->raid_disks / 3;
> +		} else
> +			return err_bad_md_layout(array);
> +		srl = DDF_2SPANNED;
> +		prl = DDF_RAID1;
> +		break;
> +	default:
> +		return err_bad_md_layout(array);
> +	}
> +	conf->prl = prl;
> +	conf->prim_elmnt_count = prim_elmnt_count;
> +	conf->rlq = rlq;
> +	conf->srl = srl;
> +	conf->sec_elmnt_count = sec_elmnt_count;
> +	return 0;
> +}
> +
>  static int err_bad_ddf_layout(const struct vd_config *conf)
>  {
>  	pr_err("DDF RAID %u qualifier %u with %u disks is unsupported\n",
> @@ -2150,59 +2251,6 @@ static int chunk_to_shift(int chunksize)
>  	return ffs(chunksize/512)-1;
>  }
>  
> -static int level_to_prl(int level)
> -{
> -	switch (level) {
> -	case LEVEL_LINEAR: return DDF_CONCAT;
> -	case 0: return DDF_RAID0;
> -	case 1: return DDF_RAID1;
> -	case 4: return DDF_RAID4;
> -	case 5: return DDF_RAID5;
> -	case 6: return DDF_RAID6;
> -	default: return -1;
> -	}
> -}
> -
> -static int layout_to_rlq(int level, int layout, int raiddisks)
> -{
> -	switch(level) {
> -	case 0:
> -		return DDF_RAID0_SIMPLE;
> -	case 1:
> -		switch(raiddisks) {
> -		case 2: return DDF_RAID1_SIMPLE;
> -		case 3: return DDF_RAID1_MULTI;
> -		default: return -1;
> -		}
> -	case 4:
> -		switch(layout) {
> -		case 0: return DDF_RAID4_N;
> -		}
> -		break;
> -	case 5:
> -		switch(layout) {
> -		case ALGORITHM_LEFT_ASYMMETRIC:
> -			return DDF_RAID5_N_RESTART;
> -		case ALGORITHM_RIGHT_ASYMMETRIC:
> -			return DDF_RAID5_0_RESTART;
> -		case ALGORITHM_LEFT_SYMMETRIC:
> -			return DDF_RAID5_N_CONTINUE;
> -		case ALGORITHM_RIGHT_SYMMETRIC:
> -			return -1; /* not mentioned in standard */
> -		}
> -	case 6:
> -		switch(layout) {
> -		case ALGORITHM_ROTATING_N_RESTART:
> -			return DDF_RAID5_N_RESTART;
> -		case ALGORITHM_ROTATING_ZERO_RESTART:
> -			return DDF_RAID6_0_RESTART;
> -		case ALGORITHM_ROTATING_N_CONTINUE:
> -			return DDF_RAID5_N_CONTINUE;
> -		}
> -	}
> -	return -1;
> -}
> -
>  #ifndef MDASSEMBLE
>  struct extent {
>  	unsigned long long start, size;
> @@ -2320,13 +2368,15 @@ static int init_super_ddf_bvd(struct supertype *st,
>  	vc->timestamp = __cpu_to_be32(time(0)-DECADE);
>  	vc->seqnum = __cpu_to_be32(1);
>  	memset(vc->pad0, 0xff, 24);
> -	vc->prim_elmnt_count = __cpu_to_be16(info->raid_disks);
>  	vc->chunk_shift = chunk_to_shift(info->chunk_size);
> -	vc->prl = level_to_prl(info->level);
> -	vc->rlq = layout_to_rlq(info->level, info->layout, info->raid_disks);
> -	vc->sec_elmnt_count = 1;
> +	if (layout_md2ddf(info, vc) == -1 ||
> +		__be16_to_cpu(vc->prim_elmnt_count) > ddf->mppe) {
> +		pr_err("%s: unsupported RAID level/layout %d/%d with %d disks\n",
> +		       __func__, info->level, info->layout, info->raid_disks);
> +		free(vcl);
> +		return 0;
> +	}
>  	vc->sec_elmnt_seq = 0;
> -	vc->srl = 0;
>  	vc->blocks = __cpu_to_be64(info->size * 2);
>  	vc->array_blocks = __cpu_to_be64(
>  		calc_array_size(info->level, info->raid_disks, info->layout,
> @@ -2990,12 +3040,12 @@ static int validate_geometry_ddf(struct supertype *st,
>  	}
>  
>  	if (!dev) {
> -		/* Initial sanity check.  Exclude illegal levels. */
> -		int i;
> -		for (i=0; ddf_level_num[i].num1 != MAXINT; i++)
> -			if (ddf_level_num[i].num2 == level)
> -				break;
> -		if (ddf_level_num[i].num1 == MAXINT) {
> +		mdu_array_info_t array = {
> +			.level = level, .layout = layout,
> +			.raid_disks = raiddisks
> +		};
> +		struct vd_config conf;
> +		if (layout_md2ddf(&array, &conf) == -1) {
>  			if (verbose)
>  				pr_err("DDF does not support level %d arrays\n",
>  				       level);


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

Re: [PATCH 11/27] DDF: layout_ddf2md: new DDF->md RAID layout conversion

[NeilBrown]

[-- Attachment #1: Type: text/plain, Size: 7867 bytes --]

On Wed,  3 Jul 2013 22:27:51 +0200 mwilck@arcor.de wrote:

> layout_ddf2md() is a new RAID layout conversion routine.
> It obsoletes the previous separate routines for obtaining
> md level and layout (map_num1, rlq_to_layout).
> 
> Signed-off-by: Martin Wilck <mwilck@arcor.de>
> ---
>  super-ddf.c |  197 ++++++++++++++++++++++++++++++-----------------------------
>  1 files changed, 100 insertions(+), 97 deletions(-)
> 
> diff --git a/super-ddf.c b/super-ddf.c
> index 0f955be..a43401e 100644
> --- a/super-ddf.c
> +++ b/super-ddf.c
> @@ -484,6 +484,102 @@ static unsigned int calc_crc(void *buf, int len)
>  	return __cpu_to_be32(newcrc);
>  }
>  
> +static int err_bad_ddf_layout(const struct vd_config *conf)
> +{
> +	pr_err("DDF RAID %u qualifier %u with %u disks is unsupported\n",
> +	       conf->prl, conf->rlq, __be16_to_cpu(conf->prim_elmnt_count));
> +	return -1;
> +}
> +
> +static int layout_ddf2md(const struct vd_config *conf,
> +			 mdu_array_info_t *array)
> +{
> +	int level = LEVEL_UNSUPPORTED;
> +	int layout = 0;
> +	int raiddisks = __be16_to_cpu(conf->prim_elmnt_count);
> +
> +	if (conf->sec_elmnt_count > 1) {
> +		/* see also check_secondary() */
> +		if (conf->prl != DDF_RAID1 ||
> +		    (conf->srl != DDF_2STRIPED && conf->srl != DDF_2SPANNED)) {
> +			pr_err("Unsupported secondary RAID level %u/%u\n",
> +			       conf->prl, conf->srl);
> +			return -1;
> +		}
> +		if (raiddisks == 2 && conf->rlq == DDF_RAID1_SIMPLE)
> +			layout = 0x102;
> +		else if  (raiddisks == 3 && conf->rlq == DDF_RAID1_MULTI)
> +			layout = 0x103;
> +		else
> +			return err_bad_ddf_layout(conf);
> +		raiddisks *= conf->sec_elmnt_count;
> +		level = 10;
> +		goto good;
> +	}
> +
> +	switch (conf->prl) {
> +	case DDF_CONCAT:
> +		level = LEVEL_LINEAR;
> +		break;
> +	case DDF_RAID0:
> +		if (conf->rlq != DDF_RAID0_SIMPLE)
> +			return err_bad_ddf_layout(conf);
> +		level = 0;
> +		break;
> +	case DDF_RAID1:
> +		if (!((conf->rlq == DDF_RAID1_SIMPLE && raiddisks == 2) ||
> +		      (conf->rlq == DDF_RAID1_MULTI && raiddisks == 3)))
> +			return err_bad_ddf_layout(conf);
> +		level = 1;
> +		break;
> +	case DDF_RAID4:
> +		if (conf->rlq != DDF_RAID4_N)
> +			return err_bad_ddf_layout(conf);
> +		level = 4;
> +		break;
> +	case DDF_RAID5:
> +		switch (conf->rlq) {
> +		case DDF_RAID5_N_RESTART:
> +			layout = ALGORITHM_LEFT_ASYMMETRIC;
> +			break;
> +		case DDF_RAID5_0_RESTART:
> +			layout = ALGORITHM_RIGHT_ASYMMETRIC;
> +			break;
> +		case DDF_RAID5_N_CONTINUE:
> +			layout = ALGORITHM_LEFT_SYMMETRIC;
> +			break;
> +		default:
> +			return err_bad_ddf_layout(conf);
> +		}
> +		level = 5;
> +		break;
> +	case DDF_RAID6:
> +		switch (conf->rlq) {
> +		case DDF_RAID5_N_RESTART:
> +			layout = ALGORITHM_ROTATING_N_RESTART;
> +			break;
> +		case DDF_RAID6_0_RESTART:
> +			layout = ALGORITHM_ROTATING_ZERO_RESTART;
> +			break;
> +		case DDF_RAID5_N_CONTINUE:
> +			layout = ALGORITHM_ROTATING_N_CONTINUE;
> +			break;
> +		default:
> +			return err_bad_ddf_layout(conf);
> +		}
> +		level = 6;
> +		break;
> +	default:
> +		return err_bad_ddf_layout(conf);
> +	};
> +
> +good:
> +	array->level = level;
> +	array->layout = layout;
> +	array->raid_disks = raiddisks;
> +	return 0;
> +}
> +
>  static int load_ddf_header(int fd, unsigned long long lba,
>  			   unsigned long long size,
>  			   int type,
> @@ -1032,33 +1128,6 @@ static mapping_t ddf_sec_level[] = {
>  };
>  #endif
>  
> -struct num_mapping {
> -	int num1, num2;
> -};
> -static struct num_mapping ddf_level_num[] = {
> -	{ DDF_RAID0, 0 },
> -	{ DDF_RAID1, 1 },
> -	{ DDF_RAID3, LEVEL_UNSUPPORTED },
> -	{ DDF_RAID4, 4 },
> -	{ DDF_RAID5, 5 },
> -	{ DDF_RAID1E, LEVEL_UNSUPPORTED },
> -	{ DDF_JBOD, LEVEL_UNSUPPORTED },
> -	{ DDF_CONCAT, LEVEL_LINEAR },
> -	{ DDF_RAID5E, LEVEL_UNSUPPORTED },
> -	{ DDF_RAID5EE, LEVEL_UNSUPPORTED },
> -	{ DDF_RAID6, 6},
> -	{ MAXINT, MAXINT }
> -};

Ooops.  You removed that while there was still a user.
Code must compile after each patch!
I've delayed the removal of this structure to the next patch, which removes
the last user.

NeilBrown


> -
> -static int map_num1(struct num_mapping *map, int num)
> -{
> -	int i;
> -	for (i=0 ; map[i].num1 != MAXINT; i++)
> -		if (map[i].num1 == num)
> -			break;
> -	return map[i].num2;
> -}
> -
>  static int all_ff(const char *guid)
>  {
>  	int i;
> @@ -1638,8 +1707,6 @@ static void getinfo_super_ddf(struct supertype *st, struct mdinfo *info, char *m
>  	}
>  }
>  
> -static int rlq_to_layout(int rlq, int prl, int raiddisks);
> -
>  static void getinfo_super_ddf_bvd(struct supertype *st, struct mdinfo *info, char *map)
>  {
>  	struct ddf_super *ddf = st->sb;
> @@ -1651,12 +1718,8 @@ static void getinfo_super_ddf_bvd(struct supertype *st, struct mdinfo *info, cha
>  	__u32 *cptr;
>  
>  	memset(info, 0, sizeof(*info));
> -	/* FIXME this returns BVD info - what if we want SVD ?? */
> -
> -	info->array.raid_disks    = __be16_to_cpu(vc->conf.prim_elmnt_count);
> -	info->array.level	  = map_num1(ddf_level_num, vc->conf.prl);
> -	info->array.layout	  = rlq_to_layout(vc->conf.rlq, vc->conf.prl,
> -						  info->array.raid_disks);
> +	if (layout_ddf2md(&vc->conf, &info->array) == -1)
> +		return;
>  	info->array.md_minor	  = -1;
>  	cptr = (__u32 *)(vc->conf.guid + 16);
>  	info->array.ctime	  = DECADE + __be32_to_cpu(*cptr);
> @@ -2140,48 +2203,6 @@ static int layout_to_rlq(int level, int layout, int raiddisks)
>  	return -1;
>  }
>  
> -static int rlq_to_layout(int rlq, int prl, int raiddisks)
> -{
> -	switch(prl) {
> -	case DDF_RAID0:
> -		return 0; /* hopefully rlq == DDF_RAID0_SIMPLE */
> -	case DDF_RAID1:
> -		return 0; /* hopefully rlq == SIMPLE or MULTI depending
> -			     on raiddisks*/
> -	case DDF_RAID4:
> -		switch(rlq) {
> -		case DDF_RAID4_N:
> -			return 0;
> -		default:
> -			/* not supported */
> -			return -1; /* FIXME this isn't checked */
> -		}
> -	case DDF_RAID5:
> -		switch(rlq) {
> -		case DDF_RAID5_N_RESTART:
> -			return ALGORITHM_LEFT_ASYMMETRIC;
> -		case DDF_RAID5_0_RESTART:
> -			return ALGORITHM_RIGHT_ASYMMETRIC;
> -		case DDF_RAID5_N_CONTINUE:
> -			return ALGORITHM_LEFT_SYMMETRIC;
> -		default:
> -			return -1;
> -		}
> -	case DDF_RAID6:
> -		switch(rlq) {
> -		case DDF_RAID5_N_RESTART:
> -			return ALGORITHM_ROTATING_N_RESTART;
> -		case DDF_RAID6_0_RESTART:
> -			return ALGORITHM_ROTATING_ZERO_RESTART;
> -		case DDF_RAID5_N_CONTINUE:
> -			return ALGORITHM_ROTATING_N_CONTINUE;
> -		default:
> -			return -1;
> -		}
> -	}
> -	return -1;
> -}
> -
>  #ifndef MDASSEMBLE
>  struct extent {
>  	unsigned long long start, size;
> @@ -3433,26 +3454,8 @@ static struct mdinfo *container_content_ddf(struct supertype *st, char *subarray
>  		this->next = rest;
>  		rest = this;
>  
> -		if (vc->conf.sec_elmnt_count == 1) {
> -			this->array.level = map_num1(ddf_level_num,
> -						     vc->conf.prl);
> -			this->array.raid_disks =
> -				__be16_to_cpu(vc->conf.prim_elmnt_count);
> -			this->array.layout =
> -				rlq_to_layout(vc->conf.rlq, vc->conf.prl,
> -					      this->array.raid_disks);
> -		} else {
> -			/* The only supported layout is RAID 10.
> -			 * Compatibility has been checked in check_secondary()
> -			 * above.
> -			 */
> -			this->array.level = 10;
> -			this->array.raid_disks =
> -				__be16_to_cpu(vc->conf.prim_elmnt_count)
> -				* vc->conf.sec_elmnt_count;
> -			this->array.layout = 0x100 |
> -				__be16_to_cpu(vc->conf.prim_elmnt_count);
> -		}
> +		if (layout_ddf2md(&vc->conf, &this->array))
> +			continue;
>  		this->array.md_minor      = -1;
>  		this->array.major_version = -1;
>  		this->array.minor_version = -2;


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

Re: DDF/RAID10 patch series

[NeilBrown]

[-- Attachment #1: Type: text/plain, Size: 2519 bytes --]

On Wed,  3 Jul 2013 22:27:40 +0200 mwilck@arcor.de wrote:

> This is the DDF/RAID10 patch series I announced.
> 
> The whole patch set is aimed at making --Create work with DDF/RAID10.
> When --Create eventually works, RAID10 can be added to the DDF
> regression test.
> 
> With this patch set, DDF is still far from the level of functionality
> present in IMSM, but getting a little closer.
> 
> The patch set also includes some cleanup patches and debug code which
> I needed to find the bugs in earlier versions, and which might be useful
> later as well.
> 
> Please review and test.
> 
> Regards
> Martin
> 
> Martin Wilck (27):
>   DDF (cleanup): use a common macro for failed searches
>   DDF: check_secondary: fix treatment of missing BVDs
>   DDF: load_ddf_headers: use secondary header as fallback
>   DDF: handle "open flag" according to spec
>   DDF: Implement store_super_ddf
>   DDF: ddf_open_new: implement minimal consistency check
>   DDF: find_vdcr: account for secondary RAID level
>   DDF: ddf_set_disk: move status logic to separate function
>   DDF: get_svd_state: Status logic for secondary RAID level
>   DDF: allow empty slots in virt disk table
>   DDF: layout_ddf2md: new DDF->md RAID layout conversion
>   DDF: layout_md2ddf: new md->DDF layout conversion
>   DDF: Simplify allocation of "other BVDs"
>   DDF: init_super_ddf_bvd: initialize other bvds
>   DDF: validate_geometry_ddf: support RAID10
>   DDF: use LBA_OFFSET macro instead of lba_offset field
>   DDF: get_extents: support secondary RAID level
>   DDF: add_to_super_ddf: allow empty slots in phys disk table
>   DDF: add_to_super_ddf: Use same amount of workspace as other disks
>   DDF: add_to_super_ddf: RAID10 changes
>   DDF: add_to_super_ddf_bvd: use get_svd_state()
>   DDF: getinfo_super_ddf_bvd: lba_offset calculation for RAID10
>   DDF: guid_str: convenience function to print GUID for debugging
>   DDF: ddf_set_array_state: more meaningful output
>   DDF: ddf_process_update: handle update of conf records for SVD
>   DDF: ddf_process_update: Fix vlist treatment for SVDs
>   tests/10ddf-create: add RAID 10 array
> 
>  super-ddf.c        | 1134 +++++++++++++++++++++++++++++++++-------------------
>  tests/10ddf-create |   15 +-
>  2 files changed, 739 insertions(+), 410 deletions(-)


Thanks. I've applied all these (with the changes that I have already
mentioned and a couple of other  tiny things).
I'll do some testing too at some stage.

NeilBrown

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

Re: [PATCH 03/27] DDF: load_ddf_headers: use secondary header as fallback

[Martin Wilck]

On 07/08/2013 07:43 AM, NeilBrown wrote:

> Should that "|| super->active == NULL" test go at the beginning of the 'if'
> rather than the set?  Currently it comparing sequence numbers between the
> successfully read secondary and the (possibly) badly read primary.

That makes certainly sense. Thanks for spotting it.

Martin

Re: [PATCH 07/27] DDF: find_vdcr: account for secondary RAID level

[Martin Wilck]

On 07/08/2013 08:16 AM, NeilBrown wrote:

>> -static struct vd_config *find_vdcr(struct ddf_super *ddf, unsigned int inst)
>> +static int find_index_in_bvd(const struct ddf_super *ddf,
>> +			     const struct vd_config *conf, unsigned int n,
>> +			     unsigned int *n_bvd)
>> +{
>> +	/*
>> +	 * Find the index of the n-th valid physical disk in this BVD
>> +	 */
>> +	unsigned int i, j;
>> +	for (i = 0, j = 0; i < ddf->mppe &&
>> +		     j < __be16_to_cpu(conf->prim_elmnt_count); i++) {
>> +		if (conf->phys_refnum[i] != 0xffffffff) {
> 
> Should that 0xffffffff be DDF_NOTFOUND?

I'd say rather not. Currently the DDF code is using 0xffffffff
explicitly for "invalid refnums" (value from the spec) in several
places. DDF_NOTFOUND is used by my code as a return code for functions
returning "unsigned int". It's similar but not quite the same.

The readability DDF code could be improved by using a symbolic name like
DDF_INVALID_REFNUM = 0xffffffff, and possibly using a different numeric
value for DDF_NOTFOUND. But that's the kind of stuff that I've avoided
so far.

> 
> (I glazed over reading the rest of this patch .. sorry.  Maybe I'll try again
> another day)

Thanks for reading it.
Martin

Re: [PATCH 11/27] DDF: layout_ddf2md: new DDF->md RAID layout conversion

[Martin Wilck]

On 07/08/2013 08:48 AM, NeilBrown wrote:

>> -static struct num_mapping ddf_level_num[] = {
>> -	{ DDF_RAID0, 0 },
>> -	{ DDF_RAID1, 1 },
>> -	{ DDF_RAID3, LEVEL_UNSUPPORTED },
>> -	{ DDF_RAID4, 4 },
>> -	{ DDF_RAID5, 5 },
>> -	{ DDF_RAID1E, LEVEL_UNSUPPORTED },
>> -	{ DDF_JBOD, LEVEL_UNSUPPORTED },
>> -	{ DDF_CONCAT, LEVEL_LINEAR },
>> -	{ DDF_RAID5E, LEVEL_UNSUPPORTED },
>> -	{ DDF_RAID5EE, LEVEL_UNSUPPORTED },
>> -	{ DDF_RAID6, 6},
>> -	{ MAXINT, MAXINT }
>> -};
> 
> Ooops.  You removed that while there was still a user.
> Code must compile after each patch!
> I've delayed the removal of this structure to the next patch, which removes
> the last user.

Sorry for that. It happened when I tried to cleanup + reorder my patches.

Martin

Fixes for the DDF patch series

[mwilck]

Hi Neil,

This series fixes some issues I found with my last series of patches.
The new series is based on your merges as of Jul 8, 2013. 
To avoid confusionwith the previous series, I started numbering at 28.
Moreover, it implements kill_subarray(), so that array creation and
deletion works with DDF.

Basically, there were two problems: 1. Array creation would fail if 
the set of devices was given explicitly rather than doing auto-layout, and 
mdmon was already running; 2. Creation of RAID10 would fail if mdmon
was already running.

29 and 30 are optional, but I think they're correct.
31 implements kill_subarray().
32 and 33 are the fixes for problem 1.
34-38 are just debug messages that I needed to track the problems down.
39 is the fix for problem 2.

Regards
Martin

Martin Wilck (12):
  test/10-ddf-create: fix comments
  Monitor: Don't write metadata in inactive array state
  DDF: write_init_super_ddf: don't zero superblocks for subarrays
  DDF: implement kill_subarray
  DDF: getinfo_super_ddf_bvd: identify disk by refnum
  DDF: getinfo_super_ddf_bvd: fix raid_disk calculation
  DDF: fix endianness of refnum in debug messages
  DDF: add debug message in add_super_ddf_bvd
  DDF: ddf_process_update: add debug messages fore adding VDs
  DDF: guid_str: more readable output
  DDF: ddf_process_update: some more debug messages
  DDF: ddf_process_update: Fix updates for SVDs

 monitor.c          |    3 +-
 super-ddf.c        |  241 ++++++++++++++++++++++++++++++++++++++++++----------
 tests/10ddf-create |    3 +-
 3 files changed, 199 insertions(+), 48 deletions(-)

[PATCH 28/39] test/10-ddf-create: fix comments

[mwilck]

Comments should reflect the changes in the script.
---
 tests/10ddf-create |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/tests/10ddf-create b/tests/10ddf-create
index 6f1f51d..be3f3c3 100644
--- a/tests/10ddf-create
+++ b/tests/10ddf-create
@@ -2,7 +2,8 @@
 # Test basic DDF functionality.
 #
 # Create a container with 5 drives
-# create a small raid0 across them all, then a 2disk raid1
+# create a small raid0 across them all, 
+# then a small raid10 using 4 drives, then a 2disk raid1
 # and a 3disk raid5 using the remaining space
 #
 # add some data, tear down the array, reassemble
-- 
1.7.1

[PATCH 29/39] Monitor: Don't write metadata in inactive array state

[mwilck]

The kernel docs state that meta data is never written in states
clear, inactive, suspended, readonly, and read_auto.
Why should this be different for containers?

We need to write metadata when the array is disabled, though.
Tested with the DDF (10*) and IMSM (9*) tests, works.
---
 monitor.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/monitor.c b/monitor.c
index 47432b2..67b40b1 100644
--- a/monitor.c
+++ b/monitor.c
@@ -416,7 +416,8 @@ static int read_and_act(struct active_array *a)
 	if (sync_completed > a->last_checkpoint)
 		a->last_checkpoint = sync_completed;
 
-	a->container->ss->sync_metadata(a->container);
+	if (deactivate || a->curr_state >= clean)
+		a->container->ss->sync_metadata(a->container);
 	dprintf("%s(%d): state:%s action:%s next(", __func__, a->info.container_member,
 		array_states[a->curr_state], sync_actions[a->curr_action]);
 
-- 
1.7.1

[PATCH 30/39] DDF: write_init_super_ddf: don't zero superblocks for subarrays

[mwilck]

commit d682f344 inserted this call to "Kill" in write_init_super_ddf:

    "Matching the functionality already in super0 and super1, when
    we first create a container, remove any other recognisable metadata to
    ensure it doesn't cause confusion."

But we should do this only at first container creation, not when
subarrays are created later.
---
 super-ddf.c |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/super-ddf.c b/super-ddf.c
index 8f60b72..87584fa 100644
--- a/super-ddf.c
+++ b/super-ddf.c
@@ -2964,8 +2964,9 @@ static int write_init_super_ddf(struct supertype *st)
 		return 0;
 	} else {
 		struct dl *d;
-		for (d = ddf->dlist; d; d=d->next)
-			while (Kill(d->devname, NULL, 0, -1, 1) == 0);
+		if (!currentconf)
+			for (d = ddf->dlist; d; d=d->next)
+				while (Kill(d->devname, NULL, 0, -1, 1) == 0);
 		return __write_init_super_ddf(st);
 	}
 }
-- 
1.7.1

[PATCH 31/39] DDF: implement kill_subarray

[mwilck]

Implement kill_subarray, for mdmon running and not running.

The way Kill_subarray() is implemented, this requires that the
DDF layer uses "currentconf" to remember the last subarray
queried with container_content(), and use it as the one to kill.
I don't like this much but IMSM does it the same way.
---
 super-ddf.c |  106 +++++++++++++++++++++++++++++++++++++++++++++++++++++++----
 1 files changed, 99 insertions(+), 7 deletions(-)

diff --git a/super-ddf.c b/super-ddf.c
index 87584fa..ee4652a 100644
--- a/super-ddf.c
+++ b/super-ddf.c
@@ -3616,7 +3616,8 @@ static struct mdinfo *container_content_ddf(struct supertype *st, char *subarray
 
 		ddf->currentconf = vc;
 		uuid_from_super_ddf(st, this->uuid);
-		ddf->currentconf = NULL;
+		if (!subarray)
+			ddf->currentconf = NULL;
 
 		sprintf(this->text_version, "/%s/%d",
 			st->container_devnm, this->container_member);
@@ -4160,6 +4161,88 @@ static void ddf_sync_metadata(struct supertype *st)
 	dprintf("ddf: sync_metadata\n");
 }
 
+static int del_from_conflist(struct vcl **list, const char *guid)
+{
+	struct vcl **p;
+	int found = 0;
+	for (p = list; p && *p; p = &((*p)->next))
+		if (!memcmp((*p)->conf.guid, guid, DDF_GUID_LEN)) {
+			found = 1;
+			*p = (*p)->next;
+		}
+	return found;
+}
+
+static int _kill_subarray_ddf(struct ddf_super *ddf, const char *guid)
+{
+	struct dl *dl;
+	unsigned int vdnum, i;
+	vdnum = find_vde_by_guid(ddf, guid);
+	if (vdnum == DDF_NOTFOUND) {
+		pr_err("%s: could not find VD %s\n", __func__,
+		       guid_str(guid));
+		return -1;
+	}
+	if (del_from_conflist(&ddf->conflist, guid) == 0) {
+		pr_err("%s: could not find conf %s\n", __func__,
+		       guid_str(guid));
+		return -1;
+	}
+	for (dl = ddf->dlist; dl; dl = dl->next)
+		for (i = 0; i < ddf->max_part; i++)
+			if (dl->vlist[i] != NULL &&
+			    !memcmp(dl->vlist[i]->conf.guid, guid,
+				    DDF_GUID_LEN))
+				dl->vlist[i] = NULL;
+	memset(ddf->virt->entries[vdnum].guid, 0xff, DDF_GUID_LEN);
+	dprintf("%s: deleted %s\n", __func__, guid_str(guid));
+	return 0;
+}
+
+static int kill_subarray_ddf(struct supertype *st)
+{
+	struct ddf_super *ddf = st->sb;
+	/*
+	 *  currentconf is set in container_content_ddf,
+	 *  called with subarray arg
+	 */
+	struct vcl *victim = ddf->currentconf;
+	struct vd_config *conf;
+	ddf->currentconf = NULL;
+	unsigned int vdnum;
+	if (!victim) {
+		pr_err("%s: nothing to kill\n", __func__);
+		return -1;
+	}
+	conf = &victim->conf;
+	vdnum = find_vde_by_guid(ddf, conf->guid);
+	if (vdnum == DDF_NOTFOUND) {
+		pr_err("%s: could not find VD %s\n", __func__,
+		       guid_str(conf->guid));
+		return -1;
+	}
+	if (st->update_tail) {
+		struct virtual_disk *vd;
+		int len = sizeof(struct virtual_disk)
+			+ sizeof(struct virtual_entry);
+		vd = xmalloc(len);
+		if (vd == NULL) {
+			pr_err("%s: failed to allocate %d bytes\n", __func__,
+			       len);
+			return -1;
+		}
+		memset(vd, 0 , len);
+		vd->magic = DDF_VIRT_RECORDS_MAGIC;
+		vd->populated_vdes = 0;
+		memcpy(vd->entries[0].guid, conf->guid, DDF_GUID_LEN);
+		/* we use DDF_state_deleted as marker */
+		vd->entries[0].state = DDF_state_deleted;
+		append_metadata_update(st, vd, len);
+	} else
+		_kill_subarray_ddf(ddf, conf->guid);
+	return 0;
+}
+
 static void ddf_process_update(struct supertype *st,
 			       struct metadata_update *update)
 {
@@ -4262,12 +4345,20 @@ static void ddf_process_update(struct supertype *st,
 			return;
 		vd = (struct virtual_disk*)update->buf;
 
-		ent = find_unused_vde(ddf);
-		if (ent == DDF_NOTFOUND)
-			return;
-		ddf->virt->entries[ent] = vd->entries[0];
-		ddf->virt->populated_vdes = __cpu_to_be16(1 +
-							  __be16_to_cpu(ddf->virt->populated_vdes));
+		if (vd->entries[0].state == DDF_state_deleted) {
+			if (_kill_subarray_ddf(ddf, vd->entries[0].guid))
+				return;
+		} else {
+
+			ent = find_unused_vde(ddf);
+			if (ent == DDF_NOTFOUND)
+				return;
+			ddf->virt->entries[ent] = vd->entries[0];
+			ddf->virt->populated_vdes =
+				__cpu_to_be16(
+					1 + __be16_to_cpu(
+						ddf->virt->populated_vdes));
+		}
 		ddf_set_updates_pending(ddf);
 		break;
 
@@ -4700,6 +4791,7 @@ struct superswitch super_ddf = {
 	.match_metadata_desc = match_metadata_desc_ddf,
 	.container_content = container_content_ddf,
 	.default_geometry = default_geometry_ddf,
+	.kill_subarray  = kill_subarray_ddf,
 
 	.external	= 1,
 
-- 
1.7.1

[PATCH 32/39] DDF: getinfo_super_ddf_bvd: identify disk by refnum

[mwilck]

Use refnum rather than raiddisk for identifying the physical disk.
raiddisk should only be used for auto-layout.
---
 super-ddf.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/super-ddf.c b/super-ddf.c
index ee4652a..bff420c 100644
--- a/super-ddf.c
+++ b/super-ddf.c
@@ -1878,7 +1878,7 @@ static void getinfo_super_ddf_bvd(struct supertype *st, struct mdinfo *info, cha
 	}
 
 	for (dl = ddf->dlist; dl ; dl = dl->next)
-		if (dl->raiddisk == ddf->currentdev)
+		if (dl->disk.refnum == conf->phys_refnum[cd])
 			break;
 
 	info->disk.major = 0;
-- 
1.7.1

[PATCH 33/39] DDF: getinfo_super_ddf_bvd: fix raid_disk calculation

[mwilck]

The return value of disk.raid_disk may be wrong.
The old code was using raiddisk, which is only valid with auto
layout. This leads to errors when arrays are created with
specified disks and mdmon is already running, like this:

mdadm -CR /dev/md/container -n5 $d1 $d2 $d3 $d4 $d5
mdadm -CR /dev/md/r5 -n5 -l5 /dev/md/container -z 5000
mdadm -CR /dev/md/r1 -n2 -l1 $d1 $d2
  => resulting array will use wrong disks

This patch fixes that.
---
 super-ddf.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/super-ddf.c b/super-ddf.c
index bff420c..7da4ce0 100644
--- a/super-ddf.c
+++ b/super-ddf.c
@@ -1887,7 +1887,8 @@ static void getinfo_super_ddf_bvd(struct supertype *st, struct mdinfo *info, cha
 	if (dl) {
 		info->disk.major = dl->major;
 		info->disk.minor = dl->minor;
-		info->disk.raid_disk = dl->raiddisk;
+		info->disk.raid_disk = cd + conf->sec_elmnt_seq
+			* __be16_to_cpu(conf->prim_elmnt_count);
 		info->disk.number = dl->pdnum;
 		info->disk.state = (1<<MD_DISK_SYNC)|(1<<MD_DISK_ACTIVE);
 	}
-- 
1.7.1

[PATCH 34/39] DDF: fix endianness of refnum in debug messages

[mwilck]

This makes it easier to match the debug output to existing
structures.
---
 super-ddf.c |   10 ++++++----
 1 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/super-ddf.c b/super-ddf.c
index 7da4ce0..c96fac5 100644
--- a/super-ddf.c
+++ b/super-ddf.c
@@ -2805,7 +2805,8 @@ static int __write_ddf_structure(struct dl *d, struct ddf_super *ddf, __u8 type,
 		}
 		if (c) {
 			dprintf("writing conf record %i on disk %08x for %s/%u\n",
-				i, d->disk.refnum, guid_str(vdc->guid),
+				i, __be32_to_cpu(d->disk.refnum),
+				guid_str(vdc->guid),
 				vdc->sec_elmnt_seq);
 			vdc->seqnum = header->seq;
 			vdc->crc = calc_crc(vdc, conf_size);
@@ -3894,7 +3895,7 @@ static int compare_super_ddf(struct supertype *st, struct supertype *tst)
 		}
 		first->dlist = dl1;
 		dprintf("%s: added disk %d: %08x\n", __func__, dl1->pdnum,
-			dl1->disk.refnum);
+			__be32_to_cpu(dl1->disk.refnum));
 	}
 
 	return 0;
@@ -4097,7 +4098,7 @@ static void ddf_set_disk(struct active_array *a, int n, int state)
 		 * If it is now in_sync, insert it. */
 		dprintf("%s: phys disk not found for %d: %d/%d ref %08x\n",
 			__func__, dl->pdnum, dl->major, dl->minor,
-			dl->disk.refnum);
+			__be32_to_cpu(dl->disk.refnum));
 		dprintf("%s: array %u disk %u ref %08x pd %d\n",
 			__func__, inst, n_bvd, vc->phys_refnum[n_bvd], pd);
 		if ((state & DS_INSYNC) && ! (state & DS_FAULTY)) {
@@ -4432,7 +4433,8 @@ static void ddf_process_update(struct supertype *st,
 				if (dn == DDF_NOTFOUND)
 					continue;
 				dprintf("dev %d/%08x has %s (sec=%u) at %d\n",
-					dl->pdnum, dl->disk.refnum,
+					dl->pdnum,
+					__be32_to_cpu(dl->disk.refnum),
 					guid_str(conf->guid),
 					conf->sec_elmnt_seq, vn);
 				/* Clear the Transition flag */
-- 
1.7.1

[PATCH 35/39] DDF: add debug message in add_super_ddf_bvd

[mwilck]

---
 super-ddf.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/super-ddf.c b/super-ddf.c
index c96fac5..1b9944f 100644
--- a/super-ddf.c
+++ b/super-ddf.c
@@ -2551,6 +2551,10 @@ static void add_to_super_ddf_bvd(struct supertype *st,
 		| get_svd_state(ddf, ddf->currentconf);
 	ddf->phys->entries[dl->pdnum].type &= ~__cpu_to_be16(DDF_Global_Spare);
 	ddf->phys->entries[dl->pdnum].type |= __cpu_to_be16(DDF_Active_in_VD);
+	dprintf("%s: added disk %d/%08x to VD %d/%s as disk %d\n",
+		__func__, dl->pdnum, __be32_to_cpu(dl->disk.refnum),
+		ddf->currentconf->vcnum, guid_str(vc->guid),
+		dk->raid_disk);
 	ddf_set_updates_pending(ddf);
 }
 
-- 
1.7.1

[PATCH 36/39] DDF: ddf_process_update: add debug messages fore adding VDs

[mwilck]

Add some debug messages for the DDF_VIRTR_RECORDS_MAGIC case.
---
 super-ddf.c |    9 +++++++++
 1 files changed, 9 insertions(+), 0 deletions(-)

diff --git a/super-ddf.c b/super-ddf.c
index 1b9944f..1cb0a9f 100644
--- a/super-ddf.c
+++ b/super-ddf.c
@@ -4356,6 +4356,13 @@ static void ddf_process_update(struct supertype *st,
 				return;
 		} else {
 
+			ent = find_vde_by_guid(ddf, vd->entries[0].guid);
+			if (ent != DDF_NOTFOUND) {
+				dprintf("%s: VD %s exists already in slot %d\n",
+					__func__, guid_str(vd->entries[0].guid),
+					ent);
+				return;
+			}
 			ent = find_unused_vde(ddf);
 			if (ent == DDF_NOTFOUND)
 				return;
@@ -4364,6 +4371,8 @@ static void ddf_process_update(struct supertype *st,
 				__cpu_to_be16(
 					1 + __be16_to_cpu(
 						ddf->virt->populated_vdes));
+			dprintf("%s: added VD %s in slot %d\n",
+				__func__, guid_str(vd->entries[0].guid), ent);
 		}
 		ddf_set_updates_pending(ddf);
 		break;
-- 
1.7.1

[PATCH 37/39] DDF: guid_str: more readable output

[mwilck]

Print ASCII characters as ASCII
---
 super-ddf.c |    9 +++++++--
 1 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/super-ddf.c b/super-ddf.c
index 1cb0a9f..428e8f3 100644
--- a/super-ddf.c
+++ b/super-ddf.c
@@ -1301,8 +1301,13 @@ static const char *guid_str(const char *guid)
 	static char buf[DDF_GUID_LEN*2+1];
 	int i;
 	char *p = buf;
-	for (i = 0; i < DDF_GUID_LEN; i++)
-		p += sprintf(p, "%02x", (unsigned char)guid[i]);
+	for (i = 0; i < DDF_GUID_LEN; i++) {
+		unsigned char c = guid[i];
+		if (c >= 32 && c < 127)
+			p += sprintf(p, "%c", c);
+		else
+			p += sprintf(p, "%02x", c);
+	}
 	*p = '\0';
 	return (const char *) buf;
 }
-- 
1.7.1

[PATCH 38/39] DDF: ddf_process_update: some more debug messages

[mwilck]

---
 super-ddf.c |   11 ++++++-----
 1 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/super-ddf.c b/super-ddf.c
index 428e8f3..b80bb3b 100644
--- a/super-ddf.c
+++ b/super-ddf.c
@@ -4376,15 +4376,15 @@ static void ddf_process_update(struct supertype *st,
 				__cpu_to_be16(
 					1 + __be16_to_cpu(
 						ddf->virt->populated_vdes));
-			dprintf("%s: added VD %s in slot %d\n",
-				__func__, guid_str(vd->entries[0].guid), ent);
+			dprintf("%s: added VD %s in slot %d(s=%02x i=%02x)\n",
+				__func__, guid_str(vd->entries[0].guid), ent,
+				ddf->virt->entries[ent].state,
+				ddf->virt->entries[ent].init_state);
 		}
 		ddf_set_updates_pending(ddf);
 		break;
 
 	case DDF_VD_CONF_MAGIC:
-		dprintf("len %d %d\n", update->len, ddf->conf_rec_len);
-
 		mppe = __be16_to_cpu(ddf->anchor.max_primary_element_entries);
 		if ((unsigned)update->len != ddf->conf_rec_len * 512)
 			return;
@@ -4392,7 +4392,8 @@ static void ddf_process_update(struct supertype *st,
 		for (vcl = ddf->conflist; vcl ; vcl = vcl->next)
 			if (memcmp(vcl->conf.guid, vc->guid, DDF_GUID_LEN) == 0)
 				break;
-		dprintf("vcl = %p\n", vcl);
+		dprintf("%s: conf update for %s (%s)\n", __func__,
+			guid_str(vc->guid), (vcl ? "old" : "new"));
 		if (vcl) {
 			/* An update, just copy the phys_refnum and lba_offset
 			 * fields
-- 
1.7.1

[PATCH 39/39] DDF: ddf_process_update: Fix updates for SVDs

[mwilck]

The "indirect" code path for adding VDs was not working correctly
for secondary RAID level. The "other BVDs" were not transmitted
to mdmon. Thus mdmon wouldn't build up correct information, and
RAID creation would fail when mdmon was already running on the container.

This patch fixes this.
---
 super-ddf.c |   86 +++++++++++++++++++++++++++++++++++++++++------------------
 1 files changed, 60 insertions(+), 26 deletions(-)

diff --git a/super-ddf.c b/super-ddf.c
index b80bb3b..b74cc51 100644
--- a/super-ddf.c
+++ b/super-ddf.c
@@ -2940,7 +2940,8 @@ static int write_init_super_ddf(struct supertype *st)
 		/* queue the virtual_disk and vd_config as metadata updates */
 		struct virtual_disk *vd;
 		struct vd_config *vc;
-		int len;
+		int len, tlen;
+		unsigned int i;
 
 		if (!currentconf) {
 			int len = (sizeof(struct phys_disk) +
@@ -2967,9 +2968,13 @@ static int write_init_super_ddf(struct supertype *st)
 
 		/* Then the vd_config */
 		len = ddf->conf_rec_len * 512;
-		vc = xmalloc(len);
+		tlen = len * currentconf->conf.sec_elmnt_count;
+		vc = xmalloc(tlen);
 		memcpy(vc, &currentconf->conf, len);
-		append_metadata_update(st, vc, len);
+		for (i = 1; i < currentconf->conf.sec_elmnt_count; i++)
+			memcpy((char *)vc + i*len, currentconf->other_bvds[i-1],
+			       len);
+		append_metadata_update(st, vc, tlen);
 
 		/* FIXME I need to close the fds! */
 		return 0;
@@ -4254,6 +4259,27 @@ static int kill_subarray_ddf(struct supertype *st)
 	return 0;
 }
 
+static void copy_matching_bvd(struct ddf_super *ddf,
+			      struct vd_config *conf,
+			      const struct metadata_update *update)
+{
+	unsigned int mppe =
+		__be16_to_cpu(ddf->anchor.max_primary_element_entries);
+	unsigned int len = ddf->conf_rec_len * 512;
+	char *p;
+	struct vd_config *vc;
+	for (p = update->buf; p < update->buf + update->len; p += len) {
+		vc = (struct vd_config *) p;
+		if (vc->sec_elmnt_seq == conf->sec_elmnt_seq) {
+			memcpy(conf->phys_refnum, vc->phys_refnum,
+			       mppe * (sizeof(__u32) + sizeof(__u64)));
+			return;
+		}
+	}
+	pr_err("%s: no match for BVD %d of %s in update\n", __func__,
+	       conf->sec_elmnt_seq, guid_str(conf->guid));
+}
+
 static void ddf_process_update(struct supertype *st,
 			       struct metadata_update *update)
 {
@@ -4293,7 +4319,7 @@ static void ddf_process_update(struct supertype *st,
 	struct dl *dl;
 	unsigned int mppe;
 	unsigned int ent;
-	unsigned int pdnum, pd2;
+	unsigned int pdnum, pd2, len;
 
 	dprintf("Process update %x\n", *magic);
 
@@ -4386,9 +4412,14 @@ static void ddf_process_update(struct supertype *st,
 
 	case DDF_VD_CONF_MAGIC:
 		mppe = __be16_to_cpu(ddf->anchor.max_primary_element_entries);
-		if ((unsigned)update->len != ddf->conf_rec_len * 512)
-			return;
 		vc = (struct vd_config*)update->buf;
+		len = ddf->conf_rec_len * 512;
+		if ((unsigned int)update->len != len * vc->sec_elmnt_count) {
+			pr_err("%s: %s: insufficient data (%d) for %u BVDs\n",
+			       __func__, guid_str(vc->guid), update->len,
+			       vc->sec_elmnt_count);
+			return;
+		}
 		for (vcl = ddf->conflist; vcl ; vcl = vcl->next)
 			if (memcmp(vcl->conf.guid, vc->guid, DDF_GUID_LEN) == 0)
 				break;
@@ -4398,36 +4429,28 @@ static void ddf_process_update(struct supertype *st,
 			/* An update, just copy the phys_refnum and lba_offset
 			 * fields
 			 */
-			struct vd_config *conf = &vcl->conf;
-			if (vcl->other_bvds != NULL &&
-			    conf->sec_elmnt_seq != vc->sec_elmnt_seq) {
-				unsigned int i;
-				for (i = 1; i < conf->sec_elmnt_count; i++)
-					if (vcl->other_bvds[i-1]->sec_elmnt_seq
-					    == vc->sec_elmnt_seq)
-						break;
-				if (i == conf->sec_elmnt_count) {
-					pr_err("%s/DDF_VD_CONF_MAGIC: BVD %u not found\n",
-					       __func__, vc->sec_elmnt_seq);
-					return;
-				}
-				conf = vcl->other_bvds[i-1];
-			}
-			memcpy(conf->phys_refnum, vc->phys_refnum,
-			       mppe * (sizeof(__u32) + sizeof(__u64)));
+			unsigned int i;
+			copy_matching_bvd(ddf, &vcl->conf, update);
+			for (i = 1; i < vc->sec_elmnt_count; i++)
+				copy_matching_bvd(ddf, vcl->other_bvds[i-1],
+						  update);
 		} else {
 			/* A new VD_CONF */
+			unsigned int i;
 			if (!update->space)
 				return;
 			vcl = update->space;
 			update->space = NULL;
 			vcl->next = ddf->conflist;
-			memcpy(&vcl->conf, vc, update->len);
+			memcpy(&vcl->conf, vc, len);
 			ent = find_vde_by_guid(ddf, vc->guid);
 			if (ent == DDF_NOTFOUND)
 				return;
 			vcl->vcnum = ent;
 			ddf->conflist = vcl;
+			for (i = 1; i < vc->sec_elmnt_count; i++)
+				memcpy(vcl->other_bvds[i-1],
+				       update->buf + len * i, len);
 		}
 		/* Set DDF_Transition on all Failed devices - to help
 		 * us detect those that are no longer in use
@@ -4539,11 +4562,22 @@ static void ddf_prepare_update(struct supertype *st,
 	 */
 	struct ddf_super *ddf = st->sb;
 	__u32 *magic = (__u32*)update->buf;
-	if (*magic == DDF_VD_CONF_MAGIC)
+	if (*magic == DDF_VD_CONF_MAGIC) {
+		struct vcl *vcl;
+		struct vd_config *conf = (struct vd_config *) update->buf;
 		if (posix_memalign(&update->space, 512,
 				   offsetof(struct vcl, conf)
-				   + ddf->conf_rec_len * 512) != 0)
+				   + ddf->conf_rec_len * 512) != 0) {
+			update->space = NULL;
+			return;
+		}
+		vcl = update->space;
+		vcl->conf.sec_elmnt_count = conf->sec_elmnt_count;
+		if (alloc_other_bvds(ddf, vcl) != 0) {
+			free(update->space);
 			update->space = NULL;
+		}
+	}
 }
 
 /*
-- 
1.7.1

Re: [PATCH 05/27] DDF: Implement store_super_ddf

[Martin Wilck]

Hi Neil,

>> This patch implements the previously unsupported case where
>> store_super_ddf is called with a non-empty superblock.
>>
>> For DDF, writing meta data to just one disk makes no sense.
>> We would run the risk of writing inconsistent meta data
>> to the devices. So just call __write_init_super_ddf and
>> write to all devices, including the one passed by the caller.
>>
>> This patch assumes that the device to store the superblock on
>> has already been added to the DDF structure. Otherwise, an
>> error message will be emitted.
>>
>> Signed-off-by: Martin Wilck <mwilck@arcor.de>
> 
> I'm not sure I really like this.  If mdadm is calling ->store_super, then it
> wants to write to just one device.  It will quite possibly call
> ->store_super on all devices, and with your change that will write to all
> devices multiple times.
> So maybe a different interface is needed?

The point is the sequence number handling. If the sequence number
increases, it would be dangerous to write only one superblock.

But perhaps it can be done otherwise, I need to examine that further.
First I need a clear understanding what this API is used for.

> What is the big-picture effect of this patch?  What mdadm function now works
> that didn't work before?

To be honest, I don't know. I came up with this patch when scanning
through the code looking for possible problems. This is not necessary to
make mdadm -e ddf -C -l10 work.

Martin

> 
> Thanks,
> NeilBrown
> 
> 
> 
>> ---
>>  super-ddf.c |   41 +++++++++++++++++++++++++++++++++++------
>>  1 files changed, 35 insertions(+), 6 deletions(-)
>>
>> diff --git a/super-ddf.c b/super-ddf.c
>> index b806949..b3c846d 100644
>> --- a/super-ddf.c
>> +++ b/super-ddf.c
>> @@ -3471,15 +3471,44 @@ static int store_super_ddf(struct supertype *st, int fd)
>>  	if (!ddf)
>>  		return 1;
>>  
>> -	/* ->dlist and ->conflist will be set for updates, currently not
>> -	 * supported
>> -	 */
>> -	if (ddf->dlist || ddf->conflist)
>> -		return 1;
>> -
>>  	if (!get_dev_size(fd, NULL, &dsize))
>>  		return 1;
>>  
>> +	if (ddf->dlist || ddf->conflist) {
>> +		struct stat sta;
>> +		struct dl *dl;
>> +		int ofd, ret;
>> +
>> +		if (fstat(fd, &sta) == -1 || !S_ISBLK(sta.st_mode)) {
>> +			pr_err("%s: file descriptor for invalid device\n",
>> +			       __func__);
>> +			return 1;
>> +		}
>> +		for (dl = ddf->dlist; dl; dl = dl->next)
>> +			if (dl->major == (int)major(sta.st_rdev) &&
>> +			    dl->minor == (int)minor(sta.st_rdev))
>> +				break;
>> +		if (!dl) {
>> +			pr_err("%s: couldn't find disk %d/%d\n", __func__,
>> +			       (int)major(sta.st_rdev),
>> +			       (int)minor(sta.st_rdev));
>> +			return 1;
>> +		}
>> +		/*
>> +		   For DDF, writing to just one disk makes no sense.
>> +		   We would run the risk of writing inconsistent meta data
>> +		   to the devices. So just call __write_init_super_ddf and
>> +		   write to all devices, including this one.
>> +		   Use the fd passed to this function, just in case dl->fd
>> +		   is invalid.
>> +		 */
>> +		ofd = dl->fd;
>> +		dl->fd = fd;
>> +		ret =  __write_init_super_ddf(st);
>> +		dl->fd = ofd;
>> +		return ret;
>> +	}
>> +
>>  	if (posix_memalign(&buf, 512, 512) != 0)
>>  		return 1;
>>  	memset(buf, 0, 512);
> 

Re: [PATCH 05/27] DDF: Implement store_super_ddf

[NeilBrown]

[-- Attachment #1: Type: text/plain, Size: 4106 bytes --]

On Wed, 10 Jul 2013 19:37:24 +0200 Martin Wilck <mwilck@arcor.de> wrote:

> Hi Neil,
> 
> >> This patch implements the previously unsupported case where
> >> store_super_ddf is called with a non-empty superblock.
> >>
> >> For DDF, writing meta data to just one disk makes no sense.
> >> We would run the risk of writing inconsistent meta data
> >> to the devices. So just call __write_init_super_ddf and
> >> write to all devices, including the one passed by the caller.
> >>
> >> This patch assumes that the device to store the superblock on
> >> has already been added to the DDF structure. Otherwise, an
> >> error message will be emitted.
> >>
> >> Signed-off-by: Martin Wilck <mwilck@arcor.de>
> > 
> > I'm not sure I really like this.  If mdadm is calling ->store_super, then it
> > wants to write to just one device.  It will quite possibly call
> > ->store_super on all devices, and with your change that will write to all
> > devices multiple times.
> > So maybe a different interface is needed?
> 
> The point is the sequence number handling. If the sequence number
> increases, it would be dangerous to write only one superblock.

->store_super should not change the sequence number (I think).  Its purpose
is to fix up the metadata in some way, typically so that a failed array
doesn't look like it is failed any more, so it can be assembled.
It should really just write back exactly the same superblock except for
whatever changes were explicitly requested via "update_super" (plus checksums
of course).

> 
> But perhaps it can be done otherwise, I need to examine that further.
> First I need a clear understanding what this API is used for.
> 
> > What is the big-picture effect of this patch?  What mdadm function now works
> > that didn't work before?
> 
> To be honest, I don't know. I came up with this patch when scanning
> through the code looking for possible problems. This is not necessary to
> make mdadm -e ddf -C -l10 work.

I've reverted it then - I really don't think it is wanted.

Thanks,
NeilBrown


> 
> Martin
> 
> > 
> > Thanks,
> > NeilBrown
> > 
> > 
> > 
> >> ---
> >>  super-ddf.c |   41 +++++++++++++++++++++++++++++++++++------
> >>  1 files changed, 35 insertions(+), 6 deletions(-)
> >>
> >> diff --git a/super-ddf.c b/super-ddf.c
> >> index b806949..b3c846d 100644
> >> --- a/super-ddf.c
> >> +++ b/super-ddf.c
> >> @@ -3471,15 +3471,44 @@ static int store_super_ddf(struct supertype *st, int fd)
> >>  	if (!ddf)
> >>  		return 1;
> >>  
> >> -	/* ->dlist and ->conflist will be set for updates, currently not
> >> -	 * supported
> >> -	 */
> >> -	if (ddf->dlist || ddf->conflist)
> >> -		return 1;
> >> -
> >>  	if (!get_dev_size(fd, NULL, &dsize))
> >>  		return 1;
> >>  
> >> +	if (ddf->dlist || ddf->conflist) {
> >> +		struct stat sta;
> >> +		struct dl *dl;
> >> +		int ofd, ret;
> >> +
> >> +		if (fstat(fd, &sta) == -1 || !S_ISBLK(sta.st_mode)) {
> >> +			pr_err("%s: file descriptor for invalid device\n",
> >> +			       __func__);
> >> +			return 1;
> >> +		}
> >> +		for (dl = ddf->dlist; dl; dl = dl->next)
> >> +			if (dl->major == (int)major(sta.st_rdev) &&
> >> +			    dl->minor == (int)minor(sta.st_rdev))
> >> +				break;
> >> +		if (!dl) {
> >> +			pr_err("%s: couldn't find disk %d/%d\n", __func__,
> >> +			       (int)major(sta.st_rdev),
> >> +			       (int)minor(sta.st_rdev));
> >> +			return 1;
> >> +		}
> >> +		/*
> >> +		   For DDF, writing to just one disk makes no sense.
> >> +		   We would run the risk of writing inconsistent meta data
> >> +		   to the devices. So just call __write_init_super_ddf and
> >> +		   write to all devices, including this one.
> >> +		   Use the fd passed to this function, just in case dl->fd
> >> +		   is invalid.
> >> +		 */
> >> +		ofd = dl->fd;
> >> +		dl->fd = fd;
> >> +		ret =  __write_init_super_ddf(st);
> >> +		dl->fd = ofd;
> >> +		return ret;
> >> +	}
> >> +
> >>  	if (posix_memalign(&buf, 512, 512) != 0)
> >>  		return 1;
> >>  	memset(buf, 0, 512);
> > 


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

[PATCH 1/3] DDF: increase seq number in ddf_set_updates_pending

[mwilck]

Increase seq number only when there's actually a metadata change.
This is better then increasing it at every write.

This also fixes another endianness bug.
---
 super-ddf.c |   12 +++++++++---
 1 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/super-ddf.c b/super-ddf.c
index a3799b8..e5007b9 100644
--- a/super-ddf.c
+++ b/super-ddf.c
@@ -464,8 +464,14 @@ static void pr_state(struct ddf_super *ddf, const char *msg)
 static void pr_state(const struct ddf_super *ddf, const char *msg) {}
 #endif
 
-#define ddf_set_updates_pending(x) \
-	do { (x)->updates_pending = 1; pr_state(x, __func__); } while (0)
+static void _ddf_set_updates_pending(struct ddf_super *ddf, const char *func)
+{
+	ddf->updates_pending = 1;
+	ddf->active->seq = __cpu_to_be32((__be32_to_cpu(ddf->active->seq)+1));
+	pr_state(ddf, func);
+}
+
+#define ddf_set_updates_pending(x) _ddf_set_updates_pending((x), __func__)
 
 static unsigned int get_pd_index_from_refnum(const struct vcl *vc,
 					     __u32 refnum, unsigned int nmax,
@@ -2867,7 +2873,7 @@ static int __write_init_super_ddf(struct supertype *st)
 	}
 	memset(null_aligned, 0xff, NULL_CONF_SZ);
 
-	seq = ddf->active->seq + 1;
+	seq = ddf->active->seq;
 
 	/* try to write updated metadata,
 	 * if we catch a failure move on to the next disk
-- 
1.7.1

[PATCH 2/3] DDF: make "null_aligned" a static buffer

[mwilck]

Use a static buffer for this "zero page". This makes it easier
to factor out the header writing code.
---
 super-ddf.c |   23 +++++++++++------------
 1 files changed, 11 insertions(+), 12 deletions(-)

diff --git a/super-ddf.c b/super-ddf.c
index e5007b9..efdba6d 100644
--- a/super-ddf.c
+++ b/super-ddf.c
@@ -2756,14 +2756,21 @@ static int remove_from_super_ddf(struct supertype *st, mdu_disk_info_t *dk)
  */
 #define NULL_CONF_SZ	4096
 
-static int __write_ddf_structure(struct dl *d, struct ddf_super *ddf, __u8 type,
-				 char *null_aligned)
+static char *null_aligned;
+static int __write_ddf_structure(struct dl *d, struct ddf_super *ddf, __u8 type)
 {
 	unsigned long long sector;
 	struct ddf_header *header;
 	int fd, i, n_config, conf_size;
 	int ret = 0;
 
+	if (null_aligned == NULL) {
+		if (posix_memalign((void **)&null_aligned, 4096, NULL_CONF_SZ)
+		    != 0)
+			return 0;
+		memset(null_aligned, 0xff, NULL_CONF_SZ);
+	}
+
 	fd = d->fd;
 
 	switch (type) {
@@ -2864,14 +2871,9 @@ static int __write_init_super_ddf(struct supertype *st)
 	int attempts = 0;
 	int successes = 0;
 	unsigned long long size;
-	char *null_aligned;
 	__u32 seq;
 
 	pr_state(ddf, __func__);
-	if (posix_memalign((void**)&null_aligned, 4096, NULL_CONF_SZ) != 0) {
-		return -ENOMEM;
-	}
-	memset(null_aligned, 0xff, NULL_CONF_SZ);
 
 	seq = ddf->active->seq;
 
@@ -2916,12 +2918,10 @@ static int __write_init_super_ddf(struct supertype *st)
 		ddf->anchor.seq = 0xFFFFFFFF; /* no sequencing in anchor */
 		ddf->anchor.crc = calc_crc(&ddf->anchor, 512);
 
-		if (!__write_ddf_structure(d, ddf, DDF_HEADER_PRIMARY,
-					   null_aligned))
+		if (!__write_ddf_structure(d, ddf, DDF_HEADER_PRIMARY))
 			continue;
 
-		if (!__write_ddf_structure(d, ddf, DDF_HEADER_SECONDARY,
-					   null_aligned))
+		if (!__write_ddf_structure(d, ddf, DDF_HEADER_SECONDARY))
 			continue;
 
 		lseek64(fd, (size-1)*512, SEEK_SET);
@@ -2929,7 +2929,6 @@ static int __write_init_super_ddf(struct supertype *st)
 			continue;
 		successes++;
 	}
-	free(null_aligned);
 
 	return attempts != successes;
 }
-- 
1.7.1

[PATCH 3/3] DDF: factor out writing super block to single disk

[mwilck]

Factor out single disk from __write_init_super_ddf to a new function
_write_super_to_disk. Use this function in store_super_ddf.
---
 super-ddf.c |  112 ++++++++++++++++++++++++++++-------------------------------
 1 files changed, 53 insertions(+), 59 deletions(-)

diff --git a/super-ddf.c b/super-ddf.c
index efdba6d..8c6fa6d 100644
--- a/super-ddf.c
+++ b/super-ddf.c
@@ -2864,70 +2864,72 @@ out:
 	return ret;
 }
 
+static int _write_super_to_disk(struct ddf_super *ddf, struct dl *d)
+{
+	unsigned long long size;
+	int fd = d->fd;
+	if (fd < 0)
+		return 0;
+
+	/* We need to fill in the primary, (secondary) and workspace
+	 * lba's in the headers, set their checksums,
+	 * Also checksum phys, virt....
+	 *
+	 * Then write everything out, finally the anchor is written.
+	 */
+	get_dev_size(fd, NULL, &size);
+	size /= 512;
+	if (d->workspace_lba != 0)
+		ddf->anchor.workspace_lba = d->workspace_lba;
+	else
+		ddf->anchor.workspace_lba =
+			__cpu_to_be64(size - 32*1024*2);
+	if (d->primary_lba != 0)
+		ddf->anchor.primary_lba = d->primary_lba;
+	else
+		ddf->anchor.primary_lba =
+			__cpu_to_be64(size - 16*1024*2);
+	if (d->secondary_lba != 0)
+		ddf->anchor.secondary_lba = d->secondary_lba;
+	else
+		ddf->anchor.secondary_lba =
+			__cpu_to_be64(size - 32*1024*2);
+	ddf->anchor.seq = ddf->active->seq;
+	memcpy(&ddf->primary, &ddf->anchor, 512);
+	memcpy(&ddf->secondary, &ddf->anchor, 512);
+
+	ddf->anchor.openflag = 0xFF; /* 'open' means nothing */
+	ddf->anchor.seq = 0xFFFFFFFF; /* no sequencing in anchor */
+	ddf->anchor.crc = calc_crc(&ddf->anchor, 512);
+
+	if (!__write_ddf_structure(d, ddf, DDF_HEADER_PRIMARY))
+		return 0;
+
+	if (!__write_ddf_structure(d, ddf, DDF_HEADER_SECONDARY))
+		return 0;
+
+	lseek64(fd, (size-1)*512, SEEK_SET);
+	if (write(fd, &ddf->anchor, 512) < 0)
+		return 0;
+
+	return 1;
+}
+
 static int __write_init_super_ddf(struct supertype *st)
 {
 	struct ddf_super *ddf = st->sb;
 	struct dl *d;
 	int attempts = 0;
 	int successes = 0;
-	unsigned long long size;
-	__u32 seq;
 
 	pr_state(ddf, __func__);
 
-	seq = ddf->active->seq;
-
 	/* try to write updated metadata,
 	 * if we catch a failure move on to the next disk
 	 */
 	for (d = ddf->dlist; d; d=d->next) {
-		int fd = d->fd;
-
-		if (fd < 0)
-			continue;
-
 		attempts++;
-		/* We need to fill in the primary, (secondary) and workspace
-		 * lba's in the headers, set their checksums,
-		 * Also checksum phys, virt....
-		 *
-		 * Then write everything out, finally the anchor is written.
-		 */
-		get_dev_size(fd, NULL, &size);
-		size /= 512;
-		if (d->workspace_lba != 0)
-			ddf->anchor.workspace_lba = d->workspace_lba;
-		else
-			ddf->anchor.workspace_lba =
-				__cpu_to_be64(size - 32*1024*2);
-		if (d->primary_lba != 0)
-			ddf->anchor.primary_lba = d->primary_lba;
-		else
-			ddf->anchor.primary_lba =
-				__cpu_to_be64(size - 16*1024*2);
-		if (d->secondary_lba != 0)
-			ddf->anchor.secondary_lba = d->secondary_lba;
-		else
-			ddf->anchor.secondary_lba =
-				__cpu_to_be64(size - 32*1024*2);
-		ddf->anchor.seq = seq;
-		memcpy(&ddf->primary, &ddf->anchor, 512);
-		memcpy(&ddf->secondary, &ddf->anchor, 512);
-
-		ddf->anchor.openflag = 0xFF; /* 'open' means nothing */
-		ddf->anchor.seq = 0xFFFFFFFF; /* no sequencing in anchor */
-		ddf->anchor.crc = calc_crc(&ddf->anchor, 512);
-
-		if (!__write_ddf_structure(d, ddf, DDF_HEADER_PRIMARY))
-			continue;
-
-		if (!__write_ddf_structure(d, ddf, DDF_HEADER_SECONDARY))
-			continue;
-
-		lseek64(fd, (size-1)*512, SEEK_SET);
-		if (write(fd, &ddf->anchor, 512) < 0)
-			continue;
-		successes++;
+		successes += _write_super_to_disk(ddf, d);
 	}
 
 	return attempts != successes;
@@ -3729,17 +3731,9 @@ static int store_super_ddf(struct supertype *st, int fd)
 			       (int)minor(sta.st_rdev));
 			return 1;
 		}
-		/*
-		   For DDF, writing to just one disk makes no sense.
-		   We would run the risk of writing inconsistent meta data
-		   to the devices. So just call __write_init_super_ddf and
-		   write to all devices, including this one.
-		   Use the fd passed to this function, just in case dl->fd
-		   is invalid.
-		 */
 		ofd = dl->fd;
 		dl->fd = fd;
-		ret =  __write_init_super_ddf(st);
+		ret = (_write_super_to_disk(ddf, dl) != 1);
 		dl->fd = ofd;
 		return ret;
 	}
-- 
1.7.1

Re: [PATCH 05/27] DDF: Implement store_super_ddf

[Martin Wilck]

On 07/15/2013 07:31 AM, NeilBrown wrote:

> I've reverted it then - I really don't think it is wanted.

AFAICS it's not reverted yet. Sending a new patch series that IMO does
the Right Thing now, and essentially reverts the part you didn't like.

Martin