Bug 888293

Created attachment 665484 [details]
program.log

Created attachment 665486 [details]
storage.log

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.

http://fpaste.org/bWhy/ appears to fix this, so it's got my ACK for master.

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.

+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 )

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.

I'm weak +1 NTH, but that VERY much hinges on the fix not being dangerous/invasive/going to cause heartburn.

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.

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.

Created attachment 674449 [details]
anaconda-tb crash

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

Crash:

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
    lvm.pvcreate(self.device)
  File "/usr/lib64/python2.7/site-packages/pyanaconda/storage/deviceaction.py", line 439, in execute
    options=self.device.formatArgs)
  File "/usr/lib64/python2.7/site-packages/pyanaconda/storage/devicetree.py", line 323, in processActions
    action.execute()
  File "/usr/lib64/python2.7/site-packages/pyanaconda/storage/__init__.py", line 323, in doIt
    self.devicetree.processActions()
  File "/usr/lib64/python2.7/site-packages/pyanaconda/storage/__init__.py", line 174, in turnOnFilesystems
    storage.doIt()
  File "/usr/lib64/python2.7/site-packages/pyanaconda/install.py", line 114, in doInstall
    turnOnFilesystems(storage)
  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).

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).

Tested a couple of kickstart installs, seem to work fine. Can't think of any other way this is going to break.

anaconda-18.37.11-1.fc18 has been submitted as an update for Fedora 18.
https://admin.fedoraproject.org/updates/anaconda-18.37.11-1.fc18

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.

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.

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.