On Fri, 2013-12-20 at 18:54 -0500, Anne Mulhern wrote:
----- Original Message -----
From: "Anne Mulhern" <amulhern(a)redhat.com>
To: "anaconda patch review" <anaconda-patches(a)lists.fedorahosted.org>
Sent: Friday, December 20, 2013 6:46:46 PM
Subject: Re: [PATCH 1/6] Fix a few minor problems introduced by recent raid level
changes.
----- Original Message -----
> From: "Brian C. Lane" <bcl(a)redhat.com>
> To: anaconda-patches(a)lists.fedorahosted.org
> Sent: Friday, December 20, 2013 6:08:53 PM
> Subject: Re: [PATCH 1/6] Fix a few minor problems introduced by recent raid level
changes.
>
>
> > diff --git a/blivet/devicelibs/raid.py b/blivet/devicelibs/raid.py
> > index b55804c..d6b4959 100644
> > --- a/blivet/devicelibs/raid.py
> > +++ b/blivet/devicelibs/raid.py
> > @@ -266,7 +266,8 @@ class RAID0(RAIDLevel):
> > return div_up(size, member_count)
> >
> > def _get_raw_array_size(self, member_count, smallest_member_size):
> > - return member_count * smallest_member_size
> > + # smallest_member_size dictates the return type
> > + return smallest_member_size * member_count
> >
> > def _get_size(self, size, chunk_size):
> > return size - size % chunk_size
> > @@ -312,7 +313,8 @@ class RAID4(RAIDLevel):
> > return div_up(size, member_count - 1)
> >
> > def _get_raw_array_size(self, member_count, smallest_member_size):
> > - return (member_count - 1) * smallest_member_size
> > + # smallest_member_size dictates the return type
> > + return smaller_member_size * (member_count - 1)
>
> smallest_member_size not smaller
Just two comments from me.
1) This causes one devicefactory_test to fail w/ a NoneType object has no...
Maybe that's really what it should do...it should be fixed or possibly just removed.
I have a patch to fix that test, but I don't understand how it could
have been broken by these patches (or working prior to them). Either
way, I'll send that out for review separately.
2) I would really prefer that the reason for reversing the order of the two arguments
to * in raid.py were in the commit message, or in the comment on the definition of the
method
in the abstract parent class, rather than in a comment on each of the individual methods.
These should be part of patch 6/6, where the comments in the base class
are updated.
Just one more, now that I realize its part of the Size patch set:
The comments on get_raw_array_size give the type of the parameters as both int, so the
order
of the _get_raw_array_size parameters shouldn't matter. Presumably, one of them has
changed type,
so the comments on get_raw_array_size in the abstract class should be updated and
probably a comment
on _get_raw_array_size in the abstract class should be added.
>
> --
> Brian C. Lane | Anaconda Team | IRC: bcl #anaconda | Port Orchard, WA
> (PST8PDT)
> _______________________________________________
> anaconda-patches mailing list
> anaconda-patches(a)lists.fedorahosted.org
>
https://lists.fedorahosted.org/mailman/listinfo/anaconda-patches
>
_______________________________________________
anaconda-patches mailing list
anaconda-patches(a)lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/anaconda-patches
_______________________________________________
anaconda-patches mailing list
anaconda-patches(a)lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/anaconda-patches