On Thu, Apr 20, 2017 at 12:05:54PM -0400, Jes Sorensen wrote:
> On 04/17/2017 07:48 PM, NeilBrown wrote:
> >On Fri, Apr 14 2017, Jes Sorensen wrote:
> >>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
--
To unsubscribe from this list: send the line "unsubscribe linux-raid" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
