ANNOUNCE: mdadm 2.5 - A tool for managing Soft RAID under Linux

ANNOUNCE: mdadm 2.5 - A tool for managing Soft RAID under Linux

[Neil Brown]

I am pleased to announce the availability of
   mdadm version 2.5

It is available at the usual places:
   http://www.cse.unsw.edu.au/~neilb/source/mdadm/
and
   http://www.{countrycode}.kernel.org/pub/linux/utils/raid/mdadm/

mdadm is a tool for creating, managing and monitoring
device arrays using the "md" driver in Linux, also
known as Software RAID arrays.

Release 2.5 adds a host of minor updates and one major update.

The major update involves an "Auto Assemble" function which will,
with certainly limits, scan all available devices for anything that
looks like an md array, and will try to assemble it.
This code should be treated with some caution as it is very new,
and could be revised in future, though hopefully not very much.

The main problem I have always had will auto-assembly is that it
is too easy for it to assemble thing that you don't want assembled,
e.g. if you have moved some devices from a different computer.
To remove this problem, arrays can now be tagged for the computer
that is their home (homehost) and only arrays with the correct
homehost will be auto-assembled.

Feedback on the effectiveness and usefulness of this feature and it's
documentation is encouraged.

Changelog Entries:
    -   Support 'mailfrom' line in mdadm.conf so the From: line in alert
	emails can be explicitly set.
    -   Arrange that SparesMissing (which is similar in import to
	DegradedArray) generates an Email.
    -   Assume "DEVICE partitions" if no DEVICE line is given.
    -   Support new 'offset' layout for raid10.
    -   When creating a bitmap file, choose a chunksize to limit number
	of bitmap chunks to 2 million.  More than this can cause kmalloc
	failure.
    -   New 'CREATE' line in mdadm.conf for defaults such as owner, group,
	mode and auto-flag
    -   --detail checks if array has been started or not and includes that
	in report.
    -   When using --update=uuid on an array with a bitmap, update the
	bitmap's uuid too.
    -   Add a copy of /proc/mdstat to the mail message sent by mdadm
        --monitor.
    -   New flag --no-degraded to avoid starting arrays if there are
	fewer devices available than last time the array was started.
	This is only needed with --scan, as with --scan, that behaviour
	is the default.
    -   Support for 'homehost' concept.  This is a fairly major update.
	It includes a configfile option and a command line option for
	specifying a homehost, records that host in the superblock,
	and reports the homehost where possible.
    -   Support for Auto Assembly.  "mdadm -As" will, if provided with
	the name of a homehost, try to assemble all arrays it can find
	that were created for that homehost.  See man pages for more details.

Development of mdadm is sponsored by
 SUSE Labs, Novell Inc.

NeilBrown  26th May 2006

[PATCH] mdadm 2.5 (Was: ANNOUNCE: mdadm 2.5 - A tool for managing Soft RAID under Linux)

[Luca Berra]

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

On Fri, May 26, 2006 at 04:33:08PM +1000, Neil Brown wrote:
>
>
>I am pleased to announce the availability of
>   mdadm version 2.5
>

hello,
i tried rebuilding mdadm 2.5 on current mandriva cooker, which uses
gcc-4.1.1, glibc-2.4 and dietlibc 0.29 and found the following issues
addressed by patches attacched to this message
I would be glad if you could review these patches and include them in
upcoming mdadm releases.

- mdadm-2.3.1-kernel-byteswap-include-fix.patch
reverts a change introduced with mdadm 2.3.1 for redhat compatibility
asm/byteorder.h is an architecture dependent file and does more
stuff than a call to the linux/byteorder/XXX_endian.h
the fact that not calling asm/byteorder.h does not define
__BYTEORDER_HAS_U64__ is just an example of issues that might arise.
if redhat is broken it should be worked around differently than breaking
mdadm.

- mdadm-2.4-snprintf.patch
this is self commenting, just an error in the snprintf call

- mdadm-2.4-strict-aliasing.patch
fix for another srict-aliasing problem, you can typecast a reference to a
void pointer to anything, you cannot typecast a reference to a struct.

- mdadm-2.5-mdassemble.patch
pass CFLAGS to mdassemble build, enabling -Wall -Werror showed some
issues also fixed by the patch.

- mdadm-2.5-rand.patch
Posix dictates rand() versus bsd random() function, and dietlibc
deprecated random(), so switch to srand()/rand() and make everybody
happy.

- mdadm-2.5-unused.patch
glibc 2.4 is pedantic on ignoring return values from fprintf, fwrite and
write, so now we check the rval and actually do something with it.
in the Grow.c case i only print a warning, since i don't think we can do
anithing in case we fail invalidating those superblocks (is should never
happen, but then...)

Regards,
L.


-- 
Luca Berra -- bluca@comedia.it
        Communication Media & Services S.r.l.
 /"\
 \ /     ASCII RIBBON CAMPAIGN
  X        AGAINST HTML MAIL
 / \

