| Summary: | make the uninstall fail more obviously | ||||||
|---|---|---|---|---|---|---|---|
| Product: | Red Hat Enterprise Linux 6 | Reporter: | Mohua Li <moli> | ||||
| Component: | ovirt-node | Assignee: | Mike Burns <mburns> | ||||
| Status: | CLOSED ERRATA | QA Contact: | Virtualization Bugs <virt-bugs> | ||||
| Severity: | medium | Docs Contact: | |||||
| Priority: | medium | ||||||
| Version: | 6.2 | CC: | apevec, cshao, gouyang, leiwang, mburns, ovirt-maint, ycui | ||||
| Target Milestone: | rc | ||||||
| Target Release: | --- | ||||||
| Hardware: | Unspecified | ||||||
| OS: | Unspecified | ||||||
| Whiteboard: | |||||||
| Fixed In Version: | ovirt-node-2.0.2-0.6.gitcc6ba8c.el6 | Doc Type: | Bug Fix | ||||
| Doc Text: | Story Points: | --- | |||||
| Clone Of: | Environment: | ||||||
| Last Closed: | 2011-12-06 19:17:42 UTC | Type: | --- | ||||
| Regression: | --- | Mount Type: | --- | ||||
| Documentation: | --- | CRM: | |||||
| Verified Versions: | Category: | --- | |||||
| oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |||||
| Cloudforms Team: | --- | Target Upstream Version: | |||||
| Attachments: |
|
||||||
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")"
;;
(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" Created attachment 522754 [details]
Patch
Testing: specify a device in storage_init that does not exist (/dev/vda on a physical machine seems like a good choice). 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 |
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: