All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC PATCH 0/5] Add aix lvm partitions support
@ 2013-04-25 21:10 Philippe De Muyter
  2013-04-25 21:10 ` [PATCH 1/5] partitions/msdos.c: end-of-line whitespace and semicolon cleanup Philippe De Muyter
                   ` (4 more replies)
  0 siblings, 5 replies; 20+ messages in thread
From: Philippe De Muyter @ 2013-04-25 21:10 UTC (permalink / raw)
  To: linux-kernel

This patches serie implements a mapping from AIX LVM logical volumes
to linux partitions, for AIX disks from versions AIX 3 & AIX 4,
maybe later versions also, but this has not been tested.

Because the LVM layer of AIX allows logical volumes to be made of
separate so-called physical partitions which can be anywhere on the
disk, and allows up to 256 logical volumes, while Linux allows
only 16 partitions per disk, partitions which must be contiguous,
only a subset of the AIX logical volumes found can be mappedi to linux
partitions, but that's enough to make it usefull in real life.

AIX partitions discovery has been grafted to the msdos partitions
code, because AIX disks often contain an msdos partition table,
and this grafting avoids removal of the AIX protection already
included in msdos.c while avoiding code duplication.

Comments welcome

Philippe

PS: a companion AIX JFS fs read-only driver will follow

[PATCH 1/5] partitions/msdos.c: end-of-line whitespace and semicolon cleanup
[PATCH 2/5] Add aix lvm partitions support files
[PATCH 3/5] partitions/msdos: enumerate also AIX LVM partitions
[PATCH 4/5] partitions/Makefile: compile aix.c if configured.
[PATCH 5/5] partitions/Kconfig: add the AIX_PARTITION entry

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH 1/5] partitions/msdos.c: end-of-line whitespace and semicolon cleanup
  2013-04-25 21:10 [RFC PATCH 0/5] Add aix lvm partitions support Philippe De Muyter
@ 2013-04-25 21:10 ` Philippe De Muyter
  2013-04-25 21:10 ` [PATCH 2/5] Add aix lvm partitions support files Philippe De Muyter
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 20+ messages in thread
From: Philippe De Muyter @ 2013-04-25 21:10 UTC (permalink / raw)
  To: linux-kernel; +Cc: Philippe De Muyter, Jens Axboe

Signed-off-by: Philippe De Muyter <phdm@macqel.be>
Cc: Jens Axboe <axboe@kernel.dk>
---
 block/partitions/msdos.c |   12 ++++++------
 1 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/block/partitions/msdos.c b/block/partitions/msdos.c
index 7681cd2..9bf19e6 100644
--- a/block/partitions/msdos.c
+++ b/block/partitions/msdos.c
@@ -90,7 +90,7 @@ static int aix_magic_present(struct parsed_partitions *state, unsigned char *p)
 		if (d[0] == '_' && d[1] == 'L' && d[2] == 'V' && d[3] == 'M')
 			ret = 1;
 		put_dev_sector(sect);
-	};
+	}
 	return ret;
 }
 
