On Wed, Oct 17, 2018 at 09:03:58PM +0200, Goffredo Baroncelli wrote:
> On 17/10/2018 16.14, Daniel Kiper wrote:
> > On Thu, Oct 11, 2018 at 08:51:01PM +0200, Goffredo Baroncelli wrote:
> >> From: Goffredo Baroncelli <kreij...@inwind.it>
> >>
> >> Add support for recovery for a RAID 5 btrfs profile. In addition
> >> it is added some code as preparatory work for RAID 6 recovery code.
> >>
> >> Signed-off-by: Goffredo Baroncelli <kreij...@inwind.it>
> >> ---
> [...]
>
> >> +
> >> + for (failed_devices = 0, i = 0; i < nstripes; i++)
> >> + {
> >> + struct grub_btrfs_chunk_stripe *stripe;
> >> + grub_disk_addr_t paddr;
> >> + grub_device_t dev;
> >> + grub_err_t err;
> >> +
> >> + /* after the struct grub_btrfs_chunk_item, there is an array of
> >> + struct grub_btrfs_chunk_stripe */
> >
> > /* Struct grub_btrfs_chunk_stripe lives behind struct
> > grub_btrfs_chunk_item. */
>
> What about
>
> /* The struct grub_btrfs_chunk_stripe array lives behind struct
> grub_btrfs_chunk_item. */
Works for me.
> [...]
>
> >> @@ -921,17 +1061,29 @@ grub_btrfs_read_logical (struct grub_btrfs_data
> >> *data, grub_disk_addr_t addr,
> >> grub_dprintf ("btrfs", "reading laddr 0x%" PRIxGRUB_UINT64_T "\n",
> >> addr);
> >>
> >> - for (i = 0; i < redundancy; i++)
> >> + if (!is_raid56)
> >
> > Why not "if (is_raid56)"? I asked about that earlier. Please change
> > this if and of course code below. It will be much easier to read. And
> > you do not need curly brackets for for loop after else.
>
> Frankly speaking I don't see any problem having a if (!...). However I
> update the code as your request, hoping to speedup this patch set
OK, it works. However, if you have "else" below then I think that it is
more natural to drop "!" here. If you would not have else I would not
complain. Well, because it would not make sense to do so... :-)))
Daniel
_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