[-- Attachment #2: mdadm-2.3.1-kernel-byteswap-include-fix.patch --]
[-- Type: text/plain, Size: 802 bytes --]

* Sat Feb 18 2006 Christiaan Welvaart <cjw@daneel.dyndns.org>
not including asm/byteorder.h will not define __BYTEORDER_HAS_U64__
causing __fswab64 to be undefined and failure compiling mdadm on
big_endian architectures like PPC

--- mdadm-2.3.1/mdadm.h.bak	2006-02-06 04:52:12.000000000 +0100
+++ mdadm-2.3.1/mdadm.h	2006-02-18 03:51:59.786926267 +0100
@@ -72,16 +72,7 @@
 #include	"bitmap.h"
 
 #include <endian.h>
-/* #include "asm/byteorder.h" Redhat don't like this so... */
-#if __BYTE_ORDER == __LITTLE_ENDIAN
-#  include <linux/byteorder/little_endian.h>
-#elif __BYTE_ORDER == __BIG_ENDIAN
-#  include <linux/byteorder/big_endian.h>
-#elif __BYTE_ORDER == __PDP_ENDIAN
-#  include <linux/byteorder/pdp_endian.h>
-#else
-#  error "unknown endianness."
-#endif
+#include <asm/byteorder.h>
 
 
 

[-- Attachment #3: mdadm-2.4-snprintf.patch --]
[-- Type: text/plain, Size: 424 bytes --]

* Sat May 27 2006 Luca Berra <bluca@vodka.it>
snprintf size should be at most the size of the buffer

--- mdadm-2.4/util.c.snprintf	2006-05-27 13:53:18.000000000 +0200
+++ mdadm-2.4/util.c	2006-05-27 13:53:38.000000000 +0200
@@ -439,7 +439,7 @@
 		}
 	if (create && !std && !nonstd) {
 		static char buf[30];
-		snprintf(buf, 1024, "%d:%d", major, minor);
+		snprintf(buf, 30, "%d:%d", major, minor);
 		nonstd = buf;
 	}
 

[-- Attachment #4: mdadm-2.4-strict-aliasing.patch --]
[-- Type: text/plain, Size: 928 bytes --]

* Sat May 27 2006 Luca Berra <bluca@vodka.it>
This is to avoid gcc warnings when building with strict-aliasing optimization

--- mdadm-2.4/dlink.h.alias	2006-05-26 21:05:07.000000000 +0200
+++ mdadm-2.4/dlink.h	2006-05-27 12:32:58.000000000 +0200
@@ -4,16 +4,16 @@
 
 struct __dl_head
 {
-    struct __dl_head *	dh_prev;
-    struct __dl_head *	dh_next;
+    void * dh_prev;
+    void * dh_next;
 };
 
 #define	dl_alloc(size)	((void*)(((char*)calloc(1,(size)+sizeof(struct __dl_head)))+sizeof(struct __dl_head)))
 #define	dl_new(t)	((t*)dl_alloc(sizeof(t)))
 #define	dl_newv(t,n)	((t*)dl_alloc(sizeof(t)*n))
 
-#define dl_next(p) *((void**)&(((struct __dl_head*)(p))[-1].dh_next))
-#define dl_prev(p) *((void**)&(((struct __dl_head*)(p))[-1].dh_prev))
+#define dl_next(p) *(&(((struct __dl_head*)(p))[-1].dh_next))
+#define dl_prev(p) *(&(((struct __dl_head*)(p))[-1].dh_prev))
 
 void *dl_head(void);
 char *dl_strdup(char *);

[-- Attachment #5: mdadm-2.5-mdassemble.patch --]
[-- Type: text/plain, Size: 2062 bytes --]

* Sat May 27 2006 Luca Berra <bluca@vodka.it>
add CFLAGS to mdassemble build and fix a couple of non-returning functions

--- mdadm-2.5/mdadm.h.bluca	2006-05-27 14:25:53.000000000 +0200
+++ mdadm-2.5/mdadm.h	2006-05-27 15:20:37.000000000 +0200
@@ -44,10 +44,8 @@
 #include	<errno.h>
 #include	<string.h>
 #include	<syslog.h>
-#ifdef __dietlibc__NONO
-int strncmp(const char *s1, const char *s2, size_t n) __THROW __pure__;
-char *strncpy(char *dest, const char *src, size_t n) __THROW;
-#include    <strings.h>
+#ifdef __dietlibc__
+#include	<strings.h>
 #endif
 
 
--- mdadm-2.5/mdassemble.c.bluca	2006-05-27 15:11:02.000000000 +0200
+++ mdadm-2.5/mdassemble.c	2006-05-27 15:15:24.000000000 +0200
@@ -54,7 +54,7 @@
 };
 
 #ifndef MDASSEMBLE_AUTO
-/* from mdadm.c */
+/* from mdopen.c */
 int open_mddev(char *dev, int autof/*unused */)
 {
 	int mdfd = open(dev, O_RDWR, 0);
@@ -79,7 +79,7 @@
 int verbose = 0;
 int force = 0;
 
-int main() {
+int main(int argc, char *argv[]) {
 	mddev_ident_t array_list =  conf_get_ident(configfile, NULL);
 	if (!array_list) {
 		fprintf(stderr, Name ": No arrays found in config file\n");
@@ -100,4 +100,5 @@
 					   NULL, NULL,
 					   readonly, runstop, NULL, NULL, verbose, force);
 		}
+	return rv;
 }
--- mdadm-2.5/Makefile.bluca	2006-05-27 14:56:07.000000000 +0200
+++ mdadm-2.5/Makefile	2006-05-27 15:24:07.000000000 +0200
@@ -73,7 +73,7 @@
 	mdopen.c super0.c super1.c bitmap.c restripe.c sysfs.c
 
 ASSEMBLE_SRCS := mdassemble.c Assemble.c config.c dlink.c util.c super0.c super1.c
-ASSEMBLE_FLAGS:= -DMDASSEMBLE
+ASSEMBLE_FLAGS:= $(CFLAGS) -DMDASSEMBLE
 ifdef MDASSEMBLE_AUTO
 ASSEMBLE_SRCS += mdopen.c mdstat.c
 ASSEMBLE_FLAGS += -DMDASSEMBLE_AUTO
--- mdadm-2.5/util.c.bluca	2006-05-27 14:25:53.000000000 +0200
+++ mdadm-2.5/util.c	2006-05-27 15:34:47.000000000 +0200
@@ -375,7 +375,7 @@
 }
 int nftw(const char *path, int (*han)(const char *name, const struct stat *stb, int flag, struct FTW *s), int nopenfd, int flags)
 {
-	ftw(path, add_dev_1, nopenfd);
+	return ftw(path, add_dev_1, nopenfd);
 }
 #endif
 

[-- Attachment #6: mdadm-2.5-rand.patch --]
[-- Type: text/plain, Size: 2845 bytes --]

* Sat May 27 2006 Luca Berra <bluca@vodka.it>
POSIX dictates rand/srand instead of BSD srandom/random

--- mdadm-2.5/Assemble.c.rand	2006-05-27 14:40:30.000000000 +0200
+++ mdadm-2.5/Assemble.c	2006-05-27 15:32:39.000000000 +0200
@@ -412,10 +412,10 @@
 				int rfd;
 				if ((rfd = open("/dev/urandom", O_RDONLY)) < 0 ||
 				    read(rfd, ident->uuid, 16) != 16) {
-					*(__u32*)(ident->uuid) = random();
-					*(__u32*)(ident->uuid+1) = random();
-					*(__u32*)(ident->uuid+2) = random();
-					*(__u32*)(ident->uuid+3) = random();
+					*(__u32*)(ident->uuid) = rand();
+					*(__u32*)(ident->uuid+1) = rand();
+					*(__u32*)(ident->uuid+2) = rand();
+					*(__u32*)(ident->uuid+3) = rand();
 				}
 				if (rfd >= 0) close(rfd);
 			}
--- mdadm-2.5/mdadm.c.rand	2006-05-26 07:43:35.000000000 +0200
+++ mdadm-2.5/mdadm.c	2006-05-27 15:31:18.000000000 +0200
@@ -107,7 +107,7 @@
 
 	int mdfd = -1;
 
-	srandom(time(0) ^ getpid());
+	srand(time(0) ^ getpid());
 
 	ident.uuid_set=0;
 	ident.level = UnSet;
--- mdadm-2.5/super0.c.rand	2006-05-27 14:34:54.000000000 +0200
+++ mdadm-2.5/super0.c	2006-05-27 15:33:08.000000000 +0200
@@ -539,7 +539,7 @@
 	sb->patch_version = 0;
 	sb->gvalid_words = 0; /* ignored */
 	if (rfd < 0 || read(rfd, &sb->set_uuid0, 4) != 4)
-		sb->set_uuid0 = random();
+		sb->set_uuid0 = rand();
 	sb->ctime = time(0);
 	sb->level = info->level;
 	if (size != info->size)
@@ -550,9 +550,9 @@
 	sb->md_minor = info->md_minor;
 	sb->not_persistent = 0;
 	if (rfd < 0 || read(rfd, &sb->set_uuid1, 12) != 12) {
-		sb->set_uuid1 = random();
-		sb->set_uuid2 = random();
-		sb->set_uuid3 = random();
+		sb->set_uuid1 = rand();
+		sb->set_uuid2 = rand();
+		sb->set_uuid3 = rand();
 	}
 	if (rfd >= 0)
 		close(rfd);
--- mdadm-2.5/super1.c.rand	2006-05-27 14:37:22.000000000 +0200
+++ mdadm-2.5/super1.c	2006-05-27 15:33:29.000000000 +0200
@@ -583,10 +583,10 @@
 
 	if ((rfd = open("/dev/urandom", O_RDONLY)) < 0 ||
 	    read(rfd, sb->set_uuid, 16) != 16) {
-		*(__u32*)(sb->set_uuid) = random();
-		*(__u32*)(sb->set_uuid+4) = random();
-		*(__u32*)(sb->set_uuid+8) = random();
-		*(__u32*)(sb->set_uuid+12) = random();
+		*(__u32*)(sb->set_uuid) = rand();
+		*(__u32*)(sb->set_uuid+4) = rand();
+		*(__u32*)(sb->set_uuid+8) = rand();
+		*(__u32*)(sb->set_uuid+12) = rand();
 	}
 	if (rfd >= 0) close(rfd);
 
@@ -751,10 +751,10 @@
 
 	if ((rfd = open("/dev/urandom", O_RDONLY)) < 0 ||
 	    read(rfd, sb->device_uuid, 16) != 16) {
-		*(__u32*)(sb->device_uuid) = random();
-		*(__u32*)(sb->device_uuid+4) = random();
-		*(__u32*)(sb->device_uuid+8) = random();
-		*(__u32*)(sb->device_uuid+12) = random();
+		*(__u32*)(sb->device_uuid) = rand();
+		*(__u32*)(sb->device_uuid+4) = rand();
+		*(__u32*)(sb->device_uuid+8) = rand();
+		*(__u32*)(sb->device_uuid+12) = rand();
 	}
 	if (rfd >= 0) close(rfd);
 	sb->events = 0;

[-- Attachment #7: mdadm-2.5-unused.patch --]
[-- Type: text/plain, Size: 3471 bytes --]

* Sat May 27 2006 Luca Berra <bluca@vodka.it>
check return status of all write/fwrite functions as required by glibc 2.4

--- mdadm-2.5/Monitor.c.unused	2006-05-27 14:28:23.000000000 +0200
+++ mdadm-2.5/Monitor.c	2006-05-27 14:28:28.000000000 +0200
@@ -521,7 +521,7 @@
 				int n;
 				fprintf(mp, "\nP.S. The /proc/mdstat file current contains the following:\n\n");
 				while ( (n=fread(buf, 1, sizeof(buf), mdstat)) > 0)
-					fwrite(buf, 1, n, mp);
+					n=fwrite(buf, 1, n, mp); /* yes, i don't care about the result */
 				fclose(mdstat);
 			}
 			fclose(mp);
--- mdadm-2.5/super1.c.unused	2006-05-27 14:36:13.000000000 +0200
+++ mdadm-2.5/super1.c	2006-05-27 14:37:22.000000000 +0200
@@ -715,7 +715,8 @@
 			(((char*)sb)+1024);
 		if (__le32_to_cpu(bm->magic) == BITMAP_MAGIC) {
 			locate_bitmap1(st, fd, sbv);
-			write(fd, bm, sizeof(*bm));
+			if (write(fd, bm, sizeof(*bm)) != sizeof(*bm))
+			    return 5;
 		}
 	}
 	fsync(fd);
--- mdadm-2.5/super0.c.unused	2006-05-27 14:31:33.000000000 +0200
+++ mdadm-2.5/super0.c	2006-05-27 14:34:54.000000000 +0200
@@ -625,7 +625,8 @@
 	if (super->state & (1<<MD_SB_BITMAP_PRESENT)) {
 		struct bitmap_super_s * bm = (struct bitmap_super_s*)(super+1);
 		if (__le32_to_cpu(bm->magic) == BITMAP_MAGIC)
-			write(fd, bm, sizeof(*bm));
+			if (write(fd, bm, sizeof(*bm)) != sizeof(*bm))
+			    return 5;
 	}
 
 	fsync(fd);
--- mdadm-2.5/Grow.c.unused	2006-05-23 06:34:37.000000000 +0200
+++ mdadm-2.5/Grow.c	2006-05-27 14:25:53.000000000 +0200
@@ -801,7 +801,10 @@
 		memset(&bsb, 0, sizeof(bsb));
 		for (i=odisks; i<d ; i++) {
 			lseek64(fdlist[i], (offsets[i]+last_block)<<9, 0);
-			write(fdlist[i], &bsb, sizeof(bsb));
+			if (write(fdlist[i], &bsb, sizeof(bsb)) < 0) {
+				fprintf(stderr, Name ": %s: failed to invalidate metadata for raid disk %d\n",
+					devname, i);
+			}
 		}
 
 		/* unsuspend. */
--- mdadm-2.5/bitmap.c.unused	2006-05-19 09:15:32.000000000 +0200
+++ mdadm-2.5/bitmap.c	2006-05-27 14:52:31.000000000 +0200
@@ -399,16 +399,22 @@
 	return rv;
 }
 
