[blivet:master 05/17] Tighten up FS.mountable().

Anne Mulhern amulhern at redhat.com
Wed Jan 7 13:23:27 UTC 2015





----- Original Message -----
> From: "Vratislav Podzimek" <vpodzime at redhat.com>
> To: "anaconda patch review" <anaconda-patches at lists.fedorahosted.org>
> Sent: Wednesday, January 7, 2015 3:07:52 AM
> Subject: Re: [blivet:master 05/17] Tighten up FS.mountable().
> 
> On Tue, 2015-01-06 at 15:38 -0500, mulhern wrote:
> > Only calculate values if needed and omit redundant startswith check.
> > 
> > Add a comment to clarify middle loop's purpose.
> > 
> > Signed-off-by: mulhern <amulhern at redhat.com>
> > ---
> >  blivet/formats/fs.py | 18 ++++++++++--------
> >  1 file changed, 10 insertions(+), 8 deletions(-)
> > 
> > diff --git a/blivet/formats/fs.py b/blivet/formats/fs.py
> > index 0c610cb..27a2bc4 100644
> > --- a/blivet/formats/fs.py
> > +++ b/blivet/formats/fs.py
> > @@ -797,14 +797,16 @@ class FS(DeviceFormat):
> >      def mountable(self):
> >          canmount = (self.mountType in kernel_filesystems) or \
> >                     (os.access("/sbin/mount.%s" % (self.mountType,),
> >                     os.X_OK))
> > -        modpath = os.path.realpath(os.path.join("/lib/modules",
> > os.uname()[2]))
> > -        modname = "%s.ko" % self.mountType
> > -
> > -        if not canmount and os.path.isdir(modpath):
> > -            for _root, _dirs, files in os.walk(modpath):
> > -                have = [x for x in files if x.startswith(modname)]
> > -                if len(have) == 1 and have[0].startswith(modname):
> > -                    return True
> > +
> > +        # Still consider the filesystem type mountable if there exists
> > +        # an appropriate filesystem driver in the kernel modules
> > directory.
> > +        if not canmount:
> > +            modpath = os.path.realpath(os.path.join("/lib/modules",
> > os.uname()[2]))
> > +            if os.path.isdir(modpath):
> > +                modname = "%s.ko" % self.mountType
> > +                for _root, _dirs, files in os.walk(modpath):
> > +                    if len([x for x in files if x.startswith(modname)]) ==
> > 1:
> > +                        return True
> I know you're not changing the behaviour in this patch, but I don't
> understand the len() == 1 check. Does having 2 or more modules starting
> with the FS name mean it's not mountable? I don't think so.
> 

Agreed.

Unless somebody objects, I'll change the above line to:

if any(x.startswith(modname) for x in files):
    return True

> --
> Vratislav Podzimek
> 
> Anaconda Rider | Red Hat, Inc. | Brno - Czech Republic
> 
> _______________________________________________
> anaconda-patches mailing list
> anaconda-patches at lists.fedorahosted.org
> https://lists.fedorahosted.org/mailman/listinfo/anaconda-patches
> 


More information about the anaconda-patches mailing list