linux-raid.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [mdadm,v1 PATCH 0/6] Extend mdadm [...] --export
@ 2012-09-26 11:42 Maciej Naruszewicz
  2012-09-26 11:42 ` [PATCH 1/6] imsm: Add --export option for --detail-platform Maciej Naruszewicz
                   ` (5 more replies)
  0 siblings, 6 replies; 14+ messages in thread
From: Maciej Naruszewicz @ 2012-09-26 11:42 UTC (permalink / raw)
  To: neilb; +Cc: linux-raid, maciej.patelczyk

The following patches:

1. Add support for --mdadm --detail-platform --export with an additional option to provide specific controller to scan. The controller might be an absolute path to an actual   device, or just a symlink.

2. Synchronize two methods of calculating size and add the possibility to choose output format (IEC or JEDEC prefixes).

3. Add an additional value to mdadm --examine --export

---

Maciej Naruszewicz (6):
      imsm: Add --export option for --detail-platform
      imsm: Add --controller-path option for --detail-platform.
      Fix return code for --detail-platform
      Synchronize size calculation in human_size and human_size_brief
      Display size with human_size_brief with a chosen prefix
      Add MD_ARRAY_SIZE for --examine --export


 Create.c      |    2 +
 Detail.c      |   13 +++++--
 Query.c       |    2 +
 ReadMe.c      |    6 ++-
 mdadm.8.in    |   10 +++++
 mdadm.c       |   32 +++++++++++++++++
 mdadm.h       |   14 ++++++--
 super-intel.c |  105 +++++++++++++++++++++++++++++++++++++++++++++++++--------
 super1.c      |   19 ++++++++++
 util.c        |   51 +++++++++++++++++++++-------
 10 files changed, 213 insertions(+), 41 deletions(-)

--
MN

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

* [PATCH 1/6] imsm: Add --export option for --detail-platform
  2012-09-26 11:42 [mdadm,v1 PATCH 0/6] Extend mdadm [...] --export Maciej Naruszewicz
@ 2012-09-26 11:42 ` Maciej Naruszewicz
  2012-10-02  6:28   ` NeilBrown
  2012-09-26 11:42 ` [PATCH 2/6] imsm: Add --controller-path " Maciej Naruszewicz
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 14+ messages in thread
From: Maciej Naruszewicz @ 2012-09-26 11:42 UTC (permalink / raw)
  To: neilb; +Cc: linux-raid, maciej.patelczyk

This option will provide most of information we can get via
mdadm --detail-platform [-e format] in the key=value format.
Example output:

$ mdadm --detail-platform
       Platform : Intel(R) Matrix Storage Manager
        Version : 9.5.0.1037
    RAID Levels : raid0 raid1 raid10 raid5
    Chunk Sizes : 4k 8k 16k 32k 64k 128k
    2TB volumes : supported
      2TB disks : not supported
      Max Disks : 7
    Max Volumes : 2 per array, 4 per controller
 I/O Controller : /sys/devices/pci0000:00/0000:00:1f.2 (SATA)

$ mdadm --detail-platform --export
MD_FIRMWARE_TYPE=imsm
IMSM_VERSION=9.5.0.1037
IMSM_SUPPORTED_RAID_LEVELS=raid0 raid1 raid10 raid5
IMSM_SUPPORTED_CHUNK_SIZES=4k 8k 16k 32k 64k 128k
IMSM_2TB_VOLUMES=yes
IMSM_2TB_DISKS=no
IMSM_MAX_DISKS=7
IMSM_MAX_VOLUMES_PER_ARRAY=2
IMSM_MAX_VOLUMES_PER_CONTROLLER=4

Signed-off-by: Maciej Naruszewicz <maciej.naruszewicz@intel.com>
---
 Detail.c      |    8 +++++--
 ReadMe.c      |    4 ++-
 mdadm.8.in    |    2 +-
 mdadm.c       |    2 +-
 mdadm.h       |    3 ++
 super-intel.c |   70 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 6 files changed, 82 insertions(+), 7 deletions(-)

diff --git a/Detail.c b/Detail.c
index f633d93..b0c31e6 100644
--- a/Detail.c
+++ b/Detail.c
@@ -616,7 +616,7 @@ out:
 	return rv;
 }
 
-int Detail_Platform(struct superswitch *ss, int scan, int verbose)
+int Detail_Platform(struct superswitch *ss, int scan, int verbose, int export)
 {
 	/* display platform capabilities for the given metadata format
 	 * 'scan' in this context means iterate over all metadata types
@@ -624,7 +624,9 @@ int Detail_Platform(struct superswitch *ss, int scan, int verbose)
 	int i;
 	int err = 1;
 
-	if (ss && ss->detail_platform)
+	if (ss && export && ss->export_detail_platform)
+		err = ss->export_detail_platform(verbose);
+	else if (ss && ss->detail_platform)
 		err = ss->detail_platform(verbose, 0);
 	else if (ss) {
 		if (verbose > 0)
@@ -650,6 +652,8 @@ int Detail_Platform(struct superswitch *ss, int scan, int verbose)
 			if (verbose > 0)
 				pr_err("%s metadata is platform independent\n",
 					meta->name ? : "[no name]");
+		} else if (export){
+			err |= meta->export_detail_platform(verbose);
 		} else
 			err |= meta->detail_platform(verbose, 0);
 	}
diff --git a/ReadMe.c b/ReadMe.c
index 35ffeaa..346f08c 100644
--- a/ReadMe.c
+++ b/ReadMe.c
@@ -238,8 +238,8 @@ char OptionHelp[] =
 "  --verbose     -v   : Be more verbose about what is happening\n"
 "  --quiet       -q   : Don't print un-necessary messages\n"
 "  --brief       -b   : Be less verbose, more brief\n"
-"  --export      -Y   : With --detail, use key=value format for easy\n"
-"                       import into environment\n"
+"  --export      -Y   : With --detail, --detail-platform or --examine use\n"
+"                       key=value format for easy import into environment\n"
 "  --force       -f   : Override normal checks and be more forceful\n"
 "\n"
 "  --assemble    -A   : Assemble an array\n"
diff --git a/mdadm.8.in b/mdadm.8.in
index 33919bd..38c8bc8 100644
--- a/mdadm.8.in
+++ b/mdadm.8.in
@@ -1321,7 +1321,7 @@ topology) for a given metadata format.
 .TP
 .BR \-Y ", " \-\-export
 When used with
-.B \-\-detail
+.B \-\-detail , \-\-detail-platform
 or
 .BR \-\-examine ,
 output will be formatted as
diff --git a/mdadm.c b/mdadm.c
index 4c7c5ea..3ee7ddb 100644
--- a/mdadm.c
+++ b/mdadm.c
@@ -1344,7 +1344,7 @@ int main(int argc, char *argv[])
 			}
 			rv = Examine(devlist, &c, ss);
 		} else if (devmode == DetailPlatform) {
-			rv = Detail_Platform(ss ? ss->ss : NULL, ss ? c.scan : 1, c.verbose);
+			rv = Detail_Platform(ss ? ss->ss : NULL, ss ? c.scan : 1, c.verbose, c.export);
 		} else if (devlist == NULL) {
 			if (devmode == 'S' && c.scan)
 				rv = stop_scan(c.verbose);
diff --git a/mdadm.h b/mdadm.h
index f202ffa..6d219f7 100644
--- a/mdadm.h
+++ b/mdadm.h
@@ -655,6 +655,7 @@ extern struct superswitch {
 
 	/* Optional: platform hardware / firmware details */
 	int (*detail_platform)(int verbose, int enumerate_only);
+	int (*export_detail_platform)(int verbose);
 
 	/* Used:
 	 *   to get uuid to storing in bitmap metadata
@@ -1121,7 +1122,7 @@ extern int Create(struct supertype *st, char *mddev,
 		  struct context *c);
 
 extern int Detail(char *dev, struct context *c);
-extern int Detail_Platform(struct superswitch *ss, int scan, int verbose);
+extern int Detail_Platform(struct superswitch *ss, int scan, int verbose, int export);
 extern int Query(char *dev);
 extern int Examine(struct mddev_dev *devlist, struct context *c,
 		   struct supertype *forcest);
diff --git a/super-intel.c b/super-intel.c
index 107550f..fdf441a 100644
--- a/super-intel.c
+++ b/super-intel.c
@@ -1798,6 +1798,41 @@ static void print_imsm_capability(const struct imsm_orom *orom)
 	return;
 }
 
+static void print_imsm_capability_export(const struct imsm_orom *orom)
+{
+	printf("MD_FIRMWARE_TYPE=imsm\n");
+	printf("IMSM_VERSION=%d.%d.%d.%d\n",orom->major_ver, orom->minor_ver,
+			orom->hotfix_ver, orom->build);
+	printf("IMSM_SUPPORTED_RAID_LEVELS=%s%s%s%s%s\n",
+			imsm_orom_has_raid0(orom) ? "raid0 " : "",
+			imsm_orom_has_raid1(orom) ? "raid1 " : "",
+			imsm_orom_has_raid1e(orom) ? "raid1e " : "",
+			imsm_orom_has_raid5(orom) ? "raid10 " : "",
+			imsm_orom_has_raid10(orom) ? "raid5 " : "");
+	printf("IMSM_SUPPORTED_CHUNK_SIZES=%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s\n",
+			imsm_orom_has_chunk(orom, 2) ? "2k " : "",
+			imsm_orom_has_chunk(orom, 4) ? "4k " : "",
+			imsm_orom_has_chunk(orom, 8) ? "8k " : "",
+			imsm_orom_has_chunk(orom, 16) ? "16k " : "",
+			imsm_orom_has_chunk(orom, 32) ? "32k " : "",
+			imsm_orom_has_chunk(orom, 64) ? "64k " : "",
+			imsm_orom_has_chunk(orom, 128) ? "128k " : "",
+			imsm_orom_has_chunk(orom, 256) ? "256k " : "",
+			imsm_orom_has_chunk(orom, 512) ? "512k " : "",
+			imsm_orom_has_chunk(orom, 1024*1) ? "1M " : "",
+			imsm_orom_has_chunk(orom, 1024*2) ? "2M " : "",
+			imsm_orom_has_chunk(orom, 1024*4) ? "4M " : "",
+			imsm_orom_has_chunk(orom, 1024*8) ? "8M " : "",
+			imsm_orom_has_chunk(orom, 1024*16) ? "16M " : "",
+			imsm_orom_has_chunk(orom, 1024*32) ? "32M " : "",
+			imsm_orom_has_chunk(orom, 1024*64) ? "64M " : "");
+	printf("IMSM_2TB_VOLUMES=%s\n",(orom->attr & IMSM_OROM_ATTR_2TB) ? "yes" : "no");
+	printf("IMSM_2TB_DISKS=%s\n",(orom->attr & IMSM_OROM_ATTR_2TB_DISK) ? "yes" : "no");
+	printf("IMSM_MAX_DISKS=%d\n",orom->tds);
+	printf("IMSM_MAX_VOLUMES_PER_ARRAY=%d\n",orom->vpa);
+	printf("IMSM_MAX_VOLUMES_PER_CONTROLLER=%d\n",orom->vphba);
+}
+
 static int detail_platform_imsm(int verbose, int enumerate_only)
 {
 	/* There are two components to imsm platform support, the ahci SATA
@@ -1871,6 +1906,40 @@ static int detail_platform_imsm(int verbose, int enumerate_only)
 	free_sys_dev(&list);
 	return result;
 }
+
+static int export_detail_platform_imsm(int verbose)
+{
+	const struct imsm_orom *orom;
+	struct sys_dev *list, *hba;
+	int result=1;
+
+	list = find_intel_devices();
+	if (!list) {
+		if (verbose > 0)
+			pr_err("IMSM_DETAIL_PLATFORM_ERROR=NO_INTEL_DEVICES\n");
+		result = 2;
+		free_sys_dev(&list);
+		return result;
+	}
+
+	for (hba = list; hba; hba = hba->next) {
+		orom = find_imsm_capability(hba->type);
+		if (!orom) {
+			if (verbose > 0)
+				pr_err("IMSM_DETAIL_PLATFORM_ERROR=NO_IMSM_CAPABLE_DEVICE_UNDER_%s\n",hba->path);
+		}
+		else {
+			print_imsm_capability_export(orom);
+			result = 0;
+		}
+	}
+
+	if (result == 1 && verbose > 0)
+		pr_err("IMSM_DETAIL_PLATFORM_ERROR=NO_IMSM_CAPABLE_DEVICES\n");
+
+	return result;
+}
+
 #endif
 
 static int match_home_imsm(struct supertype *st, char *homehost)
@@ -10397,6 +10466,7 @@ struct superswitch super_imsm = {
 	.add_to_super	= add_to_super_imsm,
 	.remove_from_super = remove_from_super_imsm,
 	.detail_platform = detail_platform_imsm,
+	.export_detail_platform = export_detail_platform_imsm,
 	.kill_subarray = kill_subarray_imsm,
 	.update_subarray = update_subarray_imsm,
 	.load_container	= load_container_imsm,


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

* [PATCH 2/6] imsm: Add --controller-path option for --detail-platform.
  2012-09-26 11:42 [mdadm,v1 PATCH 0/6] Extend mdadm [...] --export Maciej Naruszewicz
  2012-09-26 11:42 ` [PATCH 1/6] imsm: Add --export option for --detail-platform Maciej Naruszewicz
