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

David Lehman dlehman at redhat.com
Thu Feb 20 18:17:08 UTC 2014


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

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




More information about the anaconda-patches mailing list