-void bitmap_update_uuid(int fd, int *uuid)
+int bitmap_update_uuid(int fd, int *uuid)
 {
 	struct bitmap_super_s bm;
-	lseek(fd, 0, 0);
+	if (lseek(fd, 0, 0) != 0)
+		return 1;
 	if (read(fd, &bm, sizeof(bm)) != sizeof(bm))
-		return;
+		return 1;
 	if (bm.magic != __cpu_to_le32(BITMAP_MAGIC))
-		return;
+		return 1;
 	memcpy(bm.uuid, uuid, 16);
+	if (lseek(fd, 0, 0) != 0)
+		return 2;
+	if (write(fd, &bm, sizeof(bm)) != sizeof(bm)) {
+		lseek(fd, 0, 0);
+		return 2;
+	}
 	lseek(fd, 0, 0);
-	write(fd, &bm, sizeof(bm));
-	lseek(fd, 0, 0);
+	return 0;
 }
--- mdadm-2.5/mdadm.h.unused	2006-05-27 14:25:53.000000000 +0200
+++ mdadm-2.5/mdadm.h	2006-05-27 14:51:53.000000000 +0200
@@ -371,7 +371,7 @@
 			unsigned long long array_size,
 			int major);
 extern int ExamineBitmap(char *filename, int brief, struct supertype *st);
