--- a/usr/src/lib/install_target/physical.py	Fri May 27 12:13:10 2011 +0200
+++ b/usr/src/lib/install_target/physical.py	Fri May 27 14:20:49 2011 -0600
@@ -39,7 +39,8 @@
 from solaris_install.logger import INSTALL_LOGGER_NAME as ILN
 from solaris_install.target.libadm import const, cstruct, extvtoc
 from solaris_install.target.libdiskmgt import const as ldm_const
-from solaris_install.target.shadow.physical import ShadowPhysical
+from solaris_install.target.shadow.physical import LOGICAL_ADJUSTMENT, \
+    ShadowPhysical
 from solaris_install.target.size import Size
 
 FDISK = "/usr/sbin/fdisk"
@@ -186,7 +187,8 @@
 
         # re-insert it with the new size
         resized_partition = self.parent.add_partition(self.name, start_sector,
-            size, size_units, self.part_type, self.in_zpool, self.in_vdev)
+            size, size_units=size_units, partition_type=self.part_type,
+            bootid=self.bootid, in_zpool=self.in_zpool, in_vdev=self.in_vdev)
 
         return resized_partition
 
@@ -199,12 +201,12 @@
 
         # the size of the partition is already in sectors, so use that as the
         # units.
-        self.parent.add_partition(self.name, self.start_sector,
-                                  self.size.sectors, Size.sector_units,
-                                  new_type, self.in_zpool, self.in_vdev)
+        new_partition = self.parent.add_partition(self.name, self.start_sector,
+            self.size.sectors, size_units=Size.sector_units,
+            partition_type=new_type, bootid=self.bootid,
+            in_zpool=self.in_zpool, in_vdev=self.in_vdev)
 
-        # update the attributes
-        self.part_type = new_type
+        return new_partition
 
     def change_bootid(self, new_bootid):
         """ change_bootid() - method to change the partition's bootid
@@ -219,13 +221,12 @@
 
         # the size of the partition is already in sectors, so use that as the
         # units.
-        self.parent.add_partition(self.name, self.start_sector,
-                                  self.size.sectors, Size.sector_units,
-                                  self.part_type, new_bootid, self.in_zpool,
-                                  self.in_vdev)
+        new_partition = self.parent.add_partition(self.name, self.start_sector,
+            self.size.sectors, size_units=Size.sector_units,
+            partition_type=self.part_type, bootid=new_bootid,
+            in_zpool=self.in_zpool, in_vdev=self.in_vdev)
 
-        # update the attributes
-        self.bootid = new_bootid
+        return new_partition
 
     def get_gaps(self):
         """ get_gaps() - method to return a list of Holey Objects
@@ -250,14 +251,7 @@
         # sort the usage list and add bookends
         usage.sort()
         usage.insert(0, 0L)
-        # due to the possiblity that the size of the disk was artifically
-        # decreased by 3 cylinders, if the last entry in the usage list is
-        # larger than the size of the partition, append the last entry again to
-        # create a gap of 0 sectors
-        if usage[-1] > self.size.sectors:
-            usage.append(usage[-1])
-        else:
-            usage.append(self.size.sectors)
+        usage.append(self.size.sectors)
 
         holes = list()
         i = 0
@@ -776,15 +770,7 @@
         # sort the usage list and add bookends
         usage.sort()
         usage.insert(0, 0L)
-
-        # due to the possiblity that the size of the disk was artifically
-        # decreased by 3 cylinders, if the last entry in the usage list is
-        # larger than the size of the disk, append the last entry again to
-        # create a gap of 0 sectors
-        if usage[-1] > self.disk_prop.dev_size.sectors:
-            usage.append(usage[-1])
-        else:
-            usage.append(self.disk_prop.dev_size.sectors)
+        usage.append(self.disk_prop.dev_size.sectors)
 
         holes = list()
         i = 0
@@ -838,16 +824,7 @@
         # sort the usage list and add bookends
         usage.sort()
         usage.insert(0, extended_part.start_sector)
