[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [PATCH rhel6-branch] Don't allow GPT disks to be used for booting on non-EFI systems (614585)



Hi,

I'm afraid that this patch will regress bug 580404, which is
about allowing the use of gpt labelled disks for /boot.

Most modern non efi x86 systems can boot from gpt disks just fine,
and doing so is preferably when using really big disks (or when
the system is a mac using non efi BIOS emulation).

So I think that not showing gpt labeled drives to select them
as a boot drive is not a good idea.

All I can come up with is to instead pop up a warning in the
GPT bootdisk on non efi system case that that is not guaranteed
to work. So that people who want to use gpt can (by ignoring the
warning). And that others are warned.

Regards,

Hans


On 07/16/2010 11:17 PM, Brian C. Lane wrote:
In autopart, if the only disk available is GPT warn the user that it
will be re-initialized and only allow them to use the 'All Space'
option.

With multiple disks don't allow the GPT labeled disk to be selected as
the boot disk in cleardisk or when editing the bootloader settings.

These changes only apply to GPT disks on non-EFI systems.

Resolves: rhbz#614585
---
  iutil.py                  |    9 +++++
  iw/autopart_type.py       |   73 +++++++++++++++++++++++++++++----------------
  iw/bootloader_main_gui.py |    8 +++++
  iw/cleardisks_gui.py      |   12 +++++--
  4 files changed, 73 insertions(+), 29 deletions(-)

diff --git a/iutil.py b/iutil.py
index 0b74e08..b34a85c 100644
--- a/iutil.py
+++ b/iutil.py
@@ -1091,3 +1091,12 @@ def get_sysfs_attr(path, attr):
          return None

      return open(attribute, "r").read().strip()
+
+def isDiskBootable(d):
+    """  Return True if the disk is allowed to be listed as a bootable disk
+         Do not list hidden disks
+         Do not list GPT disks on non-EFI systems
+    """
+    return not d.format.hidden and \
+           (d.format.labelType != 'gpt' or isEfi())
+
diff --git a/iw/autopart_type.py b/iw/autopart_type.py
index fbc4391..0a0568f 100644
--- a/iw/autopart_type.py
+++ b/iw/autopart_type.py
@@ -25,6 +25,7 @@ import math

  from constants import *
  import gui
+import iutil
  from partition_ui_helpers_gui import *
  from pixmapRadioButtonGroup_gui import pixmapRadioButtonGroup

@@ -237,6 +238,23 @@ class PartitionTypeWindow(InstallWindow):
          self.reviewButton.set_active(not self.dispatch.stepInSkipList("partition"))
          self.encryptButton.set_active(self.storage.encryptedAutoPart)

+        # Check to see if the only disk available is GPT on non-EFI systems
+        # If so, the only option is to use all space since the mbr needs to
+        # be cleared
+        disks = filter(lambda d: not d.format.hidden, anaconda.id.storage.disks)
+        if (len(disks) == 1) and not iutil.isDiskBootable(disks[0]):
+            onlyUseAllSpace = True
+
+            self.intf.messageWindow(_("Warning"),
+                               _("Your only disk uses GPT partitioning. This is "
+                                 "not supported on non-EFI systems. It will be "
+                                 "re-initalized using MSDOS partitioning if you "
+                                 "proceed with the install."),
+                               type="warning", custom_icon="error")
+        else:
+            onlyUseAllSpace = False
+
+
          self.buttonGroup = pixmapRadioButtonGroup()
          self.buttonGroup.addEntry("all", _("Use All Space"),
                                    pixmap=gui.readImageFromFile("partscheme-all.png"),
@@ -246,31 +264,32 @@ class PartitionTypeWindow(InstallWindow):
                                            "<b>Tip:</b>  This option will remove "
                                            "data from the selected device(s).  Make "
                                            "sure you have backups."))
