Bug 720946 - make the uninstall fail more obviously
Summary: make the uninstall fail more obviously
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Red Hat Enterprise Linux 6
Classification: Red Hat
Component: ovirt-node
Version: 6.2
Hardware: Unspecified
OS: Unspecified
medium
medium
Target Milestone: rc
: ---
Assignee: Mike Burns
QA Contact: Virtualization Bugs
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2011-07-13 10:19 UTC by Mohua Li
Modified: 2011-12-06 19:17 UTC (History)
7 users (show)

Fixed In Version: ovirt-node-2.0.2-0.6.gitcc6ba8c.el6
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2011-12-06 19:17:42 UTC
Target Upstream Version:


Attachments (Terms of Use)
Patch (3.53 KB, patch)
2011-09-12 18:29 UTC, Mike Burns
no flags Details | Diff


Links
System ID Private Priority Status Summary Last Updated
Red Hat Product Errata RHBA-2011:1783 0 normal SHIPPED_LIVE rhev-hypervisor6 bug fix and enhancement update 2011-12-06 15:10:54 UTC

Description Mohua Li 2011-07-13 10:19:37 UTC
Description of problem:

in kernel boot command line, user could specify
storage_init=/dev/mapper/* uninstall to uninstall the exist HostVG,
but if there is any fail, like follow storage_init can't find the disk
we should drop to shell, or better, we could provide the command let
the user to manually uninstall the HostVG in emergency shell

in ovirt-early, we just wipe the disk's mbr, if disk could not be
found,we should call  autoinstall_failed  to drop to shell, sth like that,


        wipe_mbr "$init_disk"
        if [ $rc -ne 0 ]; then
           autoinstall_failed
        fi


Version-Release number of selected component (if applicable):
rhev-hypervisor-6.2-0.5

How reproducible:
always

Steps to Reproduce:
1,boot to rhev-hypervisor kernel command line
2,specify invalid disk in "storage_init=$invalidisk uninstall"


  
Actual results:


Expected results:


Additional info:

Comment 5 Mike Burns 2011-07-25 14:58:11 UTC
It's not that simple.

autoinstall_failed is a local function in ovirt-firstboot and isn't available in ovirt-early where this check is.  It can be refactored pretty easily though.

This is actually a bigger issue.  If you specify an invalid disk, that gets handled silently earlier in the process.  When we parse the storage_init parameter, an invalid device gets dropped


ovirt-early runs parse_disk_id from ovirt-boot-functions which has a case statement:

case
...
 /dev/*)
            disk="$(ls -1 "$i" 2>/dev/null | grep -m 1 "$dev")"
 ;;

Comment 6 Alan Pevec 2011-08-09 08:31:34 UTC
(In reply to comment #5)
>  /dev/*)
>             disk="$(ls -1 "$i" 2>/dev/null | grep -m 1 "$dev")"
>  ;;

So in case of invalid storage_init we end up with disk="", we could check for that and bail-out:
diff --git a/scripts/ovirt-early b/scripts/ovirt-early
index fa24720..47864fe 100755
--- a/scripts/ovirt-early
+++ b/scripts/ovirt-early
@@ -364,10 +364,14 @@ start_ovirt_early () {
                     IFS=,
                     init=
                     for d in $hostvgdisks; do
+                        did="$(IFS="$oldIFS" parse_disk_id "$d")"
+                        if [ -z "$did" ]; then
+                            autoinstall_failed
+                        fi
                         if [ -n "$init" ]; then
-                            init="$init${SEP}$(IFS="$oldIFS" parse_disk_id "$d")"
+                            init="$init${SEP}$did"
                         else
-                            init="$(IFS="$oldIFS" parse_disk_id "$d")"
+                            init="$did"
                         fi
                     done
                     IFS="$oldIFS"
@@ -378,10 +382,14 @@ start_ovirt_early () {
                     IFS=,
                     init_app=
                     for d in $appvgdisks; do
+                        did="$(IFS="$oldIFS" parse_disk_id "$d")"
+                        if [ -z "$did" ]; then
+                            autoinstall_failed
+                        fi
                         if [ -n "$init_app" ]; then
-                            init_app="$init_app${SEP}$(IFS="$oldIFS" parse_disk_id "$d")"
+                            init_app="$init_app${SEP}$did"
                         else
-                            init_app="$(IFS="$oldIFS" parse_disk_id "$d")"
+                            init_app="$did"
                         fi
                     done
                     IFS="$oldIFS"

Comment 9 Mike Burns 2011-09-12 18:29:59 UTC
Created attachment 522754 [details]
Patch

Comment 10 Mike Burns 2011-09-12 18:30:45 UTC
Testing: 

specify a device in storage_init that does not exist (/dev/vda on a physical machine seems like a good choice).

Comment 15 errata-xmlrpc 2011-12-06 19:17:42 UTC
Since the problem described in this bug report should be
resolved in a recent advisory, it has been closed with a
resolution of ERRATA.

For information on the advisory, and where to find the updated
files, follow the link below.

If the solution does not work for you, open a new bug report.

http://rhn.redhat.com/errata/RHBA-2011-1783.html


Note You need to log in before you can comment on or make changes to this bug.