Re: [PATCH] Create a getDeps and getTarget function for the block module.

[Joel Granados <jgranado redhat com>]

----- "Hans de Goede" <hdegoede redhat com> wrote:

> Caveat emperor: I'm not familiar with pyblock at all.
> 
> Joel Granados Moreno wrote:
> > ---
> >  __init__.py |   24 +++++++++++++++++++++---
> >  1 files changed, 21 insertions(+), 3 deletions(-)
> > 
> > diff --git a/__init__.py b/__init__.py
> > index 943e127..bb60be6 100644
> > --- a/__init__.py
> > +++ b/__init__.py
> > @@ -190,6 +189,25 @@ def getRaidSets(*disks):
> >              rsList.append(set)
> >      return rsList
> >  
> > +def getDeps(uuid = None, major = None, minor = None, name = None):
> > +    # If has dpes, return a set of maps, else return an empty set.
> > +    for map in dm.maps():
> > +        if map.name == name or \
> > +                map.uuid == uuid or \
> > +                (map.dev.minor == minor and map.dev.major ==
> major):
> > +            return map.deps
> > +    return ()
> > +
> 
> I assume from the way this function works, that it gets called with
> either a 
> name, or a uuid, or a minor major pair. Can either of these never be
> None in 
> the map itself? Otherwise the not passed in argument could match!

good catch,  I'll modify the patch :)

> 
> To me such a multiplexer function feels wrong, since the caller

Yep it is kinda strangeish.  I guess I can reduce the arguments to a *args **kwargs pair.

> already 
> indicates what he wants to use to identify the map, why not have 3
> methods, 
> which *clearly* indicate what they do:
> getDepsByName
> getDepsByUUID
> getDepsByDevno

I think this is discussable.  Its choosing between calling a getDepsByName(NAME) and getDeps(name=NAME).  IMO the two ways clearly indicate what is being done.  Additionally getDepsBy* will use getDeps so I think its overdoing it a little.  I might want to change the method name to getMapDep or getDmDeps so as to specify that it referes to all device-mapper devices.

Will post the changes.

thx.

> 
> ?
> 
> 
> > +def getTarget(uuid = None, major = None, minor = None, name =
> None):
> > +    # Return None if we don't find the map.
> > +    for map in dm.maps():
> > +        if map.name == name or \
> > +                map.uuid == uuid or \
> > +                (map.dev.minor == minor and map.dev.major ==
> major):
> > +            return map.table.type
> > +    return None
> > +
> > +
> 
> Same comment
> 
> Otherwise I see nothing wrong with this.
> 
> Regards,
> 
> Hans
> 
> _______________________________________________
> Anaconda-devel-list mailing list
> Anaconda-devel-list redhat com
> https://www.redhat.com/mailman/listinfo/anaconda-devel-list

-- 
Joel Andres Granados
Red Hat / Brno Czech Republic