-
-        # due to the possiblity that the size of the disk was artifically
-        # decreased by 3 cylinders, if the last entry in the usage list is
-        # larger than the size of the extended partition, append the last entry
-        # again to create a gap of 0 sectors
-        if usage[-1] > extended_part.start_sector + extended_part.size.sectors:
-            usage.append(usage[-1])
-        else:
-            usage.append(extended_part.start_sector + \
-                         extended_part.size.sectors)
+        usage.append(extended_part.start_sector + extended_part.size.sectors)
 
         holes = list()
         i = 0
@@ -855,16 +832,14 @@
             # subtract i+1 to get the size of this hole.
             size = usage[i + 1] - usage[i]
 
-            # if the size is 0 (for a starting sector of 0) or 1 (for adjacent
-            # children), there's no gap, so skip it
-            if size not in [0, 1]:
-                # do not correct for holes that start at 0
-                if usage[i] == 0:
-                    holes.append(HoleyObject(
-                        usage[i], Size(str(size - 1) + Size.sector_units)))
-                else:
-                    holes.append(HoleyObject(
-                        usage[i] + 1, Size(str(size - 1) + Size.sector_units)))
+            # if the size is equal to the logical partition offset, or the gap
+            # is smaller than the offset there's no gap, so skip it.
+            if size > LOGICAL_ADJUSTMENT:
+                # set the start_sector of the hole to include the offset.
+                start_sector = usage[i] + LOGICAL_ADJUSTMENT
+                size_obj = Size(str(size - 1 - LOGICAL_ADJUSTMENT) + \
+                                Size.sector_units)
+                holes.append(HoleyObject(start_sector, size_obj))
 
             # step across the size of the child
             i += 2

--- a/usr/src/lib/install_target/shadow/physical.py	Fri May 27 12:13:10 2011 +0200
+++ b/usr/src/lib/install_target/shadow/physical.py	Fri May 27 14:20:49 2011 -0600
@@ -32,6 +32,8 @@
 from solaris_install.target.shadow import ShadowList, ShadowExceptionBase
 from solaris_install.target.size import Size
 
