[virt-tools-list] [virt-bootstrap] [PATCH v2 04/14] Use python-guestfs
Cedric Bosdonnat
cbosdonnat at suse.com
Tue Aug 1 14:35:32 UTC 2017
On Tue, 2017-08-01 at 12:28 +0100, Radostin Stoyanov wrote:
> ---
> setup.py | 9 +-
> src/virtBootstrap/sources/docker_source.py | 4 +-
> src/virtBootstrap/sources/file_source.py | 7 +-
> src/virtBootstrap/utils.py | 200 +++++++++++++++++------------
> 4 files changed, 132 insertions(+), 88 deletions(-)
>
> diff --git a/setup.py b/setup.py
> index 54e24a2..f6aa9c4 100755
> --- a/setup.py
> +++ b/setup.py
> @@ -115,8 +115,13 @@ setuptools.setup(
>
> # virt-bootstrap uses passlib to compute the hash of
> # root password for root file system.
> - install_requires=['passlib>=1.6.1'],
> -
> + install_requires=[
> + 'passlib>=1.6.1',
> + 'guestfs'
> + ],
> + dependency_links=[
> + 'http://download.libguestfs.org/python#guestfs'
> + ],
> tests_require=['mock>=2.0'],
>
> extras_require={
> diff --git a/src/virtBootstrap/sources/docker_source.py b/src/virtBootstrap/sources/docker_source.py
> index 1b5bc31..9220e13 100644
> --- a/src/virtBootstrap/sources/docker_source.py
> +++ b/src/virtBootstrap/sources/docker_source.py
> @@ -65,6 +65,7 @@ class DockerSource(object):
> self.images_dir = utils.get_image_dir(self.no_cache)
> self.manifest = None
> self.layers = []
> + self.tar_files = []
>
> if self.username and not self.password:
> self.password = getpass.getpass()
> @@ -96,6 +97,7 @@ class DockerSource(object):
>
> sum_type, layer_sum = layer_digest.split(':')
> file_path = os.path.join(self.images_dir, layer_sum + '.tar')
> + self.tar_files.append(file_path)
> self.layers.append([sum_type, layer_sum, file_path, layer_size])
>
> def gen_valid_uri(self, uri):
> @@ -260,7 +262,7 @@ class DockerSource(object):
> elif self.output_format == 'qcow2':
> self.progress("Extracting container layers into qcow2 images",
> value=50, logger=logger)
> - utils.extract_layers_in_qcow2(self.layers, dest, self.progress)
> + utils.Build_QCOW2_Image(self.tar_files, dest, self.progress)
Looks like the commit is not only about using libguestfs python's binding.
In such a case some more description of the commit would be good. Otherwise
split the commit into smaller ones:
* for the refactoring
* for the guestfs python binding use.
> else:
> raise Exception("Unknown format:" + self.output_format)
>
> diff --git a/src/virtBootstrap/sources/file_source.py b/src/virtBootstrap/sources/file_source.py
> index f8cf6c4..87589f6 100644
> --- a/src/virtBootstrap/sources/file_source.py
> +++ b/src/virtBootstrap/sources/file_source.py
> @@ -62,14 +62,9 @@ class FileSource(object):
> utils.safe_untar(self.path, dest)
>
> elif self.output_format == 'qcow2':
> - # Remove the old path
> - file_name = os.path.basename(self.path)
> - qcow2_file = os.path.realpath('{}/{}.qcow2'.format(dest,
> - file_name))
> -
> self.progress("Extracting files into qcow2 image", value=0,
> logger=logger)
> - utils.create_qcow2(self.path, qcow2_file)
> + utils.Build_QCOW2_Image([self.path], dest, self.progress)
> else:
> raise Exception("Unknown format:" + self.output_format)
>
> diff --git a/src/virtBootstrap/utils.py b/src/virtBootstrap/utils.py
> index 2c79d6b..6369127 100644
> --- a/src/virtBootstrap/utils.py
> +++ b/src/virtBootstrap/utils.py
> @@ -33,13 +33,16 @@ import logging
> import re
>
> from subprocess import CalledProcessError, PIPE, Popen
> +import guestfs
> import passlib.hosts
>
> # pylint: disable=invalid-name
> # Create logger
> logger = logging.getLogger(__name__)
> +
> # Default virtual size of qcow2 image
> -DEF_QCOW2_SIZE = '5G'
> +DEF_QCOW2_SIZE = 5 * 1024 * 1024 * 1024
> +
> if os.geteuid() == 0:
> LIBVIRT_CONN = "lxc:///"
> DEFAULT_IMG_DIR = "/var/lib/virt-bootstrap/docker_images"
> @@ -49,6 +52,117 @@ else:
> DEFAULT_IMG_DIR += "/.local/share/virt-bootstrap/docker_images"
>
>
> +class Build_QCOW2_Image(object):
> + """
> + Create qcow2 image with backing chains from list of tar files.
> + """
> + def __init__(self, tar_files, dest, progress):
> + """
> + Initialize guestfs
> + """
> + if not isinstance(tar_files, list):
> + raise ValueError('tar_files must be list not %s' % type(tar_files))
> + self.tar_files = tar_files
> + self.nlayers = len(tar_files)
> + self.progress = progress
> + self.fmt = 'qcow2'
> + self.qcow2_files = [os.path.join(dest, 'layer-%s.qcow2' % i)
> + for i in range(self.nlayers)]
> +
> + self.g = guestfs.GuestFS(python_return_dict=True)
> + self.create_base_qcow2_layer(self.tar_files[0], self.qcow2_files[0])
> + if len(self.tar_files) > 1:
> + self.create_backing_chains()
> + self.g.shutdown()
> +
> + def create_and_add_disk(self, qcow2_file, backingfile=None,
> + readonly=False):
Couldn't that function name be simplified, like create_disk or add_disk?
> + """
> + Create and add qcow2 disk image.
> + """
> + if backingfile is not None:
> + size = -1
> + backingformat = self.fmt
> + else:
> + size = DEF_QCOW2_SIZE
> + backingformat = None
> +
> + self.g.disk_create(qcow2_file, self.fmt, size, backingfile,
> + backingformat)
> + self.g.add_drive_opts(qcow2_file, readonly, self.fmt)
> +
> + def tar_in(self, tar_file, dev):
> + """
> + Extract tar file in disk device.
> + """
> + self.g.mount(dev, '/')
> + # Restore extended attributes, SELinux contexts and POSIX ACLs
> + # from tar file.
> + self.g.tar_in(tar_file, '/', get_compression_type(tar_file),
> + xattrs=True, selinux=True, acls=True)
> + # Shutdown guestfs instance to avoid hot-plugging of devices.
> + self.g.umount('/')
> +
> + def create_base_qcow2_layer(self, tar_file, qcow2_file):
> + """
> + Create and format base qcow2 layer.
> +
> + Do this separatelly when extracting multiple layers to avoid
> + hot-plugging of devices.
> + """
> + self.progress("Creating base layer", logger=logger)
> + self.create_and_add_disk(qcow2_file)
> + self.g.launch()
> + dev = self.g.list_devices()[0]
> + self.progress("Formating disk image", logger=logger)
> + self.g.mkfs("ext3", dev)
> + self.tar_in(tar_file, dev)
> + self.progress("Extracting content of base layer", logger=logger)
> + self.g.shutdown()
> +
> + def create_backing_chains(self):
> + """
> + Create backing chains for all layers after following the first
> + and tar-in the content.
> + """
> + for i in range(1, self.nlayers):
> + self.progress("Creating layer %d" % i, logger=logger)
> + self.create_and_add_disk(self.qcow2_files[i],
> + backingfile=self.qcow2_files[i - 1])
> +
> + self.g.launch()
> + devices = self.g.list_devices()
> + # Iterate trough tar files of layers and skip the base layer
> + for i, tar_file in enumerate(self.tar_files[1:]):
> + self.progress("Extracting content of layer %d" % (i + 1),
> + logger=logger)
> + self.tar_in(tar_file, devices[i])
> +
> +
> +def get_compression_type(tar_file):
> + """
> + Get compression type of tar file.
> + """
> + # Get mime type of archive
> + mime_tar_file = get_mime_type(tar_file)
> + logger.debug("Detected mime type of archive: %s", mime_tar_file)
> +
> + compression_fmts = {
> + 'x-gzip': 'gzip',
> + 'gzip': 'gzip',
> + 'x-xz': 'xz',
> + 'x-bzip2': 'bzip2',
> + 'x-compress': 'compress',
> + 'x-lzop': 'lzop'
> + }
> +
> + # Check if tarball is compressed
> + mime_type, mime_subtype = mime_tar_file.split('/')
> + if mime_type == 'application' and mime_subtype in compression_fmts:
> + return compression_fmts[mime_subtype]
> + return None
> +
> +
Extracting code into a function like this one should have its own commit.
> def checksum(path, sum_type, sum_expected):
> """
> Validate file using checksum.
> @@ -148,22 +262,6 @@ def log_layer_extract(layer, current, total, progress):
> logger.debug('Untar layer: (%s:%s) %s', sum_type, sum_value, layer_file)
>
>
> -def untar_layers(layers_list, dest_dir, progress):
> - """
> - Untar each of layers from container image.
> - """
> - nlayers = len(layers_list)
> - for index, layer in enumerate(layers_list):
> - log_layer_extract(layer, index + 1, nlayers, progress)
> - layer_file = layer[2]
> -
> - # Extract layer tarball into destination directory
> - safe_untar(layer_file, dest_dir)
> -
> - # Update progress value
> - progress(value=(float(index + 1) / nlayers * 50) + 50)
> -
> -
> def get_mime_type(path):
> """
> Get the mime type of a file.
> @@ -172,73 +270,17 @@ def get_mime_type(path):
> .stdout.read().decode('utf-8').split()[1])
>
>
> -def create_qcow2(tar_file, layer_file, backing_file=None, size=DEF_QCOW2_SIZE):
> - """
> - Create qcow2 image from tarball.
> - """
> - qemu_img_cmd = ["qemu-img", "create", "-f", "qcow2", layer_file, size]
> -
> - if not backing_file:
> - logger.info("Creating base qcow2 image")
> - execute(qemu_img_cmd)
> -
> - logger.info("Formatting qcow2 image")
> - execute(['virt-format',
> - '--format=qcow2',
> - '--partition=none',
> - '--filesystem=ext3',
> - '-a', layer_file])
> - else:
> - # Add backing chain
> - qemu_img_cmd.insert(2, "-b")
> - qemu_img_cmd.insert(3, backing_file)
> -
> - logger.info("Creating qcow2 image with backing chain")
> - execute(qemu_img_cmd)
> -
> - # Get mime type of archive
> - mime_tar_file = get_mime_type(tar_file)
> - logger.debug("Detected mime type of archive: %s", mime_tar_file)
> -
> - # Extract tarball using "tar-in" command from libguestfs
> - tar_in_cmd = ["guestfish",
> - "-a", layer_file,
> - '-m', '/dev/sda',
> - 'tar-in', tar_file, "/"]
> -
> - compression_fmts = {'x-gzip': 'gzip', 'gzip': 'gzip',
> - 'x-xz': 'xz',
> - 'x-bzip2': 'bzip2',
> - 'x-compress': 'compress',
> - 'x-lzop': 'lzop'}
> -
> - # Check if tarball is compressed
> - mime_parts = mime_tar_file.split('/')
> - if mime_parts[0] == 'application' and \
> - mime_parts[1] in compression_fmts:
> - tar_in_cmd.append('compress:' + compression_fmts[mime_parts[1]])
> -
> - # Execute virt-tar-in command
> - execute(tar_in_cmd)
> -
> -
> -def extract_layers_in_qcow2(layers_list, dest_dir, progress):
> +def untar_layers(layers_list, dest_dir, progress):
> """
> - Extract docker layers in qcow2 images with backing chains.
> + Untar each of layers from container image.
> """
> - qcow2_backing_file = None
> -
> nlayers = len(layers_list)
> for index, layer in enumerate(layers_list):
> log_layer_extract(layer, index + 1, nlayers, progress)
> - tar_file = layer[2]
> -
> - # Name format for the qcow2 image
> - qcow2_layer_file = "{}/layer-{}.qcow2".format(dest_dir, index)
> - # Create the image layer
> - create_qcow2(tar_file, qcow2_layer_file, qcow2_backing_file)
> - # Keep the file path for the next layer
> - qcow2_backing_file = qcow2_layer_file
> + layer_file = layer[2]
> +
> + # Extract layer tarball into destination directory
> + safe_untar(layer_file, dest_dir)
>
> # Update progress value
> progress(value=(float(index + 1) / nlayers * 50) + 50)
It's a rather complex commit, but you can't avoid the big refactoring involved
in using libguestfs python binding instead of the tools. Better split the giant
commit into smaller chunks for them to be easier to read and review.
--
Cedric
More information about the virt-tools-list
mailing list