bug-parted
[Top][All Lists]
Advanced

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

bug#16231: [PATCH] Fix loop labels


From: Phillip Susi
Subject: bug#16231: [PATCH] Fix loop labels
Date: Sat, 29 Mar 2014 13:51:46 -0400

Loop labels were incorrectly identifying the device for the fictional
partition as $dev1 instead of just $dev. This caused other programs like
gparted to be confused, and caused parted to fail to identify the partition
as busy due to the fact that it was looking for the wrong device. Parted
also actually created the partition device so your raw fs on $dev gained an
alias as $dev1.  Next, writing the label back to the disk clobbered the
filesystem there if it used the first sector.  Several filesystems end up
using the first sector for 2048/4096 byte sectors even though they don't
for 512/1024 byte sectors.  Finally, fat and ntfs boot sectors were being
detected as msdos labels.
---
 NEWS                       |  2 +
 include/parted/device.in.h |  1 +
 libparted/arch/linux.c     | 44 +++++++++++++--------
 libparted/disk.c           |  2 +
 libparted/fs/ntfs/ntfs.c   |  2 +-
 libparted/labels/dos.c     | 29 ++++++++++++++
 libparted/labels/loop.c    | 42 +++++++-------------
 partprobe/partprobe.c      |  4 +-
 tests/Makefile.am          |  1 +
 tests/t1102-loop-label.sh  | 96 ++++++++++++++++++++++++++++++++++++++++++++++
 10 files changed, 174 insertions(+), 49 deletions(-)
 create mode 100644 tests/t1102-loop-label.sh

diff --git a/NEWS b/NEWS
index 9ef8bf4..cdd406e 100644
--- a/NEWS
+++ b/NEWS
@@ -23,6 +23,8 @@ GNU parted NEWS                                    -*- 
outline -*-
 
 ** Bug Fixes
 
+  Fix several bugs with loop labels ( whole disk filesystems )
+
   Fix linux partition sync code to flush partitions > 16
 
   Do not reject a FAT boot sector as invalid because it has no
diff --git a/include/parted/device.in.h b/include/parted/device.in.h
index 7c06a66..ff23a6c 100644
--- a/include/parted/device.in.h
+++ b/include/parted/device.in.h
@@ -92,6 +92,7 @@ struct _PedDevice {
         short           host, did;
 
         void*           arch_specific;
+        int             loop;           /* using "loop" partition table */
 };
 
 #include <parted/natmath.h>
diff --git a/libparted/arch/linux.c b/libparted/arch/linux.c
index 71f5034..50ca744 100644
--- a/libparted/arch/linux.c
+++ b/libparted/arch/linux.c
@@ -48,6 +48,7 @@
 #include "../architecture.h"
 #include "dirname.h"
 #include "xstrtol.h"
+#include "xalloc.h"
 
 #if ENABLE_NLS
 #  include <libintl.h>
@@ -2333,10 +2334,14 @@ _device_get_part_path (PedDevice const *dev, int num)
                          ? dm_canonical_path (dev) : dev->path);
         size_t path_len = strlen (devpath);
         char *result;
+
+        /* bare device, no partitions */
+        if (dev->loop)
+                result = xstrdup(devpath);
         /* Check for devfs-style /disc => /partN transformation
            unconditionally; the system might be using udev with devfs rules,
            and if not the test is harmless. */