+LOGICAL_ADJUSTMENT = 63
+
 
 class ShadowPhysical(ShadowList):
     """ ShadowPhysical - class to hold and validate Physical objects (Partition
@@ -80,10 +82,6 @@
             self.value = "Logical partition exceeds the start or end " + \
                          "sector of the extended partition"
 
-    class PartitionTooLargeError(ShadowExceptionBase):
-        def __init__(self):
-            self.value = "Partition is too large for disk"
-
     class DuplicatePartitionNameError(ShadowExceptionBase):
         def __init__(self, partition_name):
             self.value = "Partition name %s already inserted" % partition_name
@@ -322,8 +320,10 @@
         if value.part_type is None:
             self.set_error(self.PartitionTypeMissingError())
 
-        # fix the start_sector and size to align to cylinder boundaries
-        value = self.cylinder_boundary_adjustment(value)
+        # fix the start_sector and size to align to cylinder boundaries for
+        # primary partitions
+        if value.is_primary:
+            value = self.cylinder_boundary_adjustment(value)
 
         # check the bounds of the partition to be added.
         if not (isinstance(value.start_sector, int) or \
@@ -344,18 +344,56 @@
         # with part_type in Partition.EXTENDED_ID_LIST has already been
         # inserted.
         if value.is_logical:
-            found = False
+            extended_part = None
             for partition in self._shadow:
                 if partition.is_extended:
                     if partition.part_type in partition.EXTENDED_ID_LIST:
-                        found = True
-            if not found:
+                        extended_part = partition
+
+            if extended_part is None:
                 self.set_error(self.NoExtPartitionsError())
+            else:
+                # verify there are not more than MAX_EXT_PARTS
+                logical_list = [p for p in self._shadow if p.is_logical and \
+                                p.action != "delete"]
+                if len(logical_list) >= MAX_EXT_PARTS:
+                    self.set_error(self.TooManyLogicalPartitionsError())
+
+                # ensure this logical partition does not start too close to any
+                # previously inserted logical partitions.
+
+                # sort the logical list by start sector
+                slist = sorted(logical_list,
+                    lambda x, y: cmp(x.start_sector, y.start_sector))
 
-            # verify there are not more than MAX_EXT_PARTS
-            logical_list = [p for p in self._shadow if p.is_logical]
-            if len(logical_list) >= MAX_EXT_PARTS:
-                self.set_error(self.TooManyLogicalPartitionsError())
+                # find the closest logical partition within the extended
+                # partition
+                closest_endpoint = 0
+                for logical in slist:
+                    end_point = logical.start_sector + logical.size.sectors
+                    if end_point < value.start_sector and \
+                       end_point > closest_endpoint:
+                        closest_endpoint = end_point
+
+                if closest_endpoint == 0:
+                    # no logical partitions were found, so use the start of the
+                    # extended partition, if the difference is smaller than the
+                    # needed offset
+                    diff = value.start_sector - extended_part.start_sector
+                    if diff < LOGICAL_ADJUSTMENT and value.action == "create":
+                        value.start_sector = extended_part.start_sector + \
+                                             LOGICAL_ADJUSTMENT
+                        new_size = value.size.sectors - LOGICAL_ADJUSTMENT
+                        value.size = Size(str(new_size) + Size.sector_units)
+                else:
+                    diff = value.start_sector - closest_endpoint
+                    # make sure there's at least 63 sectors between logical
+                    # partitions
+                    if diff < LOGICAL_ADJUSTMENT and value.action == "create":
+                        value.start_sector += LOGICAL_ADJUSTMENT - diff
+
+                        new_size = value.size.sectors - LOGICAL_ADJUSTMENT
+                        value.size = Size(str(new_size) + Size.sector_units)
 
         # check the bootid attibute on primary partitions for multiple active
         # partitions
@@ -377,8 +415,14 @@
         # partition we're trying to insert doesn't cross boundaries.
         for partition in self._shadow:
             # start and end points of the partition to check
-            start = partition.start_sector
-            end = start + partition.size.sectors - 1
+            if partition.is_primary:
+                start = partition.start_sector
+                end = start + partition.size.sectors - 1
+            else:
+                # for logical partitions, there needs to be a buffer of
+                # LOGICAL_ADJUSTMENT on each end
+                start = partition.start_sector - LOGICAL_ADJUSTMENT
+                end = start + partition.size.sectors - 1 + LOGICAL_ADJUSTMENT
 
             if value.is_primary:
                 # do not test logical partition boundaries for primary
@@ -422,13 +466,13 @@
             self.set_error(self.DuplicatePartitionNameError(value.name))
 
         # if this is an extended partition, verify there are no other
-        # partitions of the same type.  Also verify it's at least 63 sectors in
-        # size
+        # partitions of the same type.  Also verify it's at least
+        # LOGICAL_ADJUSTMENT sectors in size
         if value.is_extended:
             for partition in self._shadow:
                 if partition.is_extended and partition.action != "delete":
                     self.set_error(self.TooManyExtPartitionsError())
-            if value.size.sectors < 63:
+            if value.size.sectors < LOGICAL_ADJUSTMENT:
                 self.set_error(self.ExtPartitionTooSmallError())
 
         # if the partition type is FAT32, make sure it's not larger than 4GB
@@ -498,6 +542,10 @@
 
         value - DOC object to adjust
         """
+        # only make adjustments when the action is 'create'
+        if value.action != "create":
+            return value
+
         # determine the cylsize based on the container object
         if hasattr(self.container, "geometry"):
             # container is a Disk object
@@ -545,7 +593,6 @@
                                cyl_boundary
                 value.size = Size(str(end_cylinder) + Size.sector_units)
         
-
         return value
 
     def __init__(self, container, *args):

--- a/usr/src/lib/install_target/test/test_shadow_list.py	Fri May 27 12:13:10 2011 +0200
+++ b/usr/src/lib/install_target/test/test_shadow_list.py	Fri May 27 14:20:49 2011 -0600
@@ -42,7 +42,8 @@
     Slice, Partition
 from solaris_install.target.libadm.const import MAX_EXT_PARTS, V_NUMPAR
 from solaris_install.target.logical import BE, Logical, Vdev, Zpool
