Re: [sandbox PATCH v2 04/19] Image: Add download function

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, Aug 10, 2015 at 05:30:52PM +0100, Daniel P. Berrange wrote:
> 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

We should use $HOME/.local/share/libvirt/templates for unprivileged users


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




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]