-extern void bitmap_update_uuid(int fd, int *uuid);
+extern int bitmap_update_uuid(int fd, int *uuid);
 
 extern int md_get_version(int fd);
 extern int get_linux_version(void);
--- mdadm-2.5/Assemble.c.unused	2006-05-26 08:28:07.000000000 +0200
+++ mdadm-2.5/Assemble.c	2006-05-27 14:40:30.000000000 +0200
@@ -451,7 +451,9 @@
 
 			if (strcmp(update, "uuid")==0 &&
 			    ident->bitmap_fd)
-				bitmap_update_uuid(ident->bitmap_fd, info.uuid);
+				if (bitmap_update_uuid(ident->bitmap_fd, info.uuid) != 0)
+					fprintf(stderr, Name ": Could not update uuid on %s.\n",
+						devname);
 		} else
 #endif
 		{

Re: [PATCH] mdadm 2.5 (Was: ANNOUNCE: mdadm 2.5 - A tool for managing Soft RAID under Linux)

[dean gaudet]

On Sun, 28 May 2006, Luca Berra wrote:

> - mdadm-2.5-rand.patch
> Posix dictates rand() versus bsd random() function, and dietlibc
> deprecated random(), so switch to srand()/rand() and make everybody
> happy.

fwiw... lots of rand()s tend to suck... and RAND_MAX may not be large 
enough for this use.  glibc rand() is the same as random().  do you know 
if dietlibc's rand() is good enough?

-dean

Re: [PATCH] mdadm 2.5 (Was: ANNOUNCE: mdadm 2.5 - A tool for managing Soft RAID under Linux)

[Luca Berra]

On Sun, May 28, 2006 at 10:08:19AM -0700, dean gaudet wrote:
>On Sun, 28 May 2006, Luca Berra wrote:
>
>> - mdadm-2.5-rand.patch
>> Posix dictates rand() versus bsd random() function, and dietlibc
>> deprecated random(), so switch to srand()/rand() and make everybody
>> happy.
>
>fwiw... lots of rand()s tend to suck... and RAND_MAX may not be large 
>enough for this use.  glibc rand() is the same as random().  do you know 
the fact that glibc rand() is the same implementation as random() was one of the
reason i believe we could switch to rand()

>if dietlibc's rand() is good enough?
dietlibc rand() and random() are the same function.
but random will throw a warning saying it is deprecated.

L.

-- 
Luca Berra -- bluca@comedia.it
        Communication Media & Services S.r.l.
 /"\
 \ /     ASCII RIBBON CAMPAIGN
  X        AGAINST HTML MAIL
 / \

Re: [PATCH] mdadm 2.5 (Was: ANNOUNCE: mdadm 2.5 - A tool for managing Soft RAID under Linux)

[dean gaudet]

On Sun, 28 May 2006, Luca Berra wrote:

> dietlibc rand() and random() are the same function.
> but random will throw a warning saying it is deprecated.

that's terribly obnoxious... it's never going to be deprecated, there are 
only approximately a bazillion programs using random().

-dean

Re: [PATCH] mdadm 2.5 (Was: ANNOUNCE: mdadm 2.5 - A tool for managing Soft RAID under Linux)

[Neil Brown]

On Sunday May 28, bluca@comedia.it wrote:
> On Fri, May 26, 2006 at 04:33:08PM +1000, Neil Brown wrote:
> >
> >
> >I am pleased to announce the availability of
> >   mdadm version 2.5
> >
> 
> hello,
> i tried rebuilding mdadm 2.5 on current mandriva cooker, which uses
> gcc-4.1.1, glibc-2.4 and dietlibc 0.29 and found the following issues
> addressed by patches attacched to this message
> I would be glad if you could review these patches and include them in
> upcoming mdadm releases.

Thanks for the patches.  They are greatly appreciated.

> 
> - mdadm-2.3.1-kernel-byteswap-include-fix.patch
> reverts a change introduced with mdadm 2.3.1 for redhat compatibility
> asm/byteorder.h is an architecture dependent file and does more
> stuff than a call to the linux/byteorder/XXX_endian.h
> the fact that not calling asm/byteorder.h does not define
> __BYTEORDER_HAS_U64__ is just an example of issues that might arise.
> if redhat is broken it should be worked around differently than breaking
> mdadm.

I don't understand the problem here.  What exactly breaks with the
code currently in 2.5?  mdadm doesn't need __BYTEORDER_HAS_U64__, so
why does not having id defined break anything?
The coomment from the patch says:
 > not including asm/byteorder.h will not define __BYTEORDER_HAS_U64__
 > causing __fswab64 to be undefined and failure compiling mdadm on
 > big_endian architectures like PPC

But again, mdadm doesn't use __fswab64 ....
More details please.

> 
> - mdadm-2.4-snprintf.patch
> this is self commenting, just an error in the snprintf call

I wonder how that snuck in...
There was an odd extra tab in the patch, but no-matter.
I changed it to use 'sizeof(buf)' to be consistent with other uses
of snprint.  Thanks.

> 
> - mdadm-2.4-strict-aliasing.patch
> fix for another srict-aliasing problem, you can typecast a reference to a
> void pointer to anything, you cannot typecast a reference to a
> struct.

Why can't I typecast a reference to a struct??? It seems very
unfair...
However I have no problem with the patch.  Applied.  Thanks.
I should really change it to use 'list.h' type lists from the linux
kernel.

> 
> - mdadm-2.5-mdassemble.patch
> pass CFLAGS to mdassemble build, enabling -Wall -Werror showed some
> issues also fixed by the patch.

yep, thanks.

> 
> - mdadm-2.5-rand.patch
> Posix dictates rand() versus bsd random() function, and dietlibc
> deprecated random(), so switch to srand()/rand() and make everybody
> happy.

Everybody?
'man 3 rand' tells me:

       Do not use this function in applications  intended  to  be
       portable when good randomness is needed.

Admittedly mdadm doesn't need to be portable - it only needs to run on
Linux.  But this line in the man page bothers me.

I guess
    -Drandom=rand -Dsrandom=srand
might work.... no.  stdlib.h doesn't like that.
'random' returns 'long int' while rand returns 'int'.
Interestingly 'random_r' returns 'int' as does 'rand_r'.

#ifdef __dietlibc__
#include	<strings.h>
/* dietlibc has deprecated random and srandom!! */
#define random rand
#define srandom srand
#endif

in mdadm.h.  Will that do you?


> 
> - mdadm-2.5-unused.patch
> glibc 2.4 is pedantic on ignoring return values from fprintf, fwrite and
> write, so now we check the rval and actually do something with it.
> in the Grow.c case i only print a warning, since i don't think we can do
> anithing in case we fail invalidating those superblocks (is should never
> happen, but then...)

Ok, thanks.


You can see these patches at
   http://neil.brown.name/cgi-bin/gitweb.cgi?p=mdadm

more welcome :-)