-from solaris_install.target.shadow.physical import ShadowPhysical
+from solaris_install.target.shadow.physical import LOGICAL_ADJUSTMENT, \
+    ShadowPhysical
 from solaris_install.target.shadow.logical import ShadowLogical
 from solaris_install.target.shadow.zpool import ShadowZpool
 from solaris_install.target.size import Size
@@ -578,9 +579,9 @@
         self.assertFalse(errsvc._ERRORS)
         self.assertEqual(p.part_type, Partition.name_to_num("Solaris2"))
 
-        p.change_type(Partition.name_to_num("WIN95 Extended(LBA)"))
+        new_p = p.change_type(Partition.name_to_num("WIN95 Extended(LBA)"))
         self.assertFalse(errsvc._ERRORS)
-        self.assertEqual(p.part_type, \
+        self.assertEqual(new_p.part_type, \
             Partition.name_to_num("WIN95 Extended(LBA)"))
 
     def test_extended_partition_too_small(self):
@@ -658,10 +659,11 @@
     def test_holey_object_logical_partitions(self):
         # add a single extended partition
         extended_part = self.disk.add_partition(1, CYLSIZE, 25, Size.gb_units,
-                                        partition_type=15)
+            partition_type=15)
 
         # add a single logical partition
         logical_part = self.disk.add_partition(5, CYLSIZE, 1, Size.gb_units)
+        self.assertFalse(errsvc._ERRORS)
 
         disksize = self.disk.disk_prop.dev_size.sectors
 
