On Fri, May 9, 2014 at 5:11 PM, Klaus Aehlig <[email protected]> wrote:

> On Fri, May 09, 2014 at 04:46:15PM +0200, 'Petr Pudlak' via ganeti-devel
> wrote:
> > This includes nested disk children.
> >
> > Signed-off-by: Petr Pudlak <[email protected]>
> > ---
> >  src/Ganeti/Config.hs | 15 +++++++++++++++
> >  1 file changed, 15 insertions(+)
>
>
> > +-- | Returns the DRBD minors of a given 'Disk'
> > +getDrbdMinorsForDisk :: Disk -> [(Int, String)]
> > +getDrbdMinorsForDisk Disk{ diskLogicalId = (LIDDrbd8 nA nB _ mnA mnB _)
>
> add sapce between "Disk" and "{" and adjust indentation of the following
> lines
> accordingly.
>

Will fix.


>
> > +                         , diskChildren = ch
> > +                         } = [(mnA, nA), (mnB, nB)] ++
> > +                             concatMap getDrbdMinorsForDisk ch
>
>
> > +getDrbdMinorsForDisk d = concatMap getDrbdMinorsForDisk (diskChildren d)
>
> This line is redundant, as "Disk" is the only constructor for "Disk".
> Remove it.
>

No, it's necessary, because the previous pattern matches on the Drbd disk
ID, and we need to handle the case for all other non-DRBD disk types.


>
> > +
> >  -- | Filters DRBD minors for a given node.
> >  getDrbdMinorsForNode :: String -> Disk -> [(Int, String)]
> >  getDrbdMinorsForNode node disk =
> > @@ -369,6 +378,12 @@ getDrbdMinorsForNode node disk =
> >            _ -> []
> >    in this_minors ++ child_minors
> >
> > +-- | Returns the DRBD minors of a given instance
> > +getDrbdMinorsForInstance :: ConfigData -> Instance
> > +                         -> ErrorResult [(Int, String)]
> > +getDrbdMinorsForInstance cfg =
> > +  liftM (concatMap getDrbdMinorsForDisk) . getInstDisksFromObj cfg
> > +
> >  -- | String for primary role.
> >  rolePrimary :: String
> >  rolePrimary = "primary"
>
> Rest LGTM. No need to resend, if you do the ammendments suggested.
>
> --
> Klaus Aehlig
> Google Germany GmbH, Dienerstr. 12, 80331 Muenchen
> Registergericht und -nummer: Hamburg, HRB 86891
> Sitz der Gesellschaft: Hamburg
> Geschaeftsfuehrer: Graham Law, Christine Elizabeth Flores
>

Reply via email to