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

[Hans de Goede <hdegoede redhat com>]

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!

To me such a multiplexer function feels wrong, since the caller 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

?


> +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