Package: parted;
Reported by: Phillip Susi <psusi <at> ubuntu.com>
Date: Mon, 23 Dec 2013 20:54:02 UTC
Severity: normal
Tags: patch
Done: Phillip Susi <psusi <at> ubuntu.com>
Bug is archived. No further changes may be made.
To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 16231 in the body.
You can then email your comments to 16231 AT debbugs.gnu.org in the normal way.
Toggle the display of automated, internal messages from the tracker.
View this report as an mbox folder, status mbox, maintainer mbox
bug-parted <at> gnu.org:bug#16231; Package parted.
(Mon, 23 Dec 2013 20:54:02 GMT) Full text and rfc822 format available.Phillip Susi <psusi <at> ubuntu.com>:bug-parted <at> gnu.org.
(Mon, 23 Dec 2013 20:54:02 GMT) Full text and rfc822 format available.Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):
From: Phillip Susi <psusi <at> ubuntu.com> To: bug-parted <at> gnu.org Subject: [PATCH] Fix loop labels Date: Mon, 23 Dec 2013 15:17:26 -0500
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.
---
NEWS | 2 +
include/parted/device.in.h | 1 +
libparted/arch/linux.c | 17 ++++++--
libparted/disk.c | 2 +
libparted/labels/loop.c | 4 +-
partprobe/partprobe.c | 4 +-
tests/Makefile.am | 1 +
tests/t1102-loop-label.sh | 96 ++++++++++++++++++++++++++++++++++++++++++++++
8 files changed, 120 insertions(+), 7 deletions(-)
create mode 100644 tests/t1102-loop-label.sh
diff --git a/NEWS b/NEWS
index 935fa33..816fb57 100644
--- a/NEWS
+++ b/NEWS
@@ -12,6 +12,8 @@ GNU parted NEWS -*- outline -*-
boot partition type.
** Bug Fixes
+ Fix several bugs with loop labels ( whole disk filesystems )
+
Fix gpt to correctly handle non ASCII charcters in partition names
If a drive was 100 times an even multiple of two, sizes specified as
diff --git a/include/parted/device.in.h b/include/parted/device.in.h
index 7c06a66..1c1765a 100644
--- a/include/parted/device.in.h
+++ b/include/parted/device.in.h
@@ -86,6 +86,7 @@ struct _PedDevice {
int external_mode;
int dirty;
int boot_dirty;
+ int loop; /* using "loop" partition table */
PedCHSGeometry hw_geom;
PedCHSGeometry bios_geom;
diff --git a/libparted/arch/linux.c b/libparted/arch/linux.c
index f43eae1..8a09763 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>
@@ -2315,10 +2316,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);
@@ -2814,7 +2819,10 @@ _disk_sync_part_table (PedDisk* disk)
int i;
/* remove old partitions first */
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;
@@ -2830,7 +2838,10 @@ _disk_sync_part_table (PedDisk* disk)
}
}
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 ce71bfc..0eecd63 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/labels/loop.c b/libparted/labels/loop.c
index ea8f007..b237c1e 100644
--- a/libparted/labels/loop.c
+++ b/libparted/labels/loop.c
@@ -121,6 +121,7 @@ loop_read (PedDisk* disk)
if (found_sig) {
ped_constraint_destroy (constraint_any);
+ dev->loop = 1;
return 1;
}
@@ -142,6 +143,7 @@ loop_read (PedDisk* disk)
if (!ped_disk_add_partition (disk, part, constraint_any))
goto error;
ped_constraint_destroy (constraint_any);
+ dev->loop = 1;
return 1;
error_free_geom:
@@ -159,7 +161,7 @@ loop_write (const PedDisk* disk)
char *buf = ped_malloc (buflen);
if (buf == NULL)
return 0;
-
+ disk->dev->loop = 1;
if (ped_disk_get_partition (disk, 1)) {
if (!ped_device_read (disk->dev, buf, 0, 1)) {
free (buf);
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 7a6fe8f..fc0b5ec 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..b26addd
--- /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): 512B/512B
+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 mkpart ext2 0% 100% || 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/sde 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
bug-parted <at> gnu.org:bug#16231; Package parted.
(Sun, 29 Dec 2013 05:26:02 GMT) Full text and rfc822 format available.Message #8 received at 16231 <at> debbugs.gnu.org (full text, mbox):
From: Phillip Susi <psusi <at> ubuntu.com> To: 16231 <at> debbugs.gnu.org Subject: Re: bug#16231: [PATCH] Fix loop labels Date: Sun, 29 Dec 2013 00:25:30 -0500
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA512 On 12/23/2013 03:17 PM, Phillip Susi wrote: > 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. I had to tweak this patch just a bit more to get it fully working. It used to let you create a loop label, which had no partition, then when you created a partition, it wiped out the loop signature. Now it implicitly creates the loop partition if you either have the loop label signature, or a raw filesystem signature, and thus, has to leave the signature intact, even if you have a partition, which may or may not have a valid filesystem signature, depending on whether or not you actually format a filesystem. -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.14 (GNU/Linux) Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQEcBAEBCgAGBQJSv7JKAAoJEI5FoCIzSKrwrnQIAJJ0lRnELDG+RUeB4JknkkPU YfkCeQXONrQ0/VUiIeCedA3PbLvPCo0T1aIHd7URp5T/Im4p2LAt4DqF98m7ih2f xbWkrMo7Tww5y06qneOphazzCOHx0/o95z2NSCNu6bDDZ+QLL/MXrOXkFhId3yR8 BFbOvUQBfMP/9oRNirvwDBRxCO9ZdZDP23tPRlrdsvau7hvZCiSWRlQVeUHHtdD7 k4LvHMlHSDIdwc5a2+Eehblb0C2KS+H2f7QUeO26glxmNNvhf1mNEWRE7l9T4Phq 54g9UDaF+Bny4qJoN1bTrKJVdj9y4wRigAZ40QPhTWgNmoPfcvxPFnZCsb/Cfpw= =yO/a -----END PGP SIGNATURE-----
bug-parted <at> gnu.org:bug#16231; Package parted.
(Sun, 29 Dec 2013 05:28:01 GMT) Full text and rfc822 format available.Message #11 received at 16231 <at> debbugs.gnu.org (full text, mbox):
From: Phillip Susi <psusi <at> ubuntu.com> To: 16231 <at> debbugs.gnu.org Subject: [PATCH] [PATCH] Fix loop labels Date: Sun, 29 Dec 2013 00:27:54 -0500
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.
---
NEWS | 2 +
include/parted/device.in.h | 1 +
libparted/arch/linux.c | 17 ++++++--
libparted/disk.c | 2 +
libparted/labels/loop.c | 25 ++----------
partprobe/partprobe.c | 4 +-
tests/Makefile.am | 1 +
tests/t1102-loop-label.sh | 96 ++++++++++++++++++++++++++++++++++++++++++++++
8 files changed, 120 insertions(+), 28 deletions(-)
create mode 100644 tests/t1102-loop-label.sh
diff --git a/NEWS b/NEWS
index 935fa33..816fb57 100644
--- a/NEWS
+++ b/NEWS
@@ -12,6 +12,8 @@ GNU parted NEWS -*- outline -*-
boot partition type.
** Bug Fixes
+ Fix several bugs with loop labels ( whole disk filesystems )
+
Fix gpt to correctly handle non ASCII charcters in partition names
If a drive was 100 times an even multiple of two, sizes specified as
diff --git a/include/parted/device.in.h b/include/parted/device.in.h
index 7c06a66..1c1765a 100644
--- a/include/parted/device.in.h
+++ b/include/parted/device.in.h
@@ -86,6 +86,7 @@ struct _PedDevice {
int external_mode;
int dirty;
int boot_dirty;
+ int loop; /* using "loop" partition table */
PedCHSGeometry hw_geom;
PedCHSGeometry bios_geom;
diff --git a/libparted/arch/linux.c b/libparted/arch/linux.c
index f43eae1..8a09763 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>
@@ -2315,10 +2316,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);
@@ -2814,7 +2819,10 @@ _disk_sync_part_table (PedDisk* disk)
int i;
/* remove old partitions first */
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;
@@ -2830,7 +2838,10 @@ _disk_sync_part_table (PedDisk* disk)
}
}
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 ce71bfc..0eecd63 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/labels/loop.c b/libparted/labels/loop.c
index ea8f007..79b817c 100644
--- a/libparted/labels/loop.c
+++ b/libparted/labels/loop.c
@@ -118,18 +118,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 +131,11 @@ 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;
return 1;
error_free_geom:
@@ -159,20 +153,7 @@ loop_write (const PedDisk* disk)
char *buf = ped_malloc (buflen);
if (buf == NULL)
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);
- }
-
+ disk->dev->loop = 1;
memset (buf, 0, buflen);
strcpy (buf, LOOP_SIGNATURE);
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 7a6fe8f..fc0b5ec 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..b26addd
--- /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): 512B/512B
+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 mkpart ext2 0% 100% || 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/sde 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
bug-parted <at> gnu.org:bug#16231; Package parted.
(Sat, 04 Jan 2014 04:39:02 GMT) Full text and rfc822 format available.Message #14 received at 16231 <at> debbugs.gnu.org (full text, mbox):
From: Phillip Susi <psusi <at> ubuntu.com> To: 16231 <at> debbugs.gnu.org Subject: Re: bug#16231: [PATCH] [PATCH] Fix loop labels Date: Fri, 03 Jan 2014 23:38:24 -0500
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA512 I had to adjust the patch slightly. The return value in loop_write was backwards, and fat and ntfs boot sectors were being misidentified as msdos partition tables. -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.14 (GNU/Linux) Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQEcBAEBCgAGBQJSx5A/AAoJEI5FoCIzSKrwQCkH/jLXeZt4aD/AqvNMftGxfJ/C CETzUCfl+IOmXRQKMA8ro/hfDzsiyy+RHv76wsue8dF+mIMHedNBQgfnMqHjAVfp wdYPIkbmqLVyvRKHXKIIcB3NU8jyD+evY2K81l4E63Z7pPa37T3aIQ3M/7h15lJK 7FJlNGLU+X/+WFw57HFBgpDT+y4gn4IzMAlDHGZYjWVPDHIaFRIDDTtYQ7ke7kY0 f3zwQH2dmfAYuq1Zh0o/vIYooCt1myrIDSQSijVwGxlYEf6wYvwBTe3Fax2XSkNS MdsdgE85k3shbZj3r30c7L68pKFGbcerVbDUILXSIA9+z2RvhQdXMb9DJmdGJFs= =/Qel -----END PGP SIGNATURE-----
bug-parted <at> gnu.org:bug#16231; Package parted.
(Sat, 04 Jan 2014 04:49:01 GMT) Full text and rfc822 format available.Message #17 received at 16231 <at> debbugs.gnu.org (full text, mbox):
From: Phillip Susi <psusi <at> ubuntu.com> To: 16231 <at> debbugs.gnu.org Subject: [PATCH 1/2] Fix loop labels Date: Fri, 3 Jan 2014 23:48:30 -0500
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 | 17 ++++++--
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, 161 insertions(+), 35 deletions(-)
create mode 100644 tests/t1102-loop-label.sh
diff --git a/NEWS b/NEWS
index 935fa33..816fb57 100644
--- a/NEWS
+++ b/NEWS
@@ -12,6 +12,8 @@ GNU parted NEWS -*- outline -*-
boot partition type.
** Bug Fixes
+ Fix several bugs with loop labels ( whole disk filesystems )
+
Fix gpt to correctly handle non ASCII charcters in partition names
If a drive was 100 times an even multiple of two, sizes specified as
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 f43eae1..8a09763 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>
@@ -2315,10 +2316,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);
@@ -2814,7 +2819,10 @@ _disk_sync_part_table (PedDisk* disk)
int i;
/* remove old partitions first */
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;
@@ -2830,7 +2838,10 @@ _disk_sync_part_table (PedDisk* disk)
}
}
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 ce71bfc..0eecd63 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 6bddd79..56df6a4 100644
--- a/libparted/labels/dos.c
+++ b/libparted/labels/dos.c
@@ -231,12 +231,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);
@@ -253,6 +264,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
@@ -299,6 +324,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..ad0db99 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 = (void *)0;
+ 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; /* don't 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)
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 7a6fe8f..fc0b5ec 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..31de186
--- /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/sde 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
bug-parted <at> gnu.org:bug#16231; Package parted.
(Mon, 03 Mar 2014 18:50:02 GMT) Full text and rfc822 format available.Message #20 received at submit <at> debbugs.gnu.org (full text, mbox):
From: "Brian C. Lane" <bcl <at> redhat.com> To: bug-parted <at> gnu.org Subject: Re: bug#16231: [PATCH 1/2] Fix loop labels Date: Mon, 3 Mar 2014 10:48:57 -0800
On Fri, Jan 03, 2014 at 11:48:30PM -0500, Phillip Susi wrote:
> 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.
I am not sure exactly what the problem is here. I think you are saying
the Disk identification is including a partition? On Fedora 20 (parted
3.1 plus a number of my patches) I am seeing this output when using
scsi_debug (and when using a /dev/mapper device and when using a disk
image directly):
[bcl <at> lister parted (master *)]$ sudo parted -s /dev/sdd p
Model: Linux scsi_debug (scsi)
Disk /dev/sdd: 94.4MB
Sector size (logical/physical): 512B/512B
Partition Table: loop
Disk Flags:
Number Start End Size File system Flags
1 0.00B 94.4MB 94.4MB ext2
This looks fine to me.
Meanwhile I do have some comments about this code.
> ---
> NEWS | 2 +
> include/parted/device.in.h | 1 +
> libparted/arch/linux.c | 17 ++++++--
> 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, 161 insertions(+), 35 deletions(-)
> create mode 100644 tests/t1102-loop-label.sh
I still think we should split up tests into their own commits.
>
> diff --git a/NEWS b/NEWS
> index 935fa33..816fb57 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -12,6 +12,8 @@ GNU parted NEWS -*- outline -*-
> boot partition type.
>
> ** Bug Fixes
> + Fix several bugs with loop labels ( whole disk filesystems )
> +
> Fix gpt to correctly handle non ASCII charcters in partition names
>
> If a drive was 100 times an even multiple of two, sizes specified as
> 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 */
> };
When we make changes like these should we be bumping the library
version? Or should that only happen at release time? I'm also not sure
how to actually do that, not being a big fan of autotools.
> @@ -2814,7 +2819,10 @@ _disk_sync_part_table (PedDisk* disk)
> int i;
> /* remove old partitions first */
> 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;
> @@ -2830,7 +2838,10 @@ _disk_sync_part_table (PedDisk* disk)
> }
> }
> 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;
We should be skipping these loops instead of letting them run with part
set to 0.
> diff --git a/libparted/labels/loop.c b/libparted/labels/loop.c
> index ea8f007..ad0db99 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 = (void *)0;
> + return disk;
> }
This seems like a misuse of the disk_specific pointer.
> @@ -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)
> 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;
It looks like this leaks buf everywhere it returns.
--
Brian C. Lane | Anaconda Team | IRC: bcl #anaconda | Port Orchard, WA (PST8PDT)
bug-parted <at> gnu.org:bug#16231; Package parted.
(Tue, 04 Mar 2014 00:31:02 GMT) Full text and rfc822 format available.Message #23 received at 16231 <at> debbugs.gnu.org (full text, mbox):
From: Phillip Susi <psusi <at> ubuntu.com> To: "Brian C. Lane" <bcl <at> redhat.com>, 16231 <at> debbugs.gnu.org Subject: Re: bug#16231: [PATCH 1/2] Fix loop labels Date: Mon, 03 Mar 2014 19:30:43 -0500
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512
On 03/03/2014 01:48 PM, Brian C. Lane wrote:
> I am not sure exactly what the problem is here. I think you are
> saying the Disk identification is including a partition? On Fedora
> 20 (parted 3.1 plus a number of my patches) I am seeing this output
> when using scsi_debug (and when using a /dev/mapper device and when
> using a disk image directly):
>
> [bcl <at> lister parted (master *)]$ sudo parted -s /dev/sdd p Model:
> Linux scsi_debug (scsi) Disk /dev/sdd: 94.4MB Sector size
> (logical/physical): 512B/512B Partition Table: loop Disk Flags:
>
> Number Start End Size File system Flags 1 0.00B
> 94.4MB 94.4MB ext2
>
> This looks fine to me.
Right, it reports a synthetic partition number 1 starting at offset 0.
GParted asks libparted for the device name of partition number 1, and
it returns /dev/sdd1, rather than /dev/sdd, which causes gparted to
run for instance, mke2fs /dev/sdd1, which of course, fails.
> I still think we should split up tests into their own commits.
I really prefer the cleaner git log keeping them together gives.
>>
>> diff --git a/NEWS b/NEWS index 935fa33..816fb57 100644 ---
>> a/NEWS +++ b/NEWS @@ -12,6 +12,8 @@ GNU parted NEWS
>> -*- outline -*- boot partition type.
>>
>> ** Bug Fixes + Fix several bugs with loop labels ( whole disk
>> filesystems ) + Fix gpt to correctly handle non ASCII charcters
>> in partition names
>>
>> If a drive was 100 times an even multiple of two, sizes specified
>> as 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 */ };
>
> When we make changes like these should we be bumping the library
> version? Or should that only happen at release time? I'm also not
> sure how to actually do that, not being a big fan of autotools.
I believe that since PedDevice is an opaque structure ( libparted
client's aren't supposed to allocate or copy them directly ), and
since the new member is at the end, it doesn't break ABI. As for how
to bump the ABI, it's a change of the CURRENT variable in
libparted/Makefile.am.
>> @@ -2814,7 +2819,10 @@ _disk_sync_part_table (PedDisk* disk) int
>> i; /* remove old partitions first */ 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; @@ -2830,7
>> +2838,10 @@ _disk_sync_part_table (PedDisk* disk) } } 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;
>
> We should be skipping these loops instead of letting them run with
> part set to 0.
The first loop we could just skip, but not the second one because we
do want to remove any existing partitions. Think of starting with an
existing partitioned disk and running mklabel loop on it. I'll revise
the patch to skip the first loop.
>> diff --git a/libparted/labels/loop.c b/libparted/labels/loop.c
>> index ea8f007..ad0db99 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 = (void *)0; + return disk; }
>
> This seems like a misuse of the disk_specific pointer.
I figured it would be silly to allocate a structure to just contain a
single boolean value when the pointer itself can be used for that
purpose. Admittedly it is a bit of a micro optimization but there's a
part of me that just hates to allocate things you don't really need.
I suppose I could add a comment or two explaining the usage.
>> @@ -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) 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;
>
> It looks like this leaks buf everywhere it returns.
alloca() allocates on the stack, so it is implicitly freed on return.
It also can't fail, so no need for annoying tests and frees
everywhere for a temporary buffer.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.14 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/
iQEcBAEBCgAGBQJTFR6zAAoJEI5FoCIzSKrwuWoH/3MinQNcW9S5jEVyMlIFtT0X
hkaKRF15EO/B8B5MJwSiQ5fQ4yrjC0EScu7bGeB9UWMkFkWcuGntp3l+kTNrxGv6
oCBxTi2yqkb9E3/bo6FdDwFjrCM2JSE6ht1h4JVZsU+77knugrmRcGtIsVEgleLD
LD/wDEsjElZzu+TB+ixv0vgDKGXxPq+hjH2eJe416IeSDRMKe/dWZpAq6DGWc6Du
Bnfr4qWjFBdDFSUNtzKmTfVxvnk/Zbtt1hdBaNQ4SgerpjOthF3ACjxkAfRs5zP9
4L4AxPUzmygEdekGO4B205AU0sU8QT6FOyMMMhbSFkGwqjCBqoQxe3+Cy62Yfp8=
=hecx
-----END PGP SIGNATURE-----
bug-parted <at> gnu.org:bug#16231; Package parted.
(Sat, 29 Mar 2014 17:52:01 GMT) Full text and rfc822 format available.Message #26 received at 16231 <at> debbugs.gnu.org (full text, mbox):
From: Phillip Susi <psusi <at> ubuntu.com> To: bcl <at> redhat.com Cc: 16231 <at> debbugs.gnu.org Subject: [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
bug-parted <at> gnu.org:bug#16231; Package parted.
(Thu, 17 Apr 2014 22:15:02 GMT) Full text and rfc822 format available.Message #29 received at 16231 <at> debbugs.gnu.org (full text, mbox):
From: "Brian C. Lane" <bcl <at> redhat.com> To: Phillip Susi <psusi <at> ubuntu.com> Cc: 16231 <at> debbugs.gnu.org Subject: Re: [PATCH] Fix loop labels Date: Thu, 17 Apr 2014 15:14:01 -0700
On Sat, Mar 29, 2014 at 01:51:46PM -0400, Phillip Susi wrote: > 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 This makes a whole pile of tests fail. I think I also previously commented that I wasn't seeing the problem as described -- I need to dig up that email and read it again. -- Brian C. Lane | Anaconda Team | IRC: bcl #anaconda | Port Orchard, WA (PST8PDT)
bug-parted <at> gnu.org:bug#16231; Package parted.
(Sat, 03 May 2014 01:52:02 GMT) Full text and rfc822 format available.Message #32 received at 16231 <at> debbugs.gnu.org (full text, mbox):
From: Phillip Susi <psusi <at> ubuntu.com> To: 16231 <at> debbugs.gnu.org Cc: bcl <at> redhat.com Subject: [PATCH 0/9] Refactored loop fixes Date: Fri, 2 May 2014 21:50:42 -0400
I have refactored the loop fixes making them cleaner, and breaking them out into separate patches for easier review. Phillip Susi (9): libparted: don't detect fat and ntfs boot sectors as dos MBR libparted: remove old partitions *first* before adding new ones libparted: remove all old partitions, even if new label allows less libparted: fix loop labels to not vanish libparted: don't create partition on loop label partprobe: do not skip loop labels libparted: give correct partition device name on loop labels libparted: don't trash filesystem when writing loop label tests: test loop labels NEWS | 22 +++++++ libparted/arch/linux.c | 67 ++++++++++++-------- libparted/fs/ntfs/ntfs.c | 2 +- libparted/labels/dos.c | 29 +++++++++ libparted/labels/loop.c | 54 ++++++++--------- partprobe/partprobe.c | 4 +- tests/Makefile.am | 2 + tests/t1102-loop-label.sh | 104 ++++++++++++++++++++++++++++++++ tests/t1104-remove-and-add-partition.sh | 50 +++++++++++++++ 9 files changed, 275 insertions(+), 59 deletions(-) create mode 100644 tests/t1102-loop-label.sh create mode 100644 tests/t1104-remove-and-add-partition.sh -- 1.9.1
bug-parted <at> gnu.org:bug#16231; Package parted.
(Sat, 03 May 2014 01:52:03 GMT) Full text and rfc822 format available.Message #35 received at 16231 <at> debbugs.gnu.org (full text, mbox):
From: Phillip Susi <psusi <at> ubuntu.com> To: 16231 <at> debbugs.gnu.org Cc: bcl <at> redhat.com Subject: [PATCH 2/9] libparted: remove old partitions *first* before adding new ones Date: Fri, 2 May 2014 21:50:44 -0400
"libparted: avoid disturbing partitions" put the remove of the old
partition in second pass. If you simultaneously removed partitions 1
and 2, and created a new partition #1 that overlapped the previous second
partition, the sync would fail because it would try to create the new,
larger partition #1 before removing the old partition #2.
---
libparted/arch/linux.c | 43 ++++++++++++++--------------
tests/Makefile.am | 1 +
tests/t1104-remove-and-add-partition.sh | 50 +++++++++++++++++++++++++++++++++
3 files changed, 72 insertions(+), 22 deletions(-)
create mode 100644 tests/t1104-remove-and-add-partition.sh
diff --git a/libparted/arch/linux.c b/libparted/arch/linux.c
index 71f5034..ced06a3 100644
--- a/libparted/arch/linux.c
+++ b/libparted/arch/linux.c
@@ -2855,23 +2855,8 @@ _disk_sync_part_table (PedDisk* disk)
&& 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);
- 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;
/* partition is unchanged, so nothing to do */
+ ok[i - 1] = 1;
continue;
}
}
@@ -2890,12 +2875,26 @@ _disk_sync_part_table (PedDisk* disk)
} while (n_sleep--);
if (!ok[i - 1] && errnums[i - 1] == ENXIO)
ok[i - 1] = 1; /* it already doesn't exist */
- if (part && ok[i - 1]) {
- /* add the (possibly modified or new) partition */
- if (!add_partition (disk, part)) {
- ok[i - 1] = 0;
- errnums[i - 1] = errno;
- }
+ }
+ for (i = 1; i <= lpn; i++) {
+ PedPartition *part = ped_disk_get_partition (disk, i);
+ if (!part)
+ continue;
+ 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;
+ /* partition is unchanged, so nothing to do */
+ continue;
+ }
+ /* add the (possibly modified or new) partition */
+ if (!add_partition (disk, part)) {
+ ok[i - 1] = 0;
+ errnums[i - 1] = errno;
}
}
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 9100a81..e064b8f 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 \
+ t1104-remove-and-add-partition.sh \
t1700-probe-fs.sh \
t2200-dos-label-recog.sh \
t2201-pc98-label-recog.sh \
diff --git a/tests/t1104-remove-and-add-partition.sh b/tests/t1104-remove-and-add-partition.sh
new file mode 100644
index 0000000..61cc392
--- /dev/null
+++ b/tests/t1104-remove-and-add-partition.sh
@@ -0,0 +1,50 @@
+#!/bin/sh
+# make sure that removing a higher numbered partition and adding a lower
+# one using that space at the same time works
+
+# Copyright (C) 2014 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_
+ss=$sector_size_
+
+d1= f1=
+cleanup_fn_()
+{
+ test -n "$d1" && losetup -d "$d1"
+ rm -f "$f1"
+}
+
+f1=$(pwd)/1; d1=$(loop_setup_ "$f1") \
+ || skip_ "is this partition mounted with 'nodev'?"
+
+require_partitionable_loop_device_ $d1
+
+# create one big partition
+parted -s $d1 mklabel msdos mkpart primary ext2 1m 10m || fail=1
+
+# save this table
+dd if=$d1 of=saved count=1 || fail=1
+
+# create two small partitions
+parted -s $d1 mklabel msdos mkpart primary ext2 1m 5m mkpart primary ext2 5m 10m || fail=1
+
+# restore first table and make sure partprobe works
+dd if=saved of=$d1 || fail=1
+partprobe $d1 || fail=1
+
+Exit $fail
--
1.9.1
bug-parted <at> gnu.org:bug#16231; Package parted.
(Sat, 03 May 2014 01:52:03 GMT) Full text and rfc822 format available.Message #38 received at 16231 <at> debbugs.gnu.org (full text, mbox):
From: Phillip Susi <psusi <at> ubuntu.com> To: 16231 <at> debbugs.gnu.org Cc: bcl <at> redhat.com Subject: [PATCH 4/9] libparted: fix loop labels to not vanish Date: Fri, 2 May 2014 21:50:46 -0400
The loop label type was using the existence of a partition as a proxy for
a filesystem being detected, and loop_write() would try to write a loop
signature if there was no filesystem, and erase it if there was. Because
of this, creating a partition without writing a filesystem to it caused
loop_write to erase the loop label.
There seems to be no reason to bother erasing the loop label if it is still
present along with a filesystem signature, so don't bother with this, and
actually check to see if a filesystem is detected in the partition rather
than using the existence of a partition to decide if writing the loop
signature is needed. Finally, since there is no way to preserve the
existence of a partition with no filesystem in it, have loop_read() always
create a partition, even if no filesystem is detected.
---
NEWS | 6 ++++++
libparted/labels/loop.c | 53 ++++++++++++++++++++++---------------------------
2 files changed, 30 insertions(+), 29 deletions(-)
diff --git a/NEWS b/NEWS
index 093314b..36536e2 100644
--- a/NEWS
+++ b/NEWS
@@ -23,6 +23,9 @@ GNU parted NEWS -*- outline -*-
** Bug Fixes
+ libparted: fix loop labels to not vanish if you don't create
+ a filesystem, and to not return an error syncing when you do.
+
libparted: remove all old partitions, even if new label does not allow
as many.
@@ -110,6 +113,9 @@ GNU parted NEWS -*- outline -*-
** Changes in behavior
+ When creating a loop label, it automatically comes with a partition
+ using the whole disk.
+
parted -l no longer lists device-mapper devices other than
dmraid whole disks.
diff --git a/libparted/labels/loop.c b/libparted/labels/loop.c
index ea8f007..8ebb1f4 100644
--- a/libparted/labels/loop.c
+++ b/libparted/labels/loop.c
@@ -80,7 +80,23 @@ 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);
+ PED_ASSERT (disk != NULL);
+ PedGeometry *geom = ped_geometry_new (dev, 0, dev->length);
+ PED_ASSERT (geom != NULL);
+ PedPartition *part = ped_partition_new (disk, PED_PARTITION_NORMAL,
+ NULL, geom->start, geom->end);
+ PED_ASSERT (part != NULL);
+ ped_geometry_destroy (geom);
+ PedConstraint *constraint_any = ped_constraint_any (dev);
+ if (!ped_disk_add_partition (disk, part, constraint_any))
+ goto error;
+ ped_constraint_destroy (constraint_any);
+ return disk;
+ error:
+ ped_constraint_destroy (constraint_any);
+ ped_disk_destroy (disk);
+ return NULL;
}
static PedDisk*
@@ -118,18 +134,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,7 +147,6 @@ 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;
@@ -156,29 +165,15 @@ static int
loop_write (const PedDisk* disk)
{
size_t buflen = disk->dev->sector_size;
- char *buf = ped_malloc (buflen);
- if (buf == NULL)
- 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);
- }
-
+ char *buf = alloca (buflen);
+ PedPartition *part = ped_disk_get_partition (disk, 1);
+ /* if there is already a filesystem on the disk, we don't need to write the signature */
+ if (part && part->fs_type)
+ return 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;
+ return ped_device_write (disk->dev, buf, 0, 1);
}
#endif /* !DISCOVER_ONLY */
--
1.9.1
bug-parted <at> gnu.org:bug#16231; Package parted.
(Sat, 03 May 2014 01:52:04 GMT) Full text and rfc822 format available.Message #41 received at 16231 <at> debbugs.gnu.org (full text, mbox):
From: Phillip Susi <psusi <at> ubuntu.com> To: 16231 <at> debbugs.gnu.org Cc: bcl <at> redhat.com Subject: [PATCH 5/9] libparted: don't create partition on loop label Date: Fri, 2 May 2014 21:50:47 -0400
The loop label represents an unpartitioned disk, but creates
a dummy partition to represent the whole disk. This dummy partition
was actually being loaded into the kernel. Don't do that.
---
NEWS | 4 ++++
libparted/arch/linux.c | 3 +++
2 files changed, 7 insertions(+)
diff --git a/NEWS b/NEWS
index 36536e2..385a120 100644
--- a/NEWS
+++ b/NEWS
@@ -23,6 +23,10 @@ GNU parted NEWS -*- outline -*-
** Bug Fixes
+ libparted: The loop label represents an unpartitioned disk, but creates
+ a dummy partition to represent the whole disk. This dummy partition
+ was actually being loaded into the kernel. Don't do that.
+
libparted: fix loop labels to not vanish if you don't create
a filesystem, and to not return an error syncing when you do.
diff --git a/libparted/arch/linux.c b/libparted/arch/linux.c
index 4cbe49b..9ae6d64 100644
--- a/libparted/arch/linux.c
+++ b/libparted/arch/linux.c
@@ -2883,6 +2883,9 @@ _disk_sync_part_table (PedDisk* disk)
lpn = PED_MIN(lpn, part_range);
else
lpn = part_range;
+ /* don't actually add partitions for loop */
+ if (strcmp (disk->type->name, "loop") == 0)
+ lpn = 0;
for (i = 1; i <= lpn; i++) {
PedPartition *part = ped_disk_get_partition (disk, i);
if (!part)
--
1.9.1
bug-parted <at> gnu.org:bug#16231; Package parted.
(Sat, 03 May 2014 01:52:05 GMT) Full text and rfc822 format available.Message #44 received at 16231 <at> debbugs.gnu.org (full text, mbox):
From: Phillip Susi <psusi <at> ubuntu.com> To: 16231 <at> debbugs.gnu.org Cc: bcl <at> redhat.com Subject: [PATCH 6/9] partprobe: do not skip loop labels Date: Fri, 2 May 2014 21:50:48 -0400
Partprobe was not syncing loop labels. This resulted it failing to remove
existing partitions when switching to a loop label.
---
NEWS | 3 +++
partprobe/partprobe.c | 4 +---
2 files changed, 4 insertions(+), 3 deletions(-)
diff --git a/NEWS b/NEWS
index 385a120..f99c6fe 100644
--- a/NEWS
+++ b/NEWS
@@ -23,6 +23,9 @@ GNU parted NEWS -*- outline -*-
** Bug Fixes
+ partprobe: when called on a disk that has become a loop label,
+ remove any partitions left over from a previous label.
+
libparted: The loop label represents an unpartitioned disk, but creates
a dummy partition to represent the whole disk. This dummy partition
was actually being loaded into the kernel. Don't do that.
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");
--
1.9.1
bug-parted <at> gnu.org:bug#16231; Package parted.
(Sat, 03 May 2014 01:52:05 GMT) Full text and rfc822 format available.Message #47 received at 16231 <at> debbugs.gnu.org (full text, mbox):
From: Phillip Susi <psusi <at> ubuntu.com> To: 16231 <at> debbugs.gnu.org Cc: bcl <at> redhat.com Subject: [PATCH 7/9] libparted: give correct partition device name on loop labels Date: Fri, 2 May 2014 21:50:49 -0400
ped_partition_get_path() was returning "/dev/foo1" instead of
"/dev/foo" on loop labels. This caused gparted to run tools like mkfs on
a device node that did not actually exist.
---
NEWS | 3 +++
libparted/arch/linux.c | 10 ++++++++--
2 files changed, 11 insertions(+), 2 deletions(-)
diff --git a/NEWS b/NEWS
index f99c6fe..c97f4eb 100644
--- a/NEWS
+++ b/NEWS
@@ -23,6 +23,9 @@ GNU parted NEWS -*- outline -*-
** Bug Fixes
+ libparted: ped_partition_get_path() was returning "/dev/foo1" instead
+ of "/dev/foo" for loop labels.
+
partprobe: when called on a disk that has become a loop label,
remove any partitions left over from a previous label.
diff --git a/libparted/arch/linux.c b/libparted/arch/linux.c
index 9ae6d64..f2e2abc 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>
@@ -2356,6 +2357,9 @@ _device_get_part_path (PedDevice const *dev, int num)
static char*
linux_partition_get_path (const PedPartition* part)
{
+ /* loop label means use the whole disk */
+ if (strcmp (part->disk->type->name, "loop") == 0)
+ return xstrdup (part->disk->dev->path);
return _device_get_part_path (part->disk->dev, part->num);
}
@@ -2424,6 +2428,8 @@ linux_partition_is_busy (const PedPartition* part)
PED_ASSERT (part != NULL);
+ if (strcmp (part->disk->type->name, "loop") == 0)
+ return linux_is_busy (part->disk->dev);
if (_partition_is_mounted (part))
return 1;
if (part->type == PED_PARTITION_EXTENDED) {
@@ -2546,7 +2552,7 @@ _sysfs_ull_entry_from_part(PedPartition const* part, const char *entry,
unsigned long long *val)
{
char path[128];
- char *part_name = linux_partition_get_path(part);
+ char *part_name = _device_get_part_path (part->disk->dev, part->num);
if (!part_name)
return false;
@@ -2581,7 +2587,7 @@ _kernel_get_partition_start_and_length(PedPartition const *part,
PED_ASSERT(start);
PED_ASSERT(length);
- char *dev_name = linux_partition_get_path (part);
+ char *dev_name = _device_get_part_path (part->disk->dev, part->num);
if (!dev_name)
return false;
--
1.9.1
bug-parted <at> gnu.org:bug#16231; Package parted.
(Sat, 03 May 2014 01:52:06 GMT) Full text and rfc822 format available.Message #50 received at 16231 <at> debbugs.gnu.org (full text, mbox):
From: Phillip Susi <psusi <at> ubuntu.com> To: 16231 <at> debbugs.gnu.org Cc: bcl <at> redhat.com Subject: [PATCH 8/9] libparted: don't trash filesystem when writing loop label Date: Fri, 2 May 2014 21:50:50 -0400
If you deleted the fake partition on a loop label, loop_write() would write
the loop signature to the device, zeroing out all other bytes in the first
sector. When the disk contained an ext[234] filesystem and was using 2k
sectors, this would trash the super block residing in the 1-2kb part of the
sector causing the disk to become unrecognized. Instead, read the existing
sector and only modify the first few bytes that contain the loop label.
---
libparted/labels/loop.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/libparted/labels/loop.c b/libparted/labels/loop.c
index 8ebb1f4..98f9f23 100644
--- a/libparted/labels/loop.c
+++ b/libparted/labels/loop.c
@@ -170,7 +170,8 @@ loop_write (const PedDisk* disk)
/* if there is already a filesystem on the disk, we don't need to write the signature */
if (part && part->fs_type)
return 1;
- memset (buf, 0, buflen);
+ if (!ped_device_read (disk->dev, buf, 0, 1))
+ return 0;
strcpy (buf, LOOP_SIGNATURE);
return ped_device_write (disk->dev, buf, 0, 1);
--
1.9.1
bug-parted <at> gnu.org:bug#16231; Package parted.
(Sat, 03 May 2014 01:52:07 GMT) Full text and rfc822 format available.Message #53 received at 16231 <at> debbugs.gnu.org (full text, mbox):
From: Phillip Susi <psusi <at> ubuntu.com> To: 16231 <at> debbugs.gnu.org Cc: bcl <at> redhat.com Subject: [PATCH 3/9] libparted: remove all old partitions, even if new label allows less Date: Fri, 2 May 2014 21:50:45 -0400
We were limiting partition sync operations to the lesser number allowed
by the device, or the label. This meant that when creating a new label
over an old label that had more partitions than the new one allows, the
higher partitions would not be removed. Use the greater of the two values
for the remove pass, and the lesser for the add.
---
NEWS | 3 +++
libparted/arch/linux.c | 11 +++++++++--
2 files changed, 12 insertions(+), 2 deletions(-)
diff --git a/NEWS b/NEWS
index 1233f1c..093314b 100644
--- a/NEWS
+++ b/NEWS
@@ -23,6 +23,9 @@ GNU parted NEWS -*- outline -*-
** Bug Fixes
+ libparted: remove all old partitions, even if new label does not allow
+ as many.
+
libparted: fat and ntfs boot sectors were misdetected as dos
partition tables instead of being treated as a loop label.
diff --git a/libparted/arch/linux.c b/libparted/arch/linux.c
index ced06a3..4cbe49b 100644
--- a/libparted/arch/linux.c
+++ b/libparted/arch/linux.c
@@ -2823,9 +2823,10 @@ _disk_sync_part_table (PedDisk* disk)
get_partition_start_and_length = _kernel_get_partition_start_and_length;
}
- /* lpn = largest partition number. */
+ /* lpn = largest partition number.
+ * for remove pass, use greater of device or label limit */
if (ped_disk_get_max_supported_partition_count(disk, &lpn))
- lpn = PED_MIN(lpn, part_range);
+ lpn = PED_MAX(lpn, part_range);
else
lpn = part_range;
@@ -2876,6 +2877,12 @@ _disk_sync_part_table (PedDisk* disk)
if (!ok[i - 1] && errnums[i - 1] == ENXIO)
ok[i - 1] = 1; /* it already doesn't exist */
}
+ /* lpn = largest partition number.
+ * for add pass, use lesser of device or label limit */
+ if (ped_disk_get_max_supported_partition_count(disk, &lpn))
+ lpn = PED_MIN(lpn, part_range);
+ else
+ lpn = part_range;
for (i = 1; i <= lpn; i++) {
PedPartition *part = ped_disk_get_partition (disk, i);
if (!part)
--
1.9.1
bug-parted <at> gnu.org:bug#16231; Package parted.
(Sat, 03 May 2014 01:52:08 GMT) Full text and rfc822 format available.Message #56 received at 16231 <at> debbugs.gnu.org (full text, mbox):
From: Phillip Susi <psusi <at> ubuntu.com> To: 16231 <at> debbugs.gnu.org Cc: bcl <at> redhat.com Subject: [PATCH 9/9] tests: test loop labels Date: Fri, 2 May 2014 21:50:51 -0400
Verify previous fixes to loop labels.
---
tests/Makefile.am | 1 +
tests/t1102-loop-label.sh | 104 ++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 105 insertions(+)
create mode 100644 tests/t1102-loop-label.sh
diff --git a/tests/Makefile.am b/tests/Makefile.am
index e064b8f..26226cf 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 \
t1104-remove-and-add-partition.sh \
t1700-probe-fs.sh \
t2200-dos-label-recog.sh \
diff --git a/tests/t1102-loop-label.sh b/tests/t1102-loop-label.sh
new file mode 100644
index 0000000..f5701a3
--- /dev/null
+++ b/tests/t1102-loop-label.sh
@@ -0,0 +1,104 @@
+#!/bin/sh
+# make sure that loop labels work correctly
+# 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% || fail=1
+if [ ! -e ${dev}1 ]; then
+ echo "Partition doesn't exist on loop device"
+ fail=1
+fi
+
+mke2fs -F $dev
+partprobe $dev || fail=1
+if [ -e ${dev}1 ]; then
+ echo "Partition should not exist on loop device"
+ fail=1
+fi
+
+# make sure new loop label removes old partitions > 1
+parted -s $dev mklabel msdos mkpart primary ext2 0% 50% mkpart primary ext2 50% 100% || fail=1
+parted -s $dev mklabel loop || fail=1
+if [ -e ${dev}2 ]; then
+ echo "Partition 2 not removed"
+ fail=1
+fi
+
+Exit $fail
--
1.9.1
bug-parted <at> gnu.org:bug#16231; Package parted.
(Sat, 03 May 2014 01:52:08 GMT) Full text and rfc822 format available.Message #59 received at 16231 <at> debbugs.gnu.org (full text, mbox):
From: Phillip Susi <psusi <at> ubuntu.com> To: 16231 <at> debbugs.gnu.org Cc: bcl <at> redhat.com Subject: [PATCH 1/9] libparted: don't detect fat and ntfs boot sectors as dos MBR Date: Fri, 2 May 2014 21:50:43 -0400
fat and ntfs boot sectors are very similar to an MBR so if you had one of
these filesystems on an unpartitioned disk, parted detected them as a dos
partition table. Have the dos label code call the fat and ntfs filesystem
probes and if they recognize the sector ( their tests are more stringent )
then don't claim it as a dos label.
---
NEWS | 3 +++
libparted/fs/ntfs/ntfs.c | 2 +-
libparted/labels/dos.c | 29 +++++++++++++++++++++++++++++
3 files changed, 33 insertions(+), 1 deletion(-)
diff --git a/NEWS b/NEWS
index 61a896c..1233f1c 100644
--- a/NEWS
+++ b/NEWS
@@ -23,6 +23,9 @@ GNU parted NEWS -*- outline -*-
** Bug Fixes
+ libparted: fat and ntfs boot sectors were misdetected as dos
+ partition tables instead of being treated as a loop label.
+
libparted: previously if you chose to ignore the warning about
the gpt thinking the disk was smaller than it appears to be on
on disk, subsequent warnings on other disks would be suppressed.
diff --git a/libparted/fs/ntfs/ntfs.c b/libparted/fs/ntfs/ntfs.c
index 3ba2683..4c154fd 100644
--- a/libparted/fs/ntfs/ntfs.c
+++ b/libparted/fs/ntfs/ntfs.c
@@ -32,7 +32,7 @@
#define NTFS_SIGNATURE "NTFS"
-static PedGeometry*
+PedGeometry*
ntfs_probe (PedGeometry* geom)
{
char *buf = alloca (geom->dev->sector_size);
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;
}
--
1.9.1
bug-parted <at> gnu.org:bug#16231; Package parted.
(Wed, 07 May 2014 22:50:02 GMT) Full text and rfc822 format available.Message #62 received at 16231 <at> debbugs.gnu.org (full text, mbox):
From: "Brian C. Lane" <bcl <at> redhat.com> To: Phillip Susi <psusi <at> ubuntu.com> Cc: 16231 <at> debbugs.gnu.org Subject: Re: [PATCH 0/9] Refactored loop fixes Date: Wed, 7 May 2014 15:49:13 -0700
On Fri, May 02, 2014 at 09:50:42PM -0400, Phillip Susi wrote: > I have refactored the loop fixes making them cleaner, and breaking them out > into separate patches for easier review. > > Phillip Susi (9): > libparted: don't detect fat and ntfs boot sectors as dos MBR > libparted: remove old partitions *first* before adding new ones > libparted: remove all old partitions, even if new label allows less > libparted: fix loop labels to not vanish > libparted: don't create partition on loop label > partprobe: do not skip loop labels > libparted: give correct partition device name on loop labels > libparted: don't trash filesystem when writing loop label > tests: test loop labels > > NEWS | 22 +++++++ > libparted/arch/linux.c | 67 ++++++++++++-------- > libparted/fs/ntfs/ntfs.c | 2 +- > libparted/labels/dos.c | 29 +++++++++ > libparted/labels/loop.c | 54 ++++++++--------- > partprobe/partprobe.c | 4 +- > tests/Makefile.am | 2 + > tests/t1102-loop-label.sh | 104 ++++++++++++++++++++++++++++++++ > tests/t1104-remove-and-add-partition.sh | 50 +++++++++++++++ > 9 files changed, 275 insertions(+), 59 deletions(-) > create mode 100644 tests/t1102-loop-label.sh > create mode 100644 tests/t1104-remove-and-add-partition.sh > > -- > 1.9.1 > NAK. These make a pile of the tests fail: ========================================== GNU parted 3.1: tests/test-suite.log ========================================== # TOTAL: 77 # PASS: 27 # SKIP: 35 # XFAIL: 0 # FAIL: 15 # XPASS: 0 # ERROR: 0 .. contents:: :depth: 2 looks like at least one core dump in the mix. -- Brian C. Lane | Anaconda Team | IRC: bcl #anaconda | Port Orchard, WA (PST8PDT)
bug-parted <at> gnu.org:bug#16231; Package parted.
(Thu, 08 May 2014 01:33:01 GMT) Full text and rfc822 format available.Message #65 received at 16231 <at> debbugs.gnu.org (full text, mbox):
From: Phillip Susi <psusi <at> ubuntu.com> To: "Brian C. Lane" <bcl <at> redhat.com> Cc: 16231 <at> debbugs.gnu.org Subject: Re: [PATCH 0/9] Refactored loop fixes Date: Wed, 07 May 2014 21:32:45 -0400
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA512 On 05/07/2014 06:49 PM, Brian C. Lane wrote: > NAK. These make a pile of the tests fail: > > ========================================== GNU parted 3.1: > tests/test-suite.log ========================================== # > TOTAL: 77 # PASS: 27 # SKIP: 35 # XFAIL: 0 # FAIL: 15 # XPASS: > 0 # ERROR: 0 .. contents:: :depth: 2 > > looks like at least one core dump in the mix. Something weird is going on.. as I mentioned before, I get that odd gnulib error after the first 3 sector size passes in a make check that seems to be a bug in the build scripts, but the first three all pass: ============================================================================ Testsuite summary for GNU parted 3.1.98-c457 ============================================================================ # TOTAL: 77 # PASS: 63 # SKIP: 13 # XFAIL: 1 # FAIL: 0 # XPASS: 0 # ERROR: 0 make[1]: Leaving directory `/home/psusi/parted' GEN public-submodule-commit Stopping at 'gnulib'; script returned non-zero status. maint.mk: found non-public submodule commit make: *** [public-submodule-commit] Error 1 And then I manually run make check-recursive to get the 512 byte sector size: ============================================================================ Testsuite summary for GNU parted 3.1.98-c457 ============================================================================ # TOTAL: 77 # PASS: 75 # SKIP: 1 # XFAIL: 1 # FAIL: 0 # XPASS: 0 # ERROR: 0 ============================================================================ Can you look into why/what fails on your system? I also notice yours says version 3.1 while mine is 3.1.98-c457. Did you check out 3.1 instead of master and then not run bootstrap/configure after applying the patches? -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQEcBAEBCgAGBQJTat69AAoJEI5FoCIzSKrw+hQH/2WyE97Vu7FADHoOo9tIl3ht bzkj2v9j9B/y0LM1dbfhUsNfOjVHFAgIpV6fVjBTS/xVFruvbGSo11WjcFsXnAGr 8PU6FW53+Sx+ELaREoJ5iLEgR37Q9cM4pbR1sGjFzAfy5yRtrGXKjee3KuEced1q N6FrbLgV2ARdTV+BHbMMy8rtjPPU1KqC/j43NlQ6jEJm19tOT08cXDReLpofzjMf 6mErkN3in9u4n5QW5M+3We2/qeA2wMV8IzKb4Xoe1/S+yXCtCJf1YYOHGyWGH3rL 6Uf5JNHY0CdEpRpfSU37lC1g0yOWTZnhXOJmpAwfCVf7fSRkp501fmHlHB5UxlU= =vd3Z -----END PGP SIGNATURE-----
bug-parted <at> gnu.org:bug#16231; Package parted.
(Thu, 08 May 2014 15:23:02 GMT) Full text and rfc822 format available.Message #68 received at 16231 <at> debbugs.gnu.org (full text, mbox):
From: "Brian C. Lane" <bcl <at> redhat.com> To: Phillip Susi <psusi <at> ubuntu.com> Cc: 16231 <at> debbugs.gnu.org Subject: Re: [PATCH 0/9] Refactored loop fixes Date: Thu, 8 May 2014 08:22:22 -0700
On Wed, May 07, 2014 at 09:32:45PM -0400, Phillip Susi wrote: > On 05/07/2014 06:49 PM, Brian C. Lane wrote: > > NAK. These make a pile of the tests fail: > > > > ========================================== GNU parted 3.1: > > tests/test-suite.log ========================================== # > > TOTAL: 77 # PASS: 27 # SKIP: 35 # XFAIL: 0 # FAIL: 15 # XPASS: > > 0 # ERROR: 0 .. contents:: :depth: 2 > > > > looks like at least one core dump in the mix. > > Something weird is going on.. as I mentioned before, I get that odd > gnulib error after the first 3 sector size passes in a make check that > seems to be a bug in the build scripts, but the first three all pass: > > ============================================================================ > Testsuite summary for GNU parted 3.1.98-c457 > ============================================================================ > # TOTAL: 77 > # PASS: 63 > # SKIP: 13 > # XFAIL: 1 > # FAIL: 0 > # XPASS: 0 > # ERROR: 0 > > make[1]: Leaving directory `/home/psusi/parted' > GEN public-submodule-commit > Stopping at 'gnulib'; script returned non-zero status. > maint.mk: found non-public submodule commit > make: *** [public-submodule-commit] Error 1 > > And then I manually run make check-recursive to get the 512 byte > sector size: > > ============================================================================ > Testsuite summary for GNU parted 3.1.98-c457 > ============================================================================ > # TOTAL: 77 > # PASS: 75 > # SKIP: 1 > # XFAIL: 1 > # FAIL: 0 > # XPASS: 0 > # ERROR: 0 > ============================================================================ > > Can you look into why/what fails on your system? I also notice yours > says version 3.1 while mine is 3.1.98-c457. Did you check out 3.1 > instead of master and then not run bootstrap/configure after applying > the patches? That's weird (and troubling that the tests wouldn't fail the same). I applied them to my fedora-21 branch and ran them via a mockbuild as a first level test. My branch is only different from master by those few test fixup patches that I sent a few weeks ago. I'll take a deeper look at it when I get a chance, maybe not until Monday. -- Brian C. Lane | Anaconda Team | IRC: bcl #anaconda | Port Orchard, WA (PST8PDT)
bug-parted <at> gnu.org:bug#16231; Package parted.
(Mon, 19 May 2014 13:42:02 GMT) Full text and rfc822 format available.Message #71 received at 16231 <at> debbugs.gnu.org (full text, mbox):
From: Phillip Susi <psusi <at> ubuntu.com> To: "Brian C. Lane" <bcl <at> redhat.com> Cc: 16231 <at> debbugs.gnu.org Subject: Re: bug#16231: [PATCH 0/9] Refactored loop fixes Date: Mon, 19 May 2014 09:41:18 -0400
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 On 5/8/2014 11:22 AM, Brian C. Lane wrote: >> Can you look into why/what fails on your system? I also notice >> yours says version 3.1 while mine is 3.1.98-c457. Did you check >> out 3.1 instead of master and then not run bootstrap/configure >> after applying the patches? > > That's weird (and troubling that the tests wouldn't fail the same). > I applied them to my fedora-21 branch and ran them via a mockbuild > as a first level test. My branch is only different from master by > those few test fixup patches that I sent a few weeks ago. > > I'll take a deeper look at it when I get a chance, maybe not until > Monday. Had a chance to look into this at all yet? -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.17 (MingW32) Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQEcBAEBAgAGBQJTegn+AAoJEI5FoCIzSKrwKYUH/1T9I2hJt37187MRVIj0iLKN Z+apMhVlcu6kgW6DmO2tpJQzWZg9iBW4aBSWqU8DSEq6yi2lMgPix93qCz8rFbpr gt8nU54EVzdA+9OajqziVxhnNLfdoCrIMlyIF8XmI94wa0U2ejpgaA2vaYBk5GYJ Xrgn8c9Pk2Q7bwHT53Iw0AffFiun3afuPujK842SkWNAkfx1QBB2iCu0bgYIRDML 9QTEucgz+B+fEdoeGe8KmzMt7E8EluiWzhLDfCjjqLOO3ZZcqW5RVGl1p4DlKqGU gnfTSBhTH0aF++V/GRLmr74dlYPJ00PYtxkgYQSepFsVyVHRQHTur9VLV8qvaKs= =9fY1 -----END PGP SIGNATURE-----
bug-parted <at> gnu.org:bug#16231; Package parted.
(Thu, 22 May 2014 00:43:01 GMT) Full text and rfc822 format available.Message #74 received at 16231 <at> debbugs.gnu.org (full text, mbox):
From: "Brian C. Lane" <bcl <at> redhat.com> To: Phillip Susi <psusi <at> ubuntu.com> Cc: 16231 <at> debbugs.gnu.org Subject: Re: bug#16231: [PATCH 0/9] Refactored loop fixes Date: Wed, 21 May 2014 17:42:30 -0700
On Mon, May 19, 2014 at 09:41:18AM -0400, Phillip Susi wrote: > On 5/8/2014 11:22 AM, Brian C. Lane wrote: > >> Can you look into why/what fails on your system? I also notice > >> yours says version 3.1 while mine is 3.1.98-c457. Did you check > >> out 3.1 instead of master and then not run bootstrap/configure > >> after applying the patches? > > > > That's weird (and troubling that the tests wouldn't fail the same). > > I applied them to my fedora-21 branch and ran them via a mockbuild > > as a first level test. My branch is only different from master by > > those few test fixup patches that I sent a few weeks ago. > > > > I'll take a deeper look at it when I get a chance, maybe not until > > Monday. > > Had a chance to look into this at all yet? > The problem is that the ntfs probe has a fixed 512b buffer and tries to read a whole sector into it. The fat probes may also have similar issues. -- Brian C. Lane | Anaconda Team | IRC: bcl #anaconda | Port Orchard, WA (PST8PDT)
bug-parted <at> gnu.org:bug#16231; Package parted.
(Thu, 22 May 2014 13:33:02 GMT) Full text and rfc822 format available.Message #77 received at 16231 <at> debbugs.gnu.org (full text, mbox):
From: Phillip Susi <psusi <at> ubuntu.com> To: "Brian C. Lane" <bcl <at> redhat.com> Cc: 16231 <at> debbugs.gnu.org Subject: Re: bug#16231: [PATCH 0/9] Refactored loop fixes Date: Thu, 22 May 2014 09:32:33 -0400
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
On 5/21/2014 8:42 PM, Brian C. Lane wrote:
> The problem is that the ntfs probe has a fixed 512b buffer and
> tries to read a whole sector into it. The fat probes may also have
> similar issues.
You must be using an out of date tree; commit 80678bdd957cf4 fixed that:
static PedGeometry*
ntfs_probe (PedGeometry* geom)
{
- - char buf[512];
+ char *buf = alloca (geom->dev->sector_size);
+ PedGeometry *newg = NULL;
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.17 (MingW32)
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/
iQEcBAEBAgAGBQJTffxxAAoJEI5FoCIzSKrwWlYH/0RnKFsvr98qEyggM65tck0C
6cWVk4v91Al5mVlu/kU9ey/6FMOj5but91oBeQHTl603tZYFE+rLWXVQ9PPBSb+h
G2lGjTl6Rj4M9pjSPh59XvnLcM5Zgn9d5bP/3i4zUkkzNtgowufTpmXgFFNib58L
nF++rhTxL2l+3XZb356HYiXCbRb+enf2PS4Jz2U1cKjj8s4tusI/3bzN2MZmxubH
XtjNqiOnyaqOKXgz1VPQaMFe99cv07wQNkUiIO9EPIPwYbev2jr7lj/6fCfv6aIX
wdI5h2xkj48qK7FRdMalIk+2Sd5PsAoScRoMSRoW6a8GlX7qeu4ONxTCTpn9BnU=
=erE8
-----END PGP SIGNATURE-----
bug-parted <at> gnu.org:bug#16231; Package parted.
(Thu, 22 May 2014 19:02:01 GMT) Full text and rfc822 format available.Message #80 received at 16231 <at> debbugs.gnu.org (full text, mbox):
From: "Brian C. Lane" <bcl <at> redhat.com> To: Phillip Susi <psusi <at> ubuntu.com> Cc: 16231 <at> debbugs.gnu.org Subject: Re: bug#16231: [PATCH 0/9] Refactored loop fixes Date: Thu, 22 May 2014 12:01:42 -0700
On Thu, May 22, 2014 at 09:32:33AM -0400, Phillip Susi wrote:
> On 5/21/2014 8:42 PM, Brian C. Lane wrote:
> > The problem is that the ntfs probe has a fixed 512b buffer and
> > tries to read a whole sector into it. The fat probes may also have
> > similar issues.
>
> You must be using an out of date tree; commit 80678bdd957cf4 fixed that:
>
> static PedGeometry*
> ntfs_probe (PedGeometry* geom)
> {
> - char buf[512];
> + char *buf = alloca (geom->dev->sector_size);
> + PedGeometry *newg = NULL;
>
Sure enough, I was missing that one. Looks good, and passes all tests
on all arches.
--
Brian C. Lane | Anaconda Team | IRC: bcl #anaconda | Port Orchard, WA (PST8PDT)
Phillip Susi <psusi <at> ubuntu.com>:Phillip Susi <psusi <at> ubuntu.com>:Message #85 received at 16231-done <at> debbugs.gnu.org (full text, mbox):
From: Phillip Susi <psusi <at> ubuntu.com> Cc: 16231-done <at> debbugs.gnu.org Subject: Re: bug#16231: [PATCH 0/9] Refactored loop fixes Date: Thu, 22 May 2014 20:06:20 -0400
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA512 On 05/22/2014 03:01 PM, Brian C. Lane wrote: > Sure enough, I was missing that one. Looks good, and passes all > tests on all arches. Patches pushed. -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQEcBAEBCgAGBQJTfpD8AAoJEI5FoCIzSKrwyYcH/A8ofwpaDKB+9sL4h/eV+Ako lNBT4qRMwS3DecOkUrhvr9xbs+qYQztqHPkyAeB/6jdbGpNZY1TgarqmOu42zU8d nq0cHZFFfnbRm7yL7Sr3aw4rl/dtFMPiCqU6TjsGEFLOWg0nIapoHUhVhLGCpTCY b5ShqxdRaTrMjj00BvyKDwPwGdsMfEXkwMfcKx8W/eG2VJnOOXHBOZVV8edJF7Ew T+LSbzMxqlRt7GCcgrZTl5Ooya5jRfp6bsqKS3ihYwfLzp9w9TMw6ozxNZEkffgV 79PVmHJMUeXKeKvwGHjhhKFUSim4oAy/HYDgO4/I12++eRJZjEe7O7k5DruPbTU= =cN6b -----END PGP SIGNATURE-----
Debbugs Internal Request <help-debbugs <at> gnu.org>
to internal_control <at> debbugs.gnu.org.
(Fri, 20 Jun 2014 11:24:04 GMT) Full text and rfc822 format available.
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.