[parted-devel] patch for GPT problems

David Cantrell dcantrell at redhat.com
Thu Jun 22 18:07:53 UTC 2006


Attached is a patch against parted-1.7.1 that addresses a problem I'm
seeing on GPT-labeled disks.  The disks are gigantic in capacity and
have 2048 byte sector sizes.  The problem is simple to recreate:

1) Run parted on the disk in question. Blank it out with a new GPT
label.

2) Create a new fat16 partition starting at 1 and running to 100.  The
partition actually starts at the 17kb boundary because of the GPT label.
Leave parted running.

3) In another terminal, mount this new partition and create a directory:

   mount /dev/hdX1 /mnt
   mkdir /mnt/testdir
   sync

4) Go back to the running parted and create a new partition running from
100 to 200 (or whatever).  Exit parted.

5) Go back to the other terminal, umount the partition, remount:

   umount /mnt
   mount -t vfat /dev/hdX1 /mnt
   ls -l /mnt

With any version of parted with GPT support, testdir will be gone and
the ls output will show question marks for all values.  Why does this
happen?  The idea I've been working with is parted is reading and
writing the first 24kb of the disk when it modifies the GPT label,
rather than the first 17kb.  This destroys the beginning of the first
partition since it starts at the 17kb boundary.

The fix?  Well, using O_DIRECT in linux.c solves the problem.  So here's
the patch, attached to this email.

You need to compile with -D_GNU_SOURCE as a gcc option, or define
_GNU_SOURCE in linux.c.  If you don't, O_DIRECT is undefined.

-- 
David Cantrell
Red Hat / Westford, MA
-------------- next part --------------
diff -urN parted-1.7.1.orig/libparted/arch/linux.c parted-1.7.1/libparted/arch/linux.c
--- parted-1.7.1.orig/libparted/arch/linux.c	2006-05-25 13:29:06.000000000 -0400
+++ parted-1.7.1/libparted/arch/linux.c	2006-06-22 13:58:52.000000000 -0400
@@ -29,6 +29,7 @@
 #include <libgen.h>
 #include <stdint.h>
 #include <stdio.h>
+#include <stdlib.h>
 #include <string.h>
 #include <syscall.h>
 #include <unistd.h>
@@ -1013,7 +1014,7 @@
                 if (!name)
                         break;
                 if (!_partition_is_mounted_by_path (name)) {
-                        fd = open (name, O_WRONLY, 0);
+                        fd = open (name, O_WRONLY | O_DIRECT, 0);
                         if (fd > 0) {
                                 ioctl (fd, BLKFLSBUF);
                                 close (fd);
@@ -1029,11 +1030,11 @@
         LinuxSpecific*  arch_specific = LINUX_SPECIFIC (dev);
 
 retry:
-        arch_specific->fd = open (dev->path, O_RDWR);
+        arch_specific->fd = open (dev->path, O_RDWR | O_DIRECT);
         if (arch_specific->fd == -1) {
                 char*   rw_error_msg = strerror (errno);
 
-                arch_specific->fd = open (dev->path, O_RDONLY);
+                arch_specific->fd = open (dev->path, O_RDONLY | O_DIRECT);
                 if (arch_specific->fd == -1) {
                         if (ped_exception_throw (
                                 PED_EXCEPTION_ERROR,
@@ -1176,6 +1177,7 @@
         int                     status;
         PedExceptionOption      ex_status;
         size_t                  read_length = count * dev->sector_size;
+        void*                   diobuf;
 
         PED_ASSERT (dev->sector_size % 512 == 0, return 0);
 
@@ -1213,8 +1215,12 @@
                 }
         }
 
+        if (posix_memalign(&diobuf, PED_SECTOR_SIZE, count * PED_SECTOR_SIZE) != 0)
+                return 0;
         while (1) {
-                status = read (arch_specific->fd, buffer, read_length);
+                status = read (arch_specific->fd, diobuf, read_length);
+                if (status > 0)
+                        memcpy(buffer, diobuf, status);
                 if (status == count * dev->sector_size) break;
                 if (status > 0) {
                         read_length -= status;
@@ -1231,6 +1237,7 @@
 
                 switch (ex_status) {
                         case PED_EXCEPTION_IGNORE:
+                                free(diobuf);
                                 return 1;
 
                         case PED_EXCEPTION_RETRY:
@@ -1239,9 +1246,11 @@
                         case PED_EXCEPTION_UNHANDLED:
                                 ped_exception_catch ();
                         case PED_EXCEPTION_CANCEL:
+                                free(diobuf);
                                 return 0;
                 }
         }
+        free(diobuf);
 
         return 1;
 }
@@ -1287,6 +1296,8 @@
         int                     status;
         PedExceptionOption      ex_status;
         size_t                  write_length = count * dev->sector_size;
+        void*                   diobuf;
+        void*                   diobuf_start;
 
         PED_ASSERT(dev->sector_size % 512 == 0, return 0);
 
@@ -1340,12 +1351,16 @@
                 dev->path, buffer, (int) start, (int) count);
 #else
         dev->dirty = 1;
+        if (posix_memalign(&diobuf, PED_SECTOR_SIZE, count * PED_SECTOR_SIZE) != 0)
+                return 0;
+        memcpy(diobuf, buffer, count * PED_SECTOR_SIZE);
+        diobuf_start = diobuf;
         while (1) {
-                status = write (arch_specific->fd, buffer, write_length);
+                status = write (arch_specific->fd, diobuf, write_length);
                 if (status == count * dev->sector_size) break;
                 if (status > 0) {
                         write_length -= status;
-                        buffer += status;
+                        diobuf += status;
                         continue;
                 }
 
@@ -1357,6 +1372,7 @@
 
                 switch (ex_status) {
                         case PED_EXCEPTION_IGNORE:
+                                free(diobuf_start);
                                 return 1;
 
                         case PED_EXCEPTION_RETRY:
@@ -1365,9 +1381,11 @@
                         case PED_EXCEPTION_UNHANDLED:
                                 ped_exception_catch ();
                         case PED_EXCEPTION_CANCEL:
+                                free(diobuf_start);
                                 return 0;
                 }
         }
+        free(diobuf_start);
 #endif /* !READ_ONLY */
         return 1;
 }
@@ -1380,18 +1398,25 @@
         LinuxSpecific*  arch_specific = LINUX_SPECIFIC (dev);
         PedSector       done = 0;
         int             status;
+        void*           diobuf;
 
         PED_ASSERT(dev != NULL, return 0);
         
         if (!_device_seek (dev, start))
                 return 0;
 
+        if (posix_memalign(&diobuf, PED_SECTOR_SIZE, count * PED_SECTOR_SIZE) != 0)
+                return 0;
+
         for (done = 0; done < count; done += status / dev->sector_size) {
-                status = read (arch_specific->fd, buffer,
+                status = read (arch_specific->fd, diobuf,
                                (size_t) ((count-done) * dev->sector_size));
+                if (status > 0)
+                        memcpy(buffer, diobuf, status);
                 if (status < 0)
                         break;
         }
+        free(diobuf);
 
         return done;
 }


More information about the parted-devel mailing list