Thanks again,
NeilBrown

Re: [PATCH] mdadm 2.5 (Was: ANNOUNCE: mdadm 2.5 - A tool for managing Soft RAID under Linux)

[Luca Berra]

On Mon, May 29, 2006 at 12:08:25PM +1000, Neil Brown wrote:
>On Sunday May 28, bluca@comedia.it wrote:
>Thanks for the patches.  They are greatly appreciated.
You're welcome
>> 
>> - mdadm-2.3.1-kernel-byteswap-include-fix.patch
>> reverts a change introduced with mdadm 2.3.1 for redhat compatibility
>> asm/byteorder.h is an architecture dependent file and does more
>> stuff than a call to the linux/byteorder/XXX_endian.h
>> the fact that not calling asm/byteorder.h does not define
>> __BYTEORDER_HAS_U64__ is just an example of issues that might arise.
>> if redhat is broken it should be worked around differently than breaking
>> mdadm.
>
>I don't understand the problem here.  What exactly breaks with the
>code currently in 2.5?  mdadm doesn't need __BYTEORDER_HAS_U64__, so
>why does not having id defined break anything?
>The coomment from the patch says:
> > not including asm/byteorder.h will not define __BYTEORDER_HAS_U64__
> > causing __fswab64 to be undefined and failure compiling mdadm on
> > big_endian architectures like PPC
>
>But again, mdadm doesn't use __fswab64 ....
>More details please.
you use __cpu_to_le64 (ie in super0.c line 987)

        bms->sync_size = __cpu_to_le64(size);

