On Thu, Jul 10, 2014 at 6:09 PM, Dimitris Aragiorgis <[email protected]>
wrote:

> The `snapshot` script is added to the interface and the new variables
> VOL_SNAPSHOT_NAME and VOL_SNAPSHOT_SIZE are exported to the
> environment.
>
> Signed-off-by: Dimitris Aragiorgis <[email protected]>
> ---
>  lib/backend.py            |    4 +++-
>  lib/masterd/instance.py   |   10 ++++++++--
>  lib/objects.py            |    1 +
>  lib/storage/extstorage.py |   41 +++++++++++++++++++++++++++++++++++++----
>  src/Ganeti/Constants.hs   |    9 ++++++++-
>  5 files changed, 57 insertions(+), 8 deletions(-)
>
> diff --git a/lib/backend.py b/lib/backend.py
> index 7a6c4bd..cbd900f 100644
> --- a/lib/backend.py
> +++ b/lib/backend.py
> @@ -3418,8 +3418,10 @@ def BlockdevSnapshot(disk):
>      return BlockdevSnapshot(disk.children[0])
>    elif disk.dev_type == constants.DT_PLAIN:
>      return _DiskSnapshot(disk)
> +  elif disk.dev_type == constants.DT_EXT:
> +    return _DiskSnapshot(disk)
>    else:
> -    _Fail("Cannot snapshot non-lvm block device '%s' of type '%s'",
> +    _Fail("Cannot snapshot block device '%s' of type '%s'",
>            disk.unique_id, disk.dev_type)
>
>
> diff --git a/lib/masterd/instance.py b/lib/masterd/instance.py
> index 54179ab..5a2dcf1 100644
> --- a/lib/masterd/instance.py
> +++ b/lib/masterd/instance.py
> @@ -1185,8 +1185,14 @@ class ExportInstanceHelper(object):
>

The docstring of this method (CreateSnapshots) still mentions LVM only -
should be fixed as well.


>                              " result '%s'", idx, src_node_name,
> result.payload)
>        else:
>          disk_id = tuple(result.payload)
> -        disk_params =
> constants.DISK_LD_DEFAULTS[constants.DT_PLAIN].copy()
> -        new_dev = objects.Disk(dev_type=constants.DT_PLAIN,
> size=disk.size,
> +        # Snapshot is currently supported for ExtStorage and
> LogicalVolume.
> +        # In case disk is of type drbd the snapshot will be of type plain.
> +        if disk.dev_type == constants.DT_EXT:
> +          dev_type = constants.DT_EXT
> +        else:
> +          dev_type = constants.DT_PLAIN

+        disk_params = constants.DISK_LD_DEFAULTS[dev_type].copy()
> +        new_dev = objects.Disk(dev_type=dev_type, size=disk.size,
>                                 logical_id=disk_id, iv_name=disk.iv_name,
>                                 params=disk_params)
>
> diff --git a/lib/objects.py b/lib/objects.py
> index c3532b0..553c5a8 100644
> --- a/lib/objects.py
> +++ b/lib/objects.py
> @@ -1345,6 +1345,7 @@ class ExtStorage(ConfigObject):
>      "detach_script",
>      "setinfo_script",
>      "verify_script",
> +    "snapshot_script",
>      "supported_parameters",
>      ]
>
> diff --git a/lib/storage/extstorage.py b/lib/storage/extstorage.py
> index 48e8f33..19692a3 100644
> --- a/lib/storage/extstorage.py
> +++ b/lib/storage/extstorage.py
> @@ -248,10 +248,26 @@ class ExtStorageDevice(base.BlockDev):
>                      " ExtStorage provider for the '%s' hypervisor"
>                      % (self.driver, hypervisor))
>
> +  def Snapshot(self, snap_name=None, snap_size=None):
> +    """Take a snapshot of the block device.
> +
> +    """
> +    provider, vol_name = self.unique_id
> +    if not snap_name:
> +      snap_name = vol_name + ".snap"
> +    if not snap_size:
> +      snap_size = self.size
> +
> +    _ExtStorageAction(constants.ES_ACTION_SNAPSHOT, self.unique_id,
> +                      self.ext_params, snap_name=snap_name,
> snap_size=snap_size)
> +
> +    return (provider, snap_name)
> +
>
>  def _ExtStorageAction(action, unique_id, ext_params,
>                        size=None, grow=None, metadata=None,
> -                      name=None, uuid=None):
> +                      name=None, uuid=None,
> +                      snap_name=None, snap_size=None):
>    """Take an External Storage action.
>
>    Take an External Storage action concerning or affecting
> @@ -259,7 +275,7 @@ def _ExtStorageAction(action, unique_id, ext_params,
>
>    @type action: string
>    @param action: which action to perform. One of:
> -                 create / remove / grow / attach / detach
> +                 create / remove / grow / attach / detach / snapshot
>    @type unique_id: tuple (driver, vol_name)
>    @param unique_id: a tuple containing the type of ExtStorage (driver)
>                      and the Volume name
> @@ -274,6 +290,10 @@ def _ExtStorageAction(action, unique_id, ext_params,
>    @type name: string
>    @param name: name of the Volume (objects.Disk.name)
>    @type uuid: string
> +  @type snap_size: integer
> +  @param snap_size: the size of the snapshot
> +  @type snap_name: string
> +  @param snap_name: the name of the snapshot
>    @param uuid: uuid of the Volume (objects.Disk.uuid)
>    @rtype: None or a block device path (during attach)
>
> @@ -287,7 +307,8 @@ def _ExtStorageAction(action, unique_id, ext_params,
>
>    # Create the basic environment for the driver's scripts
>    create_env = _ExtStorageEnvironment(unique_id, ext_params, size,
> -                                      grow, metadata, name, uuid)
> +                                      grow, metadata, name, uuid,
> +                                      snap_name, snap_size)
>
>    # Do not use log file for action `attach' as we need
>    # to get the output from RunResult
> @@ -399,13 +420,15 @@ def ExtStorageFromDisk(name, base_dir=None):
>                         detach_script=es_files[constants.ES_SCRIPT_DETACH],
>
> setinfo_script=es_files[constants.ES_SCRIPT_SETINFO],
>                         verify_script=es_files[constants.ES_SCRIPT_VERIFY],
> +
> snapshot_script=es_files[constants.ES_SCRIPT_SNAPSHOT],
>

It looks as if this change breaks backward compatibility, as it requires
the snapshot script.

If this is true, the script should be made optional in some way. It is
entirely possible that an external storage system would not support
snapshots.


>                         supported_parameters=parameters)
>    return True, es_obj
>
>
>  def _ExtStorageEnvironment(unique_id, ext_params,
>                             size=None, grow=None, metadata=None,
> -                           name=None, uuid=None):
> +                           name=None, uuid=None,
> +                           snap_name=None, snap_size=None):
>    """Calculate the environment for an External Storage script.
>
>    @type unique_id: tuple (driver, vol_name)
> @@ -422,6 +445,10 @@ def _ExtStorageEnvironment(unique_id, ext_params,
>    @param name: name of the Volume (objects.Disk.name)
>    @type uuid: string
>    @param uuid: uuid of the Volume (objects.Disk.uuid)
> +  @type snap_size: integer
> +  @param snap_size: the size of the snapshot
> +  @type snap_name: string
> +  @param snap_name: the name of the snapshot
>    @rtype: dict
>    @return: dict of environment variables
>
> @@ -450,6 +477,12 @@ def _ExtStorageEnvironment(unique_id, ext_params,
>    if uuid is not None:
>      result["VOL_UUID"] = uuid
>
> +  if snap_name is not None:
> +    result["VOL_SNAPSHOT_NAME"] = snap_name
> +
> +  if snap_size is not None:
> +    result["VOL_SNAPSHOT_SIZE"] = snap_size
> +
>    return result
>
>
> diff --git a/src/Ganeti/Constants.hs b/src/Ganeti/Constants.hs
> index 928b45d..bb12c89 100644
> --- a/src/Ganeti/Constants.hs
> +++ b/src/Ganeti/Constants.hs
> @@ -1391,6 +1391,9 @@ esActionSetinfo = "setinfo"
>  esActionVerify :: String
>  esActionVerify = "verify"
>
> +esActionSnapshot :: String
> +esActionSnapshot = "snapshot"
> +
>  esScriptCreate :: String
>  esScriptCreate = esActionCreate
>
> @@ -1412,6 +1415,9 @@ esScriptSetinfo = esActionSetinfo
>  esScriptVerify :: String
>  esScriptVerify = esActionVerify
>
> +esScriptSnapshot :: String
> +esScriptSnapshot = esActionSnapshot
> +
>  esScripts :: FrozenSet String
>  esScripts =
>    ConstantUtils.mkSet [esScriptAttach,
> @@ -1420,7 +1426,8 @@ esScripts =
>                         esScriptGrow,
>                         esScriptRemove,
>                         esScriptSetinfo,
> -                       esScriptVerify]
> +                       esScriptVerify,
> +                       esScriptSnapshot]
>
>  esParametersFile :: String
>  esParametersFile = "parameters.list"
> --
> 1.7.10.4
>
>

Reply via email to