On Tue, Aug 04, 2015 at 08:11:10PM +0000, Eren Yagdiran wrote:
> Refactor download function from virt-sandbox-image to use
> the newly introduced Source abstract class. The docker-specific
> download code is moved to a new DockerSource class.
> ---
> virt-sandbox-image/Makefile.am | 1 +
> virt-sandbox-image/sources/DockerSource.py | 227 +++++++++++++++++++++++++++++
> virt-sandbox-image/sources/Source.py | 4 +
> virt-sandbox-image/virt-sandbox-image.py | 199 ++++---------------------
> 4 files changed, 259 insertions(+), 172 deletions(-)
> create mode 100644 virt-sandbox-image/sources/DockerSource.py
>
> diff --git a/virt-sandbox-image/Makefile.am b/virt-sandbox-image/Makefile.am
> index 5ab4d2e..8188c80 100644
> --- a/virt-sandbox-image/Makefile.am
> +++ b/virt-sandbox-image/Makefile.am
> @@ -8,6 +8,7 @@ install-data-local:
> $(INSTALL) -m 0755 $(srcdir)/virt-sandbox-image.py $(DESTDIR)$(pkgpythondir)
> $(INSTALL) -m 0644 $(srcdir)/sources/__init__.py $(DESTDIR)$(pkgpythondir)/sources
> $(INSTALL) -m 0644 $(srcdir)/sources/Source.py $(DESTDIR)$(pkgpythondir)/sources
> + $(INSTALL) -m 0644 $(srcdir)/sources/DockerSource.py $(DESTDIR)$(pkgpythondir)/sources
>
> uninstall-local:
> rm -f $(DESTDIR)$(pkgpythondir)
> diff --git a/virt-sandbox-image/sources/DockerSource.py b/virt-sandbox-image/sources/DockerSource.py
> new file mode 100644
> index 0000000..cf81ffe
> --- /dev/null
> +++ b/virt-sandbox-image/sources/DockerSource.py
> @@ -0,0 +1,227 @@
> +'''
> +*
> +* libvirt-sandbox-config-diskaccess.h: libvirt sandbox configuration
> +*
> +* Copyright (C) 2015 Universitat Politècnica de Catalunya.
> +*
> +* This library is free software; you can redistribute it and/or
> +* modify it under the terms of the GNU Lesser General Public
> +* License as published by the Free Software Foundation; either
> +* version 2.1 of the License, or (at your option) any later version.
> +*
> +* This library is distributed in the hope that it will be useful,
> +* but WITHOUT ANY WARRANTY; without even the implied warranty of
> +* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> +* Lesser General Public License for more details.
> +*
> +* You should have received a copy of the GNU Lesser General Public
> +* License along with this library; if not, write to the Free Software
> +* Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
> +*
> +* Author: Eren Yagdiran <erenyagdiran@xxxxxxxxx>
> +*
> +'''
> +#!/usr/bin/python
> +
> +from Source import Source
> +import urllib2
> +import sys
> +import json
> +import traceback
> +import os
> +import subprocess
> +import shutil
> +
> +class DockerSource(Source):
> + default_index_server = "index.docker.io"
> + default_template_dir = "/var/lib/libvirt/templates"
> + default_image_path = "/var/lib/libvirt/templates"
> + default_disk_format = "qcow2"
These are class level variables
> + def __init__(self,server="index.docker.io",destdir="/var/lib/libvirt/templates"):
> + self.default_index_server = server
> + self.default_template_dir = destdir
And these are object level variables with the same name. Generally
object level variables should have a leading _ to indicate that
they are private.
I'd suggest we remove the default directories from this bit
of code though.
> def download(args):
> - info("Downloading %s from %s to %s\n" % (args.name, default_index_server, default_template_dir))
> - download_template(args.name, default_index_server, default_template_dir)
> + try:
> + dynamic_source_loader(args.source).download_template(name=args.name,
> + registry=args.registry,
> + username=args.username,
> + password=args.password,
> + templatedir=args.template_dir)
> + except IOError,e:
> + print "Source %s cannot be found in given path" %args.source
> + except Exception,e:
> + print "Download Error %s" % str(e)
>
> def delete(args):
> info("Deleting %s from %s\n" % (args.name, default_template_dir))
> @@ -355,10 +193,27 @@ def requires_name(parser):
> parser.add_argument("name",
> help=_("name of the template"))
>
> +def requires_source(parser):
> + parser.add_argument("-s","--source",
> + default="docker",
> + help=_("name of the template"))
> +
> +def requires_auth_conn(parser):
> + parser.add_argument("-r","--registry",
> + help=_("Url of the custom registry"))
> + parser.add_argument("-u","--username",
> + help=_("Username for the custom registry"))
> + parser.add_argument("-p","--password",
> + help=_("Password for the custom registry"))
> + parser.add_argument("-t","--template-dir",
> + help=_("Template directory for saving templates"))
And register a default here instead.
Also, we will want to have a different default directory for people running
as non-root, as /var/lib/libvirt/templates won't be accessible for
people using qemu:///session
> +
> def gen_download_args(subparser):
> parser = subparser.add_parser("download",
> help=_("Download template data"))
> + requires_source(parser)
> requires_name(parser)
> + requires_auth_conn(parser)
> parser.set_defaults(func=download)
>
> def gen_delete_args(subparser):
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 :|
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list