which in byteorder/big_endian.h is defined as

#define __cpu_to_le64(x) ((__force __le64)__swab64((x)))

but __swab64 is defined in byteorder/swab.h (included by
byteorder/big_endian.h) as

#if defined(__GNUC__) && (__GNUC__ >= 2) && defined(__OPTIMIZE__)
#  define __swab64(x) \
(__builtin_constant_p((__u64)(x)) ? \
___swab64((x)) : \
__fswab64((x)))
#else
#  define __swab64(x) __fswab64(x)
#endif /* OPTIMIZE */

and __fswab64 is defined further into byteorder/swab.h as

#ifdef __BYTEORDER_HAS_U64__
static __inline__ __attribute_const__ __u64 __fswab64(__u64 x)
.....
#endif /* __BYTEORDER_HAS_U64__ */

so building mdadm on a ppc (or i suppose a sparc) will break


now, if you look at /usr/src/linux/asm-*/byteorder.h you will notice
they are very different files, this makes me believe it is not a good
idea bypassing asm/byteorder.h
And no, just defining __BYTEORDER_HAS_U64__ will break on 32bit
big-endian cpus (and if i do not misread it might just compile and give
incorrect results)

>> 
>> - mdadm-2.4-snprintf.patch
>> this is self commenting, just an error in the snprintf call
>
>I wonder how that snuck in...
>There was an odd extra tab in the patch, but no-matter.
>I changed it to use 'sizeof(buf)' to be consistent with other uses
>of snprint.  Thanks.
yes, that would be better.

>> - mdadm-2.4-strict-aliasing.patch
>> fix for another srict-aliasing problem, you can typecast a reference to a
>> void pointer to anything, you cannot typecast a reference to a
>> struct.
>
>Why can't I typecast a reference to a struct??? It seems very
>unfair...
that's strict-aliasing optimization for you, i do agree it _is_ unfair.