@ 2012-09-26 11:42 ` Maciej Naruszewicz
  2012-10-02  6:36   ` NeilBrown
  2012-09-26 11:42 ` [PATCH 3/6] Fix return code " Maciej Naruszewicz
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 14+ messages in thread
From: Maciej Naruszewicz @ 2012-09-26 11:42 UTC (permalink / raw)
  To: neilb; +Cc: linux-raid, maciej.patelczyk

Usually, 'mdadm --detail-platform -e imsm' scans all the controllers
looking for IMSM capabilities. This patch provides the possibility
to specify a controller to scan, enabling custom usage by other
processes - especially with the --export switch.

$ mdadm --detail-platform
       Platform : Intel(R) Matrix Storage Manager
        Version : 9.5.0.1037
    RAID Levels : raid0 raid1 raid10 raid5
    Chunk Sizes : 4k 8k 16k 32k 64k 128k
    2TB volumes : supported
      2TB disks : not supported
      Max Disks : 7
    Max Volumes : 2 per array, 4 per controller
 I/O Controller : /sys/devices/pci0000:00/0000:00:1f.2 (SATA)

$ mdadm --detail-platform --controller-path=/sys/devices/pci0000:00/0000:00:1f.2
       Platform : Intel(R) Matrix Storage Manager
        Version : 9.5.0.1037
    RAID Levels : raid0 raid1 raid10 raid5
    Chunk Sizes : 4k 8k 16k 32k 64k 128k
    2TB volumes : supported
      2TB disks : not supported
      Max Disks : 7
    Max Volumes : 2 per array, 4 per controller
 I/O Controller : /sys/devices/pci0000:00/0000:00:1f.2 (SATA)

$ mdadm --detail-platform --controller-path=/sys/devices/pci0000:00/0000:00:1f.2 --export
MD_FIRMWARE_TYPE=imsm
IMSM_VERSION=9.5.0.1037
IMSM_SUPPORTED_RAID_LEVELS=raid0 raid1 raid10 raid5
IMSM_SUPPORTED_CHUNK_SIZES=4k 8k 16k 32k 64k 128k
IMSM_2TB_VOLUMES=yes
IMSM_2TB_DISKS=no
IMSM_MAX_DISKS=7
IMSM_MAX_VOLUMES_PER_ARRAY=2
IMSM_MAX_VOLUMES_PER_CONTROLLER=4

$ mdadm --detail-platform --controller-path=/sys/devices/pci0000:00/0000:00:1f.0 # This isn't an IMSM-capable controller
mdadm: no active Intel(R) RAID controller found under /sys/devices/pci0000:00/0000:00:1f.0

Signed-off-by: Maciej Naruszewicz <maciej.naruszewicz@intel.com>
---
 Create.c      |    2 +-
 Detail.c      |   10 +++++-----
 ReadMe.c      |    2 ++
 mdadm.8.in    |    8 ++++++++
 mdadm.c       |   32 +++++++++++++++++++++++++++++++-
 mdadm.h       |    8 +++++---
 super-intel.c |   45 +++++++++++++++++++++++++--------------------
 7 files changed, 77 insertions(+), 30 deletions(-)

diff --git a/Create.c b/Create.c
index 32683a8..2da07d0 100644
--- a/Create.c
+++ b/Create.c
@@ -492,7 +492,7 @@ int Create(struct supertype *st, char *mddev,
 		warn = 1;
 	}
 
-	if (st->ss->detail_platform && st->ss->detail_platform(0, 1) != 0) {
+	if (st->ss->detail_platform && st->ss->detail_platform(0, 1, NULL) != 0) {
 		if (c->runstop != 1 || c->verbose >= 0)
 			pr_err("%s unable to enumerate platform support\n"
 				"    array may not be compatible with hardware/firmware\n",
diff --git a/Detail.c b/Detail.c
index b0c31e6..57faf3c 100644
--- a/Detail.c
+++ b/Detail.c
@@ -616,7 +616,7 @@ out:
 	return rv;
 }
 
-int Detail_Platform(struct superswitch *ss, int scan, int verbose, int export)
+int Detail_Platform(struct superswitch *ss, int scan, int verbose, int export, char *controller_path)
 {
 	/* display platform capabilities for the given metadata format
 	 * 'scan' in this context means iterate over all metadata types
@@ -625,9 +625,9 @@ int Detail_Platform(struct superswitch *ss, int scan, int verbose, int export)
 	int err = 1;
 
 	if (ss && export && ss->export_detail_platform)
-		err = ss->export_detail_platform(verbose);
+		err = ss->export_detail_platform(verbose, controller_path);
 	else if (ss && ss->detail_platform)
-		err = ss->detail_platform(verbose, 0);
+		err = ss->detail_platform(verbose, 0, controller_path);
 	else if (ss) {
 		if (verbose > 0)
 			pr_err("%s metadata is platform independent\n",
@@ -653,9 +653,9 @@ int Detail_Platform(struct superswitch *ss, int scan, int verbose, int export)
 				pr_err("%s metadata is platform independent\n",
 					meta->name ? : "[no name]");
 		} else if (export){
-			err |= meta->export_detail_platform(verbose);
+			err |= meta->export_detail_platform(verbose, controller_path);
 		} else
-			err |= meta->detail_platform(verbose, 0);
+			err |= meta->detail_platform(verbose, 0, controller_path);
 	}
 
 	return err;
diff --git a/ReadMe.c b/ReadMe.c
index 346f08c..0c9c80b 100644
--- a/ReadMe.c
+++ b/ReadMe.c
@@ -159,6 +159,7 @@ struct option long_options[] = {
     {"sparc2.2",  0, 0, Sparc22},
     {"test",      0, 0, 't'},
     {"prefer",    1, 0, Prefer},
+    {"controller-path",1, 0, ControllerPath},
 
     /* For Follow/monitor */
     {"mail",      1, 0, EMail},
@@ -240,6 +241,7 @@ char OptionHelp[] =
 "  --brief       -b   : Be less verbose, more brief\n"
 "  --export      -Y   : With --detail, --detail-platform or --examine use\n"
 "                       key=value format for easy import into environment\n"
+"  --controller-path  : Specify controller for --detail-platform\n"
 "  --force       -f   : Override normal checks and be more forceful\n"
 "\n"
 "  --assemble    -A   : Assemble an array\n"
diff --git a/mdadm.8.in b/mdadm.8.in
index 38c8bc8..8792e54 100644
--- a/mdadm.8.in
+++ b/mdadm.8.in
@@ -1329,6 +1329,14 @@ output will be formatted as
 pairs for easy import into the environment.
 
 .TP
+.BR \-\-controller\-path
+When used with
+.BR \-\-detail\-platform ,
+mdadm will only print information about the specified controller. The
+controller should be given in form of an absolute filepath, e.g.
+.B \-\-controller\-path /sys/devices/pci0000:00/0000:00:1f.0 .
+
+.TP
 .BR \-E ", " \-\-examine
 Print contents of the metadata stored on the named device(s).
 Note the contrast between
diff --git a/mdadm.c b/mdadm.c
index 3ee7ddb..d313b76 100644
--- a/mdadm.c
+++ b/mdadm.c
@@ -93,6 +93,7 @@ int main(int argc, char *argv[])
 	int rebuild_map = 0;
 	char *remove_path = NULL;
 	char *udev_filename = NULL;
+	char *actual_controller_path;
 
 	int print_help = 0;
 	FILE *outf;
@@ -151,6 +152,35 @@ int main(int argc, char *argv[])
 		case 'Y': c.export++;
 			continue;
 
+		case ControllerPath:
+			if (c.controller_path)
+				free(c.controller_path);
+			if (asprintf(&c.controller_path, "%s", optarg) <= 0) {
+				pr_err("Empty or wrong controller path, \'%s\' ignored.\n",
+					c.controller_path);
+				c.controller_path = NULL;
+			}
+			else {
+				struct stat st;
+				if (stat(c.controller_path, &st) != 0) {
+					pr_err("Specified controller path %s doesn't exist.\n",
+						c.controller_path);
+					exit(2);
+				}
+				else {
+					actual_controller_path = realpath(c.controller_path,NULL);
+					if (actual_controller_path) {
+						if(asprintf(&c.controller_path, "%s", actual_controller_path) > 0)
+							free(actual_controller_path);
+						else
+							free(c.controller_path);
+					}
+					else
+						free(c.controller_path);
+				}
+			}
+			continue;
+
 		case HomeHost:
 			if (strcasecmp(optarg, "<ignore>") == 0)
 				c.require_homehost = 0;
@@ -1344,7 +1374,7 @@ int main(int argc, char *argv[])
 			}
 			rv = Examine(devlist, &c, ss);
 		} else if (devmode == DetailPlatform) {
-			rv = Detail_Platform(ss ? ss->ss : NULL, ss ? c.scan : 1, c.verbose, c.export);
+			rv = Detail_Platform(ss ? ss->ss : NULL, ss ? c.scan : 1, c.verbose, c.export, c.controller_path);
 		} else if (devlist == NULL) {
 			if (devmode == 'S' && c.scan)
 				rv = stop_scan(c.verbose);
diff --git a/mdadm.h b/mdadm.h
index 6d219f7..e27275b 100644
--- a/mdadm.h
+++ b/mdadm.h
@@ -326,6 +326,7 @@ enum special_options {
 	OffRootOpt,
 	Prefer,
 	KillOpt,
+	ControllerPath,
 };
 
 /* structures read from config file */
@@ -394,6 +395,7 @@ struct context {
 	int	freeze_reshape;
 	char	*backup_file;
 	int	invalid_backup;
+	char	*controller_path;
 };
 
 struct shape {
@@ -654,8 +656,8 @@ extern struct superswitch {
 	void (*export_detail_super)(struct supertype *st);
 
 	/* Optional: platform hardware / firmware details */
-	int (*detail_platform)(int verbose, int enumerate_only);
-	int (*export_detail_platform)(int verbose);
+	int (*detail_platform)(int verbose, int enumerate_only, char *controller_path);
+	int (*export_detail_platform)(int verbose, char *controller_path);
 
 	/* Used:
 	 *   to get uuid to storing in bitmap metadata
@@ -1122,7 +1124,7 @@ extern int Create(struct supertype *st, char *mddev,
 		  struct context *c);
 
 extern int Detail(char *dev, struct context *c);
-extern int Detail_Platform(struct superswitch *ss, int scan, int verbose, int export);
+extern int Detail_Platform(struct superswitch *ss, int scan, int verbose, int export, char *controller_path);
 extern int Query(char *dev);
 extern int Examine(struct mddev_dev *devlist, struct context *c,
 		   struct supertype *forcest);
diff --git a/super-intel.c b/super-intel.c
index fdf441a..cedc976 100644
--- a/super-intel.c
+++ b/super-intel.c
@@ -1833,7 +1833,7 @@ static void print_imsm_capability_export(const struct imsm_orom *orom)
 	printf("IMSM_MAX_VOLUMES_PER_CONTROLLER=%d\n",orom->vphba);
 }
 
-static int detail_platform_imsm(int verbose, int enumerate_only)
+static int detail_platform_imsm(int verbose, int enumerate_only, char *controller_path)
 {
 	/* There are two components to imsm platform support, the ahci SATA
 	 * controller and the option-rom.  To find the SATA controller we
@@ -1850,7 +1850,7 @@ static int detail_platform_imsm(int verbose, int enumerate_only)
 	struct sys_dev *list, *hba;
 	int host_base = 0;
 	int port_count = 0;
-	int result=0;
+	int result=1;
 
 	if (enumerate_only) {
 		if (check_env("IMSM_NO_PLATFORM"))
@@ -1864,6 +1864,8 @@ static int detail_platform_imsm(int verbose, int enumerate_only)
 				result = 2;
 				break;
 			}
+			else
+				result = 0;
 		}
 		free_sys_dev(&list);
 		return result;
@@ -1880,34 +1882,38 @@ static int detail_platform_imsm(int verbose, int enumerate_only)
 		print_found_intel_controllers(list);
 
 	for (hba = list; hba; hba = hba->next) {
+		if (controller_path != NULL && ( strcmp(hba->path,controller_path) != 0 ))
+			continue;
 		orom = find_imsm_capability(hba->type);
 		if (!orom)
 			pr_err("imsm capabilities not found for controller: %s (type %s)\n",
 				hba->path, get_sys_dev_type(hba->type));
-		else
+		else {
+			result = 0;
 			print_imsm_capability(orom);
-	}
-
-	for (hba = list; hba; hba = hba->next) {
-		printf(" I/O Controller : %s (%s)\n",
-			hba->path, get_sys_dev_type(hba->type));
-
-		if (hba->type == SYS_DEV_SATA) {
-			host_base = ahci_get_port_count(hba->path, &port_count);
-			if (ahci_enumerate_ports(hba->path, port_count, host_base, verbose)) {
-				if (verbose > 0)
-					pr_err("failed to enumerate "
-						"ports on SATA controller at %s.", hba->pci_id);
-				result |= 2;
+			printf(" I/O Controller : %s (%s)\n",
+				hba->path, get_sys_dev_type(hba->type));
+			if (hba->type == SYS_DEV_SATA) {
+				host_base = ahci_get_port_count(hba->path, &port_count);
+				if (ahci_enumerate_ports(hba->path, port_count, host_base, verbose)) {
+					if (verbose > 0)
+						pr_err("failed to enumerate "
+							"ports on SATA controller at %s.\n", hba->pci_id);
+					result |= 2;
+				}
 			}
 		}
 	}
 
+	if (controller_path != NULL && result == 1)
+		pr_err("no active Intel(R) RAID "
+				"controller found under %s\n",controller_path);
+
 	free_sys_dev(&list);
 	return result;
 }
 
-static int export_detail_platform_imsm(int verbose)
+static int export_detail_platform_imsm(int verbose, char *controller_path)
 {
 	const struct imsm_orom *orom;
 	struct sys_dev *list, *hba;
@@ -1923,6 +1929,8 @@ static int export_detail_platform_imsm(int verbose)
 	}
 
 	for (hba = list; hba; hba = hba->next) {
+		if (controller_path != NULL && ( strcmp(hba->path,controller_path) != 0 ))
+			continue;
 		orom = find_imsm_capability(hba->type);
 		if (!orom) {
 			if (verbose > 0)
@@ -1934,9 +1942,6 @@ static int export_detail_platform_imsm(int verbose)
 		}
 	}
 
-	if (result == 1 && verbose > 0)
-		pr_err("IMSM_DETAIL_PLATFORM_ERROR=NO_IMSM_CAPABLE_DEVICES\n");
-
 	return result;
 }
 


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

* [PATCH 3/6] Fix return code for --detail-platform
  2012-09-26 11:42 [mdadm,v1 PATCH 0/6] Extend mdadm [...] --export Maciej Naruszewicz
  2012-09-26 11:42 ` [PATCH 1/6] imsm: Add --export option for --detail-platform Maciej Naruszewicz
  2012-09-26 11:42 ` [PATCH 2/6] imsm: Add --controller-path " Maciej Naruszewicz
@ 2012-09-26 11:42 ` Maciej Naruszewicz
  2012-10-02  6:38   ` NeilBrown
  2012-09-26 11:44 ` [PATCH 4/6] Synchronize size calculation in human_size and human_size_brief Maciej Naruszewicz
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 14+ messages in thread
From: Maciej Naruszewicz @ 2012-09-26 11:42 UTC (permalink / raw)
  To: neilb; +Cc: linux-raid, maciej.patelczyk

