Re: [PATCH] Fix the filesystem migration dialog in text mode (#688314).

[David Lehman <dlehman redhat com>]

Looks good except for two things, below.

On Fri, 2011-03-18 at 17:10 -0400, Chris Lumens wrote:
> ---
>  pyanaconda/textw/upgrade_text.py |   43 +++++++++++++++++++++++---------------
>  1 files changed, 26 insertions(+), 17 deletions(-)
> 
> diff --git a/pyanaconda/textw/upgrade_text.py b/pyanaconda/textw/upgrade_text.py
> index ed00ad9..7dca48f 100644
> --- a/pyanaconda/textw/upgrade_text.py
> +++ b/pyanaconda/textw/upgrade_text.py
> @@ -36,7 +36,6 @@ seenExamineScreen = False
>  
>  class UpgradeMigrateFSWindow:
>      def __call__ (self, screen, anaconda):
> -      
>          migent = anaconda.storage.migratableDevices
>  
>  	g = GridFormHelp(screen, _("Migrate File Systems"), "upmigfs", 1, 4)
> @@ -64,15 +63,15 @@ class UpgradeMigrateFSWindow:
>                                                device.format.type,
>                                                device.format.mountpoint),
>                                                device, migrating)
> -            
> +
>  	g.add(partlist, 0, 1, padding = (0, 0, 0, 1))
> -        
> +
>  	buttons = ButtonBar(screen, [TEXT_OK_BUTTON, TEXT_BACK_BUTTON] )
>  	g.add(buttons, 0, 3, anchorLeft = 1, growx = 1)
>  
> -	while 1:
> +	while True:
>  	    result = g.run()
> -        
> +
>  	    if (buttons.buttonPressed(result)):
>  		result = buttons.buttonPressed(result)
>  
> @@ -80,20 +79,30 @@ class UpgradeMigrateFSWindow:
>  		screen.popWindow()
>  		return INSTALL_BACK
>  
> -            # reset
> -            # XXX the way to do this is by scheduling and cancelling actions
> -            #for entry in migent:
> -            #    entry.setFormat(0)
> -            #    entry.setMigrate(0)
> -            #    entry.fsystem = entry.origfsystem
> +            # Cancel any previously scheduled migrate actions first.
> +            for entry in partlist:
> +                action = anaconda.storage.devicetree.findActions(device=entry[1],
> +                                                                 type="migrate")

findActions will return a list of actions, possibly empty.

> +                if not action:
> +                    continue
> +
> +                anaconda.storage.devicetree.cancelAction(action)
>  
> +            # Then schedule an action for whatever rows were selected.
>              for entry in partlist.getSelection():
> -                try:
> -                    newfs = getFormat(entry.format.migratetofs[0])
> -                except Exception, e:
> -                    log.info("failed to get new filesystem type, defaulting to ext3: %s" %(e,))
> -                    newfs = getFormat("ext3")
> -                    anaconda.storage.migrateFormat(entry, newfs)
> +                action = anaconda.storage.devicetree.findActions(device=entry[1],
> +                                                                 type="migrate")
> +                if action:
> +                    # the migrate action has already been scheduled
> +                    continue

Surely the above won't ever happen if you've just canceled all
previously scheduled migrate actions.

> +
> +                newfs = getFormat(entry[1].format.migrationTarget)
> +                if not newfs:
> +                    log.warning("failed to get new filesystem type (%s)"
> +                                % entry[1].format.migrationTarget)
> +                    continue
> +                action = ActionMigrateFormat(entry[1])
> +                anaconda.storage.devicetree.registerAction(action)
>  
>              screen.popWindow()
>              return INSTALL_OK