On Wed, Jun 4, 2014 at 3:57 PM, Dimitris Bliablias <[email protected]>
wrote:

> Create a new constant variable named 'ddBlockSize' that holds the
> default value for the block size option of the 'dd' command, i.e., the
> 'bs' option. In Ganeti the block size is always 1MiB to match its units,
> so we create a new constant variable for it, instead of using hardcoded
> values.
>
> Signed-off-by: Dimitris Bliablias <[email protected]>
> ---
>  lib/backend.py          |    6 +++---
>  lib/storage/drbd.py     |    3 ++-
>  src/Ganeti/Constants.hs |    5 +++++
>  3 files changed, 10 insertions(+), 4 deletions(-)
>
> diff --git a/lib/backend.py b/lib/backend.py
> index bf56d6f..0e23c12 100644
> --- a/lib/backend.py
> +++ b/lib/backend.py
> @@ -2305,7 +2305,7 @@ def _DumpDevice(source_path, target_path, offset,
> size, truncate):
>    # Internal sizes are always in Mebibytes; if the following "dd" command
>    # should use a different block size the offset and size given to this
>    # function must be adjusted accordingly before being passed to "dd".
> -  block_size = 1024 * 1024
> +  block_size = constants.DD_BLOCK_SIZE
>
>    cmd = [constants.DD_CMD, "if=%s" % source_path, "seek=%d" % offset,
>           "bs=%s" % block_size, "oflag=direct", "of=%s" % target_path,
> @@ -4066,13 +4066,13 @@ def _GetImportExportIoCommand(instance, mode,
> ieio, ieargs):
>        # wrong path; we use notrunc to no attempt truncate on an LV device
>        suffix = utils.BuildShellCmd("| dd of=%s conv=nocreat,notrunc
> bs=%s",
>                                     real_disk.dev_path,
> -                                   str(1024 * 1024)) # 1 MB
> +                                   str(constants.DD_BLOCK_SIZE)) # 1 MB
>
>      elif mode == constants.IEM_EXPORT:
>        # the block size on the read dd is 1MiB to match our units
>        prefix = utils.BuildShellCmd("dd if=%s bs=%s count=%s |",
>                                     real_disk.dev_path,
> -                                   str(1024 * 1024), # 1 MB
> +                                   str(constants.DD_BLOCK_SIZE), # 1 MB
>                                     str(disk.size))
>        exp_size = disk.size
>
> diff --git a/lib/storage/drbd.py b/lib/storage/drbd.py
> index 524bf78..ead6d31 100644
> --- a/lib/storage/drbd.py
> +++ b/lib/storage/drbd.py
> @@ -1013,7 +1013,8 @@ class DRBD8Dev(base.BlockDev):
>      # can be bigger
>      result = utils.RunCmd([constants.DD_CMD,
>                             "if=/dev/zero", "of=%s" % dev_path,
> -                           "bs=1048576", "count=128", "oflag=direct"])
> +                           "bs=%s" % constants.DD_BLOCK_SIZE, "count=128",
> +                           "oflag=direct"])
>      if result.failed:
>        base.ThrowError("Can't wipe the meta device: %s", result.output)
>
> diff --git a/src/Ganeti/Constants.hs b/src/Ganeti/Constants.hs
> index 7446ce0..cbb7b49 100644
> --- a/src/Ganeti/Constants.hs
> +++ b/src/Ganeti/Constants.hs
> @@ -299,6 +299,11 @@ cpuPinningAllKvm = 0xFFFFFFFF
>  ddCmd :: String
>  ddCmd = "dd"
>
> +-- | 1 MiB
> +-- The default block size for the 'dd' command
> +ddBlockSize :: Int
> +ddBlockSize = 1048576
>

Just a very minor suggestion: Since most people don't remember the exact
number, from the readability point of view it'd be better to have 1024^2 or
something like that. If you agree, I'll make the change before pushing.


> +
>  -- | 1GB
>  maxWipeChunk :: Int
>  maxWipeChunk = 1024
> --
> 1.7.10.4
>
>
Rest LGTM

Reply via email to