[parted-devel] RFC: Define PedSector to be 512 bytes

Matt Domsch Matt_Domsch at dell.com
Mon Jan 26 14:23:22 UTC 2009


After a long hiatus, I've started looking at GPT and 4k-sector-size
disks again.

  
Conveniently, Matthew Wilcox has done work on the ATA layer, and wrote
an 'ata_ram' module which emulates an ATA disk, complete with letting
the user specify the reported logical block size.
http://git.kernel.org/?p=linux/kernel/git/willy/misc.git;a=summary
(see the ata-ram and ata-large-sectors branches; some minor fixups
needed to work on current Linus kernels which I'll work with Matthew
to get pushed).

The patch below (on top of Fedora's source RPM) is necessary to get
GPT tables written and read properly on 4k disks.  But, it's not
sufficient...

The next challenge is that anything calling ped_device_{read,write}()
does so using "PedSector count" as the implicit length of the buffer.

extern int ped_device_read (const PedDevice* dev, void* buffer,
                           PedSector start, PedSector count);
extern int ped_device_write (PedDevice* dev, const void* buffer,
                             PedSector start, PedSector count);

With only the patch below, the ext2/3 probe() code dies immediately by
reading a 4k PedSector into a 512b buffer.

Either we'll have to each every caller of these functions that buffer
needs to be arbitrarily large (to allow for PedSector to be of
variable length) and do the math to munge "PedSector start", or we're
going to have to define PedSector to be 512 bytes, and inside these
functions, adjust on a per-device basis.  I believe the latter is the
right choice, as it puts the complications in exactly one place.

Thoughts?			     

-- 
Matt Domsch
Linux Technology Strategist, Dell Office of the CTO
linux.dell.com & www.dell.com/linux

diff --git a/libparted/arch/linux.c b/libparted/arch/linux.c
index 462ab92..c48b41d 100644
--- a/libparted/arch/linux.c
+++ b/libparted/arch/linux.c
@@ -193,7 +193,8 @@ struct hd_driveid {
 #define BLKRRPART  _IO(0x12,95) /* re-read partition table */
 #define BLKGETSIZE _IO(0x12,96) /* return device size */
 #define BLKFLSBUF  _IO(0x12,97) /* flush buffer cache */
-#define BLKSSZGET  _IO(0x12,104) /* get block device sector size */
+#define BLKSSZGET  _IO(0x12,104) /* get block device physical sector size */
+#define BLKBSZGET  _IOR(0x12,112,size_t) /* get block device logical sector size */
 #define BLKGETLASTSECT  _IO(0x12,108) /* get last sector of block device */
 #define BLKSETLASTSECT  _IO(0x12,109) /* set last sector of block device */
 
@@ -539,7 +540,8 @@ static void
 _device_set_sector_size (PedDevice* dev)
 {
         LinuxSpecific*  arch_specific = LINUX_SPECIFIC (dev);
-        int sector_size;
+        int physical_sector_size=0;
+        size_t logical_sector_size=0ULL;
         
         dev->sector_size = PED_SECTOR_SIZE_DEFAULT;
         dev->phys_sector_size = PED_SECTOR_SIZE_DEFAULT;
@@ -551,15 +553,26 @@ _device_set_sector_size (PedDevice* dev)
                 return;
         }
         
-        if (ioctl (arch_specific->fd, BLKSSZGET, &sector_size)) {
+        if (ioctl (arch_specific->fd, BLKBSZGET, &logical_sector_size)) {
                 ped_exception_throw (
                         PED_EXCEPTION_WARNING,
                         PED_EXCEPTION_OK,
-                        _("Could not determine sector size for %s: %s.\n"
-                          "Using the default sector size (%lld)."),
+                        _("Could not determine logical sector size for %s: %s.\n"
+                          "Using the default logical sector size (%lld)."),
                         dev->path, strerror (errno), PED_SECTOR_SIZE_DEFAULT);
         } else {
-                dev->sector_size = (long long)sector_size;
+                dev->sector_size = (long long)logical_sector_size;
+        }
+
+        if (ioctl (arch_specific->fd, BLKSSZGET, &physical_sector_size)) {
+                ped_exception_throw (
+                        PED_EXCEPTION_WARNING,
+                        PED_EXCEPTION_OK,
+                        _("Could not determine physical sector size for %s: %s.\n"
+                          "Using the default physical sector size (%lld)."),
+                        dev->path, strerror (errno), PED_SECTOR_SIZE_DEFAULT);
+        } else {
+                dev->phys_sector_size = (long long)physical_sector_size;
         }
 
         /* Return PED_SECTOR_SIZE_DEFAULT for DASDs. */
@@ -1447,11 +1460,12 @@ linux_read (const PedDevice* dev, void* buffer, PedSector start,
         LinuxSpecific*          arch_specific = LINUX_SPECIFIC (dev);
         PedExceptionOption      ex_status;
         void*                   diobuf = NULL;
+	size_t                  buffer_length = PED_SECTOR_SIZE_DEFAULT * count;
 
         PED_ASSERT (dev != NULL, return 0);
         PED_ASSERT (dev->sector_size % PED_SECTOR_SIZE_DEFAULT == 0, return 0);
 
-        if (_get_linux_version() < KERNEL_VERSION (2,6,0)) {
+        if (_get_linux_version() < KERNEL_VERSION (2,6,0) && dev->sector_size == PED_SECTOR_SIZE_DEFAULT) {
                 /* Kludge.  This is necessary to read/write the last
                    block of an odd-sized disk, until Linux 2.5.x kernel fixes.
                 */
@@ -1491,6 +1505,7 @@ linux_read (const PedDevice* dev, void* buffer, PedSector start,
         size_t read_length = count * dev->sector_size;
         if (posix_memalign (&diobuf, dev->sector_size, read_length) != 0)
                 return 0;
+	memset(diobuf, 0, read_length);
 
         while (1) {
                 ssize_t status = read (arch_specific->fd, diobuf, read_length);
@@ -1594,7 +1609,7 @@ linux_write (PedDevice* dev, const void* buffer, PedSector start,
                         return 1;
         }
 
-        if (_get_linux_version() < KERNEL_VERSION (2,6,0)) {
+        if (_get_linux_version() < KERNEL_VERSION (2,6,0) && dev->sector_size == PED_SECTOR_SIZE_DEFAULT) {
                 /* Kludge.  This is necessary to read/write the last
                    block of an odd-sized disk, until Linux 2.5.x kernel fixes.
                 */
@@ -1635,10 +1650,11 @@ linux_write (PedDevice* dev, const void* buffer, PedSector start,
                 dev->path, buffer, (int) start, (int) count);
 #else
         dev->dirty = 1;
-        if (posix_memalign(&diobuf, PED_SECTOR_SIZE_DEFAULT,
-                           count * PED_SECTOR_SIZE_DEFAULT) != 0)
+        if (posix_memalign(&diobuf, dev->sector_size,
+                           count * dev->sector_size) != 0)
                 return 0;
-        memcpy(diobuf, buffer, count * PED_SECTOR_SIZE_DEFAULT);
+	memset(diobuf, 0, count * dev->sector_size);
+        memcpy(diobuf, buffer, count * dev->sector_size);
         diobuf_start = diobuf;
         while (1) {
                 status = write (arch_specific->fd, diobuf, write_length);
@@ -1693,9 +1709,10 @@ linux_check (PedDevice* dev, void* buffer, PedSector start, PedSector count)
         if (!_device_seek (dev, start))
                 return 0;
 
-        if (posix_memalign(&diobuf, PED_SECTOR_SIZE_DEFAULT,
-                           count * PED_SECTOR_SIZE_DEFAULT) != 0)
+        if (posix_memalign(&diobuf, dev->sector_size,
+                           count * dev->sector_size) != 0)
                 return 0;
+	memset(diobuf, 0, count * dev->sector_size);
 
         for (done = 0; done < count; done += status / dev->sector_size) {
                 status = read (arch_specific->fd, diobuf,
diff --git a/libparted/labels/gpt.c b/libparted/labels/gpt.c
index f113bcc..4b892ac 100644
--- a/libparted/labels/gpt.c
+++ b/libparted/labels/gpt.c
@@ -430,6 +430,7 @@ gpt_probe (const PedDevice * dev)
 {
 	GuidPartitionTableHeader_t* gpt = NULL;
         uint8_t* pth_raw = ped_malloc (pth_get_size (dev));
+	uint8_t* legacy_mbr_buf;
 	LegacyMBR_t legacy_mbr;
 	int gpt_sig_found = 0;
 
@@ -451,7 +452,11 @@ gpt_probe (const PedDevice * dev)
 	if (!gpt_sig_found)
 		return 0;
 
-	if (ped_device_read(dev, &legacy_mbr, 0, GPT_HEADER_SECTORS)) {
+
+	legacy_mbr_buf = ped_malloc(dev->sector_size);
+	if (ped_device_read(dev, legacy_mbr_buf, 0, GPT_HEADER_SECTORS)) {
+		memcpy(&legacy_mbr, legacy_mbr_buf, sizeof(legacy_mbr));
+		ped_free(legacy_mbr_buf);
 		if (!_pmbr_is_valid (&legacy_mbr)) {
 			int ex_status = ped_exception_throw (
 				PED_EXCEPTION_WARNING,



More information about the parted-devel mailing list