[libvirt] [sandbox PATCH v2 05/19] Image: Refactor create function

[Daniel P. Berrange]

On Tue, Aug 04, 2015 at 08:11:11PM +0000, Eren Yagdiran wrote:
> Move the docker-related code to the DockerSource and use
> the Source mechanism
> ---
>  virt-sandbox-image/sources/DockerSource.py | 100 +++++++++++++++++++++++++++++
>  virt-sandbox-image/sources/Source.py       |   4 ++
>  virt-sandbox-image/virt-sandbox-image.py   |  70 ++++----------------
>  3 files changed, 118 insertions(+), 56 deletions(-)
> 

> diff --git a/virt-sandbox-image/virt-sandbox-image.py b/virt-sandbox-image/virt-sandbox-image.py
> index 2573c29..6f65445 100755
> --- a/virt-sandbox-image/virt-sandbox-image.py
> +++ b/virt-sandbox-image/virt-sandbox-image.py
> @@ -116,59 +116,6 @@ def delete_template(name, destdir):
>                  parent = None
>          imagetagid = parent
>  
> -
> -def get_image_list(name, destdir):
> -    imageparent = {}
> -    imagenames = {}
> -    imagedirs = os.listdir(destdir)
> -    for imagetagid in imagedirs:
> -        indexfile = destdir + "/" + imagetagid + "/index.json"
> -        if os.path.exists(indexfile):
> -            with open(indexfile, "r") as f:
> -                index = json.load(f)
> -            imagenames[index["name"]] = imagetagid
> -        jsonfile = destdir + "/" + imagetagid + "/template.json"
> -        if os.path.exists(jsonfile):
> -            with open(jsonfile, "r") as f:
> -                template = json.load(f)
> -
> -            parent = template.get("parent", None)
> -            if parent:
> -                imageparent[imagetagid] = parent
> -
> -    if not name in imagenames:
> -        raise ValueError(["Image %s does not exist locally" % name])
> -
> -    imagetagid = imagenames[name]
> -    imagelist = []
> -    while imagetagid != None:
> -        imagelist.append(imagetagid)
> -        parent = imageparent.get(imagetagid, None)
> -        imagetagid = parent
> -
> -    return imagelist
> -
> -def create_template(name, imagepath, format, destdir):
> -    if not format in ["qcow2"]:
> -        raise ValueError(["Unsupported image format %s" % format])
> -
> -    imagelist = get_image_list(name, destdir)
> -    imagelist.reverse()
> -
> -    parentImage = None
> -    for imagetagid in imagelist:
> -        templateImage = destdir + "/" + imagetagid + "/template." + format
> -        cmd = ["qemu-img", "create", "-f", "qcow2"]
> -        if parentImage is not None:
> -            cmd.append("-o")
> -            cmd.append("backing_fmt=qcow2,backing_file=%s" % parentImage)
> -        cmd.append(templateImage)
> -        if parentImage is None:
> -            cmd.append("10G")
> -        debug("Run %s\n" % " ".join(cmd))
> -        subprocess.call(cmd)
> -        parentImage = templateImage
> -
>  def download(args):
>      try:
>          dynamic_source_loader(args.source).download_template(name=args.name,
> @@ -186,8 +133,13 @@ def delete(args):
>      delete_template(args.name, default_template_dir)
>  
>  def create(args):
> -    info("Creating %s from %s in format %s\n" % (args.imagepath, args.name, args.format))
> -    create_template(args.name, args.imagepath, args.format, default_template_dir)
> +    try:
> +        dynamic_source_loader(args.source).create_template(name=args.name,
> +                                                           connect=args.connect,
> +                                                           imagepath=args.imagepath,
> +                                                           format=args.format)
> +    except Exception,e:
> +        print "Create Error %s" % str(e)
>  
>  def requires_name(parser):
>      parser.add_argument("name",
> @@ -198,6 +150,10 @@ def requires_source(parser):
>                          default="docker",
>                          help=_("name of the template"))
>  
> +def requires_connect(parser):
> +    parser.add_argument("-c","--connect",
> +                        help=_("Connect string for libvirt"))
> +
>  def requires_auth_conn(parser):
>      parser.add_argument("-r","--registry",
>                          help=_("Url of the custom registry"))
> @@ -226,10 +182,12 @@ def gen_create_args(subparser):
>      parser = subparser.add_parser("create",
>                                     help=_("Create image from template data"))
>      requires_name(parser)
> +    requires_source(parser)
> +    requires_connect(parser)
>      parser.add_argument("imagepath",
>                          help=_("path for image"))
>      parser.add_argument("format",
> -                        help=_("format"))
> +                        help=_("format format for image"))

For the previous 'download' command we called the directory 'templatedir'
but for 'create' we are calling it 'imagepath'.  We should be consistent
about what we do and so call it 'templatedir' for all commands, and
also make sure it uses '--templatedir' and not a positional argument.
The same applies to the 'run' command.

Also we should really use '--format=qcow2' too, so people don't have
to pass it by default

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|