Bug 888293 - anaconda entering and leaving STORAGE: INSTALLATION DESTINATION unlocks 'begin installation'
Summary: anaconda entering and leaving STORAGE: INSTALLATION DESTINATION unlocks 'begi...
Alias: None
Product: Fedora
Classification: Fedora
Component: anaconda
Version: 18
Hardware: x86_64
OS: Linux
Target Milestone: ---
Assignee: David Lehman
QA Contact: Fedora Extras Quality Assurance
Whiteboard: AcceptedNTH
Depends On:
Blocks: F18-accepted, F18FinalFreezeExcept
TreeView+ depends on / blocked
Reported: 2012-12-18 13:10 UTC by Reartes Guillermo
Modified: 2013-01-08 21:40 UTC (History)
9 users (show)

Fixed In Version: anaconda-18.37.11-1
Doc Type: Bug Fix
Doc Text:
Clone Of:
Last Closed: 2013-01-08 21:31:48 UTC
Type: Bug

Attachments (Terms of Use)
anaconda.log (11.70 KB, text/x-log)
2012-12-18 13:10 UTC, Reartes Guillermo
no flags Details
program.log (45.29 KB, text/plain)
2012-12-18 13:10 UTC, Reartes Guillermo
no flags Details
storage.log (165.56 KB, text/plain)
2012-12-18 13:11 UTC, Reartes Guillermo
no flags Details
anaconda-tb crash (464.41 KB, text/plain)
2013-01-07 23:26 UTC, Reartes Guillermo
no flags Details

Description Reartes Guillermo 2012-12-18 13:10:28 UTC
Created attachment 665483 [details]

Description of problem:

I found a case when by just entering and leaving storage: installation destination causes the 'begin installation' button to be unlocked with 'automatic partitioning' selected. This might cause unwanted data loss, since there was no partitioning selection by the part of the user.

Version-Release number of selected component (if applicable):
F18 TC3 (anaconda 18.37.3)

How reproducible:

Steps to Reproduce:
0. Boot with 'gpt' boot parameter.
1. The guest has one disk with a gpt labeled disk but no partitions.
2. Select 'spanish' and 'set keyboard'
3. Select spanish in keyboard. (did i do it in .2?)
4. Select America/Buenos_Aires and disable ntp.
5. Enter Storage and leave storage
6. 'automatic partitioning selected' is shown and 'begin installation' is unlocked.
7. Enter Storage and leave storage
8. 'error cheching storage configuration' is shown
9. Enter Storage and leave storage
10. 'automatic partitioning selected' is shown and 'begin installation' is unlocked.
11. Enter Storage and leave storage
12. 'error cheching storage configuration' is shown
13. Enter Storage and leave storage
14. 'automatic partitioning selected' is shown and 'begin installation' is unlocked.
15. etc
Actual results:
one can start installing by just entering and leaving the STORAGE: INSTALLATION DESTINATION

Expected results:
anaconda should always reject such storage configuration. 

Additional info:

Comment 1 Reartes Guillermo 2012-12-18 13:10:50 UTC
Created attachment 665484 [details]

Comment 2 Reartes Guillermo 2012-12-18 13:11:16 UTC
Created attachment 665486 [details]

Comment 3 Reartes Guillermo 2012-12-18 13:22:11 UTC
I just reasoned that the disk is empty so no data loss here.

But it kinda does not feel right for some reason. If one retries, it alternates between rejecting and accepting the storage configuration.

Comment 4 Chris Lumens 2013-01-07 18:56:33 UTC
http://fpaste.org/bWhy/ appears to fix this, so it's got my ACK for master.

Comment 5 Adam Williamson 2013-01-07 21:48:57 UTC
Proposing as NTH. Data loss is actually very unlikely in this case, as no 'clearpart' operation happens - it won't destroy any existing partitions, just try and install into any available free space. clumens says it could only possibly lead to any data loss in a case where there was some unrecognized data on the disk (pretty unlikely). Still, someone could conceivably get angry about this, I guess, and it is wrong behaviour. On fix safety:

<adamw> ok
 i'm probably a weak +1 nth if the fix is safe
 how worrying is teh fix? any worries it could stop you installing in some valid case, for e.g.?
 any possible interference with kickstarts?
<dlehman> doesn't seem dangerous at all to me
<adamw> well, we've said that before, but :)
 i'll ask the others to vote
<dlehman> the fix is to add to the set of conditions for storage completeness "either one or more disks have been selected or this is a kickstart install"
<adamw> okay.
 seems reasonable.

I'm a weak +1 nth, as stated in that log.

Comment 6 Jóhann B. Guðmundsson 2013-01-07 22:00:44 UTC
+1 NTH since the fix is ready does not seem dangerous and *might* if the stars align right lead to dataloss ( which arguably is being expected anyway since you know this is an installer and you are about to install a *new* OS to your hd )

Comment 7 Tim Flink 2013-01-07 22:03:26 UTC
I'm slightly -1 NTH on this but if there are enough votes, I'm not going to fight it.

While this is not behaving correctly, I'm a little worried about taking non-blocker fixes 2 days before go/no-go is scheduled. Especially for something that seems to have so little risk of data loss.

