[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] osdep/linux: Fix md array device enumeration
From: |
Petr Vorel |
Subject: |
Re: [PATCH] osdep/linux: Fix md array device enumeration |
Date: |
Wed, 6 Oct 2021 09:28:32 +0200 |
Hi Kees, Daniel,
> On Sat, Sep 25, 2021 at 07:03:35PM -0700, kees@ubuntu.com wrote:
> > From: Kees Cook <kees@ubuntu.com>
> > GET_ARRAY_INFO's info.nr_disks does not map to GET_DISK_INFO's
> > disk.number, which is an internal kernel index. If an array has had drives
> > added, removed, etc, there may be gaps in GET_DISK_INFO's results. But
> > since the consumer of devicelist cannot tolerate gaps (it expects to walk
> > a NULL-terminated list of device name strings), the devicelist index (j)
> > must be tracked separately from the disk.number index (i). But grub
> > wants to only examine active (i.e. present and non-failed) disks, so how
> > many disks are left to be queried must be also separately tracked
> > (remaining).
Kees, thanks a lot for taking care for this.
<snip>
> > + /* Skip empty slot: MD_DISK_REMOVED slots don't count toward
> > nr_disks. */
> > if (disk.state & (1 << MD_DISK_REMOVED))
> > continue;
> > + remaining--;
> > - if (disk.state & (1 << MD_DISK_ACTIVE))
> > - devicelist[j] = grub_find_device (NULL,
> > - makedev (disk.major, disk.minor));
> > - else
> > - devicelist[j] = NULL;
> > - j++;
> > + /* Only examine disks that are actively participating in the array.
> > */
> > + if (!(disk.state & (1 << MD_DISK_ACTIVE)))
> > + continue;
> > +
> > + devicelist[j++] = grub_find_device (NULL, makedev (disk.major,
> > + disk.minor));
> I would prefer if you leave original if statement as is and update
> grub_find_device() call accordingly... And of course drop else as
> needed... :-)
I suppose you'll need to send v2 due this, but for now:
Reviewed-by: Petr Vorel <pvorel@suse.cz>
Kind regards,
Petr