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

[Cole Robinson]

On 06/22/2017 10:54 AM, Radostin Stoyanov wrote:
> ---
>  virtManager/create.py | 74 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 74 insertions(+)
> 
> diff --git a/virtManager/create.py b/virtManager/create.py
> index 658135c..4bedcfe 100644
> --- a/virtManager/create.py
> +++ b/virtManager/create.py
> @@ -22,6 +22,7 @@ import logging
>  import threading
>  import time
>  import subprocess
> +from os.path import exists, isdir

Not really a fan of this style of importing, I suggest just doing 'import os'
and spelling out the full function names at the call sites

>  try:
>      import commands  # Python2 only
>  except Exception:
> @@ -1963,6 +1964,26 @@ class vmmCreate(vmmGObjectUI):
>              if not fs:
>                  return self.err.val_err(_("An OS directory path is required."))
>  
> +            if self.widget("install-oscontainer-bootstrap").get_active():
> +
> +                src_url = self.widget("install-oscontainer-source-url-entry"
> +                ).get_text().strip()

Indent these second lines at least 4 chars. We aren't that consistent in the
code base but continued lines are typically indented in some way

> +                user = self.widget("install-oscontainer-source-user"
> +                ).get_text().strip()
> +                passwd = self.widget("install-oscontainer-source-passwd"
> +                ).get_text()
> +
> +                # Check if the source path was provided
> +                if not src_url:
> +                    return self.err.val_err(_("Source URL is required"))
> +
> +                # Show error if destination path exist and is not folder
> +                if exists(fs) and not isdir(fs):
> +                    return self.err.val_err(_("Path already exist and"
> +                                              " it is not directory: " + fs))

'Path already exists but is not a directory'

Hmm, we will need to think about how to handle things for paths like
/var/lib/libvirt/filesystems which a user running virt-manager might not be
able to access, so could give false results here. But that should come later

> +                # Start container bootstrap
> +                self._container_image_bootstrap(src_url, fs, user, passwd)
> +
>          elif instmethod == INSTALL_PAGE_VZ_TEMPLATE:
>              instclass = virtinst.ContainerInstaller
>              template = self.widget("install-container-template").get_text()
> @@ -2517,3 +2538,56 @@ 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):
> +        """
> +        Call virt-bootstrap and wait until exitCode is returned
> +        """
> +
> +        cmd = ["virt-bootstrap", src, dest]
> +        if user:
> +            cmd += ["--username", user]
> +        if passwd:
> +            cmd += ["--password", passwd]
> +
> +        logging.debug("Start asyncjob job: %s", " ".join(cmd))
> +
> +        bootstrap_proc = subprocess.Popen(cmd,
> +                                          stdout=subprocess.PIPE,
> +                                          stderr=subprocess.PIPE)
> +
> +        stdout, stderr = bootstrap_proc.communicate()
> +
> +        if bootstrap_proc.returncode != 0:
> +            asyncjob.set_error("virt-bootstrap did not complete successfully",
> +                               "{}\n{}".format(stdout, stderr))
> +

Add a ": " at the end of 'successfully'

Thanks,
Cole