-        self.buttonGroup.addEntry("replace", _("Replace Existing Linux System(s)"),
-                                  pixmap=gui.readImageFromFile("partscheme-replace.png"),
-                                  descr=_("Removes only Linux partitions (created from "
-                                          "a previous Linux installation).  This does "
-                                          "not remove other partitions you may have "
-                                          "on your storage device(s) (such as VFAT or "
-                                          "FAT32).\n\n"
-                                          "<b>Tip:</b>  This option will remove "
-                                          "data from the selected device(s).  Make "
-                                          "sure you have backups."))
-        self.buttonGroup.addEntry("shrink", _("Shrink Current System"),
-                                  pixmap=gui.readImageFromFile("partscheme-shrink.png"),
-                                  descr=_("Shrinks existing partitions to create free "
-                                          "space for the default layout."))
-        self.buttonGroup.addEntry("freespace", _("Use Free Space"),
-                                  pixmap=gui.readImageFromFile("partscheme-freespace.png"),
-                                  descr=_("Retains your current data and partitions and "
-                                          "uses only the unpartitioned space on the "
-                                          "selected device(s), assuming you have enough "
-                                          "free space available."))
-        self.buttonGroup.addEntry("custom", _("Create Custom Layout"),
-                                  pixmap=gui.readImageFromFile("partscheme-custom.png"),
-                                  descr=_("Manually create your own custom layout on "
-                                          "the selected device(s) using our partitioning "
-                                          "tool."))
+        if not onlyUseAllSpace:
+            self.buttonGroup.addEntry("replace", _("Replace Existing Linux System(s)"),
+                                      pixmap=gui.readImageFromFile("partscheme-replace.png"),
+                                      descr=_("Removes only Linux partitions (created from "
+                                              "a previous Linux installation).  This does "
+                                              "not remove other partitions you may have "
+                                              "on your storage device(s) (such as VFAT or "
+                                              "FAT32).\n\n"
+                                              "<b>Tip:</b>  This option will remove "
+                                              "data from the selected device(s).  Make "
+                                              "sure you have backups."))
+            self.buttonGroup.addEntry("shrink", _("Shrink Current System"),
+                                      pixmap=gui.readImageFromFile("partscheme-shrink.png"),
+                                      descr=_("Shrinks existing partitions to create free "
+                                              "space for the default layout."))
+            self.buttonGroup.addEntry("freespace", _("Use Free Space"),
+                                      pixmap=gui.readImageFromFile("partscheme-freespace.png"),
+                                      descr=_("Retains your current data and partitions and "
+                                              "uses only the unpartitioned space on the "
+                                              "selected device(s), assuming you have enough "
+                                              "free space available."))
+            self.buttonGroup.addEntry("custom", _("Create Custom Layout"),
+                                      pixmap=gui.readImageFromFile("partscheme-custom.png"),
+                                      descr=_("Manually create your own custom layout on "
+                                              "the selected device(s) using our partitioning "
+                                              "tool."))

          self.buttonGroup.setToggleCallback(self.typeChanged)

@@ -278,7 +297,9 @@ class PartitionTypeWindow(InstallWindow):
          self.table.attach(widget, 0, 1, 1, 2)

          # if not set in ks, use UI default
-        if self.storage.clearPartChoice:
+        if onlyUseAllSpace:
+                self.buttonGroup.setCurrent("all")
+        elif self.storage.clearPartChoice:
              self.buttonGroup.setCurrent(self.storage.clearPartChoice)
          else:
              if self.storage.clearPartType is None or self.storage.clearPartType == CLEARPART_TYPE_LINUX:
diff --git a/iw/bootloader_main_gui.py b/iw/bootloader_main_gui.py
index fa5d554..fdb40d1 100644
--- a/iw/bootloader_main_gui.py
+++ b/iw/bootloader_main_gui.py
@@ -22,6 +22,7 @@
  import gtk
  import gobject
  import gui
+import iutil
  from iw_gui import *
  from constants import *

@@ -137,6 +138,10 @@ class MainBootloaderWindow(InstallWindow):
              partitioned = anaconda.id.storage.partitioned
              disks = anaconda.id.storage.disks
              bl_disks = [d for d in disks if d in partitioned]
+
+            # Filter out GPT disks on non-EFI systems
+            bl_disks = filter(iutil.isDiskBootable, bl_disks)
+
              m = __genStore(combo, bl_disks, self.driveorder[i - 1])

          dxml.get_widget("bd1Combo").connect("changed", __driveChange, dxml, choices)
@@ -197,6 +202,9 @@ class MainBootloaderWindow(InstallWindow):
              disks = anaconda.id.storage.disks
              self.driveorder = [d.name for d in disks if d in partitioned]

+        # Filter out the GPT drives on non-EFI systems
+        self.driveorder = filter(lambda d: iutil.isDiskBootable(anaconda.id.storage.devicetree.getDeviceByName(d)), self.driveorder)
+
          if self.bl.getPassword():
              self.usePass = 1
              self.password = self.bl.getPassword()
diff --git a/iw/cleardisks_gui.py b/iw/cleardisks_gui.py
index 926f613..af64f9a 100644
--- a/iw/cleardisks_gui.py
+++ b/iw/cleardisks_gui.py
@@ -23,6 +23,7 @@ import gui
  from DeviceSelector import *
  from constants import *
  import isys
+import iutil
  from iw_gui import *
  from storage.devices import deviceNameToDiskByPath
  from storage.udev import *
@@ -150,8 +151,11 @@ class ClearDisksWindow (InstallWindow):

          # Store the first disk (according to our detected BIOS order) for
          # auto boot device selection
-        names = map(lambda d: d.name, disks)
-        self.bootDisk = sorted(names, self.anaconda.id.storage.compareDisks)[0]
+        names = map(lambda d: d.name, filter(iutil.isDiskBootable, disks))
+        if len(names)>  0:
+            self.bootDisk = sorted(names, self.anaconda.id.storage.compareDisks)[0]
+        else:
+            self.bootDisk = None

          # The device filtering UI set up exclusiveDisks as a list of the names
          # of all the disks we should use later on.  Now we need to go get those,
@@ -173,8 +177,10 @@ class ClearDisksWindow (InstallWindow):
                  except DeviceNotFoundError:
                      ident = d.name

+            # Don't allow GPT partitions on non-EFI systems to be bootable
+            immutable = not iutil.isDiskBootable(d)
              self.store.append(None, (d,
-                                     leftVisible, True, False,
+                                     leftVisible, True, immutable,
                                       rightVisible, rightActive,
                                       d.model,
                                       str(int(d.size)) + " MB",


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]