[libvirt] [sandbox PATCH v2 03/19] Image: Add Hooking Mechanism

[Daniel P. Berrange]

On Tue, Aug 04, 2015 at 08:11:09PM +0000, Eren Yagdiran wrote:
> Any custom source provider can be added to virt-sandbox-image as a source
> ---
>  .gitignore                               |  1 +
>  bin/Makefile.am                          | 16 ++++++++++++----
>  bin/virt-sandbox-image.in                |  3 +++
>  configure.ac                             |  2 ++
>  virt-sandbox-image/Makefile.am           | 13 +++++++++++++
>  virt-sandbox-image/sources/Source.py     | 31 +++++++++++++++++++++++++++++++
>  virt-sandbox-image/sources/__init__.py   | 29 +++++++++++++++++++++++++++++
>  virt-sandbox-image/virt-sandbox-image.py | 13 ++++++++++++-
>  8 files changed, 103 insertions(+), 5 deletions(-)
>  create mode 100644 bin/virt-sandbox-image.in
>  create mode 100644 virt-sandbox-image/Makefile.am
>  create mode 100644 virt-sandbox-image/sources/Source.py
>  create mode 100644 virt-sandbox-image/sources/__init__.py
>  mode change 100644 => 100755 virt-sandbox-image/virt-sandbox-image.py

> diff --git a/virt-sandbox-image/sources/Source.py b/virt-sandbox-image/sources/Source.py
> new file mode 100644
> index 0000000..43f0720
> --- /dev/null
> +++ b/virt-sandbox-image/sources/Source.py
> @@ -0,0 +1,31 @@
> +'''
> +*
> +* 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 at gmail.com>
> +*
> +'''

Python comment syntax is '# '

What you have done here is declare a string.

Also since the copyright line has accents in it, you need
to add

 #coding=utf-8

as the 2nd line in the file, otherwise python refuses to load
the code at all saying it doens't know the character encoding.

> +#!/usr/bin/python

THis should be the first line

> +
> +from abc import ABCMeta, abstractmethod
> +
> +class Source():
> +    __metaclass__ = ABCMeta
> +    def __init__(self):
> +        pass
> diff --git a/virt-sandbox-image/sources/__init__.py b/virt-sandbox-image/sources/__init__.py
> new file mode 100644
> index 0000000..4830c7a
> --- /dev/null
> +++ b/virt-sandbox-image/sources/__init__.py
> @@ -0,0 +1,29 @@
> +'''
> +*
> +* 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 at gmail.com>
> +*
> +'''
> +#!/usr/bin/python

Same here about comment syntax, encoding and #!/usr being the
first line.

> +
> +import os
> +import glob
> +modules = glob.glob(os.path.dirname(__file__)+"/*.py")
> +__all__ = [ os.path.basename(f)[:-3] for f in modules]

I don't think any of this stuff is needed, since we are
directly importing the module we want based on its name


> diff --git a/virt-sandbox-image/virt-sandbox-image.py b/virt-sandbox-image/virt-sandbox-image.py
> old mode 100644
> new mode 100755
> index a9cb0ff..b20b240
> --- a/virt-sandbox-image/virt-sandbox-image.py
> +++ b/virt-sandbox-image/virt-sandbox-image.py
> @@ -1,5 +1,5 @@
>  #!/usr/bin/python -Es
> -#
> +# -*- coding: utf-8 -*-
>  # Authors: Daniel P. Berrange <berrange at redhat.com>
>  #          Eren Yagdiran <erenyagdiran at gmail.com>
>  #
> @@ -38,6 +38,17 @@ default_template_dir = "/var/lib/libvirt/templates"
>  debug = True
>  verbose = True
>  
> +sys.dont_write_bytecode = True

This kind of change doesn't belong in the code.

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 :|