[PATCH 2/6] driver-updates: refactor dd_scan

Vratislav Podzimek vpodzime at redhat.com
Wed Mar 5 10:31:29 UTC 2014


On Tue, 2014-03-04 at 10:53 -0500, Will Woods wrote:
> Three refactoring changes:
> 
> 1) Make default value for skip_dds=set()
> The docstring says the value will be a set(), so make it a set().
> 
> 2) Set default state for dd_todo/dd_finished outside the if/else
> This is just for clarity, mostly.
> 
> 2) split the dd autoload loop into its own function, dd_load()
> This is useful because we might want to do the dd_load() loop in more
> than one place. Like, say, after each DVD, if we were loading drivers
> off sequential DVDs.
> 
> Note that we're filtering the skip_dds out of dd_todo twice:
> 
>   * dd_scan() removes dd_finished (an alias for skip_dds) from dd_todo
>   * dd_load() skips anything in skip_dds
> 
> This is redundant, and one of them can probably be removed.
> ---
>  dracut/driver-updates | 28 ++++++++++++++++++++++------
>  1 file changed, 22 insertions(+), 6 deletions(-)
> 
> diff --git a/dracut/driver-updates b/dracut/driver-updates
> index d7226f0..a95c4d3 100755
> --- a/dracut/driver-updates
> +++ b/dracut/driver-updates
> @@ -700,7 +700,7 @@ def select_iso():
>  
>      return (os.path.join(mnt, dd_iso), "/media/DD-search")
>  
> -def dd_scan(skip_dds=None, scan_dd_args=True):
> +def dd_scan(skip_dds=set(), scan_dd_args=True):
This would create a single set that will be used every time this
functioni is called without the skip_dds argument. Is that what you want
it to do? Or is this one of the "common Python bugs" and should be
replaced with None which will be replaced with set() at the very
beginning of the function? I see no point in having this single
permanent set used over and over again.

>      """ Scan the system for OEMDRV devices and and specified by dd=/dev/<device>
>  
>          :param skip_dds:     devices to skip when checking for OEMDRV label
> @@ -709,14 +709,13 @@ def dd_scan(skip_dds=None, scan_dd_args=True):
>          :type scan_dd_args:  bool
>          :returns:            None
>      """
> +    dd_todo = set(oemdrv_list())
> +    dd_finished = set()
>      if skip_dds:
>          dd_finished = skip_dds
>          dd_todo = set(oemdrv_list()).difference(dd_finished)
>          if dd_todo:
>              log.info("Found new OEMDRV device(s) - %s", ", ".join(dd_todo))
> -    else:
> -        dd_finished = set()
> -        dd_todo = set(oemdrv_list())
>  
>      if scan_dd_args:
>          # Add the user specified devices
> @@ -725,6 +724,7 @@ def dd_scan(skip_dds=None, scan_dd_args=True):
>          dd_todo.update(dd_devs)
>          log.info("Checking devices %s", ", ".join(dd_todo))
>  
> +    # Handle interactive driver selection, if needed
>      mount_point = None
>      if not dd_todo and is_interactive():
>          iso, mount_point = select_iso()
> @@ -732,8 +732,25 @@ def dd_scan(skip_dds=None, scan_dd_args=True):
>              dd_todo.add(iso)
>  
>      # Process each Driver Disk, checking for new disks after each one
> +    dd_load(dd_todo, skip_dds=skip_dds)
> +
> +    umount(mount_point)
> +
> +def dd_load(dd_todo, skip_dds=set()):
Same question/issue here.

-- 
Vratislav Podzimek

Anaconda Rider | Red Hat, Inc. | Brno - Czech Republic



More information about the anaconda-patches mailing list