@@ -678,9 +680,11 @@
             disksize - extended_part.size.sectors - CYLSIZE - 1)
 
         self.assertEqual(logical_holey_list[0].start_sector, \
-            CYLSIZE + logical_part.size.sectors + 1)
-        self.assertEqual(logical_holey_list[0].size.sectors, \
-            extended_part.size.sectors - logical_part.size.sectors - 1)
+            extended_part.start_sector + LOGICAL_ADJUSTMENT + \
+            logical_part.size.sectors + LOGICAL_ADJUSTMENT)
+        self.assertEqual(logical_holey_list[0].size.sectors,
+            extended_part.size.sectors - (2 * LOGICAL_ADJUSTMENT) - \
+                logical_part.size.sectors - 1)
 
     def test_add_two_active_partitions(self):
         self.disk.add_partition(1, 0, 5, Size.gb_units,
@@ -1061,8 +1065,9 @@
         # verify there are no errors
         self.assertFalse(errsvc._ERRORS)
 
-        # verify the size of the slice, rounding for cylinder size
-        self.assertEqual(disksize / CYLSIZE * CYLSIZE, s.size.sectors)
+        # verify the size of the slice, rounding for cylinder size and
+        # maximum size limits
+        self.assertEqual(((disksize / CYLSIZE) - 4) * CYLSIZE, s.size.sectors)
 
     def test_no_validate_children(self):
         self.disk.validate_children = False
@@ -1326,6 +1331,62 @@
         self.disk.add_partition(5, 0, 1, Size.gb_units)
         self.assertFalse(errsvc._ERRORS)
 
+    def test_add_three_logical_partitions_in_order(self):
+        # add a 10 GB extended partition (type is 0xf or "15")
+        ep = self.disk.add_partition(1, CYLSIZE, 10, Size.gb_units,
+            Partition.name_to_num("WIN95 Extended(LBA)"))
+        self.assertFalse(errsvc._ERRORS)
+
+        # add a logical partition to the disk
+        l1 = self.disk.add_partition(5, CYLSIZE, 1, Size.gb_units)
+        self.assertFalse(errsvc._ERRORS)
+        self.assertEqual(l1.start_sector, ep.start_sector + LOGICAL_ADJUSTMENT)
+        self.assertEqual(l1.size.sectors, GBSECTOR - LOGICAL_ADJUSTMENT)
+
+        # add a second logical partition to the disk
+        l2 = self.disk.add_partition(6, CYLSIZE + GBSECTOR + 64, 1,
+                                     Size.gb_units)
+        self.assertFalse(errsvc._ERRORS)
+        self.assertEqual(l2.start_sector, \
+            l1.start_sector + l1.size.sectors + LOGICAL_ADJUSTMENT + 1)
+        self.assertEqual(l2.size.sectors, GBSECTOR)
+
+        # add a third logical partition to the disk
+        l3 = self.disk.add_partition(8, CYLSIZE + (GBSECTOR * 2) + 128, 1,
+                                     Size.gb_units)
+        self.assertFalse(errsvc._ERRORS)
+        self.assertEqual(l3.start_sector, \
+            l2.start_sector + l2.size.sectors + LOGICAL_ADJUSTMENT + 1)
+        self.assertEqual(l3.size.sectors, GBSECTOR)
+
+    def test_add_three_logical_partitions_out_of_order(self):
+        # add a 10 GB extended partition (type is 0xf or "15")
+        ep = self.disk.add_partition(1, CYLSIZE, 10, Size.gb_units,
+            Partition.name_to_num("WIN95 Extended(LBA)"))
+        self.assertFalse(errsvc._ERRORS)
+
+        # add a logical partition to the disk (at the 'end')
+        l1 = self.disk.add_partition(5, CYLSIZE + (GBSECTOR * 2) + 128, 1,
+                                     Size.gb_units)
+        self.assertFalse(errsvc._ERRORS)
+        self.assertEqual(l1.start_sector, CYLSIZE + (GBSECTOR * 2) + 128)
+        self.assertEqual(l1.size.sectors, GBSECTOR)
+
+        # add a second logical partition to the disk (at the 'beginning')
+        l2 = self.disk.add_partition(6, CYLSIZE, 1, Size.gb_units)
+        self.assertFalse(errsvc._ERRORS)
+        self.assertEqual(l2.start_sector, ep.start_sector + LOGICAL_ADJUSTMENT)
+        self.assertEqual(l2.size.sectors, GBSECTOR - LOGICAL_ADJUSTMENT)
+
+        # add a third logical partition to the disk (in the 'middle')
+        l3 = self.disk.add_partition(8, CYLSIZE + GBSECTOR + 64, 1,
+                                     Size.gb_units)
+        self.assertFalse(errsvc._ERRORS)
+        self.assertEqual(l3.start_sector, \
+            l2.start_sector + l2.size.sectors + LOGICAL_ADJUSTMENT + 1)
+        self.assertEqual(l3.size.sectors, GBSECTOR)
+
+
     def test_add_logical_without_extended(self):
         # add a logical partition to the disk
         self.disk.add_partition(5, 0, 1, Size.gb_units)
@@ -1337,23 +1398,6 @@
         self.assertTrue(isinstance(error.error_data[ES_DATA_EXCEPTION],
                                    ShadowPhysical.NoExtPartitionsError))
 
-    def test_add_logical_outside_extended_partition(self):
-        # add a 10 GB extended partition (type is 0xf or "15")
-        self.disk.add_partition(1, 0, 10, Size.gb_units,
-                                Partition.name_to_num("WIN95 Extended(LBA)"))
-        self.assertFalse(errsvc._ERRORS)
-
-        # add a logical partition to the disk outside of the extended partition
-        # boundary
-        self.disk.add_partition(5, 20 * GBSECTOR, 1, Size.gb_units)
-
-        # verify there is only one error in the errsvc list and that it is the
-        # proper error
-        self.assertEqual(len(errsvc._ERRORS), 1)
-        error = errsvc._ERRORS[0]
-        self.assertTrue(isinstance(error.error_data[ES_DATA_EXCEPTION],
-            ShadowPhysical.LogicalPartitionOverlapError))
-
     def test_add_too_many_logical_partitions(self):
         # add a 50 GB extended partition (type is 0xf or "15")
         self.disk.add_partition(1, 0, 50, Size.gb_units,