[virt-tools-list] [virt-manager] [PATCH v3 1/4] create: Call virt-bootstrap asynchronously

[Cole Robinson]

On 07/10/2017 08:07 PM, Radostin Stoyanov wrote:
> The bootstrap method is called at the end of the "create dialog" (when
> "Finish" button is clicked).
> 
> We handle two cases:
> 
> 1. When virt-bootstrap fails. User re-clicks the 'Finish' button and we
> reattempt the container bootstrap.
> 
> 2. When virt-bootstrap succeeds, but something later in the install
> fails, like XML define. If user re-clicks 'Finish' we don't attempt
> virt-bootstrap again, just use the directory path.
> This is achieved by unchecking the 'install-oscontainer-bootstrap'
> checkbox is unchecked if virt-bootstrap succeeds.
> 
> Log messages from the virtBootstrap's logger are stored in string
> buffer and shown in case of failure.
> ---
>  virtManager/create.py | 46 ++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 46 insertions(+)
> 
> diff --git a/virtManager/create.py b/virtManager/create.py
> index 662c200..75fd553 100644
> --- a/virtManager/create.py
> +++ b/virtManager/create.py
> @@ -21,6 +21,7 @@
>  import logging
>  import pkgutil
>  import os
> +import cStringIO
>  import threading
>  import time
>  
> @@ -2494,6 +2495,19 @@ class vmmCreate(vmmGObjectUI):
>          """
>          meter = asyncjob.get_meter()
>  
> +        container_bootstrap = self._get_config_oscontainer_bootstrap()
> +        # If creating new container and "container bootstrap" is enabled
> +        if self._guest.os.is_container() and container_bootstrap:
> +            # Start container bootstrap
> +            src_url = self._get_config_oscontainer_source_url()
> +            dest = self.widget("install-oscontainer-fs").get_text()
> +            user = self._get_config_oscontainer_source_username()
> +            passwd = self._get_config_oscontainer_source_password()
> +            insecure = self._get_config_oscontainer_isecure()
> +
> +            self._create_directory_tree(asyncjob, src_url, dest, user, passwd,
> +                                        insecure)
> +

Looks better, but some of this is problematic. Functions run via async jobs
are run in a thread, and if they access the UI you can get random gtk aborts
since it may collide with the main thread. So basically any UI interaction
needs to be deferred outside of the async job function.

I suggest moving all those get_config calls to _start_install. If we are
creating a container store all the values in a tuple called bootstrap_args,
otherwise use None or an empty tuple, and pass it to _do_async_install. If the
tuple was passed, async_install will invoke
self._create_directory_tree(asyncjob, *bootstrap_args)

>          # Build a list of pools we should refresh, if we are creating storage
>          refresh_pools = []
>          for disk in guest.get_devices("disk"):
> @@ -2578,3 +2592,35 @@ class vmmCreate(vmmGObjectUI):
>              self.err.show_err(_("Error continue install: %s") % str(e))
>  
>          return True
> +
> +
> +    def _create_directory_tree(self, asyncjob, src, dest, user, passwd, insecure):
> +        """
> +        Call bootstrap method from virtBootstrap.
> +        """
> +        import virtBootstrap
> +
> +        # Use string buffer to store log messages
> +        log_stream = cStringIO.StringIO()
> +
> +        # Get virt-bootstrap logger
> +        vbLogger = logging.getLogger('virtBootstrap')
> +        vbLogger.setLevel(logging.DEBUG)
> +        vbLogger.addHandler(logging.StreamHandler(log_stream))
> +
> +        # Key word arguments to be passed
> +        kwargs = {'uri': src, 'dest': dest, 'not_secure': insecure}
> +        if user and passwd:
> +            kwargs['username'] = user
> +            kwargs['password'] = passwd
> +
> +        logging.debug('Start container bootstrap')
> +        try:
> +            virtBootstrap.bootstrap(**kwargs)
> +            # Success - uncheck the 'install-oscontainer-bootstrap' checkbox
> +            self.widget("install-oscontainer-bootstrap").set_active(False)

This is another place where we are touching the UI from the async thread.
Instead do this to push it to the main thread

def cb():
    self.widget("install-oscontainer-bootstrap").set_active(False)
self.idle_add(cb)

> +        except Exception as err:
> +            asyncjob.set_error(err, log_stream.getvalue())
> +        except:
> +            asyncjob.set_error("Container bootstrap has failed",
> +                               log_stream.getvalue())
> 

Is this double except: block actually catching anything? I think just the
first two lines are sufficient

Thanks,
Cole