Variable 'err' is initially set to 1, so changing its value with
'|=' won't set it to 0 even if the operation is successful.

Signed-off-by: Maciej Naruszewicz <maciej.naruszewicz@intel.com>
---
 Detail.c |    1 +
 1 file changed, 1 insertion(+)

diff --git a/Detail.c b/Detail.c
index 57faf3c..96d1ff6 100644
--- a/Detail.c
+++ b/Detail.c
@@ -640,6 +640,7 @@ int Detail_Platform(struct superswitch *ss, int scan, int verbose, int export, c
 	if (!scan)
 		return err;
 
+	err = 0;
 	for (i = 0; superlist[i]; i++) {
 		struct superswitch *meta = superlist[i];
 


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

* [PATCH 4/6] Synchronize size calculation in human_size and human_size_brief
  2012-09-26 11:42 [mdadm,v1 PATCH 0/6] Extend mdadm [...] --export Maciej Naruszewicz
                   ` (2 preceding siblings ...)
  2012-09-26 11:42 ` [PATCH 3/6] Fix return code " Maciej Naruszewicz
@ 2012-09-26 11:44 ` Maciej Naruszewicz
  2012-10-02  6:40   ` NeilBrown
  2012-09-26 11:44 ` [PATCH 5/6] Display size with human_size_brief with a chosen prefix Maciej Naruszewicz
  2012-09-26 11:44 ` [PATCH 6/6] Add MD_ARRAY_SIZE for --examine --export Maciej Naruszewicz
  5 siblings, 1 reply; 14+ messages in thread
From: Maciej Naruszewicz @ 2012-09-26 11:44 UTC (permalink / raw)
  To: neilb; +Cc: linux-raid, maciej.patelczyk

It would be better if two size-calculating methods had the same
calculating algorithm. The human_size way of calculation seems
more readable, so let's use it for both methods.

Signed-off-by: Maciej Naruszewicz <maciej.naruszewicz@intel.com>
---
 util.c |   33 ++++++++++++++++++++-------------
 1 file changed, 20 insertions(+), 13 deletions(-)

diff --git a/util.c b/util.c
index c63a232..09971a2 100644
--- a/util.c
+++ b/util.c
@@ -686,20 +686,27 @@ char *human_size_brief(long long bytes)
 {
 	static char buf[30];
 
+	/* We convert bytes to either centi-M{ega,ibi}bytes or
+	 * centi-G{igi,ibi}bytes, with appropriate rounding,
+	 * and then print 1/100th of those as a decimal.
+	 * We allow upto 2048Megabytes before converting to
+	 * gigabytes, as that shows more precision and isn't
+	 * too large a number.
+	 * Terabytes are not yet handled.
+	 */
+
 	if (bytes < 5000*1024)
-		snprintf(buf, sizeof(buf), "%ld.%02ldKiB",
-			(long)(bytes>>10), (long)(((bytes&1023)*100+512)/1024)
-			);
-	else if (bytes < 2*1024LL*1024LL*1024LL)
-		snprintf(buf, sizeof(buf), "%ld.%02ldMiB",
-			(long)(bytes>>20),
-			(long)((bytes&0xfffff)+0x100000/200)/(0x100000/100)
-			);
-	else
-		snprintf(buf, sizeof(buf), "%ld.%02ldGiB",
-			(long)(bytes>>30),
-			(long)(((bytes>>10)&0xfffff)+0x100000/200)/(0x100000/100)
-			);
+		buf[0] = 0;
+	else if (bytes < 2*1024LL*1024LL*1024LL) {
+		long cMiB = (bytes / ( (1LL<<20) / 200LL ) +1) /2;
+		snprintf(buf, sizeof(buf), " (%ld.%02ldMiB)",
+			cMiB/100 , cMiB % 100);
+	} else {
+		long cGiB = (bytes / ( (1LL<<30) / 200LL ) +1) /2;
+		snprintf(buf, sizeof(buf), " (%ld.%02ldGiB)",
+				cGiB/100 , cGiB % 100);
+	}
+
 	return buf;
 }
 


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

* [PATCH 5/6] Display size with human_size_brief with a chosen prefix
  2012-09-26 11:42 [mdadm,v1 PATCH 0/6] Extend mdadm [...] --export Maciej Naruszewicz
                   ` (3 preceding siblings ...)
  2012-09-26 11:44 ` [PATCH 4/6] Synchronize size calculation in human_size and human_size_brief Maciej Naruszewicz
@ 2012-09-26 11:44 ` Maciej Naruszewicz
  2012-10-02  6:41   ` NeilBrown
  2012-09-26 11:44 ` [PATCH 6/6] Add MD_ARRAY_SIZE for --examine --export Maciej Naruszewicz
  5 siblings, 1 reply; 14+ messages in thread
From: Maciej Naruszewicz @ 2012-09-26 11:44 UTC (permalink / raw)
  To: neilb; +Cc: linux-raid, maciej.patelczyk

When using human_size_brief, only IEC prefixes were supported. Now
it's possible to specify which format we want to see - either IEC
(kibi, mibi, gibi) or JEDEC (kilo, mega, giga).

Signed-off-by: Maciej Naruszewicz <maciej.naruszewicz@intel.com>
---
 Query.c |    2 +-
 mdadm.h |    7 ++++++-
 util.c  |   36 +++++++++++++++++++++++++++---------
 3 files changed, 34 insertions(+), 11 deletions(-)

diff --git a/Query.c b/Query.c
index b9c209f..329e583 100644
--- a/Query.c
+++ b/Query.c
@@ -76,7 +76,7 @@ int Query(char *dev)
 	else {
 		printf("%s: %s %s %d devices, %d spare%s. Use mdadm --detail for more detail.\n",
 		       dev,
-		       human_size_brief(larray_size),
+		       human_size_brief(larray_size,IEC),
 		       map_num(pers, array.level),
 		       array.raid_disks,
 		       array.spare_disks, array.spare_disks==1?"":"s");
diff --git a/mdadm.h b/mdadm.h
index e27275b..a24b803 100644
--- a/mdadm.h
+++ b/mdadm.h
@@ -329,6 +329,11 @@ enum special_options {
 	ControllerPath,
 };
 
+enum prefix_standard {
+	JEDEC,
+	IEC
+};
+
 /* structures read from config file */
 /* List of mddevice names and identifiers
  * Identifiers can be:
@@ -1242,7 +1247,7 @@ extern int set_array_info(int mdfd, struct supertype *st, struct mdinfo *info);
 unsigned long long min_recovery_start(struct mdinfo *array);
 
 extern char *human_size(long long bytes);
-extern char *human_size_brief(long long bytes);
+extern char *human_size_brief(long long bytes, int prefix);
 extern void print_r10_layout(int layout);
 
 #define NoMdDev (1<<23)
diff --git a/util.c b/util.c
index 09971a2..cb97816 100644
--- a/util.c
+++ b/util.c
@@ -682,7 +682,7 @@ char *human_size(long long bytes)
 	return buf;
 }
 
-char *human_size_brief(long long bytes)
+char *human_size_brief(long long bytes, int prefix)
 {
 	static char buf[30];
 
@@ -693,19 +693,37 @@ char *human_size_brief(long long bytes)
 	 * gigabytes, as that shows more precision and isn't
 	 * too large a number.
 	 * Terabytes are not yet handled.
+	 *
+	 * If prefix == IEC, we mean prefixes like kibi,mebi,gibi etc.
+	 * If prefix == JEDEC, we mean prefixes like kilo,mega,giga etc.
 	 */
 
 	if (bytes < 5000*1024)
 		buf[0] = 0;
-	else if (bytes < 2*1024LL*1024LL*1024LL) {
-		long cMiB = (bytes / ( (1LL<<20) / 200LL ) +1) /2;
-		snprintf(buf, sizeof(buf), " (%ld.%02ldMiB)",
-			cMiB/100 , cMiB % 100);
-	} else {
-		long cGiB = (bytes / ( (1LL<<30) / 200LL ) +1) /2;
-		snprintf(buf, sizeof(buf), " (%ld.%02ldGiB)",
-				cGiB/100 , cGiB % 100);
+	else if (prefix == IEC) {
+		if (bytes < 2*1024LL*1024LL*1024LL) {
+			long cMiB = (bytes / ( (1LL<<20) / 200LL ) +1) /2;
+			snprintf(buf, sizeof(buf), "%ld.%02ldMiB",
+				cMiB/100 , cMiB % 100);
+		} else {
+			long cGiB = (bytes / ( (1LL<<30) / 200LL ) +1) /2;
+			snprintf(buf, sizeof(buf), "%ld.%02ldGiB",
+					cGiB/100 , cGiB % 100);
+		}
+	}
+	else if (prefix == JEDEC) {
+		if (bytes < 2*1024LL*1024LL*1024LL) {
+			long cMB  = (bytes / ( 1000000LL / 200LL ) +1) /2;
+			snprintf(buf, sizeof(buf), "%ld.%02ldMB",
+					cMB/100, cMB % 100);
+		} else {
+			long cGB  = (bytes / (1000000000LL/200LL ) +1) /2;
+			snprintf(buf, sizeof(buf), "%ld.%02ldGB",
+					cGB/100 , cGB % 100);
+		}
 	}
+	else
+		buf[0] = 0;
 
 	return buf;
 }


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

* [PATCH 6/6] Add MD_ARRAY_SIZE for --examine --export
  2012-09-26 11:42 [mdadm,v1 PATCH 0/6] Extend mdadm [...] --export Maciej Naruszewicz
                   ` (4 preceding siblings ...)
  2012-09-26 11:44 ` [PATCH 5/6] Display size with human_size_brief with a chosen prefix Maciej Naruszewicz
@ 2012-09-26 11:44 ` Maciej Naruszewicz
  2012-10-02  6:42   ` NeilBrown
  5 siblings, 1 reply; 14+ messages in thread
From: Maciej Naruszewicz @ 2012-09-26 11:44 UTC (permalink / raw)
  To: neilb; +Cc: linux-raid, maciej.patelczyk

An additional pair of key=value for --examine --export.

Signed-off-by: Maciej Naruszewicz <maciej.naruszewicz@intel.com>
---
 super1.c |   19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/super1.c b/super1.c
index b04945f..0df5bee 100644
--- a/super1.c
+++ b/super1.c
@@ -495,6 +495,7 @@ static void export_examine_super1(struct supertype *st)
 	struct mdp_superblock_1 *sb = st->sb;
 	int i;
 	int len = 32;
+	int layout;
 
 	printf("MD_LEVEL=%s\n", map_num(pers, __le32_to_cpu(sb->level)));
 	printf("MD_DEVICES=%d\n", __le32_to_cpu(sb->raid_disks));
@@ -506,6 +507,24 @@ static void export_examine_super1(struct supertype *st)
 		}
 	if (len)
 		printf("MD_NAME=%.*s\n", len, sb->set_name);
+	if (__le32_to_cpu(sb->level) > 0) {
+		int ddsks = 0, ddsks_denom = 1;
+		switch(__le32_to_cpu(sb->level)) {
+			case 1: ddsks=1;break;
+			case 4:
+			case 5: ddsks = __le32_to_cpu(sb->raid_disks)-1; break;
+			case 6: ddsks = __le32_to_cpu(sb->raid_disks)-2; break;
+			case 10:
+				layout = __le32_to_cpu(sb->layout);
+				ddsks = __le32_to_cpu(sb->raid_disks);
+				ddsks_denom = (layout&255) * ((layout>>8)&255);
+			}
+		if (ddsks) {
+			long long asize = __le64_to_cpu(sb->size);
+			asize = (asize << 9) * ddsks / ddsks_denom;
+			printf("MD_ARRAY_SIZE=%s\n",human_size_brief(asize,JEDEC));
+		}
+	}
 	printf("MD_UUID=");
 	for (i=0; i<16; i++) {
 		if ((i&3)==0 && i != 0) printf(":");


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

* Re: [PATCH 1/6] imsm: Add --export option for --detail-platform
  2012-09-26 11:42 ` [PATCH 1/6] imsm: Add --export option for --detail-platform Maciej Naruszewicz
@ 2012-10-02  6:28   ` NeilBrown
  0 siblings, 0 replies; 14+ messages in thread
From: NeilBrown @ 2012-10-02  6:28 UTC (permalink / raw)
  To: Maciej Naruszewicz; +Cc: linux-raid, maciej.patelczyk

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

On Wed, 26 Sep 2012 13:42:43 +0200 Maciej Naruszewicz
<maciej.naruszewicz@intel.com> wrote:

> This option will provide most of information we can get via
> mdadm --detail-platform [-e format] in the key=value format.
> Example output:
> 
> $ mdadm --detail-platform
>        Platform : Intel(R) Matrix Storage Manager
>         Version : 9.5.0.1037
>     RAID Levels : raid0 raid1 raid10 raid5
>     Chunk Sizes : 4k 8k 16k 32k 64k 128k
>     2TB volumes : supported
>       2TB disks : not supported
>       Max Disks : 7
>     Max Volumes : 2 per array, 4 per controller
>  I/O Controller : /sys/devices/pci0000:00/0000:00:1f.2 (SATA)
> 
> $ mdadm --detail-platform --export
> MD_FIRMWARE_TYPE=imsm
> IMSM_VERSION=9.5.0.1037
> IMSM_SUPPORTED_RAID_LEVELS=raid0 raid1 raid10 raid5
> IMSM_SUPPORTED_CHUNK_SIZES=4k 8k 16k 32k 64k 128k
> IMSM_2TB_VOLUMES=yes
> IMSM_2TB_DISKS=no
> IMSM_MAX_DISKS=7
> IMSM_MAX_VOLUMES_PER_ARRAY=2
> IMSM_MAX_VOLUMES_PER_CONTROLLER=4
> 
> Signed-off-by: Maciej Naruszewicz <maciej.naruszewicz@intel.com>
> ---
>  Detail.c      |    8 +++++--
>  ReadMe.c      |    4 ++-
>  mdadm.8.in    |    2 +-
>  mdadm.c       |    2 +-
>  mdadm.h       |    3 ++
>  super-intel.c |   70 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  6 files changed, 82 insertions(+), 7 deletions(-)
> 
> diff --git a/Detail.c b/Detail.c
> index f633d93..b0c31e6 100644
> --- a/Detail.c
> +++ b/Detail.c
> @@ -616,7 +616,7 @@ out:
>  	return rv;
>  }
>  
> -int Detail_Platform(struct superswitch *ss, int scan, int verbose)
> +int Detail_Platform(struct superswitch *ss, int scan, int verbose, int export)
>  {
>  	/* display platform capabilities for the given metadata format
>  	 * 'scan' in this context means iterate over all metadata types
> @@ -624,7 +624,9 @@ int Detail_Platform(struct superswitch *ss, int scan, int verbose)
>  	int i;
>  	int err = 1;
>  
> -	if (ss && ss->detail_platform)
> +	if (ss && export && ss->export_detail_platform)
> +		err = ss->export_detail_platform(verbose);
> +	else if (ss && ss->detail_platform)
>  		err = ss->detail_platform(verbose, 0);
>  	else if (ss) {
>  		if (verbose > 0)
> @@ -650,6 +652,8 @@ int Detail_Platform(struct superswitch *ss, int scan, int verbose)
>  			if (verbose > 0)
>  				pr_err("%s metadata is platform independent\n",
>  					meta->name ? : "[no name]");
> +		} else if (export){
> +			err |= meta->export_detail_platform(verbose);
>  		} else
>  			err |= meta->detail_platform(verbose, 0);
>  	}

I changed this to:

  } else if (export && meta->export_detail_platform) {

otherwise looks good,
thanks,
NeilBrown

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

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

* Re: [PATCH 2/6] imsm: Add --controller-path option for --detail-platform.
  2012-09-26 11:42 ` [PATCH 2/6] imsm: Add --controller-path " Maciej Naruszewicz
@ 2012-10-02  6:36   ` NeilBrown
  2012-10-02 10:54     ` Maciej Naruszewicz
  0 siblings, 1 reply; 14+ messages in thread
From: NeilBrown @ 2012-10-02  6:36 UTC (permalink / raw)
  To: Maciej Naruszewicz; +Cc: linux-raid, maciej.patelczyk

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

On Wed, 26 Sep 2012 13:42:50 +0200 Maciej Naruszewicz
<maciej.naruszewicz@intel.com> wrote:

> Usually, 'mdadm --detail-platform -e imsm' scans all the controllers
> looking for IMSM capabilities. This patch provides the possibility
> to specify a controller to scan, enabling custom usage by other
> processes - especially with the --export switch.
> 
> $ mdadm --detail-platform
>        Platform : Intel(R) Matrix Storage Manager
>         Version : 9.5.0.1037
>     RAID Levels : raid0 raid1 raid10 raid5
>     Chunk Sizes : 4k 8k 16k 32k 64k 128k
>     2TB volumes : supported
>       2TB disks : not supported
>       Max Disks : 7
>     Max Volumes : 2 per array, 4 per controller
>  I/O Controller : /sys/devices/pci0000:00/0000:00:1f.2 (SATA)
> 
> $ mdadm --detail-platform --controller-path=/sys/devices/pci0000:00/0000:00:1f.2
>        Platform : Intel(R) Matrix Storage Manager
>         Version : 9.5.0.1037
>     RAID Levels : raid0 raid1 raid10 raid5
>     Chunk Sizes : 4k 8k 16k 32k 64k 128k
>     2TB volumes : supported
>       2TB disks : not supported
>       Max Disks : 7
>     Max Volumes : 2 per array, 4 per controller
>  I/O Controller : /sys/devices/pci0000:00/0000:00:1f.2 (SATA)
> 
> $ mdadm --detail-platform --controller-path=/sys/devices/pci0000:00/0000:00:1f.2 --export
> MD_FIRMWARE_TYPE=imsm
> IMSM_VERSION=9.5.0.1037
> IMSM_SUPPORTED_RAID_LEVELS=raid0 raid1 raid10 raid5
> IMSM_SUPPORTED_CHUNK_SIZES=4k 8k 16k 32k 64k 128k
> IMSM_2TB_VOLUMES=yes
> IMSM_2TB_DISKS=no
> IMSM_MAX_DISKS=7
> IMSM_MAX_VOLUMES_PER_ARRAY=2
> IMSM_MAX_VOLUMES_PER_CONTROLLER=4
> 
> $ mdadm --detail-platform --controller-path=/sys/devices/pci0000:00/0000:00:1f.0 # This isn't an IMSM-capable controller
> mdadm: no active Intel(R) RAID controller found under /sys/devices/pci0000:00/0000:00:1f.0

hi,
 I think this functionality is good.  I think the implementation is not.

1/ Don't have a "--controller-path" option, just require any name
   given as a bare argument to the controller path - and probably require
   there is either 0 or 1.
   So:
      mdadm --detail-platform  /sys/devices/pci0000:00/0000:00:1f.0

   is the correct usage.

2/ You really don't need to 
     if (asprintf(&c.controller_path, "%s", optarg) <= 0) {

   just use 'optarg' directly.

3/ Don't use 'realpath' and 'strcmp'.  To compare two paths, use "stat" and
    compare st_dev and st_ino.

So: not applied.

Thanks,
NeilBrown



> 
> Signed-off-by: Maciej Naruszewicz <maciej.naruszewicz@intel.com>
> ---
>  Create.c      |    2 +-
>  Detail.c      |   10 +++++-----
>  ReadMe.c      |    2 ++
>  mdadm.8.in    |    8 ++++++++
>  mdadm.c       |   32 +++++++++++++++++++++++++++++++-
>  mdadm.h       |    8 +++++---
>  super-intel.c |   45 +++++++++++++++++++++++++--------------------
>  7 files changed, 77 insertions(+), 30 deletions(-)
> 
> diff --git a/Create.c b/Create.c
> index 32683a8..2da07d0 100644
> --- a/Create.c
> +++ b/Create.c
> @@ -492,7 +492,7 @@ int Create(struct supertype *st, char *mddev,
>  		warn = 1;
>  	}
>  
> -	if (st->ss->detail_platform && st->ss->detail_platform(0, 1) != 0) {
> +	if (st->ss->detail_platform && st->ss->detail_platform(0, 1, NULL) != 0) {
>  		if (c->runstop != 1 || c->verbose >= 0)
>  			pr_err("%s unable to enumerate platform support\n"
>  				"    array may not be compatible with hardware/firmware\n",
> diff --git a/Detail.c b/Detail.c
> index b0c31e6..57faf3c 100644
> --- a/Detail.c
> +++ b/Detail.c
> @@ -616,7 +616,7 @@ out:
>  	return rv;
>  }
>  
> -int Detail_Platform(struct superswitch *ss, int scan, int verbose, int export)
> +int Detail_Platform(struct superswitch *ss, int scan, int verbose, int export, char *controller_path)
>  {
>  	/* display platform capabilities for the given metadata format
>  	 * 'scan' in this context means iterate over all metadata types
> @@ -625,9 +625,9 @@ int Detail_Platform(struct superswitch *ss, int scan, int verbose, int export)
>  	int err = 1;
>  
>  	if (ss && export && ss->export_detail_platform)
> -		err = ss->export_detail_platform(verbose);
> +		err = ss->export_detail_platform(verbose, controller_path);
>  	else if (ss && ss->detail_platform)
> -		err = ss->detail_platform(verbose, 0);
> +		err = ss->detail_platform(verbose, 0, controller_path);
>  	else if (ss) {
>  		if (verbose > 0)
>  			pr_err("%s metadata is platform independent\n",
> @@ -653,9 +653,9 @@ int Detail_Platform(struct superswitch *ss, int scan, int verbose, int export)
>  				pr_err("%s metadata is platform independent\n",
>  					meta->name ? : "[no name]");
>  		} else if (export){
> -			err |= meta->export_detail_platform(verbose);
> +			err |= meta->export_detail_platform(verbose, controller_path);
>  		} else
> -			err |= meta->detail_platform(verbose, 0);
> +			err |= meta->detail_platform(verbose, 0, controller_path);
>  	}
>  
>  	return err;
> diff --git a/ReadMe.c b/ReadMe.c
> index 346f08c..0c9c80b 100644
> --- a/ReadMe.c
> +++ b/ReadMe.c
> @@ -159,6 +159,7 @@ struct option long_options[] = {
>      {"sparc2.2",  0, 0, Sparc22},
>      {"test",      0, 0, 't'},
>      {"prefer",    1, 0, Prefer},
> +    {"controller-path",1, 0, ControllerPath},
>  
>      /* For Follow/monitor */
>      {"mail",      1, 0, EMail},
> @@ -240,6 +241,7 @@ char OptionHelp[] =
>  "  --brief       -b   : Be less verbose, more brief\n"
>  "  --export      -Y   : With --detail, --detail-platform or --examine use\n"
>  "                       key=value format for easy import into environment\n"
> +"  --controller-path  : Specify controller for --detail-platform\n"
>  "  --force       -f   : Override normal checks and be more forceful\n"
>  "\n"
>  "  --assemble    -A   : Assemble an array\n"
> diff --git a/mdadm.8.in b/mdadm.8.in
> index 38c8bc8..8792e54 100644
> --- a/mdadm.8.in
> +++ b/mdadm.8.in
> @@ -1329,6 +1329,14 @@ output will be formatted as
>  pairs for easy import into the environment.
>  
>  .TP
> +.BR \-\-controller\-path
> +When used with
> +.BR \-\-detail\-platform ,
> +mdadm will only print information about the specified controller. The
> +controller should be given in form of an absolute filepath, e.g.
> +.B \-\-controller\-path /sys/devices/pci0000:00/0000:00:1f.0 .
> +
> +.TP
>  .BR \-E ", " \-\-examine
>  Print contents of the metadata stored on the named device(s).
>  Note the contrast between
> diff --git a/mdadm.c b/mdadm.c
> index 3ee7ddb..d313b76 100644
> --- a/mdadm.c
> +++ b/mdadm.c
> @@ -93,6 +93,7 @@ int main(int argc, char *argv[])
>  	int rebuild_map = 0;
>  	char *remove_path = NULL;
>  	char *udev_filename = NULL;
> +	char *actual_controller_path;
>  
>  	int print_help = 0;
>  	FILE *outf;
> @@ -151,6 +152,35 @@ int main(int argc, char *argv[])
>  		case 'Y': c.export++;
>  			continue;
>  
> +		case ControllerPath:
> +			if (c.controller_path)
> +				free(c.controller_path);
> +			if (asprintf(&c.controller_path, "%s", optarg) <= 0) {
> +				pr_err("Empty or wrong controller path, \'%s\' ignored.\n",
> +					c.controller_path);
> +				c.controller_path = NULL;
> +			}
> +			else {
> +				struct stat st;
> +				if (stat(c.controller_path, &st) != 0) {
> +					pr_err("Specified controller path %s doesn't exist.\n",
> +						c.controller_path);
> +					exit(2);
> +				}
> +				else {
> +					actual_controller_path = realpath(c.controller_path,NULL);
> +					if (actual_controller_path) {
> +						if(asprintf(&c.controller_path, "%s", actual_controller_path) > 0)
> +							free(actual_controller_path);
> +						else
> +							free(c.controller_path);
> +					}
> +					else
> +						free(c.controller_path);
> +				}
> +			}
> +			continue;
> +
>  		case HomeHost:
>  			if (strcasecmp(optarg, "<ignore>") == 0)
>  				c.require_homehost = 0;
> @@ -1344,7 +1374,7 @@ int main(int argc, char *argv[])
>  			}
>  			rv = Examine(devlist, &c, ss);
>  		} else if (devmode == DetailPlatform) {
> -			rv = Detail_Platform(ss ? ss->ss : NULL, ss ? c.scan : 1, c.verbose, c.export);
> +			rv = Detail_Platform(ss ? ss->ss : NULL, ss ? c.scan : 1, c.verbose, c.export, c.controller_path);
>  		} else if (devlist == NULL) {
>  			if (devmode == 'S' && c.scan)
>  				rv = stop_scan(c.verbose);
> diff --git a/mdadm.h b/mdadm.h
> index 6d219f7..e27275b 100644
> --- a/mdadm.h
> +++ b/mdadm.h
> @@ -326,6 +326,7 @@ enum special_options {
>  	OffRootOpt,
>  	Prefer,
>  	KillOpt,
> +	ControllerPath,
>  };
>  
>  /* structures read from config file */
> @@ -394,6 +395,7 @@ struct context {
>  	int	freeze_reshape;
>  	char	*backup_file;
>  	int	invalid_backup;
> +	char	*controller_path;
>  };
>  
>  struct shape {
> @@ -654,8 +656,8 @@ extern struct superswitch {
>  	void (*export_detail_super)(struct supertype *st);
>  
>  	/* Optional: platform hardware / firmware details */
> -	int (*detail_platform)(int verbose, int enumerate_only);
> -	int (*export_detail_platform)(int verbose);
> +	int (*detail_platform)(int verbose, int enumerate_only, char *controller_path);
> +	int (*export_detail_platform)(int verbose, char *controller_path);
>  
>  	/* Used:
>  	 *   to get uuid to storing in bitmap metadata
> @@ -1122,7 +1124,7 @@ extern int Create(struct supertype *st, char *mddev,
>  		  struct context *c);
>  
>  extern int Detail(char *dev, struct context *c);
> -extern int Detail_Platform(struct superswitch *ss, int scan, int verbose, int export);
> +extern int Detail_Platform(struct superswitch *ss, int scan, int verbose, int export, char *controller_path);
>  extern int Query(char *dev);
>  extern int Examine(struct mddev_dev *devlist, struct context *c,
>  		   struct supertype *forcest);
> diff --git a/super-intel.c b/super-intel.c
> index fdf441a..cedc976 100644
> --- a/super-intel.c
> +++ b/super-intel.c
> @@ -1833,7 +1833,7 @@ static void print_imsm_capability_export(const struct imsm_orom *orom)
>  	printf("IMSM_MAX_VOLUMES_PER_CONTROLLER=%d\n",orom->vphba);
>  }
>  
> -static int detail_platform_imsm(int verbose, int enumerate_only)
> +static int detail_platform_imsm(int verbose, int enumerate_only, char *controller_path)
>  {
>  	/* There are two components to imsm platform support, the ahci SATA
>  	 * controller and the option-rom.  To find the SATA controller we
> @@ -1850,7 +1850,7 @@ static int detail_platform_imsm(int verbose, int enumerate_only)
>  	struct sys_dev *list, *hba;
>  	int host_base = 0;
>  	int port_count = 0;
> -	int result=0;
> +	int result=1;
>  
>  	if (enumerate_only) {
>  		if (check_env("IMSM_NO_PLATFORM"))
> @@ -1864,6 +1864,8 @@ static int detail_platform_imsm(int verbose, int enumerate_only)
>  				result = 2;
>  				break;
>  			}
> +			else
> +				result = 0;
>  		}
>  		free_sys_dev(&list);
>  		return result;
> @@ -1880,34 +1882,38 @@ static int detail_platform_imsm(int verbose, int enumerate_only)
>  		print_found_intel_controllers(list);
>  
>  	for (hba = list; hba; hba = hba->next) {
> +		if (controller_path != NULL && ( strcmp(hba->path,controller_path) != 0 ))
> +			continue;
>  		orom = find_imsm_capability(hba->type);
>  		if (!orom)
>  			pr_err("imsm capabilities not found for controller: %s (type %s)\n",
>  				hba->path, get_sys_dev_type(hba->type));
> -		else
> +		else {
> +			result = 0;
>  			print_imsm_capability(orom);
> -	}
> -
> -	for (hba = list; hba; hba = hba->next) {
> -		printf(" I/O Controller : %s (%s)\n",
> -			hba->path, get_sys_dev_type(hba->type));
> -
> -		if (hba->type == SYS_DEV_SATA) {
> -			host_base = ahci_get_port_count(hba->path, &port_count);
> -			if (ahci_enumerate_ports(hba->path, port_count, host_base, verbose)) {
> -				if (verbose > 0)
> -					pr_err("failed to enumerate "
> -						"ports on SATA controller at %s.", hba->pci_id);
> -				result |= 2;
> +			printf(" I/O Controller : %s (%s)\n",
> +				hba->path, get_sys_dev_type(hba->type));
> +			if (hba->type == SYS_DEV_SATA) {
> +				host_base = ahci_get_port_count(hba->path, &port_count);
> +				if (ahci_enumerate_ports(hba->path, port_count, host_base, verbose)) {
> +					if (verbose > 0)
> +						pr_err("failed to enumerate "
> +							"ports on SATA controller at %s.\n", hba->pci_id);
> +					result |= 2;
> +				}
>  			}
>  		}
>  	}
>  
> +	if (controller_path != NULL && result == 1)
> +		pr_err("no active Intel(R) RAID "
> +				"controller found under %s\n",controller_path);
> +
>  	free_sys_dev(&list);
>  	return result;
>  }
>  
> -static int export_detail_platform_imsm(int verbose)
> +static int export_detail_platform_imsm(int verbose, char *controller_path)
>  {
>  	const struct imsm_orom *orom;
>  	struct sys_dev *list, *hba;
> @@ -1923,6 +1929,8 @@ static int export_detail_platform_imsm(int verbose)
>  	}
>  
>  	for (hba = list; hba; hba = hba->next) {
> +		if (controller_path != NULL && ( strcmp(hba->path,controller_path) != 0 ))
> +			continue;
>  		orom = find_imsm_capability(hba->type);
>  		if (!orom) {
>  			if (verbose > 0)
> @@ -1934,9 +1942,6 @@ static int export_detail_platform_imsm(int verbose)
>  		}
>  	}
>  
> -	if (result == 1 && verbose > 0)
> -		pr_err("IMSM_DETAIL_PLATFORM_ERROR=NO_IMSM_CAPABLE_DEVICES\n");
> -
>  	return result;
>  }
>  
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-raid" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

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

* Re: [PATCH 3/6] Fix return code for --detail-platform
  2012-09-26 11:42 ` [PATCH 3/6] Fix return code " Maciej Naruszewicz
@ 2012-10-02  6:38   ` NeilBrown
  0 siblings, 0 replies; 14+ messages in thread
From: NeilBrown @ 2012-10-02  6:38 UTC (permalink / raw)
  To: Maciej Naruszewicz; +Cc: linux-raid, maciej.patelczyk

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

On Wed, 26 Sep 2012 13:42:58 +0200 Maciej Naruszewicz
<maciej.naruszewicz@intel.com> wrote:

> Variable 'err' is initially set to 1, so changing its value with
> '|=' won't set it to 0 even if the operation is successful.
> 
> Signed-off-by: Maciej Naruszewicz <maciej.naruszewicz@intel.com>
> ---
>  Detail.c |    1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/Detail.c b/Detail.c
> index 57faf3c..96d1ff6 100644
> --- a/Detail.c
> +++ b/Detail.c
> @@ -640,6 +640,7 @@ int Detail_Platform(struct superswitch *ss, int scan, int verbose, int export, c
>  	if (!scan)
>  		return err;
>  
> +	err = 0;
>  	for (i = 0; superlist[i]; i++) {
>  		struct superswitch *meta = superlist[i];
>  
Good catch.

Applied, thanks.
NeilBrown

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

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

* Re: [PATCH 4/6] Synchronize size calculation in human_size and human_size_brief
  2012-09-26 11:44 ` [PATCH 4/6] Synchronize size calculation in human_size and human_size_brief Maciej Naruszewicz
@ 2012-10-02  6:40   ` NeilBrown
  0 siblings, 0 replies; 14+ messages in thread
From: NeilBrown @ 2012-10-02  6:40 UTC (permalink / raw)
  To: Maciej Naruszewicz; +Cc: linux-raid, maciej.patelczyk

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

On Wed, 26 Sep 2012 13:44:08 +0200 Maciej Naruszewicz
<maciej.naruszewicz@intel.com> wrote:

> It would be better if two size-calculating methods had the same
> calculating algorithm. The human_size way of calculation seems
> more readable, so let's use it for both methods.
> 
> Signed-off-by: Maciej Naruszewicz <maciej.naruszewicz@intel.com>
> ---
>  util.c |   33 ++++++++++++++++++++-------------
>  1 file changed, 20 insertions(+), 13 deletions(-)
> 
> diff --git a/util.c b/util.c
> index c63a232..09971a2 100644
> --- a/util.c
> +++ b/util.c
> @@ -686,20 +686,27 @@ char *human_size_brief(long long bytes)
>  {
>  	static char buf[30];
>  
> +	/* We convert bytes to either centi-M{ega,ibi}bytes or
> +	 * centi-G{igi,ibi}bytes, with appropriate rounding,
> +	 * and then print 1/100th of those as a decimal.
> +	 * We allow upto 2048Megabytes before converting to
> +	 * gigabytes, as that shows more precision and isn't
> +	 * too large a number.
> +	 * Terabytes are not yet handled.
> +	 */
> +
>  	if (bytes < 5000*1024)
> -		snprintf(buf, sizeof(buf), "%ld.%02ldKiB",
> -			(long)(bytes>>10), (long)(((bytes&1023)*100+512)/1024)
> -			);
> -	else if (bytes < 2*1024LL*1024LL*1024LL)
> -		snprintf(buf, sizeof(buf), "%ld.%02ldMiB",
> -			(long)(bytes>>20),
> -			(long)((bytes&0xfffff)+0x100000/200)/(0x100000/100)
> -			);
> -	else
> -		snprintf(buf, sizeof(buf), "%ld.%02ldGiB",
> -			(long)(bytes>>30),
> -			(long)(((bytes>>10)&0xfffff)+0x100000/200)/(0x100000/100)
> -			);
> +		buf[0] = 0;
> +	else if (bytes < 2*1024LL*1024LL*1024LL) {
> +		long cMiB = (bytes / ( (1LL<<20) / 200LL ) +1) /2;
> +		snprintf(buf, sizeof(buf), " (%ld.%02ldMiB)",
> +			cMiB/100 , cMiB % 100);
> +	} else {
> +		long cGiB = (bytes / ( (1LL<<30) / 200LL ) +1) /2;
> +		snprintf(buf, sizeof(buf), " (%ld.%02ldGiB)",
> +				cGiB/100 , cGiB % 100);
> +	}
> +
>  	return buf;
>  }
>  

Applied,
thanks,
NeilBrown


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

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

* Re: [PATCH 5/6] Display size with human_size_brief with a chosen prefix
  2012-09-26 11:44 ` [PATCH 5/6] Display size with human_size_brief with a chosen prefix Maciej Naruszewicz
@ 2012-10-02  6:41   ` NeilBrown
  0 siblings, 0 replies; 14+ messages in thread
From: NeilBrown @ 2012-10-02  6:41 UTC (permalink / raw)
  To: Maciej Naruszewicz; +Cc: linux-raid, maciej.patelczyk

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

On Wed, 26 Sep 2012 13:44:15 +0200 Maciej Naruszewicz
<maciej.naruszewicz@intel.com> wrote:

> When using human_size_brief, only IEC prefixes were supported. Now
> it's possible to specify which format we want to see - either IEC
> (kibi, mibi, gibi) or JEDEC (kilo, mega, giga).
> 
> Signed-off-by: Maciej Naruszewicz <maciej.naruszewicz@intel.com>
> ---
>  Query.c |    2 +-
>  mdadm.h |    7 ++++++-
>  util.c  |   36 +++++++++++++++++++++++++++---------
>  3 files changed, 34 insertions(+), 11 deletions(-)
> 
> diff --git a/Query.c b/Query.c
> index b9c209f..329e583 100644
> --- a/Query.c
> +++ b/Query.c
> @@ -76,7 +76,7 @@ int Query(char *dev)
>  	else {
>  		printf("%s: %s %s %d devices, %d spare%s. Use mdadm --detail for more detail.\n",
>  		       dev,
> -		       human_size_brief(larray_size),
> +		       human_size_brief(larray_size,IEC),
>  		       map_num(pers, array.level),
>  		       array.raid_disks,
>  		       array.spare_disks, array.spare_disks==1?"":"s");
> diff --git a/mdadm.h b/mdadm.h
> index e27275b..a24b803 100644
> --- a/mdadm.h
> +++ b/mdadm.h
> @@ -329,6 +329,11 @@ enum special_options {
>  	ControllerPath,
>  };
>  
> +enum prefix_standard {
> +	JEDEC,
> +	IEC
> +};
> +
>  /* structures read from config file */
>  /* List of mddevice names and identifiers
>   * Identifiers can be:
> @@ -1242,7 +1247,7 @@ extern int set_array_info(int mdfd, struct supertype *st, struct mdinfo *info);
>  unsigned long long min_recovery_start(struct mdinfo *array);
>  
>  extern char *human_size(long long bytes);
> -extern char *human_size_brief(long long bytes);
> +extern char *human_size_brief(long long bytes, int prefix);
>  extern void print_r10_layout(int layout);
>  
>  #define NoMdDev (1<<23)
> diff --git a/util.c b/util.c
> index 09971a2..cb97816 100644
> --- a/util.c
> +++ b/util.c
> @@ -682,7 +682,7 @@ char *human_size(long long bytes)
>  	return buf;
>  }
>  
> -char *human_size_brief(long long bytes)
> +char *human_size_brief(long long bytes, int prefix)
>  {
>  	static char buf[30];
>  
> @@ -693,19 +693,37 @@ char *human_size_brief(long long bytes)
>  	 * gigabytes, as that shows more precision and isn't
>  	 * too large a number.
>  	 * Terabytes are not yet handled.
> +	 *
> +	 * If prefix == IEC, we mean prefixes like kibi,mebi,gibi etc.
> +	 * If prefix == JEDEC, we mean prefixes like kilo,mega,giga etc.
>  	 */
>  
>  	if (bytes < 5000*1024)
>  		buf[0] = 0;
> -	else if (bytes < 2*1024LL*1024LL*1024LL) {
> -		long cMiB = (bytes / ( (1LL<<20) / 200LL ) +1) /2;
> -		snprintf(buf, sizeof(buf), " (%ld.%02ldMiB)",
> -			cMiB/100 , cMiB % 100);
> -	} else {
> -		long cGiB = (bytes / ( (1LL<<30) / 200LL ) +1) /2;
> -		snprintf(buf, sizeof(buf), " (%ld.%02ldGiB)",
> -				cGiB/100 , cGiB % 100);
> +	else if (prefix == IEC) {
> +		if (bytes < 2*1024LL*1024LL*1024LL) {
> +			long cMiB = (bytes / ( (1LL<<20) / 200LL ) +1) /2;
> +			snprintf(buf, sizeof(buf), "%ld.%02ldMiB",
> +				cMiB/100 , cMiB % 100);
> +		} else {
> +			long cGiB = (bytes / ( (1LL<<30) / 200LL ) +1) /2;
> +			snprintf(buf, sizeof(buf), "%ld.%02ldGiB",
> +					cGiB/100 , cGiB % 100);
> +		}
> +	}
> +	else if (prefix == JEDEC) {
> +		if (bytes < 2*1024LL*1024LL*1024LL) {
> +			long cMB  = (bytes / ( 1000000LL / 200LL ) +1) /2;
> +			snprintf(buf, sizeof(buf), "%ld.%02ldMB",
> +					cMB/100, cMB % 100);
> +		} else {
> +			long cGB  = (bytes / (1000000000LL/200LL ) +1) /2;
> +			snprintf(buf, sizeof(buf), "%ld.%02ldGB",
> +					cGB/100 , cGB % 100);
> +		}
>  	}
> +	else
> +		buf[0] = 0;
>  
>  	return buf;
>  }

Yes, I guess that make sense.

Applied,
thanks.
NeilBrown

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

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

* Re: [PATCH 6/6] Add MD_ARRAY_SIZE for --examine --export
  2012-09-26 11:44 ` [PATCH 6/6] Add MD_ARRAY_SIZE for --examine --export Maciej Naruszewicz
@ 2012-10-02  6:42   ` NeilBrown
  0 siblings, 0 replies; 14+ messages in thread
From: NeilBrown @ 2012-10-02  6:42 UTC (permalink / raw)
  To: Maciej Naruszewicz; +Cc: linux-raid, maciej.patelczyk

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

On Wed, 26 Sep 2012 13:44:22 +0200 Maciej Naruszewicz
<maciej.naruszewicz@intel.com> wrote:

> An additional pair of key=value for --examine --export.
> 
> Signed-off-by: Maciej Naruszewicz <maciej.naruszewicz@intel.com>
> ---
>  super1.c |   19 +++++++++++++++++++
>  1 file changed, 19 insertions(+)
> 
> diff --git a/super1.c b/super1.c
> index b04945f..0df5bee 100644
> --- a/super1.c
> +++ b/super1.c
> @@ -495,6 +495,7 @@ static void export_examine_super1(struct supertype *st)
>  	struct mdp_superblock_1 *sb = st->sb;
>  	int i;
>  	int len = 32;
> +	int layout;
>  
>  	printf("MD_LEVEL=%s\n", map_num(pers, __le32_to_cpu(sb->level)));
>  	printf("MD_DEVICES=%d\n", __le32_to_cpu(sb->raid_disks));
> @@ -506,6 +507,24 @@ static void export_examine_super1(struct supertype *st)
>  		}
>  	if (len)
>  		printf("MD_NAME=%.*s\n", len, sb->set_name);
> +	if (__le32_to_cpu(sb->level) > 0) {
> +		int ddsks = 0, ddsks_denom = 1;
> +		switch(__le32_to_cpu(sb->level)) {
> +			case 1: ddsks=1;break;
> +			case 4:
> +			case 5: ddsks = __le32_to_cpu(sb->raid_disks)-1; break;
> +			case 6: ddsks = __le32_to_cpu(sb->raid_disks)-2; break;
> +			case 10:
> +				layout = __le32_to_cpu(sb->layout);
> +				ddsks = __le32_to_cpu(sb->raid_disks);
> +				ddsks_denom = (layout&255) * ((layout>>8)&255);
> +			}
> +		if (ddsks) {
> +			long long asize = __le64_to_cpu(sb->size);
> +			asize = (asize << 9) * ddsks / ddsks_denom;
> +			printf("MD_ARRAY_SIZE=%s\n",human_size_brief(asize,JEDEC));
> +		}
> +	}
>  	printf("MD_UUID=");
>  	for (i=0; i<16; i++) {
>  		if ((i&3)==0 && i != 0) printf(":");


Applied,
thanks,
NeilBrown

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

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

* Re: [PATCH 2/6] imsm: Add --controller-path option for --detail-platform.
  2012-10-02  6:36   ` NeilBrown
@ 2012-10-02 10:54     ` Maciej Naruszewicz
  0 siblings, 0 replies; 14+ messages in thread
From: Maciej Naruszewicz @ 2012-10-02 10:54 UTC (permalink / raw)
  To: NeilBrown; +Cc: linux-raid, maciej.patelczyk

W dniu 10/2/2012 8:36 AM, NeilBrown pisze:
> On Wed, 26 Sep 2012 13:42:50 +0200 Maciej Naruszewicz
> <maciej.naruszewicz@intel.com> wrote:
>
>> Usually, 'mdadm --detail-platform -e imsm' scans all the controllers
>> looking for IMSM capabilities. This patch provides the possibility
>> to specify a controller to scan, enabling custom usage by other
>> processes - especially with the --export switch.
>>
>> $ mdadm --detail-platform
>>         Platform : Intel(R) Matrix Storage Manager
>>          Version : 9.5.0.1037
>>      RAID Levels : raid0 raid1 raid10 raid5
>>      Chunk Sizes : 4k 8k 16k 32k 64k 128k
>>      2TB volumes : supported
>>        2TB disks : not supported
>>        Max Disks : 7
>>      Max Volumes : 2 per array, 4 per controller
>>   I/O Controller : /sys/devices/pci0000:00/0000:00:1f.2 (SATA)
>>
>> $ mdadm --detail-platform --controller-path=/sys/devices/pci0000:00/0000:00:1f.2
>>         Platform : Intel(R) Matrix Storage Manager
>>          Version : 9.5.0.1037
>>      RAID Levels : raid0 raid1 raid10 raid5
>>      Chunk Sizes : 4k 8k 16k 32k 64k 128k
>>      2TB volumes : supported
>>        2TB disks : not supported
>>        Max Disks : 7
>>      Max Volumes : 2 per array, 4 per controller
>>   I/O Controller : /sys/devices/pci0000:00/0000:00:1f.2 (SATA)
>>
>> $ mdadm --detail-platform --controller-path=/sys/devices/pci0000:00/0000:00:1f.2 --export
>> MD_FIRMWARE_TYPE=imsm
>> IMSM_VERSION=9.5.0.1037
>> IMSM_SUPPORTED_RAID_LEVELS=raid0 raid1 raid10 raid5
>> IMSM_SUPPORTED_CHUNK_SIZES=4k 8k 16k 32k 64k 128k
>> IMSM_2TB_VOLUMES=yes
>> IMSM_2TB_DISKS=no
>> IMSM_MAX_DISKS=7
>> IMSM_MAX_VOLUMES_PER_ARRAY=2
>> IMSM_MAX_VOLUMES_PER_CONTROLLER=4
>>
>> $ mdadm --detail-platform --controller-path=/sys/devices/pci0000:00/0000:00:1f.0 # This isn't an IMSM-capable controller
>> mdadm: no active Intel(R) RAID controller found under /sys/devices/pci0000:00/0000:00:1f.0
>
> hi,
>   I think this functionality is good.  I think the implementation is not.
>
> 1/ Don't have a "--controller-path" option, just require any name
>     given as a bare argument to the controller path - and probably require
>     there is either 0 or 1.
>     So:
>        mdadm --detail-platform  /sys/devices/pci0000:00/0000:00:1f.0
>
>     is the correct usage.
>
> 2/ You really don't need to
>       if (asprintf(&c.controller_path, "%s", optarg) <= 0) {
>
>     just use 'optarg' directly.
>
> 3/ Don't use 'realpath' and 'strcmp'.  To compare two paths, use "stat" and
>      compare st_dev and st_ino.
>
> So: not applied.
>

Thanks for the input, I'll fix this and send a second version today.

MN

> Thanks,
> NeilBrown
>
>
>
>>
>> Signed-off-by: Maciej Naruszewicz <maciej.naruszewicz@intel.com>
>> ---
>>   Create.c      |    2 +-
>>   Detail.c      |   10 +++++-----
>>   ReadMe.c      |    2 ++
>>   mdadm.8.in    |    8 ++++++++
>>   mdadm.c       |   32 +++++++++++++++++++++++++++++++-
>>   mdadm.h       |    8 +++++---
>>   super-intel.c |   45 +++++++++++++++++++++++++--------------------
>>   7 files changed, 77 insertions(+), 30 deletions(-)
>>
>> diff --git a/Create.c b/Create.c
>> index 32683a8..2da07d0 100644
>> --- a/Create.c
>> +++ b/Create.c
>> @@ -492,7 +492,7 @@ int Create(struct supertype *st, char *mddev,
>>   		warn = 1;
>>   	}
>>
>> -	if (st->ss->detail_platform && st->ss->detail_platform(0, 1) != 0) {
>> +	if (st->ss->detail_platform && st->ss->detail_platform(0, 1, NULL) != 0) {
>>   		if (c->runstop != 1 || c->verbose >= 0)
>>   			pr_err("%s unable to enumerate platform support\n"
>>   				"    array may not be compatible with hardware/firmware\n",
>> diff --git a/Detail.c b/Detail.c
>> index b0c31e6..57faf3c 100644
>> --- a/Detail.c
>> +++ b/Detail.c
>> @@ -616,7 +616,7 @@ out:
>>   	return rv;
>>   }
>>
>> -int Detail_Platform(struct superswitch *ss, int scan, int verbose, int export)
>> +int Detail_Platform(struct superswitch *ss, int scan, int verbose, int export, char *controller_path)
>>   {
>>   	/* display platform capabilities for the given metadata format
>>   	 * 'scan' in this context means iterate over all metadata types
>> @@ -625,9 +625,9 @@ int Detail_Platform(struct superswitch *ss, int scan, int verbose, int export)
>>   	int err = 1;
>>
>>   	if (ss && export && ss->export_detail_platform)
>> -		err = ss->export_detail_platform(verbose);
>> +		err = ss->export_detail_platform(verbose, controller_path);
>>   	else if (ss && ss->detail_platform)
>> -		err = ss->detail_platform(verbose, 0);
>> +		err = ss->detail_platform(verbose, 0, controller_path);
>>   	else if (ss) {
>>   		if (verbose > 0)
>>   			pr_err("%s metadata is platform independent\n",
>> @@ -653,9 +653,9 @@ int Detail_Platform(struct superswitch *ss, int scan, int verbose, int export)
>>   				pr_err("%s metadata is platform independent\n",
>>   					meta->name ? : "[no name]");
>>   		} else if (export){
>> -			err |= meta->export_detail_platform(verbose);
>> +			err |= meta->export_detail_platform(verbose, controller_path);
>>   		} else
>> -			err |= meta->detail_platform(verbose, 0);
>> +			err |= meta->detail_platform(verbose, 0, controller_path);
>>   	}
>>
>>   	return err;
>> diff --git a/ReadMe.c b/ReadMe.c
>> index 346f08c..0c9c80b 100644
>> --- a/ReadMe.c
>> +++ b/ReadMe.c
>> @@ -159,6 +159,7 @@ struct option long_options[] = {
>>       {"sparc2.2",  0, 0, Sparc22},
>>       {"test",      0, 0, 't'},
>>       {"prefer",    1, 0, Prefer},
>> +    {"controller-path",1, 0, ControllerPath},
>>
>>       /* For Follow/monitor */
>>       {"mail",      1, 0, EMail},
>> @@ -240,6 +241,7 @@ char OptionHelp[] =
>>   "  --brief       -b   : Be less verbose, more brief\n"
>>   "  --export      -Y   : With --detail, --detail-platform or --examine use\n"
>>   "                       key=value format for easy import into environment\n"
>> +"  --controller-path  : Specify controller for --detail-platform\n"
>>   "  --force       -f   : Override normal checks and be more forceful\n"
>>   "\n"
>>   "  --assemble    -A   : Assemble an array\n"
>> diff --git a/mdadm.8.in b/mdadm.8.in
>> index 38c8bc8..8792e54 100644
>> --- a/mdadm.8.in
>> +++ b/mdadm.8.in
>> @@ -1329,6 +1329,14 @@ output will be formatted as
>>   pairs for easy import into the environment.
>>
>>   .TP
>> +.BR \-\-controller\-path
>> +When used with
>> +.BR \-\-detail\-platform ,
>> +mdadm will only print information about the specified controller. The
>> +controller should be given in form of an absolute filepath, e.g.
>> +.B \-\-controller\-path /sys/devices/pci0000:00/0000:00:1f.0 .
>> +
>> +.TP
>>   .BR \-E ", " \-\-examine
>>   Print contents of the metadata stored on the named device(s).
>>   Note the contrast between
>> diff --git a/mdadm.c b/mdadm.c
>> index 3ee7ddb..d313b76 100644
>> --- a/mdadm.c
>> +++ b/mdadm.c
>> @@ -93,6 +93,7 @@ int main(int argc, char *argv[])
>>   	int rebuild_map = 0;
>>   	char *remove_path = NULL;
>>   	char *udev_filename = NULL;
>> +	char *actual_controller_path;
>>
>>   	int print_help = 0;
>>   	FILE *outf;
>> @@ -151,6 +152,35 @@ int main(int argc, char *argv[])
>>   		case 'Y': c.export++;
>>   			continue;
>>
>> +		case ControllerPath:
>> +			if (c.controller_path)
>> +				free(c.controller_path);
>> +			if (asprintf(&c.controller_path, "%s", optarg) <= 0) {
>> +				pr_err("Empty or wrong controller path, \'%s\' ignored.\n",
>> +					c.controller_path);
>> +				c.controller_path = NULL;
>> +			}
>> +			else {
>> +				struct stat st;
>> +				if (stat(c.controller_path, &st) != 0) {
>> +					pr_err("Specified controller path %s doesn't exist.\n",
>> +						c.controller_path);
>> +					exit(2);
>> +				}
>> +				else {
>> +					actual_controller_path = realpath(c.controller_path,NULL);
>> +					if (actual_controller_path) {
>> +						if(asprintf(&c.controller_path, "%s", actual_controller_path) > 0)
>> +							free(actual_controller_path);
>> +						else
>> +							free(c.controller_path);
>> +					}
>> +					else
>> +						free(c.controller_path);
>> +				}
>> +			}
>> +			continue;
>> +
>>   		case HomeHost:
>>   			if (strcasecmp(optarg, "<ignore>") == 0)
>>   				c.require_homehost = 0;
>> @@ -1344,7 +1374,7 @@ int main(int argc, char *argv[])
>>   			}
>>   			rv = Examine(devlist, &c, ss);
>>   		} else if (devmode == DetailPlatform) {
>> -			rv = Detail_Platform(ss ? ss->ss : NULL, ss ? c.scan : 1, c.verbose, c.export);
>> +			rv = Detail_Platform(ss ? ss->ss : NULL, ss ? c.scan : 1, c.verbose, c.export, c.controller_path);
>>   		} else if (devlist == NULL) {
>>   			if (devmode == 'S' && c.scan)
>>   				rv = stop_scan(c.verbose);
>> diff --git a/mdadm.h b/mdadm.h
>> index 6d219f7..e27275b 100644
>> --- a/mdadm.h
>> +++ b/mdadm.h
>> @@ -326,6 +326,7 @@ enum special_options {
>>   	OffRootOpt,
>>   	Prefer,
>>   	KillOpt,
>> +	ControllerPath,
>>   };
>>
>>   /* structures read from config file */
>> @@ -394,6 +395,7 @@ struct context {
>>   	int	freeze_reshape;
>>   	char	*backup_file;
>>   	int	invalid_backup;
>> +	char	*controller_path;
>>   };
>>
>>   struct shape {
>> @@ -654,8 +656,8 @@ extern struct superswitch {
>>   	void (*export_detail_super)(struct supertype *st);
>>
>>   	/* Optional: platform hardware / firmware details */
>> -	int (*detail_platform)(int verbose, int enumerate_only);
>> -	int (*export_detail_platform)(int verbose);
>> +	int (*detail_platform)(int verbose, int enumerate_only, char *controller_path);
>> +	int (*export_detail_platform)(int verbose, char *controller_path);
>>
>>   	/* Used:
>>   	 *   to get uuid to storing in bitmap metadata
>> @@ -1122,7 +1124,7 @@ extern int Create(struct supertype *st, char *mddev,
>>   		  struct context *c);
>>
>>   extern int Detail(char *dev, struct context *c);
>> -extern int Detail_Platform(struct superswitch *ss, int scan, int verbose, int export);
>> +extern int Detail_Platform(struct superswitch *ss, int scan, int verbose, int export, char *controller_path);
>>   extern int Query(char *dev);
>>   extern int Examine(struct mddev_dev *devlist, struct context *c,
>>   		   struct supertype *forcest);
>> diff --git a/super-intel.c b/super-intel.c
>> index fdf441a..cedc976 100644
>> --- a/super-intel.c
>> +++ b/super-intel.c
>> @@ -1833,7 +1833,7 @@ static void print_imsm_capability_export(const struct imsm_orom *orom)
>>   	printf("IMSM_MAX_VOLUMES_PER_CONTROLLER=%d\n",orom->vphba);
>>   }
>>
>> -static int detail_platform_imsm(int verbose, int enumerate_only)
>> +static int detail_platform_imsm(int verbose, int enumerate_only, char *controller_path)
>>   {
>>   	/* There are two components to imsm platform support, the ahci SATA
>>   	 * controller and the option-rom.  To find the SATA controller we
>> @@ -1850,7 +1850,7 @@ static int detail_platform_imsm(int verbose, int enumerate_only)
>>   	struct sys_dev *list, *hba;
>>   	int host_base = 0;
>>   	int port_count = 0;
>> -	int result=0;
>> +	int result=1;
>>
>>   	if (enumerate_only) {
>>   		if (check_env("IMSM_NO_PLATFORM"))
>> @@ -1864,6 +1864,8 @@ static int detail_platform_imsm(int verbose, int enumerate_only)
>>   				result = 2;
>>   				break;
>>   			}
>> +			else
>> +				result = 0;
>>   		}
>>   		free_sys_dev(&list);
>>   		return result;
>> @@ -1880,34 +1882,38 @@ static int detail_platform_imsm(int verbose, int enumerate_only)
>>   		print_found_intel_controllers(list);
>>
>>   	for (hba = list; hba; hba = hba->next) {
>> +		if (controller_path != NULL && ( strcmp(hba->path,controller_path) != 0 ))
>> +			continue;
>>   		orom = find_imsm_capability(hba->type);
>>   		if (!orom)
>>   			pr_err("imsm capabilities not found for controller: %s (type %s)\n",
>>   				hba->path, get_sys_dev_type(hba->type));
>> -		else
>> +		else {
>> +			result = 0;
>>   			print_imsm_capability(orom);
>> -	}
>> -
>> -	for (hba = list; hba; hba = hba->next) {
>> -		printf(" I/O Controller : %s (%s)\n",
>> -			hba->path, get_sys_dev_type(hba->type));
>> -
>> -		if (hba->type == SYS_DEV_SATA) {
>> -			host_base = ahci_get_port_count(hba->path, &port_count);
>> -			if (ahci_enumerate_ports(hba->path, port_count, host_base, verbose)) {
>> -				if (verbose > 0)
>> -					pr_err("failed to enumerate "
>> -						"ports on SATA controller at %s.", hba->pci_id);
>> -				result |= 2;
>> +			printf(" I/O Controller : %s (%s)\n",
>> +				hba->path, get_sys_dev_type(hba->type));
>> +			if (hba->type == SYS_DEV_SATA) {
>> +				host_base = ahci_get_port_count(hba->path, &port_count);
>> +				if (ahci_enumerate_ports(hba->path, port_count, host_base, verbose)) {
>> +					if (verbose > 0)
>> +						pr_err("failed to enumerate "
>> +							"ports on SATA controller at %s.\n", hba->pci_id);
>> +					result |= 2;
>> +				}
>>   			}
>>   		}
>>   	}
>>
>> +	if (controller_path != NULL && result == 1)
>> +		pr_err("no active Intel(R) RAID "
>> +				"controller found under %s\n",controller_path);
>> +
>>   	free_sys_dev(&list);
>>   	return result;
>>   }
>>
>> -static int export_detail_platform_imsm(int verbose)
>> +static int export_detail_platform_imsm(int verbose, char *controller_path)
>>   {
>>   	const struct imsm_orom *orom;
>>   	struct sys_dev *list, *hba;
>> @@ -1923,6 +1929,8 @@ static int export_detail_platform_imsm(int verbose)
>>   	}
>>
>>   	for (hba = list; hba; hba = hba->next) {
>> +		if (controller_path != NULL && ( strcmp(hba->path,controller_path) != 0 ))
>> +			continue;
>>   		orom = find_imsm_capability(hba->type);
>>   		if (!orom) {
>>   			if (verbose > 0)
>> @@ -1934,9 +1942,6 @@ static int export_detail_platform_imsm(int verbose)
>>   		}
>>   	}
>>
>> -	if (result == 1 && verbose > 0)
>> -		pr_err("IMSM_DETAIL_PLATFORM_ERROR=NO_IMSM_CAPABLE_DEVICES\n");
>> -
>>   	return result;
>>   }
>>
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-raid" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

end of thread, other threads:[~2012-10-02 10:54 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-09-26 11:42 [mdadm,v1 PATCH 0/6] Extend mdadm [...] --export Maciej Naruszewicz
2012-09-26 11:42 ` [PATCH 1/6] imsm: Add --export option for --detail-platform Maciej Naruszewicz
2012-10-02  6:28   ` NeilBrown
2012-09-26 11:42 ` [PATCH 2/6] imsm: Add --controller-path " Maciej Naruszewicz
2012-10-02  6:36   ` NeilBrown
2012-10-02 10:54     ` Maciej Naruszewicz
2012-09-26 11:42 ` [PATCH 3/6] Fix return code " Maciej Naruszewicz
2012-10-02  6:38   ` NeilBrown
2012-09-26 11:44 ` [PATCH 4/6] Synchronize size calculation in human_size and human_size_brief Maciej Naruszewicz
2012-10-02  6:40   ` NeilBrown
2012-09-26 11:44 ` [PATCH 5/6] Display size with human_size_brief with a chosen prefix Maciej Naruszewicz
2012-10-02  6:41   ` NeilBrown
2012-09-26 11:44 ` [PATCH 6/6] Add MD_ARRAY_SIZE for --examine --export Maciej Naruszewicz
2012-10-02  6:42   ` NeilBrown

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).