[f21-branch][PATCH] clean up stage1 volume check, check format as well as mountpoint
Vratislav Podzimek
vpodzime at redhat.com
Mon Dec 1 09:18:09 UTC 2014
On Mon, 2014-12-01 at 00:22 -0800, Adam Williamson wrote:
> On Mon, 2014-12-01 at 09:12 +0100, Vratislav Podzimek wrote:
> >
> > > This also debug logs the result of the 'disks with potential
> > > stage1 targets' check.
>
> > I like this improvement with two comments:
> > 1) using the word 'volume' and accordingly named variables makes me
> > think these are (only) LVM or BTRFS volumes which is not true.
> > 'partition' is also problematic, because LVM could, in some cases, be
> > used, but it's more typical to use partitions for stage1 devices. Or
> > what about 'device' or 'dev'? That fits best I'd say.
>
> I thought 'volume' was more or less the standard term anaconda uses in
> this case - you know, when the first word that comes into your head is
> 'partition', but it's not actually quite correct =). So that's why I
> used it.
Standard term anaconda uses for something? That's hard to believe. ;) I
still think 'dev' would be best.
>
> > 2) see one neat pick below:
>
> > > + if platform.bootStage1ConstraintDict["format_types"]:
> > > + # sort the list of disks with those that contain possibly-valid
> > > + # stage1 partitions first, so we'll pick a disk with one as
> > > + # bootDrive if it has not been explicitly set: RHBZ #1168118
> > > + s1vols = [vol for vol in storage.devices if
> > > + getattr(vol.format, "type", None) in
> > > + platform.bootStage1ConstraintDict["format_types"]]
> > This, after having a better name, could be a generator instead of a list
> > comprehension (using parentheses instead of square brackets).
>
> What would be the advantage of that, in this case? The list isn't likely
> to be particularly long, or anything. I don't mind doing it, I'd just
> like to learn why ;)
The only difference is that there won't be a temporary list created
(allocated, initialized,...) in the process. The expression is only used
to pre-filter some list for some other expression. Such "pre-filters"
are better done as generators instead of lists. It would make absolutely
no real difference in this case, however, that's why I called it a
"neat-pick".
>
> > In relation to this I'd like to mention that your Python skills became
> > waaaay better over the recent months. Nice!
>
> Thanks! It's not hard when the starting point is 'non-existent' ;)
That's hard to argue about. :)
--
Vratislav Podzimek
Anaconda Rider | Red Hat, Inc. | Brno - Czech Republic
More information about the anaconda-patches
mailing list