Attached are the two patches I have come up with. They are self
described. I have already been asked by Colin Watson to clean it up a
bit so the coding style conforms ot the surrounding code, which I will
be doing tomorrow. Please let me know what you think about them.

I have done some preliminary testing both using parted and gparted
consiting of having parted add a new partition to the tail end of my
disk with an active lvm partition holding my root volume. I had to use
-a none and switch to sector units to get parted to let me create a
partition in the last cyllinder. I was able to remove the partition with
parted or gparted without warning. Mounting the partion resulted in
parted throwing the warning when trying to delete it, and gparted
showing the locked icon. I believe that it will correctly handle the
case of removing an extended partition causing renumbering because when
it tries to add renumbered partition to the lower slot, it will fail and
throw the warning telling the user to reboot.

From: Phillip Susi <ps...@cfl.rr.com>
Description: Upstream removed the code using the new BLKPG
  ioctls instead of the old BLKRRPART ioctl to update the
  in-kernel partition table.  This patch reverses that change.
  The reason for this is because BLKRRPART fails if any
  partition on the disk is in use, but the BLKPG ioctls allow
  you to manipulate the other partitions on the disk without
  requiring a reboot.  Also BLKRRPART requires that the kernel
  understand the partition table on the disk, which may not
  always be the case.
Index: parted-2.2/libparted/arch/linux.c
===================================================================
--- parted-2.2.orig/libparted/arch/linux.c	2010-03-23 11:36:28.000000000 -0400
+++ parted-2.2/libparted/arch/linux.c	2010-03-23 11:42:58.000000000 -0400
@@ -19,7 +19,7 @@
 
 #include <config.h>
 #include <arch/linux.h>
-
+#include <linux/blkpg.h>
 #include <parted/parted.h>
 #include <parted/debug.h>
 #if defined __s390__ || defined __s390x__
@@ -2299,6 +2299,176 @@
         return 0;
 }
 