@@ -142,7 +142,7 @@ static void parse_extended(struct parsed_partitions *state,
 			return;
 
 		if (!msdos_magic_present(data + 510))
-			goto done; 
+			goto done;
 
 		p = (struct partition *) (data + 0x1be);
 
@@ -155,7 +155,7 @@ static void parse_extended(struct parsed_partitions *state,
 		 * and OS/2 seems to use all four entries.
 		 */
 
-		/* 
+		/*
 		 * First process the data partition(s)
 		 */
 		for (i=0; i<4; i++, p++) {
@@ -263,7 +263,7 @@ static void parse_solaris_x86(struct parsed_partitions *state,
 }
 
 #if defined(CONFIG_BSD_DISKLABEL)
-/* 
+/*
  * Create devices for BSD partitions listed in a disklabel, under a
  * dos-like partition. See parse_extended() for more information.
  */
@@ -294,7 +294,7 @@ static void parse_bsd(struct parsed_partitions *state,
 
 		if (state->next == state->limit)
 			break;
-		if (p->p_fstype == BSD_FS_UNUSED) 
+		if (p->p_fstype == BSD_FS_UNUSED)
 			continue;
 		bsd_start = le32_to_cpu(p->p_offset);
 		bsd_size = le32_to_cpu(p->p_size);
@@ -441,7 +441,7 @@ static struct {
 	{NEW_SOLARIS_X86_PARTITION, parse_solaris_x86},
 	{0, NULL},
 };
- 
+
 int msdos_partition(struct parsed_partitions *state)
 {
 	sector_t sector_size = bdev_logical_block_size(state->bdev) / 512;
-- 
1.7.1


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 2/5] Add aix lvm partitions support files
  2013-04-25 21:10 [RFC PATCH 0/5] Add aix lvm partitions support Philippe De Muyter
  2013-04-25 21:10 ` [PATCH 1/5] partitions/msdos.c: end-of-line whitespace and semicolon cleanup Philippe De Muyter
@ 2013-04-25 21:10 ` Philippe De Muyter
  2013-04-29  9:37   ` Karel Zak
  2013-04-25 21:10 ` [PATCH 3/5] partitions/msdos: enumerate also AIX LVM partitions Philippe De Muyter
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 20+ messages in thread
From: Philippe De Muyter @ 2013-04-25 21:10 UTC (permalink / raw)
  To: linux-kernel; +Cc: Philippe De Muyter, Jens Axboe

adding partitions/aix.h and partitions/aix.c

Partitions (called Logical Volumes in AIX) can be non-contiguous or
even split on more than one disk.  Altough we detect such partitions,
we cannot describe them to the Linux partitions layer, so we simply
discard them and issue a diagnose message.

Signed-off-by: Philippe De Muyter <phdm@macqel.be>
Cc: Jens Axboe <axboe@kernel.dk>
---
 block/partitions/aix.c |  279 ++++++++++++++++++++++++++++++++++++++++++++++++
 block/partitions/aix.h |    1 +
 2 files changed, 280 insertions(+), 0 deletions(-)
 create mode 100644 block/partitions/aix.c
 create mode 100644 block/partitions/aix.h

diff --git a/block/partitions/aix.c b/block/partitions/aix.c
new file mode 100644
index 0000000..c4c72ed
--- /dev/null
+++ b/block/partitions/aix.c
@@ -0,0 +1,279 @@
+/*
+ *  fs/partitions/aix.c
+ *
+ *  Copyright (C) 2012-2013 Philippe De Muyter <phdm@macqel.be>
+ */
+
+#include "check.h"
+#include "aix.h"
+
+struct lvm_rec {
+	char lvm_id[4]; /* "_LVM" */
+	char reserved4[16];
+	__be32 lvmarea_len;
+	__be32 vgda_len;
+	__be32 vgda_psn[2];
+	char reserved36[10];
+	__be16 pp_size; /* log2(pp_size) */
+	char reserved46[12];
+	__be16 version;
+	};
+
+struct vgda {
+	__be32 secs;
+	__be32 usec;
+	char reserved8[16];
+	__be16 numlvs;
+	__be16 maxlvs;
+	__be16 pp_size;
+	__be16 numpvs;
+	__be16 total_vgdas;
+	__be16 vgda_size;
+	};
+
+struct lvd {
+	__be16 lv_ix;
+	__be16 res2;
+	__be16 res4;
+	__be16 maxsize;
+	__be16 lv_state;
+	__be16 mirror;
+	__be16 mirror_policy;
+	__be16 num_lps;
+	__be16 res10[8];
+	};
+
+struct lvname {
+	char name[64];
+	};
+
+struct ppe {
+	__be16 lv_ix;
+	unsigned short res2;
+	unsigned short res4;
+	__be16 lp_ix;
+	unsigned short res8[12];
+	};
+
+struct pvd {
+	char reserved0[16];
+	__be16 pp_count;
+	char reserved18[2];
+	__be32 psn_part1;
+	char reserved24[8];
+	struct ppe ppe[1016];
+	};
+
+/**
+ * last_lba(): return number of last logical block of device
+ * @bdev: block device
+ *
+ * Description: Returns last LBA value on success, 0 on error.
+ * This is stored (by sd and ide-geometry) in
+ *  the part[0] entry for this disk, and is the number of
+ *  physical sectors available on the disk.
+ */
+static u64 last_lba(struct block_device *bdev)
+{
+	if (!bdev || !bdev->bd_inode)
+		return 0;
+	return (bdev->bd_inode->i_size >> 9) - 1ULL;
+}
+
+/**
+ * read_lba(): Read bytes from disk, starting at given LBA
+ * @state
+ * @lba
+ * @buffer
+ * @count
+ *
+ * Description:  Reads @count bytes from @state->bdev into @buffer.
+ * Returns number of bytes read on success, 0 on error.
+ */
+static size_t read_lba(struct parsed_partitions *state, u64 lba, u8 * buffer, size_t count)
+{
+	size_t totalreadcount = 0;
+
+	if (!buffer || lba + count / 512 > last_lba(state->bdev))
+                return 0;
+
+	while (count) {
+		int copied = 512;
+		Sector sect;
+		unsigned char *data = read_part_sector(state, lba++, &sect);
+		if (!data)
+			break;
+		if (copied > count)
+			copied = count;
+		memcpy(buffer, data, copied);
+		put_dev_sector(sect);
+		buffer += copied;
+		totalreadcount +=copied;
+		count -= copied;
+	}
+	return totalreadcount;
+}
+
+/**
+ * alloc_pvd(): reads physical volume descriptor
+ * @state
+ * @lba
+ *
+ * Description: Returns pvd on success,  NULL on error.
+ * Allocates space for pvd and fill it with disk blocks at @lba
+ * Notes: remember to free pvd when you're done!
+ */
+static struct pvd *alloc_pvd(struct parsed_partitions *state, u32 lba)
+{
+	size_t count = sizeof(struct pvd);
+	struct pvd *p;
+
+	p = kzalloc(count, GFP_KERNEL);
+	if (!p)
+		return NULL;
+
+	if (read_lba(state, lba, (u8 *) p, count) < count) {
+		kfree(p);
+		return NULL;
+	}
+	return p;
+}
+
+/**
+ * alloc_lvn(): reads logical volume names
+ * @state
+ * @lba
+ *
+ * Description: Returns lvn on success,  NULL on error.
+ * Allocates space for lvn and fill it with disk blocks at @lba
+ * Notes: remember to free lvn when you're done!
+ */
+static struct lvname *alloc_lvn(struct parsed_partitions *state, u32 lba)
+{
+	size_t count = sizeof(struct lvname) * 256;
+	struct lvname *p;
+
+	p = kzalloc(count, GFP_KERNEL);
+	if (!p)
+		return NULL;
+
+	if (read_lba(state, lba, (u8 *) p, count) < count) {
+		kfree(p);
+		return NULL;
+	}
+	return p;
+}
+
+int aix_partition(struct parsed_partitions *state)
+{
+	int ret = 0;
+	Sector sect;
+	unsigned char *d;
+	u32 pp_bytes_size;
+	u32 pp_blocks_size = 0;
+	u32 vgda_sector = 0;
+	u32 vgda_len = 0;
+	int numlvs = 0;
+	struct pvd *pvd;
+	unsigned short pps_per_lv[16];
+	unsigned short pps_found[16];
+	unsigned char lv_is_contiguous[16];
+	struct lvname *n = NULL;
+
+	d = read_part_sector(state, 7, &sect);
+	if (d) {
+		struct lvm_rec *p = (struct lvm_rec *)d;
+		u16 lvm_version = be16_to_cpu(p->version);
+		char tmp[64];
+
+		if (lvm_version == 1) {
+			int pp_size_log2 = be16_to_cpu(p->pp_size);
+
+			pp_bytes_size = 1 << pp_size_log2;
+			pp_blocks_size = pp_bytes_size / 512;
+			snprintf(tmp, sizeof(tmp), " AIX LVM header version %u found\n", lvm_version);
+			vgda_len = be32_to_cpu(p->vgda_len);
+			vgda_sector = be32_to_cpu(p->vgda_psn[0]);
+		} else {
+			snprintf(tmp, sizeof(tmp), " unsupported AIX LVM version %d found\n",
+				lvm_version);
+			strlcat(state->pp_buf, tmp, PAGE_SIZE);
+		}
+		put_dev_sector(sect);
+	}
+	if (vgda_sector && (d = read_part_sector(state, vgda_sector, &sect))) {
+		struct vgda *p = (struct vgda *)d;
+
+		numlvs = be16_to_cpu(p->numlvs);
+		put_dev_sector(sect);
+	}
+	if (numlvs && (d = read_part_sector(state, vgda_sector + 1, &sect))) {
+		struct lvd *p = (struct lvd *)d;
+		int i;
+
+		n = alloc_lvn(state, vgda_sector + vgda_len - 33);
+		if (n) {
+			int j = 0;
+
+			memset(lv_is_contiguous, 0, 16);
+			for (i = 0; i < 16; i += 1)
+				pps_found[i] = 0;
+			for (i = 0; j < numlvs && i < 16; i += 1) {
+				pps_per_lv[i] = be16_to_cpu(p[i].num_lps);
+				if (pps_per_lv[i])
+					j += 1;
+			}
+			while (i < 16)
+				pps_per_lv[i++] = 0;
+		}
+		put_dev_sector(sect);
+	}
+	pvd = alloc_pvd(state, vgda_sector + 17);
+	if (pvd) {
+		int numpps = be16_to_cpu(pvd->pp_count);
+		int psn_part1 = be32_to_cpu(pvd->psn_part1);
+		int i;
+		int cur_lv_ix = -1;
+		int next_lp_ix = 1;
+		int lp_ix;
+
+		for (i = 0; i < numpps; i += 1) {
+			struct ppe *p = pvd->ppe + i;
+			int lv_ix;
+
+			lp_ix = be16_to_cpu(p->lp_ix);
+			if (!lp_ix) {
+				next_lp_ix = 1;
+				continue;
+			}
+			lv_ix = be16_to_cpu(p->lv_ix) - 1;
+			pps_found[lv_ix] += 1;
+			if (lp_ix != next_lp_ix)
+				continue;
+			if (lp_ix == 1)
+				cur_lv_ix = lv_ix;
+			else if (lv_ix != cur_lv_ix)
+				next_lp_ix = 1;
+			if (lp_ix == pps_per_lv[lv_ix]) {
+				char tmp[70];
+
+				put_partition(state, lv_ix + 1,
+					(i + 1 - lp_ix) * pp_blocks_size + psn_part1,
+					pps_per_lv[lv_ix] * pp_blocks_size);
+				snprintf(tmp, sizeof(tmp), " <%s>\n", n[lv_ix].name);
+				strlcat(state->pp_buf, tmp, PAGE_SIZE);
+				lv_is_contiguous[lv_ix] = 1;
+				ret = 1;
+				next_lp_ix = 1;
+			} else
+				next_lp_ix += 1;
+		}
+		for (i = 0; i < 16; i += 1)
+			if (pps_found[i] && !lv_is_contiguous[i])
+				printk("partition %s (%d pp's found) is not contiguous\n", n[i].name, pps_found[i]);
+		kfree(pvd);
+	}
+	if (n)
+		kfree(n);
+	return ret;
+}
diff --git a/block/partitions/aix.h b/block/partitions/aix.h
new file mode 100644
index 0000000..e0c66a9
--- /dev/null
+++ b/block/partitions/aix.h
@@ -0,0 +1 @@
+extern int aix_partition(struct parsed_partitions *state);
-- 
1.7.1


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 3/5] partitions/msdos: enumerate also AIX LVM partitions
  2013-04-25 21:10 [RFC PATCH 0/5] Add aix lvm partitions support Philippe De Muyter
  2013-04-25 21:10 ` [PATCH 1/5] partitions/msdos.c: end-of-line whitespace and semicolon cleanup Philippe De Muyter
  2013-04-25 21:10 ` [PATCH 2/5] Add aix lvm partitions support files Philippe De Muyter
@ 2013-04-25 21:10 ` Philippe De Muyter
  2013-04-25 21:10 ` [PATCH 4/5] partitions/Makefile: compile aix.c if configured Philippe De Muyter
  2013-04-25 21:10 ` [PATCH 5/5] partitions/Kconfig: add the AIX_PARTITION entry Philippe De Muyter
  4 siblings, 0 replies; 20+ messages in thread
From: Philippe De Muyter @ 2013-04-25 21:10 UTC (permalink / raw)
  To: linux-kernel; +Cc: Philippe De Muyter, Jens Axboe

Graft AIX partitions enumeration in partitions/msdos.c

There is already a AIX disks detection logic in msdos.c.  When an
AIX disk has been found, and if configured to, call the aix partitions
recognizer.  This avoids removal of AIX disks protection from msdos.c,
avoids code duplication, and ensures that AIX partitions enumeration
is called before plain msdos partitions enumeration.

Signed-off-by: Philippe De Muyter <phdm@macqel.be>
Cc: Jens Axboe <axboe@kernel.dk>
---
 block/partitions/msdos.c |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/block/partitions/msdos.c b/block/partitions/msdos.c
index 9bf19e6..9123f25 100644
--- a/block/partitions/msdos.c
+++ b/block/partitions/msdos.c
@@ -23,6 +23,7 @@
 #include "check.h"
 #include "msdos.h"
 #include "efi.h"
+#include "aix.h"
 
 /*
  * Many architectures don't like unaligned accesses, while
@@ -462,8 +463,12 @@ int msdos_partition(struct parsed_partitions *state)
 	 */
 	if (aix_magic_present(state, data)) {
 		put_dev_sector(sect);
+#ifdef CONFIG_AIX_PARTITION
+		return aix_partition(state);
+#else
 		strlcat(state->pp_buf, " [AIX]", PAGE_SIZE);
 		return 0;
+#endif
 	}
 
 	if (!msdos_magic_present(data + 510)) {
-- 
1.7.1


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 4/5] partitions/Makefile: compile aix.c if configured.
  2013-04-25 21:10 [RFC PATCH 0/5] Add aix lvm partitions support Philippe De Muyter
                   ` (2 preceding siblings ...)
  2013-04-25 21:10 ` [PATCH 3/5] partitions/msdos: enumerate also AIX LVM partitions Philippe De Muyter
@ 2013-04-25 21:10 ` Philippe De Muyter
  2013-04-25 21:10 ` [PATCH 5/5] partitions/Kconfig: add the AIX_PARTITION entry Philippe De Muyter
  4 siblings, 0 replies; 20+ messages in thread
From: Philippe De Muyter @ 2013-04-25 21:10 UTC (permalink / raw)
  To: linux-kernel; +Cc: Philippe De Muyter, Jens Axboe

Signed-off-by: Philippe De Muyter <phdm@macqel.be>
Cc: Jens Axboe <axboe@kernel.dk>
---
 block/partitions/Makefile |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/block/partitions/Makefile b/block/partitions/Makefile
index 03af8ea..2be4d7b 100644
--- a/block/partitions/Makefile
+++ b/block/partitions/Makefile
@@ -7,6 +7,7 @@ obj-$(CONFIG_BLOCK) := check.o
 obj-$(CONFIG_ACORN_PARTITION) += acorn.o
 obj-$(CONFIG_AMIGA_PARTITION) += amiga.o
 obj-$(CONFIG_ATARI_PARTITION) += atari.o
+obj-$(CONFIG_AIX_PARTITION) += aix.o
 obj-$(CONFIG_MAC_PARTITION) += mac.o
 obj-$(CONFIG_LDM_PARTITION) += ldm.o
 obj-$(CONFIG_MSDOS_PARTITION) += msdos.o
-- 
1.7.1


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 5/5] partitions/Kconfig: add the AIX_PARTITION entry
  2013-04-25 21:10 [RFC PATCH 0/5] Add aix lvm partitions support Philippe De Muyter
                   ` (3 preceding siblings ...)
  2013-04-25 21:10 ` [PATCH 4/5] partitions/Makefile: compile aix.c if configured Philippe De Muyter
@ 2013-04-25 21:10 ` Philippe De Muyter
  4 siblings, 0 replies; 20+ messages in thread
From: Philippe De Muyter @ 2013-04-25 21:10 UTC (permalink / raw)
  To: linux-kernel; +Cc: Philippe De Muyter, Jens Axboe

This is the final patch enabling a user to select AIX lvm partitions
detection.

Signed-off-by: Philippe De Muyter <phdm@macqel.be>
Cc: Jens Axboe <axboe@kernel.dk>
---
 block/partitions/Kconfig |   11 +++++++++++
 1 files changed, 11 insertions(+), 0 deletions(-)

diff --git a/block/partitions/Kconfig b/block/partitions/Kconfig
index 75a54e1..4cebb2f 100644
--- a/block/partitions/Kconfig
+++ b/block/partitions/Kconfig
@@ -68,6 +68,17 @@ config ACORN_PARTITION_RISCIX
 	  of machines called RISCiX.  If you say 'Y' here, Linux will be able
 	  to read disks partitioned under RISCiX.
 
+config AIX_PARTITION
+	bool "AIX basic partition table support" if PARTITION_ADVANCED
+	help
+	  Say Y here if you would like to be able to read the hard disk
+	  partition table format used by IBM or Motorola PowerPC machines
+	  running AIX.  AIX actually uses a Logical Volume Manager, where
+	  "logical volumes" can be spread across one or multiple disks,
+	  but this driver works only for the simple case of partitions which
+	  are contiguous.
+	  Otherwise, say N.
+
 config OSF_PARTITION
 	bool "Alpha OSF partition support" if PARTITION_ADVANCED
 	default y if ALPHA
-- 
1.7.1


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Re: [PATCH 2/5] Add aix lvm partitions support files
  2013-04-25 21:10 ` [PATCH 2/5] Add aix lvm partitions support files Philippe De Muyter
@ 2013-04-29  9:37   ` Karel Zak
  2013-04-29 11:40     ` Philippe De Muyter
  0 siblings, 1 reply; 20+ messages in thread
From: Karel Zak @ 2013-04-29  9:37 UTC (permalink / raw)
  To: Philippe De Muyter; +Cc: linux-kernel, Jens Axboe

On Thu, Apr 25, 2013 at 11:10:26PM +0200, Philippe De Muyter wrote:
> +int aix_partition(struct parsed_partitions *state)
> +{
> +	int ret = 0;
> +	Sector sect;
> +	unsigned char *d;
> +	u32 pp_bytes_size;
> +	u32 pp_blocks_size = 0;
> +	u32 vgda_sector = 0;
> +	u32 vgda_len = 0;
> +	int numlvs = 0;
> +	struct pvd *pvd;
> +	unsigned short pps_per_lv[16];
> +	unsigned short pps_found[16];
> +	unsigned char lv_is_contiguous[16];
> +	struct lvname *n = NULL;
> +
> +	d = read_part_sector(state, 7, &sect);
> +	if (d) {
> +		struct lvm_rec *p = (struct lvm_rec *)d;
> +		u16 lvm_version = be16_to_cpu(p->version);
> +		char tmp[64];
> +
> +		if (lvm_version == 1) {
> +			int pp_size_log2 = be16_to_cpu(p->pp_size);
> +
> +			pp_bytes_size = 1 << pp_size_log2;
> +			pp_blocks_size = pp_bytes_size / 512;
> +			snprintf(tmp, sizeof(tmp), " AIX LVM header version %u found\n", lvm_version);

'tmp' is nowhere used, maybe you want to use strlcat(state->pp_buf, tmp, PAGE_SIZE); too.

> +			vgda_len = be32_to_cpu(p->vgda_len);
> +			vgda_sector = be32_to_cpu(p->vgda_psn[0]);
> +		} else {
> +			snprintf(tmp, sizeof(tmp), " unsupported AIX LVM version %d found\n",
> +				lvm_version);
> +			strlcat(state->pp_buf, tmp, PAGE_SIZE);
> +		}
> +		put_dev_sector(sect);
> +	}
> +	if (vgda_sector && (d = read_part_sector(state, vgda_sector, &sect))) {
> +		struct vgda *p = (struct vgda *)d;
> +
> +		numlvs = be16_to_cpu(p->numlvs);
> +		put_dev_sector(sect);
> +	}
> +	if (numlvs && (d = read_part_sector(state, vgda_sector + 1, &sect))) {
> +		struct lvd *p = (struct lvd *)d;
> +		int i;
> +
> +		n = alloc_lvn(state, vgda_sector + vgda_len - 33);
> +		if (n) {
> +			int j = 0;
> +
> +			memset(lv_is_contiguous, 0, 16);
> +			for (i = 0; i < 16; i += 1)
> +				pps_found[i] = 0;

why not memset(pps_found, ....)? I also see magical constant 16
everywhere, maybe you can use sizeof() and ARRAY_SIZE().

> +			for (i = 0; j < numlvs && i < 16; i += 1) {
> +				pps_per_lv[i] = be16_to_cpu(p[i].num_lps);
> +				if (pps_per_lv[i])
> +					j += 1;
> +			}

hmm, what's wrong with j++ and i++, "j += 1" seems like old Python :-)

> +			while (i < 16)
> +				pps_per_lv[i++] = 0;
> +		}
> +		put_dev_sector(sect);
> +	}
> +	pvd = alloc_pvd(state, vgda_sector + 17);
> +	if (pvd) {
> +		int numpps = be16_to_cpu(pvd->pp_count);
> +		int psn_part1 = be32_to_cpu(pvd->psn_part1);
> +		int i;
> +		int cur_lv_ix = -1;
> +		int next_lp_ix = 1;
> +		int lp_ix;
> +
> +		for (i = 0; i < numpps; i += 1) {
> +			struct ppe *p = pvd->ppe + i;
> +			int lv_ix;
> +
> +			lp_ix = be16_to_cpu(p->lp_ix);
> +			if (!lp_ix) {
> +				next_lp_ix = 1;
> +				continue;
> +			}
> +			lv_ix = be16_to_cpu(p->lv_ix) - 1;
> +			pps_found[lv_ix] += 1;

 Would be better to be a little paranoid when you read the data from
 disk and check that lv_ix is in range 0..16 ?

> +			if (lp_ix != next_lp_ix)
> +				continue;
> +			if (lp_ix == 1)
> +				cur_lv_ix = lv_ix;
> +			else if (lv_ix != cur_lv_ix)
> +				next_lp_ix = 1;
> +			if (lp_ix == pps_per_lv[lv_ix]) {
> +				char tmp[70];
> +
> +				put_partition(state, lv_ix + 1,
> +					(i + 1 - lp_ix) * pp_blocks_size + psn_part1,
> +					pps_per_lv[lv_ix] * pp_blocks_size);
> +				snprintf(tmp, sizeof(tmp), " <%s>\n", n[lv_ix].name);
> +				strlcat(state->pp_buf, tmp, PAGE_SIZE);
> +				lv_is_contiguous[lv_ix] = 1;
> +				ret = 1;
> +				next_lp_ix = 1;
> +			} else
> +				next_lp_ix += 1;
> +		}
> +		for (i = 0; i < 16; i += 1)
> +			if (pps_found[i] && !lv_is_contiguous[i])
> +				printk("partition %s (%d pp's found) is not contiguous\n", n[i].name, pps_found[i]);
> +		kfree(pvd);
> +	}
> +	if (n)
> +		kfree(n);
> +	return ret;
> +}

Philippe, do you have any disk image with AIX LVM? It would be nice to
have a way how to test the code. I'd like to add support for AIX to
libblkid too.

    Karel

-- 
 Karel Zak  <kzak@redhat.com>
 http://karelzak.blogspot.com

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 2/5] Add aix lvm partitions support files
  2013-04-29  9:37   ` Karel Zak
@ 2013-04-29 11:40     ` Philippe De Muyter
  2013-04-29 12:36       ` Karel Zak
  2013-04-29 21:50       ` Philippe De Muyter
  0 siblings, 2 replies; 20+ messages in thread
From: Philippe De Muyter @ 2013-04-29 11:40 UTC (permalink / raw)
  To: Karel Zak; +Cc: linux-kernel, Jens Axboe

On Mon, Apr 29, 2013 at 11:37:56AM +0200, Karel Zak wrote:
> On Thu, Apr 25, 2013 at 11:10:26PM +0200, Philippe De Muyter wrote:

Thanks for the interest and the quick reply.

> > +int aix_partition(struct parsed_partitions *state)
> > +{
> > +	int ret = 0;
> > +	Sector sect;
> > +	unsigned char *d;
> > +	u32 pp_bytes_size;
> > +	u32 pp_blocks_size = 0;
> > +	u32 vgda_sector = 0;
> > +	u32 vgda_len = 0;
> > +	int numlvs = 0;
> > +	struct pvd *pvd;
> > +	unsigned short pps_per_lv[16];
> > +	unsigned short pps_found[16];
> > +	unsigned char lv_is_contiguous[16];
> > +	struct lvname *n = NULL;
> > +
> > +	d = read_part_sector(state, 7, &sect);
> > +	if (d) {
> > +		struct lvm_rec *p = (struct lvm_rec *)d;
> > +		u16 lvm_version = be16_to_cpu(p->version);
> > +		char tmp[64];
> > +
> > +		if (lvm_version == 1) {
> > +			int pp_size_log2 = be16_to_cpu(p->pp_size);
> > +
> > +			pp_bytes_size = 1 << pp_size_log2;
> > +			pp_blocks_size = pp_bytes_size / 512;
> > +			snprintf(tmp, sizeof(tmp), " AIX LVM header version %u found\n", lvm_version);
> 
> 'tmp' is nowhere used, maybe you want to use strlcat(state->pp_buf, tmp, PAGE_SIZE); too.

Oops, of course :)
> 
> > +			vgda_len = be32_to_cpu(p->vgda_len);
> > +			vgda_sector = be32_to_cpu(p->vgda_psn[0]);
> > +		} else {
> > +			snprintf(tmp, sizeof(tmp), " unsupported AIX LVM version %d found\n",
> > +				lvm_version);
> > +			strlcat(state->pp_buf, tmp, PAGE_SIZE);
> > +		}
> > +		put_dev_sector(sect);
> > +	}
> > +	if (vgda_sector && (d = read_part_sector(state, vgda_sector, &sect))) {
> > +		struct vgda *p = (struct vgda *)d;
> > +
> > +		numlvs = be16_to_cpu(p->numlvs);
> > +		put_dev_sector(sect);
> > +	}
> > +	if (numlvs && (d = read_part_sector(state, vgda_sector + 1, &sect))) {
> > +		struct lvd *p = (struct lvd *)d;
> > +		int i;
> > +
> > +		n = alloc_lvn(state, vgda_sector + vgda_len - 33);
> > +		if (n) {
> > +			int j = 0;
> > +
> > +			memset(lv_is_contiguous, 0, 16);
> > +			for (i = 0; i < 16; i += 1)
> > +				pps_found[i] = 0;
> 
> why not memset(pps_found, ....)? I also see magical constant 16

Actually 16 is the maximum partition count allowed in a disk by linux,
or should it be 15 ?  Is there already a constant for that ?
The AIX disk I tested with had only :) 11 partitions.

> everywhere, maybe you can use sizeof() and ARRAY_SIZE().

Will do.
> 
> > +			for (i = 0; j < numlvs && i < 16; i += 1) {
> > +				pps_per_lv[i] = be16_to_cpu(p[i].num_lps);
> > +				if (pps_per_lv[i])
> > +					j += 1;
> > +			}
> 
> hmm, what's wrong with j++ and i++, "j += 1" seems like old Python :-)
> 

What's wrong with "j += 1" ?
I only use ++ for side effects; I personaly find "j += 1" more readable :)
But I should rename 'j' to be more explicit.


> > +			while (i < 16)
> > +				pps_per_lv[i++] = 0;
> > +		}
> > +		put_dev_sector(sect);
> > +	}
> > +	pvd = alloc_pvd(state, vgda_sector + 17);
> > +	if (pvd) {
> > +		int numpps = be16_to_cpu(pvd->pp_count);
> > +		int psn_part1 = be32_to_cpu(pvd->psn_part1);
> > +		int i;
> > +		int cur_lv_ix = -1;
> > +		int next_lp_ix = 1;
> > +		int lp_ix;
> > +
> > +		for (i = 0; i < numpps; i += 1) {
> > +			struct ppe *p = pvd->ppe + i;
> > +			int lv_ix;
> > +
> > +			lp_ix = be16_to_cpu(p->lp_ix);
> > +			if (!lp_ix) {
> > +				next_lp_ix = 1;
> > +				continue;
> > +			}
> > +			lv_ix = be16_to_cpu(p->lv_ix) - 1;
> > +			pps_found[lv_ix] += 1;
> 
>  Would be better to be a little paranoid when you read the data from
>  disk and check that lv_ix is in range 0..16 ?

Of course.

> 
> > +			if (lp_ix != next_lp_ix)
> > +				continue;
> > +			if (lp_ix == 1)
> > +				cur_lv_ix = lv_ix;
> > +			else if (lv_ix != cur_lv_ix)
> > +				next_lp_ix = 1;
> > +			if (lp_ix == pps_per_lv[lv_ix]) {
> > +				char tmp[70];
> > +
> > +				put_partition(state, lv_ix + 1,
> > +					(i + 1 - lp_ix) * pp_blocks_size + psn_part1,
> > +					pps_per_lv[lv_ix] * pp_blocks_size);
> > +				snprintf(tmp, sizeof(tmp), " <%s>\n", n[lv_ix].name);
> > +				strlcat(state->pp_buf, tmp, PAGE_SIZE);
> > +				lv_is_contiguous[lv_ix] = 1;
> > +				ret = 1;
> > +				next_lp_ix = 1;
> > +			} else
> > +				next_lp_ix += 1;
> > +		}
> > +		for (i = 0; i < 16; i += 1)
> > +			if (pps_found[i] && !lv_is_contiguous[i])
> > +				printk("partition %s (%d pp's found) is not contiguous\n", n[i].name, pps_found[i]);
> > +		kfree(pvd);
> > +	}
> > +	if (n)
> > +		kfree(n);
> > +	return ret;
> > +}
> 
> Philippe, do you have any disk image with AIX LVM? It would be nice to
> have a way how to test the code. I'd like to add support for AIX to
> libblkid too.

Of course.  But that's not a couple of blocks.  I'll try to cut the
slice that you need.

Philippe

-- 
Philippe De Muyter +32 2 6101532 Macq SA rue de l'Aeronef 2 B-1140 Bruxelles

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 2/5] Add aix lvm partitions support files
  2013-04-29 11:40     ` Philippe De Muyter
@ 2013-04-29 12:36       ` Karel Zak
  2013-04-29 15:34         ` Philippe De Muyter
  2013-04-29 21:50       ` Philippe De Muyter
  1 sibling, 1 reply; 20+ messages in thread
From: Karel Zak @ 2013-04-29 12:36 UTC (permalink / raw)
  To: Philippe De Muyter; +Cc: linux-kernel, Jens Axboe

On Mon, Apr 29, 2013 at 01:40:41PM +0200, Philippe De Muyter wrote:
> > why not memset(pps_found, ....)? I also see magical constant 16
> 
> Actually 16 is the maximum partition count allowed in a disk by linux,
> or should it be 15 ?  Is there already a constant for that ?
> The AIX disk I tested with had only :) 11 partitions.

I don't think it's correct to expect any hardcoded limit. 
 
The struct parsed_partitions->parts is allocated according to
disk_max_parts() where the limit depends on number of minor numbers or
it's DISK_MAX_PARTS (=256).

There is no problem to create disk with many partitions:

 # modprobe scsi_debug dev_size_mb=300
 # (echo -e 'g\n'; for i in {1..100}; do echo -e "n\n\n\n+1M"; done; \
    echo -e 'w\nq\n') | fdisk /dev/sdb

 # lsblk -n /dev/sdb | wc -l
 101

 Note it's fdisk with GPT support.

> > Philippe, do you have any disk image with AIX LVM? It would be nice to
> > have a way how to test the code. I'd like to add support for AIX to
> > libblkid too.
> 
> Of course.  But that's not a couple of blocks.  I'll try to cut the
> slice that you need.

 Cool!

    Karel

-- 
 Karel Zak  <kzak@redhat.com>
 http://karelzak.blogspot.com

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 2/5] Add aix lvm partitions support files
  2013-04-29 12:36       ` Karel Zak
@ 2013-04-29 15:34         ` Philippe De Muyter
  2013-04-30  6:41           ` Jens Axboe
  0 siblings, 1 reply; 20+ messages in thread
From: Philippe De Muyter @ 2013-04-29 15:34 UTC (permalink / raw)
  To: Karel Zak; +Cc: linux-kernel, Jens Axboe

Hi Karel

On Mon, Apr 29, 2013 at 02:36:51PM +0200, Karel Zak wrote:
> On Mon, Apr 29, 2013 at 01:40:41PM +0200, Philippe De Muyter wrote:
> > > why not memset(pps_found, ....)? I also see magical constant 16
> > 
> > Actually 16 is the maximum partition count allowed in a disk by linux,
> > or should it be 15 ?  Is there already a constant for that ?
> > The AIX disk I tested with had only :) 11 partitions.
> 
> I don't think it's correct to expect any hardcoded limit. 
>  
> The struct parsed_partitions->parts is allocated according to
> disk_max_parts() where the limit depends on number of minor numbers or
> it's DISK_MAX_PARTS (=256).
> 
> There is no problem to create disk with many partitions:
> 
>  # modprobe scsi_debug dev_size_mb=300
>  # (echo -e 'g\n'; for i in {1..100}; do echo -e "n\n\n\n+1M"; done; \
>     echo -e 'w\nq\n') | fdisk /dev/sdb
> 
>  # lsblk -n /dev/sdb | wc -l
>  101

how are they named then ?

on my system (a 2.6.24 kernel which is the last one that support my
powerpc PReP machine because PReP support got removed with the merge
of /arch/ppc and /arch/powerpc :( ), I get :

root:~# ls -l /dev/sd[ab]*
brw-r----- 1 root disk 8,  0 Apr 25 22:22 /dev/sda
brw-r----- 1 root disk 8, 10 Apr 25 22:22 /dev/sda10
brw-r----- 1 root disk 8, 11 Apr 25 22:22 /dev/sda11
brw-r----- 1 root disk 8,  3 Apr 25 22:22 /dev/sda3
brw-r----- 1 root disk 8,  4 Apr 25 22:22 /dev/sda4
brw-r----- 1 root disk 8,  5 Apr 25 22:22 /dev/sda5
brw-r----- 1 root disk 8,  6 Apr 25 22:22 /dev/sda6
brw-r----- 1 root disk 8,  7 Apr 25 22:22 /dev/sda7
brw-r----- 1 root disk 8,  8 Apr 25 22:22 /dev/sda8
brw-r----- 1 root disk 8,  9 Apr 25 22:22 /dev/sda9
brw-r----- 1 root disk 8, 16 Apr 25 22:22 /dev/sdb
brw-r----- 1 root disk 8, 26 Apr 25 22:22 /dev/sdb10
brw-r----- 1 root disk 8, 27 Apr 25 22:22 /dev/sdb11
brw-r----- 1 root disk 8, 19 Apr 25 22:22 /dev/sdb3
brw-r----- 1 root disk 8, 20 Apr 25 22:22 /dev/sdb4
brw-r----- 1 root disk 8, 21 Apr 25 22:22 /dev/sdb5
brw-r----- 1 root disk 8, 22 Apr 25 22:22 /dev/sdb6
brw-r----- 1 root disk 8, 23 Apr 25 22:22 /dev/sdb7
brw-r----- 1 root disk 8, 24 Apr 25 22:22 /dev/sdb8
brw-r----- 1 root disk 8, 25 Apr 25 22:22 /dev/sdb9
root:~#

so sda is 8,0 and sdb is 8,16

and if, while discovering partitions of /dev/sda, I try to make a
partition 16 or higher, it is silently discarded by 'put_partition'.

Is that changed ?

Philippe

-- 
Philippe De Muyter +32 2 6101532 Macq SA rue de l'Aeronef 2 B-1140 Bruxelles

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 2/5] Add aix lvm partitions support files
  2013-04-29 11:40     ` Philippe De Muyter
  2013-04-29 12:36       ` Karel Zak
@ 2013-04-29 21:50       ` Philippe De Muyter
  2013-04-30  7:08         ` Philippe De Muyter
  1 sibling, 1 reply; 20+ messages in thread
From: Philippe De Muyter @ 2013-04-29 21:50 UTC (permalink / raw)
  To: Karel Zak; +Cc: linux-kernel, Jens Axboe

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

On Mon, Apr 29, 2013 at 01:40:41PM +0200, Philippe De Muyter wrote:
> On Mon, Apr 29, 2013 at 11:37:56AM +0200, Karel Zak wrote:
> > 
> > Philippe, do you have any disk image with AIX LVM? It would be nice to
> > have a way how to test the code. I'd like to add support for AIX to
> > libblkid too.
> 
> Of course.  But that's not a couple of blocks.  I'll try to cut the
> slice that you need.

Here is one example.  I'll try to send another one tomorrow.

Philippe

[-- Attachment #2: aixlvmhdr-128g.bz2 --]
[-- Type: application/x-bzip, Size: 1893 bytes --]

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 2/5] Add aix lvm partitions support files
  2013-04-29 15:34         ` Philippe De Muyter
@ 2013-04-30  6:41           ` Jens Axboe
  2013-04-30  6:45             ` Philippe De Muyter
  0 siblings, 1 reply; 20+ messages in thread
From: Jens Axboe @ 2013-04-30  6:41 UTC (permalink / raw)
  To: Philippe De Muyter; +Cc: Karel Zak, linux-kernel

On Mon, Apr 29 2013, Philippe De Muyter wrote:
> Hi Karel
> 
> On Mon, Apr 29, 2013 at 02:36:51PM +0200, Karel Zak wrote:
> > On Mon, Apr 29, 2013 at 01:40:41PM +0200, Philippe De Muyter wrote:
> > > > why not memset(pps_found, ....)? I also see magical constant 16
> > > 
> > > Actually 16 is the maximum partition count allowed in a disk by linux,
> > > or should it be 15 ?  Is there already a constant for that ?
> > > The AIX disk I tested with had only :) 11 partitions.
> > 
> > I don't think it's correct to expect any hardcoded limit. 
> >  
> > The struct parsed_partitions->parts is allocated according to
> > disk_max_parts() where the limit depends on number of minor numbers or
> > it's DISK_MAX_PARTS (=256).
> > 
> > There is no problem to create disk with many partitions:
> > 
> >  # modprobe scsi_debug dev_size_mb=300
> >  # (echo -e 'g\n'; for i in {1..100}; do echo -e "n\n\n\n+1M"; done; \
> >     echo -e 'w\nq\n') | fdisk /dev/sdb
> > 
> >  # lsblk -n /dev/sdb | wc -l
> >  101
> 
> how are they named then ?
> 
> on my system (a 2.6.24 kernel which is the last one that support my
> powerpc PReP machine because PReP support got removed with the merge
> of /arch/ppc and /arch/powerpc :( ), I get :
> 
> root:~# ls -l /dev/sd[ab]*
> brw-r----- 1 root disk 8,  0 Apr 25 22:22 /dev/sda
> brw-r----- 1 root disk 8, 10 Apr 25 22:22 /dev/sda10
> brw-r----- 1 root disk 8, 11 Apr 25 22:22 /dev/sda11
> brw-r----- 1 root disk 8,  3 Apr 25 22:22 /dev/sda3
> brw-r----- 1 root disk 8,  4 Apr 25 22:22 /dev/sda4
> brw-r----- 1 root disk 8,  5 Apr 25 22:22 /dev/sda5
> brw-r----- 1 root disk 8,  6 Apr 25 22:22 /dev/sda6
> brw-r----- 1 root disk 8,  7 Apr 25 22:22 /dev/sda7
> brw-r----- 1 root disk 8,  8 Apr 25 22:22 /dev/sda8
> brw-r----- 1 root disk 8,  9 Apr 25 22:22 /dev/sda9
> brw-r----- 1 root disk 8, 16 Apr 25 22:22 /dev/sdb
> brw-r----- 1 root disk 8, 26 Apr 25 22:22 /dev/sdb10
> brw-r----- 1 root disk 8, 27 Apr 25 22:22 /dev/sdb11
> brw-r----- 1 root disk 8, 19 Apr 25 22:22 /dev/sdb3
> brw-r----- 1 root disk 8, 20 Apr 25 22:22 /dev/sdb4
> brw-r----- 1 root disk 8, 21 Apr 25 22:22 /dev/sdb5
> brw-r----- 1 root disk 8, 22 Apr 25 22:22 /dev/sdb6
> brw-r----- 1 root disk 8, 23 Apr 25 22:22 /dev/sdb7
> brw-r----- 1 root disk 8, 24 Apr 25 22:22 /dev/sdb8
> brw-r----- 1 root disk 8, 25 Apr 25 22:22 /dev/sdb9
> root:~#
> 
> so sda is 8,0 and sdb is 8,16
> 
> and if, while discovering partitions of /dev/sda, I try to make a
> partition 16 or higher, it is silently discarded by 'put_partition'.
> 
> Is that changed ?

That's a set limitation of sd, it does not apply to other devices. The
legacy IDE code used 64 for max partitions, for instance. So Karel is
right, you should not make any assumptions about the max number of
partitions, it is driver dependent.

-- 
Jens Axboe


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 2/5] Add aix lvm partitions support files
  2013-04-30  6:41           ` Jens Axboe
@ 2013-04-30  6:45             ` Philippe De Muyter
  0 siblings, 0 replies; 20+ messages in thread
From: Philippe De Muyter @ 2013-04-30  6:45 UTC (permalink / raw)
  To: Jens Axboe; +Cc: Karel Zak, linux-kernel

On Tue, Apr 30, 2013 at 08:41:52AM +0200, Jens Axboe wrote:
> On Mon, Apr 29 2013, Philippe De Muyter wrote:
> > 
> > so sda is 8,0 and sdb is 8,16
> > 
> > and if, while discovering partitions of /dev/sda, I try to make a
> > partition 16 or higher, it is silently discarded by 'put_partition'.
> > 
> > Is that changed ?
> 
> That's a set limitation of sd, it does not apply to other devices. The
> legacy IDE code used 64 for max partitions, for instance. So Karel is
> right, you should not make any assumptions about the max number of
> partitions, it is driver dependent.

I replaced that by state->limit in the v2 series.

Philippe

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 2/5] Add aix lvm partitions support files
  2013-04-29 21:50       ` Philippe De Muyter
@ 2013-04-30  7:08         ` Philippe De Muyter
  2013-04-30  7:18           ` Philippe De Muyter
  0 siblings, 1 reply; 20+ messages in thread
From: Philippe De Muyter @ 2013-04-30  7:08 UTC (permalink / raw)
  To: Karel Zak; +Cc: linux-kernel, Jens Axboe

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

On Mon, Apr 29, 2013 at 11:50:51PM +0200, Philippe De Muyter wrote:
> On Mon, Apr 29, 2013 at 01:40:41PM +0200, Philippe De Muyter wrote:
> > On Mon, Apr 29, 2013 at 11:37:56AM +0200, Karel Zak wrote:
> > > 
> > > Philippe, do you have any disk image with AIX LVM? It would be nice to
> > > have a way how to test the code. I'd like to add support for AIX to
> > > libblkid too.
> > 
> > Of course.  But that's not a couple of blocks.  I'll try to cut the
> > slice that you need.
> 
> Here is one example.  I'll try to send another one tomorrow.

Here is the other example.

Philippe
-- 
Philippe De Muyter +32 2 6101532 Macq SA rue de l'Aeronef 2 B-1140 Bruxelles

[-- Attachment #2: aixlvmhdr-goofy.bz2 --]
[-- Type: application/x-bzip, Size: 1823 bytes --]

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 2/5] Add aix lvm partitions support files
  2013-04-30  7:08         ` Philippe De Muyter
@ 2013-04-30  7:18           ` Philippe De Muyter
  2013-05-01  5:35             ` Philippe De Muyter
  0 siblings, 1 reply; 20+ messages in thread
From: Philippe De Muyter @ 2013-04-30  7:18 UTC (permalink / raw)
  To: Karel Zak; +Cc: linux-kernel, Jens Axboe

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

On Tue, Apr 30, 2013 at 09:08:40AM +0200, Philippe De Muyter wrote:
> On Mon, Apr 29, 2013 at 11:50:51PM +0200, Philippe De Muyter wrote:
> > On Mon, Apr 29, 2013 at 01:40:41PM +0200, Philippe De Muyter wrote:
> > > On Mon, Apr 29, 2013 at 11:37:56AM +0200, Karel Zak wrote:
> > > > 
> > > > Philippe, do you have any disk image with AIX LVM? It would be nice to
> > > > have a way how to test the code. I'd like to add support for AIX to
> > > > libblkid too.
> > > 
> > > Of course.  But that's not a couple of blocks.  I'll try to cut the
> > > slice that you need.
> > 
> > Here is one example.  I'll try to send another one tomorrow.
> 
> Here is the other example.

And here is a third one

Philippe

-- 
Philippe De Muyter +32 2 6101532 Macq SA rue de l'Aeronef 2 B-1140 Bruxelles

[-- Attachment #2: aixlvmhdr-jori.bz2 --]
[-- Type: application/x-bzip, Size: 7533 bytes --]

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 2/5] Add aix lvm partitions support files
  2013-04-30  7:18           ` Philippe De Muyter
@ 2013-05-01  5:35             ` Philippe De Muyter
  0 siblings, 0 replies; 20+ messages in thread
From: Philippe De Muyter @ 2013-05-01  5:35 UTC (permalink / raw)
  To: Karel Zak; +Cc: linux-kernel, Jens Axboe

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

On Tue, Apr 30, 2013 at 09:18:40AM +0200, Philippe De Muyter wrote:
> On Tue, Apr 30, 2013 at 09:08:40AM +0200, Philippe De Muyter wrote:
> > On Mon, Apr 29, 2013 at 11:50:51PM +0200, Philippe De Muyter wrote:
> > > On Mon, Apr 29, 2013 at 01:40:41PM +0200, Philippe De Muyter wrote:
> > > > On Mon, Apr 29, 2013 at 11:37:56AM +0200, Karel Zak wrote:
> > > > > 
> > > > > Philippe, do you have any disk image with AIX LVM? It would be nice to
> > > > > have a way how to test the code. I'd like to add support for AIX to
> > > > > libblkid too.
> > > > 
> > > > Of course.  But that's not a couple of blocks.  I'll try to cut the
> > > > slice that you need.
> > > 
> > > Here is one example.  I'll try to send another one tomorrow.
> > 
> > Here is the other example.
> 
> And here is a third one

And now a fourth one, this one with partitions that are not contiguous.

Philippe

-- 
Philippe De Muyter +32 2 6101532 Macq SA rue de l'Aeronef 2 B-1140 Bruxelles

[-- Attachment #2: aixlvmhdr-136g.bz2 --]
[-- Type: application/x-bzip, Size: 2934 bytes --]

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 3/5] partitions/msdos: enumerate also AIX LVM partitions
  2013-05-20 23:41   ` Andrew Morton
@ 2013-05-21  7:16     ` Philippe De Muyter
  0 siblings, 0 replies; 20+ messages in thread
From: Philippe De Muyter @ 2013-05-21  7:16 UTC (permalink / raw)
  To: Andrew Morton; +Cc: linux-kernel, Karel Zak, Jens Axboe

Hi Andrew,

On Mon, May 20, 2013 at 04:41:52PM -0700, Andrew Morton wrote:
> On Mon, 29 Apr 2013 23:18:31 +0200 Philippe De Muyter <phdm@macqel.be> wrote:
> 
> > Graft AIX partitions enumeration in partitions/msdos.c
> > 
> > There is already a AIX disks detection logic in msdos.c.  When an
> > AIX disk has been found, and if configured to, call the aix partitions
> > recognizer.  This avoids removal of AIX disks protection from msdos.c,
> > avoids code duplication, and ensures that AIX partitions enumeration
> > is called before plain msdos partitions enumeration.
> > 
> > ...
> >
> > --- a/block/partitions/msdos.c
> > +++ b/block/partitions/msdos.c
> > @@ -23,6 +23,7 @@
> >  #include "check.h"
> >  #include "msdos.h"
> >  #include "efi.h"
> > +#include "aix.h"
> >  
> >  /*
> >   * Many architectures don't like unaligned accesses, while
> > @@ -462,8 +463,12 @@ int msdos_partition(struct parsed_partitions *state)
> >  	 */
> >  	if (aix_magic_present(state, data)) {
> >  		put_dev_sector(sect);
> > +#ifdef CONFIG_AIX_PARTITION
> > +		return aix_partition(state);
> > +#else
> >  		strlcat(state->pp_buf, " [AIX]", PAGE_SIZE);
> >  		return 0;
> > +#endif
> >  	}
> >  
> >  	if (!msdos_magic_present(data + 510)) {
> 
> hm, what's going on here.
> 
> Why does partitions/msdos.c know about AIX at all?  Is there something
> special about AIX partitioning which ties it in with msdos?

Well, PowerPC BIOS (PPCBUG or Open Firmware) mimics PC BIOS and requires the
first block of the disk to describe the boot partition the same way that
PC BIOS does.  Remember, the aim of IBM, Motorola and Apple was to replace
the PC's by PowerPC's :).  So, an AIX disk can erroneously be recognized as
a dos disk with two partitions (two times the same boot partition).  That's
the reason why there is already code in block/partitions/msdos.c to avoid
that.
> 
> Now that we have AIX partitioning support, can we simply remove the AIX
> code from msdos.c?  So msdos.c will say "I don't know what that is",
> and we fall through to aix.c which says "that's mine!"?

I didn't want to remove the AIX protection from msdos.c, for the likely case
that someone would compile the kernel without AIX_PARTITION support,  And, as
a part of the AIX detection was already done by 'aix_magic_present', I did
not want to duplicate it.  I can move the AIX detection logic to aix.c and
still keep the AIX protection for msdos.c with some more #ifdef's if you prefer.

Philippe

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 3/5] partitions/msdos: enumerate also AIX LVM partitions
  2013-04-29 21:18 ` [PATCH 3/5] partitions/msdos: enumerate also AIX LVM partitions Philippe De Muyter
@ 2013-05-20 23:41   ` Andrew Morton
  2013-05-21  7:16     ` Philippe De Muyter
  0 siblings, 1 reply; 20+ messages in thread
From: Andrew Morton @ 2013-05-20 23:41 UTC (permalink / raw)
  To: Philippe De Muyter; +Cc: linux-kernel, Karel Zak, Jens Axboe

On Mon, 29 Apr 2013 23:18:31 +0200 Philippe De Muyter <phdm@macqel.be> wrote:

> Graft AIX partitions enumeration in partitions/msdos.c
> 
> There is already a AIX disks detection logic in msdos.c.  When an
> AIX disk has been found, and if configured to, call the aix partitions
> recognizer.  This avoids removal of AIX disks protection from msdos.c,
> avoids code duplication, and ensures that AIX partitions enumeration
> is called before plain msdos partitions enumeration.
> 
> ...
>
> --- a/block/partitions/msdos.c
> +++ b/block/partitions/msdos.c
> @@ -23,6 +23,7 @@
>  #include "check.h"
>  #include "msdos.h"
>  #include "efi.h"
> +#include "aix.h"
>  
>  /*
>   * Many architectures don't like unaligned accesses, while
> @@ -462,8 +463,12 @@ int msdos_partition(struct parsed_partitions *state)
>  	 */
>  	if (aix_magic_present(state, data)) {
>  		put_dev_sector(sect);
> +#ifdef CONFIG_AIX_PARTITION
> +		return aix_partition(state);
> +#else
>  		strlcat(state->pp_buf, " [AIX]", PAGE_SIZE);
>  		return 0;
> +#endif
>  	}
>  
>  	if (!msdos_magic_present(data + 510)) {

hm, what's going on here.

Why does partitions/msdos.c know about AIX at all?  Is there something
special about AIX partitioning which ties it in with msdos?

Now that we have AIX partitioning support, can we simply remove the AIX
code from msdos.c?  So msdos.c will say "I don't know what that is",
and we fall through to aix.c which says "that's mine!"?


^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH 3/5] partitions/msdos: enumerate also AIX LVM partitions
  2013-05-01  6:09 [PATCH v3 0/5] partitions: Add aix lvm partitions support Philippe De Muyter
@ 2013-05-01  6:09 ` Philippe De Muyter
  0 siblings, 0 replies; 20+ messages in thread
From: Philippe De Muyter @ 2013-05-01  6:09 UTC (permalink / raw)
  To: linux-kernel; +Cc: Jens Axboe, Karel Zak, Andrew Morton, Philippe De Muyter

From: Philippe De Muyter <phdm@macqel.be>

Graft AIX partitions enumeration in partitions/msdos.c

There is already a AIX disks detection logic in msdos.c.  When an
AIX disk has been found, and if configured to, call the aix partitions
recognizer.  This avoids removal of AIX disks protection from msdos.c,
avoids code duplication, and ensures that AIX partitions enumeration
is called before plain msdos partitions enumeration.

Signed-off-by: Philippe De Muyter <phdm@macqel.be>
Cc: Karel Zak <kzak@redhat.com>
Cc: Jens Axboe <axboe@kernel.dk>
Cc: Andrew Morton <akpm@linux-foundation.org>
---
 block/partitions/msdos.c |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/block/partitions/msdos.c b/block/partitions/msdos.c
index 9bf19e6..9123f25 100644
--- a/block/partitions/msdos.c
+++ b/block/partitions/msdos.c
@@ -23,6 +23,7 @@
 #include "check.h"
 #include "msdos.h"
 #include "efi.h"
+#include "aix.h"
 
 /*
  * Many architectures don't like unaligned accesses, while
@@ -462,8 +463,12 @@ int msdos_partition(struct parsed_partitions *state)
 	 */
 	if (aix_magic_present(state, data)) {
 		put_dev_sector(sect);
+#ifdef CONFIG_AIX_PARTITION
+		return aix_partition(state);
+#else
 		strlcat(state->pp_buf, " [AIX]", PAGE_SIZE);
 		return 0;
+#endif
 	}
 
 	if (!msdos_magic_present(data + 510)) {
-- 
1.7.1


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 3/5] partitions/msdos: enumerate also AIX LVM partitions
  2013-04-29 21:18 [PATCH v2 0/5] partitions: add AIX LVM support Philippe De Muyter
@ 2013-04-29 21:18 ` Philippe De Muyter
  2013-05-20 23:41   ` Andrew Morton
  0 siblings, 1 reply; 20+ messages in thread
From: Philippe De Muyter @ 2013-04-29 21:18 UTC (permalink / raw)
  To: linux-kernel; +Cc: Philippe De Muyter, Karel Zak, Jens Axboe, Andrew Morton

Graft AIX partitions enumeration in partitions/msdos.c

There is already a AIX disks detection logic in msdos.c.  When an
AIX disk has been found, and if configured to, call the aix partitions
recognizer.  This avoids removal of AIX disks protection from msdos.c,
avoids code duplication, and ensures that AIX partitions enumeration
is called before plain msdos partitions enumeration.

Signed-off-by: Philippe De Muyter <phdm@macqel.be>
Cc: Karel Zak <kzak@redhat.com>
Cc: Jens Axboe <axboe@kernel.dk>
Cc: Andrew Morton <akpm@linux-foundation.org>
---
 block/partitions/msdos.c |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/block/partitions/msdos.c b/block/partitions/msdos.c
index 9bf19e6..9123f25 100644
--- a/block/partitions/msdos.c
+++ b/block/partitions/msdos.c
@@ -23,6 +23,7 @@
 #include "check.h"
 #include "msdos.h"
 #include "efi.h"
+#include "aix.h"
 
 /*
  * Many architectures don't like unaligned accesses, while
@@ -462,8 +463,12 @@ int msdos_partition(struct parsed_partitions *state)
 	 */
 	if (aix_magic_present(state, data)) {
 		put_dev_sector(sect);
+#ifdef CONFIG_AIX_PARTITION
+		return aix_partition(state);
+#else
 		strlcat(state->pp_buf, " [AIX]", PAGE_SIZE);
 		return 0;
+#endif
 	}
 
 	if (!msdos_magic_present(data + 510)) {
-- 
1.7.1


^ permalink raw reply related	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2013-05-21  7:16 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-04-25 21:10 [RFC PATCH 0/5] Add aix lvm partitions support Philippe De Muyter
2013-04-25 21:10 ` [PATCH 1/5] partitions/msdos.c: end-of-line whitespace and semicolon cleanup Philippe De Muyter
2013-04-25 21:10 ` [PATCH 2/5] Add aix lvm partitions support files Philippe De Muyter
2013-04-29  9:37   ` Karel Zak
2013-04-29 11:40     ` Philippe De Muyter
2013-04-29 12:36       ` Karel Zak
2013-04-29 15:34         ` Philippe De Muyter
2013-04-30  6:41           ` Jens Axboe
2013-04-30  6:45             ` Philippe De Muyter
2013-04-29 21:50       ` Philippe De Muyter
2013-04-30  7:08         ` Philippe De Muyter
2013-04-30  7:18           ` Philippe De Muyter
2013-05-01  5:35             ` Philippe De Muyter
2013-04-25 21:10 ` [PATCH 3/5] partitions/msdos: enumerate also AIX LVM partitions Philippe De Muyter
2013-04-25 21:10 ` [PATCH 4/5] partitions/Makefile: compile aix.c if configured Philippe De Muyter
2013-04-25 21:10 ` [PATCH 5/5] partitions/Kconfig: add the AIX_PARTITION entry Philippe De Muyter
2013-04-29 21:18 [PATCH v2 0/5] partitions: add AIX LVM support Philippe De Muyter
2013-04-29 21:18 ` [PATCH 3/5] partitions/msdos: enumerate also AIX LVM partitions Philippe De Muyter
2013-05-20 23:41   ` Andrew Morton
2013-05-21  7:16     ` Philippe De Muyter
2013-05-01  6:09 [PATCH v3 0/5] partitions: Add aix lvm partitions support Philippe De Muyter
2013-05-01  6:09 ` [PATCH 3/5] partitions/msdos: enumerate also AIX LVM partitions Philippe De Muyter

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.