-        if (5 < path_len && !strcmp (devpath + path_len - 5, "/disc")) {
+        else if (5 < path_len && !strcmp (devpath + path_len - 5, "/disc")) {
                 /* replace /disc with /part%d */
                 result = zasprintf ("%.*s/part%d",
                                     (int) (path_len - 5), devpath, num);
@@ -2843,25 +2848,30 @@ _disk_sync_part_table (PedDisk* disk)
                 goto cleanup;
 
         int i;
-        /* remove old partitions first */
-        for (i = 1; i <= lpn; i++) {
-                PedPartition *part = ped_disk_get_partition (disk, i);
-                if (part) {
-                        unsigned long long length;
-                        unsigned long long start;
-                        /* get start and length of existing partition */
-                        if (get_partition_start_and_length(part,
-                                                           &start, &length)
-                            && start == part->geom.start
-                            && length == part->geom.length)
-                        {
-                                ok[i - 1] = 1;
-                                continue;
+        /* skip unmodified partitions, unless it's a loop label */
+        if (!disk->dev->loop)
+                for (i = 1; i <= lpn; i++) {
+                        PedPartition *part;
+                        part = ped_disk_get_partition (disk, i);
+                        if (part) {
+                                unsigned long long length;
+                                unsigned long long start;
+                                /* get start and length of existing partition 
*/
+                                if (get_partition_start_and_length(part,
+                                                                   &start, 
&length)
+                                    && start == part->geom.start
+                                    && length == part->geom.length)
+                                {
+                                        ok[i - 1] = 1;
+                                        continue;
+                                }
                         }
                 }
-       }
         for (i = 1; i <= lpn; i++) {
-                PedPartition *part = ped_disk_get_partition (disk, i);
+                PedPartition *part;
+                if (disk->dev->loop)
+                        part = 0;
+                else part = ped_disk_get_partition (disk, i);
                 if (part) {
                         unsigned long long length;
                         unsigned long long start;
diff --git a/libparted/disk.c b/libparted/disk.c
index 5421c03..d3a90e4 100644
--- a/libparted/disk.c
+++ b/libparted/disk.c
@@ -197,6 +197,7 @@ ped_disk_new (PedDevice* dev)
        disk = ped_disk_new_fresh (dev, type);
        if (!disk)
                goto error_close_dev;
+       dev->loop = 0;
        if (!type->ops->read (disk))
                goto error_destroy_disk;
        disk->needs_clobber = 0;
@@ -497,6 +498,7 @@ ped_disk_commit_to_dev (PedDisk* disk)
                        goto error_close_dev;
                disk->needs_clobber = 0;
        }
+       disk->dev->loop = 0;
        if (!disk->type->ops->write (disk))
                goto error_close_dev;
        ped_device_close (disk->dev);
diff --git a/libparted/fs/ntfs/ntfs.c b/libparted/fs/ntfs/ntfs.c
index 05d7f36..19e990a 100644
--- a/libparted/fs/ntfs/ntfs.c
+++ b/libparted/fs/ntfs/ntfs.c
@@ -34,7 +34,7 @@
 
 #define NTFS_SIGNATURE "NTFS"
 
-static PedGeometry*
+PedGeometry*
 ntfs_probe (PedGeometry* geom)
 {
        char    buf[512];
diff --git a/libparted/labels/dos.c b/libparted/labels/dos.c
index eff1c03..295fcf3 100644
--- a/libparted/labels/dos.c
+++ b/libparted/labels/dos.c
@@ -235,12 +235,23 @@ maybe_FAT (unsigned char const *s)
   return true;
 }
 
+PedGeometry*
+fat_probe_fat16 (PedGeometry* geom);
+
+PedGeometry*
+fat_probe_fat32 (PedGeometry* geom);
+
+PedGeometry*
+ntfs_probe (PedGeometry* geom);
+
 static int
 msdos_probe (const PedDevice *dev)
 {
        PedDiskType*    disk_type;
        DosRawTable*    part_table;
        int             i;
+       PedGeometry *geom = NULL;
+       PedGeometry *fsgeom = NULL;
 
        PED_ASSERT (dev != NULL);
 
@@ -257,6 +268,20 @@ msdos_probe (const PedDevice *dev)
        if (PED_LE16_TO_CPU (part_table->magic) != MSDOS_MAGIC)
                goto probe_fail;
 
+       geom = ped_geometry_new (dev, 0, dev->length);
+       PED_ASSERT (geom);
+       fsgeom = fat_probe_fat16 (geom);
+       if (fsgeom)
+               goto probe_fail; /* fat fs looks like dos mbr */
+       fsgeom = fat_probe_fat32 (geom);
+       if (fsgeom)
+               goto probe_fail; /* fat fs looks like dos mbr */
+       fsgeom = ntfs_probe (geom);
+       if (fsgeom)
+               goto probe_fail; /* ntfs fs looks like dos mbr */
+       ped_geometry_destroy (geom);
+       geom = NULL;
+
        /* If this is a FAT fs, fail here.  Checking for the FAT signature
         * has some false positives; instead, do what the Linux kernel does
         * and ensure that each partition has a boot indicator that is
@@ -303,6 +328,10 @@ msdos_probe (const PedDevice *dev)
        return 1;
 
  probe_fail:
+       if (geom)
+               ped_geometry_destroy (geom);
+       if (fsgeom)
+               ped_geometry_destroy (fsgeom);
        free (label);
        return 0;
 }
diff --git a/libparted/labels/loop.c b/libparted/labels/loop.c
index ea8f007..3ef4fc1 100644
--- a/libparted/labels/loop.c
+++ b/libparted/labels/loop.c
@@ -80,7 +80,10 @@ loop_alloc (const PedDevice* dev)
 
        if (dev->length < 256)
                return NULL;
-       return _ped_disk_alloc ((PedDevice*)dev, &loop_disk_type);
+       PedDisk *disk = _ped_disk_alloc ((PedDevice*)dev, &loop_disk_type);
+       if (disk)
+               disk->disk_specific = NULL;
+       return disk;
 }
 
 static PedDisk*
@@ -118,18 +121,12 @@ loop_read (PedDisk* disk)
 
         int found_sig = !strncmp (buf, LOOP_SIGNATURE, strlen 
(LOOP_SIGNATURE));
         free (buf);
-
-        if (found_sig) {
-               ped_constraint_destroy (constraint_any);
-               return 1;
-        }
-
        geom = ped_geometry_new (dev, 0, dev->length);
        if (!geom)
                goto error;
 
        fs_type = ped_file_system_probe (geom);
-       if (!fs_type)
+       if (!fs_type && !found_sig)
                goto error_free_geom;
 
        part = ped_partition_new (disk, PED_PARTITION_NORMAL,
@@ -137,11 +134,12 @@ loop_read (PedDisk* disk)
        ped_geometry_destroy (geom);
        if (!part)
                goto error;
-       part->fs_type = fs_type;
 
        if (!ped_disk_add_partition (disk, part, constraint_any))
                goto error;
        ped_constraint_destroy (constraint_any);
+       dev->loop = 1;
+       disk->disk_specific = (void *)1; /* not a pointer but a flag to not 
rewrite label */
        return 1;
 
 error_free_geom:
@@ -156,29 +154,17 @@ static int
 loop_write (const PedDisk* disk)
 {
        size_t buflen = disk->dev->sector_size;
-       char *buf = ped_malloc (buflen);
-       if (buf == NULL)
+       char *buf = alloca (buflen);
+       disk->dev->loop = 1;
+       /* only write label after creating it new */
+       if (disk->disk_specific) /* not a pointer but a flag to not rewrite 
label */
                return 0;
-
-       if (ped_disk_get_partition (disk, 1)) {
-               if (!ped_device_read (disk->dev, buf, 0, 1)) {
-                       free (buf);
-                       return 0;
-               }
-               if (strncmp (buf, LOOP_SIGNATURE, strlen (LOOP_SIGNATURE)) != 
0) {
-                       free (buf);
-                       return 1;
-                }
-               memset (buf, 0, strlen (LOOP_SIGNATURE));
-               return ped_device_write (disk->dev, buf, 0, 1);
-       }
-
        memset (buf, 0, buflen);
        strcpy (buf, LOOP_SIGNATURE);
 
-        int write_ok = ped_device_write (disk->dev, buf, 0, 1);
-        free (buf);
-       return write_ok;
+        if (ped_device_write (disk->dev, buf, 0, 1))
+               return 1;
+       return 0;
 }
 #endif /* !DISCOVER_ONLY */
 
diff --git a/partprobe/partprobe.c b/partprobe/partprobe.c
index 4da4fb7..8b744b5 100644
--- a/partprobe/partprobe.c
+++ b/partprobe/partprobe.c
@@ -106,9 +106,7 @@ process_dev (PedDevice* dev)
        PedDisk*        disk;
 
        disk_type = ped_disk_probe (dev);
-       if (disk_type && !strcmp (disk_type->name, "loop"))
-               return 1;
-       else if (!disk_type) {
+       if (!disk_type) {
                /* Partition table not found, so create dummy,
                   empty one */
                disk_type = ped_disk_type_get("msdos");
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 9100a81..ba99a3a 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -40,6 +40,7 @@ TESTS = \
   t0501-duplicate.sh \
   t1100-busy-label.sh \
   t1101-busy-partition.sh \
+  t1102-loop-label.sh \
   t1700-probe-fs.sh \
   t2200-dos-label-recog.sh \
   t2201-pc98-label-recog.sh \
diff --git a/tests/t1102-loop-label.sh b/tests/t1102-loop-label.sh
new file mode 100644
index 0000000..243248f
--- /dev/null
+++ b/tests/t1102-loop-label.sh
@@ -0,0 +1,96 @@
+#!/bin/sh
+# make sure that loop labels do busy detection and don't
+# create an actual partition
+
+# Copyright (C) 2013 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+. "${srcdir=.}/init.sh"; path_prepend_ ../parted
+path_prepend_ ../partprobe
+require_root_
+require_scsi_debug_module_
+ss=$sector_size_
+
+scsi_debug_setup_ sector_size=$ss dev_size_mb=90 > dev-name ||
+  skip_ 'failed to create scsi_debug device'
+dev=$(cat dev-name)
+
+mke2fs -F $dev
+parted -s "$dev" print > out 2>&1 || fail=1
+cat <<EOF > exp
+Model: Linux scsi_debug (scsi)
+Disk DEVICE: 94.4MB
+Sector size (logical/physical): ${ss}B/${ss}B
+Partition Table: loop
+Disk Flags: 
+
+Number  Start  End     Size    File system  Flags
+ 1      0.00B  94.4MB  94.4MB  ext2
+
+EOF
+mv out o2 && sed -e "s,$dev,DEVICE,;" o2 > out
+
+compare exp out || fail=1
+parted -s $dev rm 1 || fail=1
+if [ -e ${dev}1 ]; then
+    echo "Partition should not exist on loop device"
+    fail=1
+fi
+partprobe $dev || fail=1
+if [ -e ${dev}1 ]; then
+    echo "Partition should not exist on loop device"
+    fail=1
+fi
+
+mount_point="`pwd`/mnt"
+
+# Be sure to unmount upon interrupt, failure, etc.
+cleanup_fn_() { umount "$mount_point" > /dev/null 2>&1; }
+
+# create mount point dir. and mount the just-created partition on it
+mkdir $mount_point || fail=1
+mount -t ext2 "${dev}" $mount_point || fail=1
+
+# now that a partition is mounted, mklabel attempt must fail
+parted -s "$dev" mklabel msdos > out 2>&1; test $? = 1 || fail=1
+
+# create expected output file
+echo "Error: Partition(s) on $dev are being used." > exp
+compare exp out || fail=1
+
+# make sure partition busy check works ( mklabel checks whole disk )
+parted -s "$dev" rm 1 > out 2>&1; test $? = 1 || fail=1
+# create expected output file
+echo "Error: Partition $dev is being used. You must unmount it before you 
modify \
+it with Parted." > exp
+compare exp out || fail=1
+
+umount "$mount_point"
+
+# make sure partprobe cleans up stale partition devices
+parted -s $dev mklabel msdos mkpart primary ext2 0% 100%
+if [ ! -e ${dev}1 ]; then
+    echo "Partition doesn't exist on loop device"
+    fail=1
+fi
+
+mke2fs -F $dev
+partprobe $dev
+if [ -e ${dev}1 ]; then
+    echo "Partition should not exist on loop device"
+    fail=1
+fi
+
+Exit $fail
-- 
1.8.3.2






reply via email to

[Prev in Thread] Current Thread [Next in Thread]