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

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.

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)
+    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
                 break
                 ;;
             N|n)  return ;;
@@ -363,8 +380,10 @@ fi
 
 if [ "$1" == "AUTO" ]; then
     check_partition_sizes
-    printf "Partitioning hard disk..."
-    perform_partitioning
+    if [ $? = 0 ]; then
+        printf "Partitioning hard disk..."
+        perform_partitioning
+    fi
 else
     OPTIONS="Configure Review Partition Quit"
     PS3="Choose an option: "
-- 
1.6.0.6

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