Hi!

You are right about breaking compatibility. Are you OK with these fixes?
If yes I can squash the relevant diffs with the previous commits, rebase
the whole patch set with current master and resend it.

Let me know, thanks,
dimara



* Dimitris Aragiorgis <[email protected]> [2014-07-14 09:10:46 +0300]:

> ..for the ExtStorage providers. This way we do not break
> compatibility with existing providers that do not implement
> such a functionality.
> 
> Signed-off-by: Dimitris Aragiorgis <[email protected]>
> ---
>  doc/design-shared-storage.rst       |    6 +++++-
>  lib/masterd/instance.py             |    4 +++-
>  lib/storage/extstorage.py           |    7 ++++++-
>  man/ganeti-extstorage-interface.rst |    4 ++++
>  4 files changed, 18 insertions(+), 3 deletions(-)
> 
> diff --git a/doc/design-shared-storage.rst b/doc/design-shared-storage.rst
> index c1b3c01..793c522 100644
> --- a/doc/design-shared-storage.rst
> +++ b/doc/design-shared-storage.rst
> @@ -198,7 +198,7 @@ provider is expected to provide the following scripts:
>  - ``detach``
>  - ``setinfo``
>  - ``verify``
> -- ``snapshot``
> +- ``snapshot`` (optional)
>  
>  All scripts will be called with no arguments and get their input via
>  environment variables. A common set of variables will be exported for
> @@ -233,6 +233,10 @@ error, accompanied by an appropriate error message on 
> stderr. The
>  the block device's full path, after it has been successfully attached to
>  the host node. On error it should return non-zero.
>  
> +To keep backwards compatibility we let the ``snapshot`` script be
> +optional. If present then the provider will support instance backup
> +export as well.
> +
>  Implementation
>  --------------
>  
> diff --git a/lib/masterd/instance.py b/lib/masterd/instance.py
> index c1639e3..ebd563c 100644
> --- a/lib/masterd/instance.py
> +++ b/lib/masterd/instance.py
> @@ -1157,7 +1157,9 @@ class ExportInstanceHelper(object):
>      self._removed_snaps = [False] * len(instance.disks)
>  
>    def CreateSnapshots(self):
> -    """Creates an LVM snapshot for every disk of the instance.
> +    """Creates a snapshot for every disk of the instance.
> +
> +    Currently support drbd, plain and ext disk templates.
>  
>      """
>      assert not self._snap_disks
> diff --git a/lib/storage/extstorage.py b/lib/storage/extstorage.py
> index 19692a3..71b4f1b 100644
> --- a/lib/storage/extstorage.py
> +++ b/lib/storage/extstorage.py
> @@ -381,14 +381,19 @@ def ExtStorageFromDisk(name, base_dir=None):
>    # an optional one
>    es_files = dict.fromkeys(constants.ES_SCRIPTS, True)
>  
> +  # Let the snapshot script be optional
> +  es_files[constants.ES_SCRIPT_SNAPSHOT] = False
> +
>    es_files[constants.ES_PARAMETERS_FILE] = True
>  
> -  for (filename, _) in es_files.items():
> +  for (filename, required) in es_files.items():
>      es_files[filename] = utils.PathJoin(es_dir, filename)
>  
>      try:
>        st = os.stat(es_files[filename])
>      except EnvironmentError, err:
> +      if not required:
> +        continue
>        return False, ("File '%s' under path '%s' is missing (%s)" %
>                       (filename, es_dir, utils.ErrnoOrStr(err)))
>  
> diff --git a/man/ganeti-extstorage-interface.rst 
> b/man/ganeti-extstorage-interface.rst
> index 44a1adb..97ce14f 100644
> --- a/man/ganeti-extstorage-interface.rst
> +++ b/man/ganeti-extstorage-interface.rst
> @@ -220,6 +220,10 @@ respectively (see above).
>  
>  The script returns ``0`` on success.
>  
> +Please note that this script is optional and not all providers should
> +implement it. Of course if it is not present, instance backup export
> +will not be supported for the given provider.
> +
>  TEXT FILES
>  ----------
>  
> -- 
> 1.7.10.4

Attachment: signature.asc
Description: Digital signature

Reply via email to