mdadm: ioctl(GET_DISK_INFO) question

mdadm: ioctl(GET_DISK_INFO) question

[Alexander Lyakas]

Hello,

what is the reason that in some places (like Detail()), the code tests
for all desc_nrs possible (value read from superblock or MaxDisks
macro):
for (d = 0; d < max_disks; d++) {
    disc.number = d;
    ioctl(fd, GET_DISK_INFO, &disk)

and in other places (like Monitor()), the loop goes only up to :
array.raid_disks + array.nr_disks.

After all, disk.number (which is rdev->desc_nr in kernel) is merely a
position of the disk entry in the dev_roles[] array in the superblock,
and has no relation to the raid slot of the disk. Shouldn't the code
always check max_disks possible?

Thanks,
  Alex.

Re: mdadm: ioctl(GET_DISK_INFO) question

[NeilBrown]

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

On Fri, 27 Apr 2012 17:14:28 +0300 Alexander Lyakas <alex.bolshoy@gmail.com>
wrote:

> Hello,
> 
> what is the reason that in some places (like Detail()), the code tests
> for all desc_nrs possible (value read from superblock or MaxDisks
> macro):
> for (d = 0; d < max_disks; d++) {
>     disc.number = d;
>     ioctl(fd, GET_DISK_INFO, &disk)
> 
> and in other places (like Monitor()), the loop goes only up to :
> array.raid_disks + array.nr_disks.

Bug.

> 
> After all, disk.number (which is rdev->desc_nr in kernel) is merely a
> position of the disk entry in the dev_roles[] array in the superblock,
> and has no relation to the raid slot of the disk. Shouldn't the code
> always check max_disks possible?

Yes.

Thanks,
NeilBrown


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

Re: mdadm: ioctl(GET_DISK_INFO) question

[Alexander Lyakas]

Thanks for the clarification, Neil. I will really try to provide (at
least a partial) patch, together with the other patch I promised to
test...

On Sat, Apr 28, 2012 at 1:09 AM, NeilBrown <neilb@suse.de> wrote:
> On Fri, 27 Apr 2012 17:14:28 +0300 Alexander Lyakas <alex.bolshoy@gmail.com>
> wrote:
>
>> Hello,
>>
>> what is the reason that in some places (like Detail()), the code tests
>> for all desc_nrs possible (value read from superblock or MaxDisks
>> macro):
>> for (d = 0; d < max_disks; d++) {
>>     disc.number = d;
>>     ioctl(fd, GET_DISK_INFO, &disk)
>>
>> and in other places (like Monitor()), the loop goes only up to :
>> array.raid_disks + array.nr_disks.
>
> Bug.
>
>>
>> After all, disk.number (which is rdev->desc_nr in kernel) is merely a
>> position of the disk entry in the dev_roles[] array in the superblock,
>> and has no relation to the raid slot of the disk. Shouldn't the code
>> always check max_disks possible?
>
> Yes.
>
> Thanks,
> NeilBrown
>
--
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