>However I have no problem with the patch.  Applied.  Thanks.
>I should really change it to use 'list.h' type lists from the linux
>kernel.
hopefull redhat would not object :)

>> - mdadm-2.5-mdassemble.patch
>> pass CFLAGS to mdassemble build, enabling -Wall -Werror showed some
>> issues also fixed by the patch.
>
>yep, thanks.
>
>> 
>> - mdadm-2.5-rand.patch
>> Posix dictates rand() versus bsd random() function, and dietlibc
>> deprecated random(), so switch to srand()/rand() and make everybody
>> happy.
>
>Everybody?
>'man 3 rand' tells me:
>
>       Do not use this function in applications  intended  to  be
>       portable when good randomness is needed.
>
>Admittedly mdadm doesn't need to be portable - it only needs to run on
>Linux.  But this line in the man page bothers me.
>
>I guess
>    -Drandom=rand -Dsrandom=srand
>might work.... no.  stdlib.h doesn't like that.
>'random' returns 'long int' while rand returns 'int'.
>Interestingly 'random_r' returns 'int' as does 'rand_r'.
>
>#ifdef __dietlibc__
>#include	<strings.h>
>/* dietlibc has deprecated random and srandom!! */
>#define random rand
>#define srandom srand
>#endif
>
>in mdadm.h.  Will that do you?
>

yes, mdassemble will build, so it is ok for me.


>> 
>> - mdadm-2.5-unused.patch
>> glibc 2.4 is pedantic on ignoring return values from fprintf, fwrite and
>> write, so now we check the rval and actually do something with it.
>> in the Grow.c case i only print a warning, since i don't think we can do
>> anithing in case we fail invalidating those superblocks (is should never
>> happen, but then...)
>
>Ok, thanks.
>
>
>You can see these patches at
>   http://neil.brown.name/cgi-bin/gitweb.cgi?p=mdadm

Thanks.


-- 
Luca Berra -- bluca@comedia.it
        Communication Media & Services S.r.l.
 /"\
 \ /     ASCII RIBBON CAMPAIGN
  X        AGAINST HTML MAIL
 / \

Re: [PATCH] mdadm 2.5 (Was: ANNOUNCE: mdadm 2.5 - A tool for managing Soft RAID under Linux)

[Neil Brown]

On Monday May 29, bluca@comedia.it wrote:
> On Mon, May 29, 2006 at 12:08:25PM +1000, Neil Brown wrote:
> >On Sunday May 28, bluca@comedia.it wrote:
> >Thanks for the patches.  They are greatly appreciated.
> You're welcome
> >> 
> >> - mdadm-2.3.1-kernel-byteswap-include-fix.patch
> >> reverts a change introduced with mdadm 2.3.1 for redhat compatibility
> >> asm/byteorder.h is an architecture dependent file and does more
> >> stuff than a call to the linux/byteorder/XXX_endian.h
> >> the fact that not calling asm/byteorder.h does not define
> >> __BYTEORDER_HAS_U64__ is just an example of issues that might arise.
> >> if redhat is broken it should be worked around differently than breaking
> >> mdadm.
> >
> >I don't understand the problem here.  What exactly breaks with the
> >code currently in 2.5?  mdadm doesn't need __BYTEORDER_HAS_U64__, so
> >why does not having id defined break anything?
> >The coomment from the patch says:
> > > not including asm/byteorder.h will not define __BYTEORDER_HAS_U64__
> > > causing __fswab64 to be undefined and failure compiling mdadm on
> > > big_endian architectures like PPC
> >
> >But again, mdadm doesn't use __fswab64 ....
> >More details please.
> you use __cpu_to_le64 (ie in super0.c line 987)
> 
>         bms->sync_size = __cpu_to_le64(size);
> 
> which in byteorder/big_endian.h is defined as
> 
> #define __cpu_to_le64(x) ((__force __le64)__swab64((x)))
> 
> but __swab64 is defined in byteorder/swab.h (included by
> byteorder/big_endian.h) as
> 
> #if defined(__GNUC__) && (__GNUC__ >= 2) && defined(__OPTIMIZE__)
> #  define __swab64(x) \
> (__builtin_constant_p((__u64)(x)) ? \
> ___swab64((x)) : \
> __fswab64((x)))
> #else
> #  define __swab64(x) __fswab64(x)
> #endif /* OPTIMIZE */


Grrr......

Thanks for the details.  I think I'll just give up and do it myself.
e.g.
short swap16(short in)
{
	int i;
	short out=0;
	for (i=0; i<4; i++) {
		out = out<<8 | (in&255);
		in = in >> 8;
	}
	return out;
}

I don't need top performance and at least this should be portable...

NeilBrown

Re: [PATCH] mdadm 2.5 (Was: ANNOUNCE: mdadm 2.5 - A tool for managing Soft RAID under Linux)

[Alex Davis]

>short swap16(short in)
>{
>        int i;
>        short out=0;
>        for (i=0; i<4; i++) {
>                out = out<<8 | (in&255);
>                in = in >> 8;
>        }
>        return out;
>}