+static int
+_blkpg_part_command (PedDevice* dev, struct blkpg_partition* part, int op)
+{
+        LinuxSpecific*          arch_specific = LINUX_SPECIFIC (dev);
+        struct blkpg_ioctl_arg  ioctl_arg;
+
+        ioctl_arg.op = op;
+        ioctl_arg.flags = 0;
+        ioctl_arg.datalen = sizeof (struct blkpg_partition);
+        ioctl_arg.data = (void*) part;
+
+        return ioctl (arch_specific->fd, BLKPG, &ioctl_arg) == 0;
+}
+
+static int
+_blkpg_add_partition (PedDisk* disk, const PedPartition *part)
+{
+        struct blkpg_partition  linux_part;
+        const char*             vol_name;
+        char*                   dev_name;
+
+        PED_ASSERT(disk != NULL, return 0);
+        PED_ASSERT(disk->dev->sector_size % PED_SECTOR_SIZE_DEFAULT == 0,
+                   return 0);
+
+        if (ped_disk_type_check_feature (disk->type,
+                                         PED_DISK_TYPE_PARTITION_NAME))
+                vol_name = ped_partition_get_name (part);
+        else
+                vol_name = NULL;
+
+        dev_name = _device_get_part_path (disk->dev, part->num);
+        if (!dev_name)
+                return 0;
+
+        memset (&linux_part, 0, sizeof (linux_part));
+        linux_part.start = part->geom.start * disk->dev->sector_size;
+        /* see fs/partitions/msdos.c:msdos_partition(): "leave room for LILO" */
+        if (part->type & PED_PARTITION_EXTENDED)
+                linux_part.length = part->geom.length == 1 ? 512 : 1024;
+        else
+                linux_part.length = part->geom.length * disk->dev->sector_size;
+        linux_part.pno = part->num;
+        strncpy (linux_part.devname, dev_name, BLKPG_DEVNAMELTH);
+        if (vol_name)
+                strncpy (linux_part.volname, vol_name, BLKPG_VOLNAMELTH);
+
+        free (dev_name);
+
+        if (!_blkpg_part_command (disk->dev, &linux_part,
+                                  BLKPG_ADD_PARTITION)) {
+                return ped_exception_throw (
+                        PED_EXCEPTION_ERROR,
+                        PED_EXCEPTION_IGNORE_CANCEL,
+                        _("Error informing the kernel about modifications to "
+                          "partition %s -- %s.  This means Linux won't know "
+                          "about any changes you made to %s until you reboot "
+                          "-- so you shouldn't mount it or use it in any way "
+                          "before rebooting."),
+                        linux_part.devname,
+                        strerror (errno),
+                        linux_part.devname)
+                                == PED_EXCEPTION_IGNORE;
+        }
+
+        return 1;
+}
+
+static int
+_blkpg_remove_partition (PedDisk* disk, int n)
+{
+        struct blkpg_partition  linux_part;
+
+        memset (&linux_part, 0, sizeof (linux_part));
+        linux_part.pno = n;
+        return _blkpg_part_command (disk->dev, &linux_part,
+                                    BLKPG_DEL_PARTITION);
+}
+
+/*
+ * The number of partitions that a device can have depends on the kernel.
+ * If we don't find this value in /sys/block/DEV/range, we will use our own
+ * value.
+ */
+static unsigned int
+_device_get_partition_range(PedDevice* dev)
+{
+        int         range, r;
+        char        path[128];
+        FILE*       fp;
+        bool        ok;
+
+        r = snprintf(path, sizeof(path), "/sys/block/%s/range",
+                     last_component(dev->path));
+        if(r < 0 || r >= sizeof(path))
+                return MAX_NUM_PARTS;
+
+        fp = fopen(path, "r");
+        if(!fp)
+                return MAX_NUM_PARTS;
+
+        ok = fscanf(fp, "%d", &range) == 1;
+        fclose(fp);
+
+        /* (range <= 0) is none sense.*/
+        return ok && range > 0 ? range : MAX_NUM_PARTS;
+}
+
+/*
+ * Sync the partition table in two step process:
+ * 1. Remove all of the partitions from the kernel's tables, but do not attempt
+ *    removal of any partition for which the corresponding ioctl call fails.
+ * 2. Add all the partitions that we hold in disk.
+ *
+ * To achieve this two step process we must calculate the minimum number of
+ * maximum possible partitions between what linux supports and what the label
+ * type supports. EX:
+ *
+ * number=MIN(max_parts_supported_in_linux,max_parts_supported_in_msdos_tables)
+ */
+static int
+_disk_sync_part_table (PedDisk* disk)
+{
+        PED_ASSERT(disk != NULL, return 0);
+        PED_ASSERT(disk->dev != NULL, return 0);
+        int lpn;
+
+        /* lpn = largest partition number. */
+        if(ped_disk_get_max_supported_partition_count(disk, &lpn))
+                lpn = PED_MIN(lpn, _device_get_partition_range(disk->dev));
+        else
+                lpn = _device_get_partition_range(disk->dev);
+
+        /* Its not possible to support largest_partnum < 0.
+         * largest_partnum == 0 would mean does not support partitions.
+         * */
+        if(lpn < 0)
+                return 0;
+
+        int *rets = ped_malloc(sizeof(int) * lpn);
+        int *errnums = ped_malloc(sizeof(int) * lpn);
+        int ret = 1;
+        int i;
+
+        for (i = 1; i <= lpn; i++) {
+                rets[i - 1] = _blkpg_remove_partition (disk, i);
+                errnums[i - 1] = errno;
+        }
+
+        for (i = 1; i <= lpn; i++) {
+                const PedPartition *part = ped_disk_get_partition (disk, i);
+                if (part) {
+                        /* busy... so we won't (can't!) disturb ;)  Prolly
+                         * doesn't matter anyway, because users shouldn't be
+                         * changing mounted partitions anyway...
+                         */
+                        if (!rets[i - 1] && errnums[i - 1] == EBUSY)
+                                        continue;
+
+                        /* add the (possibly modified or new) partition */
+                        if (!_blkpg_add_partition (disk, part))
+                                ret = 0;
+                }
+        }
+
+        free (rets);
+        free (errnums);
+        return ret;
+}
+
 #ifdef ENABLE_DEVICE_MAPPER
 static int
 _dm_remove_map_name(char *name)
@@ -2546,16 +2716,39 @@
 }
 
 static int
-linux_disk_commit (PedDisk* disk)
+_have_blkpg ()
 {
-       if (!_has_partitions (disk))
-               return 1;
+        static int have_blkpg = -1;
+        int kver;
+
+        if (have_blkpg != -1)
+                return have_blkpg;
+
+        kver = _get_linux_version();
+        return have_blkpg = kver >= KERNEL_VERSION (2,4,0) ? 1 : 0;
+}
 
