On Thu, Apr 20, 2017 at 12:05:54PM -0400, Jes Sorensen wrote:

On 04/17/2017 07:48 PM, NeilBrown wrote:

quoted

On Fri, Apr 14 2017, Jes Sorensen wrote:

quoted

Looking some more at this, it may be simpler than I thought. How about
this approach (only compile tested):

int md_array_active(int fd)
{
struct mdinfo *sra;
struct mdu_array_info_s array;
int ret;

sra = sysfs_read(fd, NULL, GET_VERSION | GET_DISKS);
if (sra) {
	if (!sra->array.raid_disks &&
	    !(sra->array.major_version == -1 &&
	      sra->array.minor_version == -2))
		ret = -ENODEV;
	else
		ret = 0;

	free(sra);
} else {
	ret = ioctl(fd, GET_ARRAY_INFO, &array);
}

return !ret;
}

Note 'major = -1 && minor = -2' is sysfs_read's way of saying 'external'.

This pretty much mimics what the kernel does in the ioctl handler for
GET_ARRAY_INFO:

case GET_ARRAY_INFO:
	if (!mddev->raid_disks && !mddev->external)
		err = -ENODEV;
	else
		err = get_array_info(mddev, argp);
	goto out;

What do you think?

I think that it accurately mimics what the current code does.
I'm not sure that is what we really want.
For testing in Incremental.c if an array is "active" we really
should be testing more than "raid_disks != 0".
We should be testing, as Shaohua suggested, if
array_state != 'clear' or 'inactive'.
You cannot get that info through the ioctl interface, so I suppose
I decided the current test was 'close enough'.
If we are going to stop supported kernels that don't have (e.g.)
array_state, then we should really fo the right thing and test
array_state.

I think I got it right this time and pushed it into git. It made
things a lot prettier too IMHO :)

In the process I also changed the behavior of
sysfs_read(GET_ARRAY_STATE) as I really didn't like how it was
copying in the string rather than parsing it.

I am traveling at the moment and don't yet have my new raid test box
setup back at the office, so my testing is limited. If I broke
something badly, feel free to throw rotten tomatoes at me.

Hi Jes,

Compilation with "-O2" flag fails:

CXFLAGS=-O2 make

cc -Wall -Werror -Wstrict-prototypes -Wextra -Wno-unused-parameter -O2
-DSendmail=\""/usr/sbin/sendmail -t"\" -DCONFFILE=\"/etc/mdadm.conf\"
-DCONFFILE2=\"/etc/mdadm/mdadm.conf\" -DMAP_DIR=\"/run/mdadm\"
-DMAP_FILE=\"map\" -DMDMON_DIR=\"/run/mdadm\"
-DFAILED_SLOTS_DIR=\"/run/mdadm/failed-slots\" -DNO_COROSYNC -DNO_DLM
-DVERSION=\"4.0-85-g4435675\" -DVERS_DATE="\"2017-04-20\"" -DUSE_PTHREADS
-DBINDIR=\"/sbin\"  -c -o Query.o Query.c

Query.c: In function ‘Query’:
Query.c:92:9: error: ‘level’ may be used uninitialized in this function
[-Werror=maybe-uninitialized]
   printf("%s: %s %s %d devices, %d spare%s. Use mdadm --detail for more
   detail.\n",
            ^
Query.c:92:9: error: ‘spare_disks’ may be used uninitialized in
this function [-Werror=maybe-uninitialized]
Query.c:92:9: error: ‘raid_disks’ may be used uninitialized in
this function [-Werror=maybe-uninitialized]
cc1: all warnings being treated as errors
make: *** [Query.o] Error 1

Regards,

Tomek