Shouldn't that be "for (i=0; i<2; i++) {..." ? 

I code, therefore I am

__________________________________________________
Do You Yahoo!?
Tired of spam?  Yahoo! Mail has the best spam protection around 
http://mail.yahoo.com 

Re: [PATCH] mdadm 2.5 (Was: ANNOUNCE: mdadm 2.5 - A tool for managing Soft RAID under Linux)

[Eyal Lebedinsky]

Alex Davis wrote:
>>short swap16(short in)
>>{
>>       int i;
>>       short out=0;
>>       for (i=0; i<4; i++) {
>>               out = out<<8 | (in&255);
>>               in = in >> 8;
>>       }
>>       return out;
>>}
> 
> Shouldn't that be "for (i=0; i<2; i++) {..." ? 

In which case, do we really need this complexity rather than
a clear swap, e.g.
	return (short)(((in&0x0ff)<<8) | ((in>>8)&0x0ff))

Simple enough for a macro too.

-- 
Eyal Lebedinsky (eyal@eyal.emu.id.au) <http://samba.org/eyal/>
	attach .zip as .dat

Re: [PATCH] mdadm 2.5 (Was: ANNOUNCE: mdadm 2.5 - A tool for managing Soft RAID under Linux)

[Neil Brown]

On Thursday June 1, eyal@eyal.emu.id.au wrote:
> Alex Davis wrote:
> >>short swap16(short in)
> >>{
> >>       int i;
> >>       short out=0;
> >>       for (i=0; i<4; i++) {
> >>               out = out<<8 | (in&255);
> >>               in = in >> 8;
> >>       }
> >>       return out;
> >>}
> > 
> > Shouldn't that be "for (i=0; i<2; i++) {..." ? 

Well, yes... if you want the code to actually work, I guess that might
be an improvement....


> 
> In which case, do we really need this complexity rather than
> a clear swap, e.g.
> 	return (short)(((in&0x0ff)<<8) | ((in>>8)&0x0ff))

What I like about the above is that the difference between swap16,
swap32 and swap64 is just one number.  In this case I think clarity
and maintainability are much more important than efficiency.

Thanks for the comment,
NeilBrown

Re: [PATCH] mdadm 2.5 (Was: ANNOUNCE: mdadm 2.5 - A tool for managing Soft RAID under Linux)

[Luca Berra]

On Thu, Jun 01, 2006 at 11:22:09AM +1000, Neil Brown wrote:
>On Thursday June 1, eyal@eyal.emu.id.au wrote:
>> Alex Davis wrote:
>> >>short swap16(short in)
>> >>{
>> >>       int i;
>> >>       short out=0;
>> >>       for (i=0; i<4; i++) {
>> >>               out = out<<8 | (in&255);
>> >>               in = in >> 8;
>> >>       }
>> >>       return out;
>> >>}
>> > 
>> > Shouldn't that be "for (i=0; i<2; i++) {..." ? 
>
>Well, yes... if you want the code to actually work, I guess that might
>be an improvement....
most important,
shouldn't that be:
__u16 swap16(__u16 in)

?

Regards,
L.

-- 
Luca Berra -- bluca@comedia.it
        Communication Media & Services S.r.l.
 /"\
 \ /     ASCII RIBBON CAMPAIGN
  X        AGAINST HTML MAIL
 / \

Re: [PATCH] mdadm 2.5

[Nix]

On 29 May 2006, Neil Brown suggested tentatively:
> On Sunday May 28, bluca@comedia.it wrote:
>> - mdadm-2.4-strict-aliasing.patch
>> fix for another srict-aliasing problem, you can typecast a reference to a
>> void pointer to anything, you cannot typecast a reference to a
>> struct.
> 
> Why can't I typecast a reference to a struct??? It seems very
> unfair...

ISO C forbids it. Pretty much the only time you're allowed to cast a pointer
to A to a pointer to B is if one or the other of them is a pointer to void
or char.

The compiler can (and does) assume that all other pointers-to-T are in
different `aliasing sets', i.e. that a modification through a
pointer-to-T1 cannot affect variables of any other type (so they can
be cached in registers, moved around without reference to each other,
and so on).

It's *very* advantageous, and with the interprocedural optimizations in
GCC 4.1+ is getting more so all the time (the compiler can now move code
between *functions* if it needs to, or propagate constants consistently
used as function arguments down int into the function themselves; pointer
aliasing torpedoes both of these optimizations, and many more.)

(One particularly evil thing is the glibc printf handler extension. As
a result of that, we must assume that any call to printf() or scanf()
can potentially modify any global variable and any local to which a
pointer may have escaped. This is decidedly suboptimal given that
nobody sane ever uses those extensions!)


I think I'll dare to upgrade mdadm soon. I tried upgrading from 2.3.1 to
2.4 (with uClibc from SVN dated 2006-03-28) and it promptly segfaulted
at array assembly time. If it happens with 2.5 I'll get a backtrace
(hard though that is to get when you don't have any accessible storage
to put the core dump on... I upgraded uClibc at the same time so it may
well be an uClibc bug.)

-- 
`Voting for any American political party is fundamentally
 incomprehensible.' --- Vadik