On Thu, Jul 24, 2014 at 2:31 AM, Yuto KAWAMURA(kawamuray) <
[email protected]> wrote:

> Added two functions to utils.storage namespace.
>

... to the ...


> - CreateBdevPartitionMapping will allocate a loopback device and a
>   device-mapper device to provide a mapping device for each partition of
>   the block device.
> - ReleaseBdevPartitionMapping is inverse function of
>

... is the inverse ...

  CreateBdevPartitionMapping. This function call is required to
>   release allocated resources by CreateBdevPartitionMapping.
>

s/allocated resources/resources allocated/


> These functions depend on losetup(1) and kpartx(1).
>
> Signed-off-by: Yuto KAWAMURA(kawamuray) <[email protected]>
> ---
>  lib/utils/storage.py | 68
> ++++++++++++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 66 insertions(+), 2 deletions(-)
>
> diff --git a/lib/utils/storage.py b/lib/utils/storage.py
> index 64e2ec2..7af2baf 100644
> --- a/lib/utils/storage.py
> +++ b/lib/utils/storage.py
> @@ -25,7 +25,9 @@
>  import logging
>
>  from ganeti import constants
> -
> +from ganeti import errors
> +from ganeti.utils import io as utils_io
> +from ganeti.utils import process as utils_process
>
>  def GetDiskTemplatesOfStorageTypes(*storage_types):
>    """Given the storage type, returns a list of disk templates based on
> that
> @@ -174,7 +176,6 @@ def LookupSpaceInfoByStorageType(storage_space_info,
> storage_type):
>                          " ambiguous storage type '%s'.", storage_type)
>    return result
>
> -
>

The Ganeti style guide specifies 2 lines between functions. Lint would have
told you this ;)

Please fix other occurrences as well.

 def GetDiskLabels(prefix, num_disks, start=0):
>    """Generate disk labels for a number of disks
>
> @@ -206,3 +207,66 @@ def GetDiskLabels(prefix, num_disks, start=0):
>
>    for i in range(start, num_disks):
>      yield prefix + _GetDiskSuffix(i)
> +
> +def CreateBdevPartitionMapping(image_path):
> +  """Create dm device for each partition of disk image.
> +
> +  This operation will allocate a loopback and device-mapper device to map
>

... a device-mapper device ...


> +  partitions.
> +  You must call L{ReleaseBdevPartitionMapping} to clean up resources
> allocated
> +  by this function call.
> +
> +  @type image_path: string
> +  @param image_path: path of multi-partition disk image
> +  @rtype: tuple(string, list(string)) or NoneType
> +  @return: returns the tuple(loopback_device, list(device_mapper_files))
> if
> +    image_path is a multi-partition disk image. otherwise, returns None.
> +
> +  """
> +  # Unfortunately, there are two different losetup command in this world.
>

s/command/commands/

+  # One has '-s' switch and the other has '--show' switch to provide the
> same
>

... the '-s' switch .... the '--show' switch ...


> +  # functionality.
> +  result = utils_process.RunCmd(["losetup", "-f", "-s", image_path])
> +  if result.failed and "invalid option -- 's'" in result.stderr:
> +    result = utils_process.RunCmd(["losetup", "-f", "--show", image_path])
> +  if result.failed:
> +    raise errors.CommandError("Failed to setup loop device for %s: %s" %
> +                              (image_path, result.output))
> +  loop_dev_path = result.stdout.strip()
> +  logging.debug("Loop dev %s allocated for %s", loop_dev_path, image_path)
> +
> +  result = utils_process.RunCmd(["kpartx", "-a", "-v", loop_dev_path])
> +  if result.failed:
> +    # Just try to cleanup allocated loop device
> +    utils_process.RunCmd(["losetup", "-d", loop_dev_path])
> +    raise errors.CommandError("Failed to add partition mapping for %s:
> %s" %
> +                              (image_path, result.output))
> +  dm_devs = [x.split(" ") for x in result.stdout.split("\n") if x]
> +  if dm_devs:
> +    dm_dev_paths = [utils_io.PathJoin("/dev/mapper", x[2]) for x in
> dm_devs]
> +    return (loop_dev_path, dm_dev_paths)
> +  else:
> +    # image_path is not a multi partition disk image, no need to use
> +    # device-mapper.
> +    logging.debug("Release loop dev %s allocated for %s",
> +                  loop_dev_path, image_path)
> +    ReleaseBdevPartitionMapping(loop_dev_path)
> +    return None
> +
> +def ReleaseBdevPartitionMapping(loop_dev_path):
> +  """Release allocated dm devices and loopback devices.
> +
> +  @type loop_dev_path: string
> +  @param loop_dev_path: path of loopback device returned by
> +  L{CreateBdevPartitionMapping}
> +
> +  """
> +  result = utils_process.RunCmd(["kpartx", "-d", loop_dev_path])

+  if result.failed:
> +    raise errors.CommandError("Failed to release partition mapping of %s:
> %s"
>

Missing % operator here. Remember to lint, as it catches such errors and
helps keep the worst out of Python code!


> +                              (loop_dev_path, result.output))
> +
> +  result = utils_process.RunCmd(["losetup", "-d", loop_dev_path])
> +  if result.failed:
> +    raise errors.CommandError("Failed to detach %s: %s" %
> +                              (loop_dev_path, result.output))
> --
> 1.8.5.5
>
>

Hrvoje Ribicic
Ganeti Engineering
Google Germany GmbH
Dienerstr. 12, 80331, München

Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg
Geschäftsführer: Graham Law, Christine Elizabeth Flores
Steuernummer: 48/725/00206
Umsatzsteueridentifikationsnummer: DE813741370

Reply via email to