[anaconda:rhel7/master] Always have a "New ..." page even if it has no devices (#1043763)

David Lehman dlehman at redhat.com
Thu Feb 20 21:33:50 UTC 2014


On Thu, 2014-02-20 at 16:12 -0500, Anne Mulhern wrote:
> 
> 
> 
> ----- Original Message -----
> > From: "David Lehman" <dlehman at redhat.com>
> > To: "anaconda patch review" <anaconda-patches at lists.fedorahosted.org>
> > Sent: Thursday, February 20, 2014 1:17:08 PM
> > Subject: Re: [anaconda:rhel7/master] Always have a "New ..." page even if it	has no devices (#1043763)
> > 
> > On Thu, 2014-02-20 at 11:16 -0500, Anne Mulhern wrote:
> > > 
> > > 
> > > 
> > > ----- Original Message -----
> > > > From: "David Lehman" <dlehman at redhat.com>
> > > > To: "anaconda patch review" <anaconda-patches at lists.fedorahosted.org>
> > > > Sent: Thursday, February 20, 2014 9:56:13 AM
> > > > Subject: Re: [anaconda:rhel7/master] Always have a "New ..." page even if
> > > > it	has no devices (#1043763)
> > > > 
> > > > On Thu, 2014-02-20 at 08:18 -0500, mulhern wrote:
> > > > > Resolves: rhbz#1043763
> > > > > 
> > > > > There should always be a page for the "New ..." Fedora or RHEL7 install
> > > > > because that's our target, i.e., what we're trying to install with this
> > > > > installer.
> > > > 
> > > > If there are no devices to go in the "New ..." page it should be handled
> > > > above in the block that starts with "if not new_devices:". If there are
> > > > new devices, the page for the new installation should be getting created
> > > > already. Either way, this patch should not be necessary. I suggest you
> > > > examine the debug lines that list devices, unused, and new_devices --
> > > > something is apparently wrong in those lists.
> > > > 
> > > > David
> > > > 
> > > 
> > > In this case, where we really have just Unknown OS with a single Unknown
> > > format partition on the one checked disk..
> > > 
> > > devices is the usual (it excludes all unchecked disks and their partitions
> > > if any, which is AFAIK correct), unused is ['sdc1'] which corresponds to
> > > the unique partition on the Unknown OS, and new_devices is the same as
> > > unused. I'm not sure of the exact meaning of unused, but these values seem
> > > plausible to me.
> > > 
> > > Under these conditions the else branch of the "if not new_devices:" block
> > > is taken and a Root object corresponding to the "New ..." is made and
> > > shoved in at the front of ui_roots, which was previously empty. This root
> > > however, contains no mounts or swaps of any kind. AFAIK, this is correct.
> > > 
> > > Without the patch, this root object is skipped because it contains no
> > > mounts or swaps in the "for root in ui_roots:" loop which follows the if
> > > not new_devices: block and no page is constructed for it. With the patch,
> > > a page is constructed for the root that was created in the previous else
> > > block.
> > 
> > No device should simultaneously be in unused and new_devices (except
> > possibly bootloader-specific partitions like PReP Boot, which is a bug
> > if it is the case) -- they are disjoint by definition. Unused means it
> > isn't set to be used in the new installation in any way. new_devices
> > should contain only devices that are set to be used in the new
> > installation (created, reformatted, mounted, or used for bootloader
> > IIRC).
> > 
> > David
> 
> OK. Should the sdc1 device be in unused or in new_devices?

It depends on the device. If device.format.exists is False, it should
probably be in new_devices.

> 
> We got to this situation by:
> 
> 1) Using the special CreateNew page to create an LVM partition scheme that spanned two disks (sda and sdc)
> 2) Going back into partitioning and unselecting sda
>   -- This caused a bunch of actions to be removed, leaving only two, a Destroy something and a create disk label for sdc
> 
> on arriving at the custom spoke and without the patch, we see the Unknown OS/Unknown format configuration and nothing else.
> 
> Should we be seeing a special CreateNew page and Unknown OS/ Unknown Format because there are no new_devices, or should we be
> seeing a "New RHEL7..." because sdc1 is in new_devices?

What is on sdc1?

> 
> I believe the first is correct because the original partitioning scheme that we had previously is totally gone
> 
> - mulhern
> 
> > 
> > > 
> > > - mulhern
> > > 
> > > > > 
> > > > > Several methods assume that a search in the accordion for the page that
> > > > > corresponds to self.translated_new_install_name will always be
> > > > > successful
> > > > > and will crash if none is found.
> > > > > 
> > > > > Signed-off-by: mulhern <amulhern at redhat.com>
> > > > > ---
> > > > >  pyanaconda/ui/gui/spokes/custom.py | 10 +++++-----
> > > > >  1 file changed, 5 insertions(+), 5 deletions(-)
> > > > > 
> > > > > diff --git a/pyanaconda/ui/gui/spokes/custom.py
> > > > > b/pyanaconda/ui/gui/spokes/custom.py
> > > > > index 61506fc..f72cae9 100644
> > > > > --- a/pyanaconda/ui/gui/spokes/custom.py
> > > > > +++ b/pyanaconda/ui/gui/spokes/custom.py
> > > > > @@ -974,11 +974,11 @@ class CustomPartitioningSpoke(NormalSpoke,
> > > > > StorageChecker):
> > > > >  
> > > > >          # Add in all the existing (or autopart-created) operating
> > > > >          systems.
> > > > >          for root in ui_roots:
> > > > > -            # Don't make a page if none of the root's devices are
> > > > > left.
> > > > > -            # Also, only include devices in an old page if the format
> > > > > is
> > > > > intact.
> > > > > -            if not [d for d in root.swaps + root.mounts.values()
> > > > > -                        if d in self._devices and d.disks and
> > > > > -                           (root.name ==
> > > > > self.translated_new_install_name
> > > > > or d.format.exists)]:
> > > > > +            # Omit a page for the root if the root has no devices
> > > > > +            #  * unless it is new_root
> > > > > +            if not (root.name == self.translated_new_install_name or \
> > > > > +               [d for d in root.swaps + root.mounts.values()
> > > > > +                  if d in self._devices and d.disks and
> > > > > d.format.exists]):
> > > > >                  continue
> > > > >  
> > > > >              page = Page(root.name)
> > > > 
> > > > 
> > > > _______________________________________________
> > > > anaconda-patches mailing list
> > > > anaconda-patches at lists.fedorahosted.org
> > > > https://lists.fedorahosted.org/mailman/listinfo/anaconda-patches
> > > > 
> > > _______________________________________________
> > > anaconda-patches mailing list
> > > anaconda-patches at lists.fedorahosted.org
> > > https://lists.fedorahosted.org/mailman/listinfo/anaconda-patches
> > 
> > 
> > _______________________________________________
> > anaconda-patches mailing list
> > anaconda-patches at lists.fedorahosted.org
> > https://lists.fedorahosted.org/mailman/listinfo/anaconda-patches
> > 
> _______________________________________________
> anaconda-patches mailing list
> anaconda-patches at lists.fedorahosted.org
> https://lists.fedorahosted.org/mailman/listinfo/anaconda-patches




More information about the anaconda-patches mailing list