[Ovirt-devel] [PATCH node] Checks the request partitions against the size of the disk.

[Jim Meyering]

Perry Myers <pmyers at redhat.com> wrote:
> NOTE: This is Darryl's patch rebased on top of recent pushes to next as well
> as the two patches I posted earlier this evening.  I've tested this patch and
> it works for me.  Unless I hear objections, I'll push this when I get ack's
> on my previous two patches.
>
> Before partitioning is performed, the request size of the partitions are
> compared to the size of the disk itself. If the partitions combined are
> larger than the disk then an error message is presented to the user and
> they are returned to the main menu.

ACK.
modulo minor suggestions

> Signed-off-by: Darryl L. Pierce <dpierce at redhat.com>
> Signed-off-by: Perry Myers <pmyers at redhat.com>
> ---
>  scripts/ovirt-config-storage |   49 +++++++++++++++++++++++++++++------------
>  1 files changed, 34 insertions(+), 15 deletions(-)
>
> diff --git a/scripts/ovirt-config-storage b/scripts/ovirt-config-storage
> index 7489c68..e3acc05 100755
> --- a/scripts/ovirt-config-storage
> +++ b/scripts/ovirt-config-storage
> @@ -20,17 +20,6 @@ default_root_size=256
>  default_config_size=5
>  default_logging_size=256
>
> -check_partition_sizes()
> -{
> -    # FIXME: use this function before performing any partitioning, auto or not
> -    :
> -    # Perform some sanity checks.  E.g.,
> -    # What if DATA_SIZE ends up zero or negative?
> -    # What if any of those partition sizes is smaller than
> -    # the minimum size for an ext3 partition?  Diagnose it here
> -    # or just let the mke2fs failure suffice.
> -}
> -
>  get_selected_drive_size()
>  {
>      local udi=$(hal-find-by-property --key block.device --string $DRIVE)
> @@ -47,6 +36,32 @@ get_selected_drive_size()
>      echo "selected device: $DRIVE ($SPACE MB)"
>  }
>
> +check_partition_sizes()
> +{
> +    local disk_size need_size
> +
> +    get_selected_drive_size
> +    disk_size=$SPACE
> +    need_size=$(echo "scale=0; $BOOT_SIZE+$SWAP_SIZE+$ROOT_SIZE+$CONFIG_SIZE+$LOGGING_SIZE;" | bc -l)

it's a little easier to read this way:

    need_size=$(echo "scale=0;" \
                     "$BOOT_SIZE + $SWAP_SIZE + $ROOT_SIZE" \
                     "+ $CONFIG_SIZE + $LOGGING_SIZE" | bc -l)

> +    printf "disk_size=$disk_size\n"
> +    printf "need_size=$need_size\n"
> +
> +    if [ $need_size -gt $disk_size ]; then
> +        gap_size=$(echo "scale=0; $need_size-$disk_size;" | bc -l)
> +        printf "\n"
> +        printf "=============================================================\n"
> +        printf "The target storage device is too small for the desired sizes:\n"
> +        printf " Size of target storage device: $disk_size MB\n"
> +        printf " Total storage size to be used: $need_size MB\n"
> +        printf "\n"
> +        printf "You need an addition $gap_size MB of storage.\n"
> +        printf "\n"
> +        return 1
> +    fi
> +
> +    return 0
> +}
> +
>  # Find a usable/selected storage device.
>  # If there are none, give a diagnostic and return nonzero.
>  # If there is just one, e.g., /dev/sda, treat it as selected (see below).
> @@ -309,8 +324,10 @@ do_confirm()
>          case $REPLY in
>              Y|y)
>                  check_partition_sizes
> -                perform_partitioning
> -		        exit 0
> +                if [ $? = 0 ]; then
> +                    perform_partitioning
> +                    exit 0
> +                fi

However, this is more maintainable:

              if check_partition_sizes; then
                  perform_partitioning
                  exit 0
              fi

ovirt has already seen cases where the command and the
following test of "$?" were accidentally separated.
When you combine running the command and testing its
exit status on the same line, that can't happen.

...
>  if [ "$1" == "AUTO" ]; then
>      check_partition_sizes
> -    printf "Partitioning hard disk..."
> -    perform_partitioning
> +    if [ $? = 0 ]; then
> +        printf "Partitioning hard disk..."
> +        perform_partitioning
> +    fi

    if check_partition_sizes; then
        printf "Partitioning hard disk..."
        perform_partitioning
    fi