+static int
+linux_disk_commit (PedDisk* disk)
+{
 #ifdef ENABLE_DEVICE_MAPPER
         if (disk->dev->type == PED_DEVICE_DM)
                 return _dm_reread_part_table (disk);
 #endif
         if (disk->dev->type != PED_DEVICE_FILE) {
+                /* The ioctl() command BLKPG_ADD_PARTITION does not notify
+                 * the devfs system; consequently, /proc/partitions will not
+                 * be up to date, and the proper links in /dev are not
+                 * created.  Therefore, if using DevFS, we must get the kernel
+                 * to re-read and grok the partition table.
+                 */
+                /* Work around kernel dasd problem so we really do BLKRRPART */
+                if (disk->dev->type != PED_DEVICE_DASD &&
+                    _have_blkpg () ) {
+                        if (_disk_sync_part_table (disk))
+                                return 1;
+                }
+
                 return _kernel_reread_part_table (disk->dev);
         }
 
From: Phillip Susi <ps...@cfl.rr.com>
Description: In the event that some partitions are in use,
  they can not be modified in the running kernel usign BLKPG.
  Warn the user that this is the case and advise them to reboot,
  just like we do when BLKRRPART fails for the same reason, unless
  the partition in question is unchanged.
Index: parted-2.2/libparted/arch/linux.c
===================================================================
--- parted-2.2.orig/libparted/arch/linux.c	2010-03-25 20:19:29.407917597 -0400
+++ parted-2.2/libparted/arch/linux.c	2010-03-25 20:22:08.487917004 -0400
@@ -2411,7 +2411,9 @@
  * Sync the partition table in two step process:
  * 1. Remove all of the partitions from the kernel's tables, but do not attempt
  *    removal of any partition for which the corresponding ioctl call fails.
- * 2. Add all the partitions that we hold in disk.
+ * 2. Add all the partitions that we hold in disk, throwing a warning
+ *    if we can not beacuse step 1 failed to remove it and it is not being
+ *    added back with the same start and length.
  *
  * To achieve this two step process we must calculate the minimum number of
  * maximum possible partitions between what linux supports and what the label
@@ -2444,24 +2446,46 @@
         int i;
 
         for (i = 1; i <= lpn; i++) {
-                rets[i - 1] = _blkpg_remove_partition (disk, i);
-                errnums[i - 1] = errno;
+		  rets[i - 1] = _blkpg_remove_partition (disk, i);
+		  errnums[i - 1] = errno;
         }
 
         for (i = 1; i <= lpn; i++) {
-                const PedPartition *part = ped_disk_get_partition (disk, i);
-                if (part) {
-                        /* busy... so we won't (can't!) disturb ;)  Prolly
-                         * doesn't matter anyway, because users shouldn't be
-                         * changing mounted partitions anyway...
-                         */
-                        if (!rets[i - 1] && errnums[i - 1] == EBUSY)
-                                        continue;
-
-                        /* add the (possibly modified or new) partition */
-                        if (!_blkpg_add_partition (disk, part))
-                                ret = 0;
-                }
+		  const PedPartition *part = ped_disk_get_partition (disk, i);
+		  if (part) {
+			if (!rets[i - 1] && errnums[i - 1] == EBUSY)
+			  {
+				struct hd_geometry geom;
+				unsigned long long length;
+				int fd;
+				char *dev_name;
+				
+				memset( &geom, 0, sizeof( geom ) );
+				length = 0;
+				/* get start and length of existing partition */
+				dev_name = _device_get_part_path (disk->dev, i);
+				fd = open( dev_name, O_RDONLY );
+				ioctl( fd, HDIO_GETGEO, &geom );
+				ioctl( fd, BLKGETSIZE64, &length );
+				length /= disk->dev->sector_size;
+				close( fd );
+				if( geom.start != part->geom.start ||
+					length != part->geom.length )
+				  ped_exception_throw(
+					PED_EXCEPTION_WARNING,
+					PED_EXCEPTION_IGNORE,
+					_("WARNING: partition %d on %s could not be modified, probably "
+					  "because it is in use.  As a result, the old partition "
+					  "will remain in use until after reboot. You should reboot "
+					  "now before making further changes."),
+					i, disk->dev->path);
+				continue;
+			  }
+			
+			/* add the (possibly modified or new) partition */
+			if (!_blkpg_add_partition (disk, part))
+			  ret = 0;
+		  }
         }
 
         free (rets);
_______________________________________________
parted-devel mailing list
parted-devel@lists.alioth.debian.org
http://lists.alioth.debian.org/mailman/listinfo/parted-devel

Reply via email to