If we end up slipping again (hopefully we won't, though), I'd be more OK with it as long as it went into a new build on Thursday or Friday.

Comment 8 Robyn Bergeron 2013-01-07 22:07:27 UTC
I'm weak +1 NTH, but that VERY much hinges on the fix not being dangerous/invasive/going to cause heartburn.

Comment 9 Reartes Guillermo 2013-01-07 22:48:32 UTC
Please consider bug 869482.

If it boot on a system with many disks:

* sda: solaris os
* sdb and beyond are part of an zpool (efi labeled)

And enter and leave STORAGE: Installation Destination, the 'begin installation' button is UNLOCKED. 

Bug 869482 sees free space where there is not, so it appears that it will see free space and wipe sda wich contains solaris. I first need to backup the guest before trying.

Comment 10 Adam Williamson 2013-01-07 23:00:39 UTC
we had a lot of 'weak' votes on this one, but in the end slightly on the positive side, so marking as acceptedNTH.

The fix is simple and 'obvious': I really can't see how it can break anything, and even if it does, it should be pretty visible, because the code it affects is the 'is this spoke complete?' code. So if it breaks something, it should be pretty obvious, as you'll be trapped at the hub. So we *should* be able to catch any breakage in testing.

Comment 11 Reartes Guillermo 2013-01-07 23:26:19 UTC
Created attachment 674449 [details]
anaconda-tb crash

I tested what happens in the case of comment #9,
and i got bug 885492 comment #37


anaconda 18.37.10 exception report
Traceback (most recent call first):
  File "/usr/lib64/python2.7/site-packages/pyanaconda/storage/devicelibs/lvm.py", line 178, in pvcreate
    raise LVMError("pvcreate failed for %s: %s" % (device, msg))
  File "/usr/lib64/python2.7/site-packages/pyanaconda/storage/formats/lvmpv.py", line 113, in create
  File "/usr/lib64/python2.7/site-packages/pyanaconda/storage/deviceaction.py", line 439, in execute
  File "/usr/lib64/python2.7/site-packages/pyanaconda/storage/devicetree.py", line 323, in processActions
  File "/usr/lib64/python2.7/site-packages/pyanaconda/storage/__init__.py", line 323, in doIt
  File "/usr/lib64/python2.7/site-packages/pyanaconda/storage/__init__.py", line 174, in turnOnFilesystems
  File "/usr/lib64/python2.7/site-packages/pyanaconda/install.py", line 114, in doInstall
  File "/usr/lib64/python2.7/threading.py", line 504, in run
    self.__target(*self.__args, **self.__kwargs)
  File "/usr/lib64/python2.7/site-packages/pyanaconda/threads.py", line 91, in run
    threading.Thread.run(self, *args, **kwargs)
LVMError: pvcreate failed for /dev/sdd1: running lvm pvcreate --config  devices { filter=["r|/sdb1$|","r|/sdb9$|","r|/sdc1$|","r|/sdc9$|","r|/sdd1$|","r|/sdd9$|","r|/loop3$|","r|/loop4$|","r|/loop5$|","r|/loop6$|","r|/loop7$|"] }  --dataalignment 1024k /dev/sdd1 failed

The solaris os disks survived but the zpools did not, some ended with the primarty gpt header borked (since an msdos disklabel was applied but the secondary gpt header was still there).

I agree that this bug-report itself is not a blocker, but if left unfixed it might help other bugs (like bug 869482).

Comment 12 Adam Williamson 2013-01-07 23:30:02 UTC
Testing confirms the fix for the bug fixes the bug. I built an updates.img to test it. Without the updates.img, in a single disk case, I can de-select the disk in the 'Installation Source' spoke, and still start the install, as described in the bug. With the updates.img applied, the spoke displays the yellow triangle when no disk is selected and I cannot start the install.

Now checking nothing else is broken (as much as I can).

Comment 13 Adam Williamson 2013-01-07 23:42:01 UTC
Tested a couple of kickstart installs, seem to work fine. Can't think of any other way this is going to break.

Comment 14 Fedora Update System 2013-01-08 01:18:51 UTC
anaconda-18.37.11-1.fc18 has been submitted as an update for Fedora 18.

Comment 15 Kamil Páral 2013-01-08 13:12:00 UTC
I don't understand this bug at all and I don't know how to verify it.

If I boot anaconda 18.37.11 with a clean disk, enter Storage screen, hit Done, then the installation can be started. But it always worked that way. Is it supposed to work another way? If it is not supposed to perform autopart, and Continue is required for that, then the top left button should be called Back, not Done.

Adam, please explain more, thanks.

Comment 16 Fedora Update System 2013-01-08 21:31:51 UTC
anaconda-18.37.11-1.fc18 has been pushed to the Fedora 18 stable repository.  If problems still persist, please make note of it in this bug report.

Comment 17 Adam Williamson 2013-01-08 21:40:57 UTC
kparal: on a single-disk configuration, to see the bug, you have to go into the Installation Destination spoke, *un-select* the disk, and hit Done. Now you have no disks selected as install targets, and it shouldn't be possible to proceed with install, but you can - the spoke shows 'No disks selected', but no warning triangle, and the "Begin Installation" button isn't locked.

It's easier to see in a multi-disk case because anaconda doesn't 'pre-select' any disks for you, it waits for you to make a choice. So to see the bug in a multi-disk case, you can just boot the installer and immediately click Begin Installation. But it's the same bug: you can begin installation with no disks selected as install targets.

I've just verified that this is correctly fixed in RC2. 'No disks selected' is an 'error' case and you can't begin installation until it's resolved. So this can stay closed.

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